> > rather than anything architectural. The only bit which worries me is
> > the xenbus-level protocol for adding and removing LUNs, and I suspect
> > that's just because I've not understood what you're actually doing.
> > If you could briefly explain how you expect this to work, at the level
> > of individual xenstore operations, I expect that would make things a
> > lot clearer.
> I will briefly explain the xenbus-level protocol below.
>
> I have to say first is that the protocol uses following two kinds of
> states, "XenBusState" and "DeviceState".
> (Please do not being confused, because the two states use similar
> notation (XenBusState*ing or XenBusState*ed) as state name.)
>
> 1.) XenbusState
> The XenBusState is used to control frontend's and backend's
> automata.
> The newly defined two states, "XenBusStateReconfiguring" and
> "XenBusStateReconfigured", are used for attach/detach protocol.
>
> 2.) DeviceState
> In addition to the XenBusState, we need another kind of state
> for each LUN to be attached and detached. The "DeviceState" is
> used for such a purpose.
> It uses "XenBusStateInitializing", "XenBusStateIntialized" and
> "XenBusStateConnected" in normal case, and they are stored at
> "vscsi-devs/dev-*/state" in xenstore.
It might be a bit clearer if the device state states had different
names to the xenbus states? They don't really mean the same thing,
and overloading the names like that is kind of confusing.
> Following is a protocol for attaching a LUN. Please note that error
> case is not described for simplicity.
>
> xend backend frontend
> -----------------------------------------------------------------------
> set DeviceState to
> XenBusStateInitializing.
> set XenBusState to
> XenBusStateReconfiguring.
>
> detect backend has
> changed.
> set XenBusState to
> XenBusStateReconfiguring.
>
> detect frontend has
> changed.
> export a specified LUN.
> (call scsi_device_lookup(),
> maintain translation table
> and so on.)
> set DeviceState to
> XenBusStateInitialized.
> set XenBusState to
> XenBusStateReconfigured.
>
> detect backend has
> changed.
> import the exported LUN.
> (call scsi_add_device().)
> set DeviceState to
> XenBusStateConnected.
> set XenBusState to
> XenBusStateConnected.
>
> detect frontend has
> changed.
> set DeviceState to
> XenBusStateConnected.
> set XenBusState to
> XenBusStateConnected.
> -----------------------------------------------------------------------
Okay, that doesn't sound unreasonable. Presumably xend will be
responsible for making sure that this only happens when both endpoints
are in state XenbusStateConnected?
> > > /* quoted scsi_lib.c/scsi_req_map_sg . */
> > > static int requset_map_sg(struct request *rq, pending_req_t *pending_req,
> > > unsigned int count)
> > > {
> > I don't quite understand why it was necessary to duplicate all of this
> > code. Would you mind explaining why you were unable to use the
> > existing implementation, please?
> In general, each segment of scatter/gather may start at offset
> non-zero like following figure.
>
> segment #0 segemnt #1 segemnt #2
> |~~~~~~| |~~~~~~| |~~~~~~|
> | | | | |~~~~~~|
> |~~~~~~| |______| | |
> | Data | | Data | | Data |
> |______| |~~~~~~| | |
> | | | | |______|
> |______| |______| |______|
>
> However, I suppose that the existing requset_map_sg() implementation
> assumes following structure, segment #0 may start offset non-zero but
> segment #1 and later should start at offset zero.
>
> segment #0 segemnt #1 segemnt #2
> |~~~~~~| |~~~~~~| |~~~~~~|
> | | | | | |
> |~~~~~~| | | | |
> | | | Data | | Data |
> | Data | | | | |
> | | | | |______|
> |______| |______| |______|
Ah, yes, I see now. I had thought that scsi_req_map_sg() could handle
arbitrary scatterlists (because it uses the offset field of each SGL
entry), but it looks like it actually doesn't, because nr_pages is
calculated assuming that only the first entry has a non-zero offset.
A little bit of googling suggests that this behaviour is apparently
by design, as bizarre as it may seem:
http://marc.info/?l=linux-scsi&m=116464965414878&w=2
So I guess that duplicating scsi_req_map_sg() in a non-broken way is
justified here.
It looks like you only need your requset_map_sg() in the NO_ASYNC
case, which I'm still not entirely convinced I understand. It looks
kind of like NO_ASYNC mode is some kind of debugging aide, which will
eventually be turned off, at which point all of this will just go
away; is that accurate? If so, it really isn't worth worrying about.
> Of cource, we can create a later type of scatter/gather structure on
> backend driver by copying data, however performance will be degraded
> maybe.
> On the other hand, we can also take another way that we define
> scatter/gather interface between frontend and backend (defined in
> vscsiif.h) should be later type. However, it will spoil flexibility
> of the interface.
>
> That is the reason why we use duplicate (and bit modify) the existing
> implementation.
Sure.
> Could you please suggest another good way?
No, what you've got is good enough.
Steven.
signature.asc
Description: Digital signature
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home