|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/4] pvSCSI driver
Hi Steven-san,
On Tue, 24 Jun 2008 14:13:13 +0100
Steven Smith <steven.smith@xxxxxxxxxx> wrote:
> 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.
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.
-----------------------------------------------------------------------
> > /* 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 | | | | |
| | | | |______|
|______| |______| |______|
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.
Could you please suggest another good way?
> Also, there's a typo in the name of the function.
>
> > struct request_queue *q = rq->q;
> > int nr_pages;
> > unsigned int nsegs = count;
> >
> > unsigned int data_len = 0, len, bytes, off;
> > struct page *page;
> > struct bio *bio = NULL;
> > int i, err, nr_vecs = 0;
> >
> > for (i = 0; i < nsegs; i++) {
> > page = pending_req->sgl[i].page;
> > off = (unsigned int)pending_req->sgl[i].offset;
> > len = (unsigned int)pending_req->sgl[i].length;
> > data_len += len;
> >
> > nr_pages = (len + off + PAGE_SIZE - 1) >> PAGE_SHIFT;
> > while (len > 0) {
> > bytes = min_t(unsigned int, len, PAGE_SIZE - off);
> >
> > if (!bio) {
> > nr_vecs = min_t(int, BIO_MAX_PAGES, nr_pages);
> > nr_pages -= nr_vecs;
> > bio = bio_alloc(GFP_KERNEL, nr_vecs);
> > if (!bio) {
> > err = -ENOMEM;
> > goto free_bios;
> > }
> > bio->bi_end_io = scsiback_bi_endio;
> > }
> >
> > if (bio_add_pc_page(q, bio, page, bytes, off) !=
> > bytes) {
> > bio_put(bio);
> > err = -EINVAL;
> > goto free_bios;
> > }
> >
> > if (bio->bi_vcnt >= nr_vecs) {
> > err = scsiback_merge_bio(rq, bio);
> > if (err) {
> > bio_endio(bio, bio->bi_size, 0);
> > goto free_bios;
> > }
> > bio = NULL;
> > }
> >
> > page++;
> > len -= bytes;
> > off = 0;
> > }
> > }
> >
> > rq->buffer = rq->data = NULL;
> > rq->data_len = data_len;
> >
> > return 0;
> >
> > free_bios:
> > while ((bio = rq->bio) != NULL) {
> > rq->bio = bio->bi_next;
> > /*
> > * call endio instead of bio_put incase it was bounced
> > */
> > bio_endio(bio, bio->bi_size, 0);
> > }
> >
> > return err;
> > }
> >
> > #endif
Best regards,
-----
Jun Kamada
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |