Hi Steven-san,
Thank you for your comments.
However, the deadline of Xen 3.3 will come soon, so we would like to
fix them on next patch with few remaining fixes that you pointed out
on June 24.
On Thu, 3 Jul 2008 10:47:29 +0100
Steven Smith <steven.smith@xxxxxxxxxx> wrote:
> > 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.
Yes, I understand. We would like to fix on next patch.
# Actually, we imitated following patches for PCI attach/detach.
#
# http://lists.xensource.com/archives/html/xen-devel/2008-02/msg00737.html
# http://lists.xensource.com/archives/html/xen-devel/2008-02/msg00736.html
# http://lists.xensource.com/archives/html/xen-devel/2008-02/msg00735.html
# http://lists.xensource.com/archives/html/xen-devel/2008-02/msg00734.html
> > 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?
Sorry, current "xend" (I will post it soon) does not have such a
functionality. We would like to add it on next patch.
> > > > /* 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.
Cleanup patch, I will post soon, cleanups such the "#ifdef".
Best regards.
-----
Jun Kamada
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home