> > I have a couple of comments on the design:
> >
> > -- You've ended up re-implementing a lot of Linux SCSI stuff in the
> > backend. I don't understand why this was necessary. Would you
> > mind explaining, please?
>
> # If I misunderstood your question, please inform.
>
> In order to provide LUN assignment to guest domain, backend driver is
> modified. Modification point is as follows.
>
> - Ring, EventChannel and GrantTable are independently allocated for
> each LUN assignment. Previsous version used them for each host (HBA).
> This is the main difference between previous and new version.
>
> And also, we removed code for FC transport layer.
I'm afraid I haven't looked closely at the previous revisions of this
patch, so I don't know about any differences between them. I was
referring more to bits like requset_map_sg, which is an almost direct
copy and paste of drivers/scsi/scsi_lib.c::scsi_req_map_sg, and
scsiback_merge_bio(), which is identical to scsi_merge_bio() except
for some whitespace changes. Having to carry our own implementation
of core Linux SCSI support seems like it'll be a significant
maintenance burden, and I'd like to understand why it was necessary.
> > -- The code seems to be a bit undecided about whether the exposed
> > devices are supposed to represent SCSI adapters or SCSI targets.
> > It looks like the frontend initially tries to treat them as a bunch
> > of targets, and then conditionally gloms them back together into
> > hosts depending on xenstore fields? Having a host per target would
> > make sense, as would having a single host with all of the targets
> > hanging off of it, but I don't understand why this split model is
> > useful. Perhaps I'm just missing something.
> Frontend driver try to attach each LUN according to the information on
> xenstore. New version support only LUN assignment to guest domain. If
> you want SCSI device/host assignment to guest, you have to specify all
> LUNs under the device/host. I understand wildcard description, such
> like "1:*:*:*", is needed.
Okay, so a device in xenstore corresponds to a LUN, and you map them
to particular hosts based on the device name? That's kind of ugly,
but it's probably the most direct way of doing it through xenstore.
What I don't understand is why you need this at all. It seems like it
would make more sense to either:
a) Hang every LUN off of the same scsi host, or
b) Give each LUN its own scsi host.
Is there some reason why you might want to do something like this:
Host A -------+----- LUN 1
|
+----- LUN 2
Host B ------------- LUN 3
i.e. partition the virtual LUNs between multiple hosts in the guest,
but keeping some of them together? Perhaps I'm just missing
something, but I can't think of any use cases which would benefit from
that, and trying to support it noticeably complicates the frontend.
> > -- I don't understand the distinction between comfront and scsifront.
> > What was the reason for this split?
> I intended to seperate two types of code, primitive code for
> communication between frontend and backend, and SCSI specific code.
> However, the separation may be incomplete.
Okay.
> > -- There don't seem to be many comments in these patches. Xen and
> > Linux are both generally pretty comment-light, but an entire new
> > device class without a single meaningful comment still kind of
> > stands out.
> I agree to your comment completely. I should add more comment to
> source code.
Thanks.
Steven.
signature.asc
Description: Digital signature
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home