Re: [Patch 0/7] pvSCSI driver

[Jun Kamada <kama@jp.fujitsu.com>]

Hi Steven-san,

I appreciate your sending a lot of helpful comments. I will answer
the questions and comments described below now. For the other
questions/comments about each source code, I would like to reply on
another mail later.

On Wed, 27 Feb 2008 11:16:10 +0000
Steven Smith <steven.smith@eu.citrix.com> wrote:
> 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.

> -- 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.

> -- 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.

> -- 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.

> I'll reply to the individual patches with more detailed comments.  A
> lot of my complaints will doubtless turn out to just be because I'm
> not very used to Linux SCSI.  I've not looked at the xend changes,
> because I'm not really competent to evaluate them.
> 
> Steven.


Best regards,

-----
Jun Kamada