Re: [PATCH 0/6] pvSCSI (SCSI pass through) driver

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

Hi Keir-san,

Thank you for your comments.

On Tue, 30 Oct 2007 10:56:57 +0000
Keir Fraser <Keir.Fraser@cl.cam.ac.uk> wrote:

> Vscsiif.h still needs clean up:
>  1. You cannot have Linux-specific stuff in that header file. It's supposed
> to be generic.

I understood. I would like to remove Linux-specific definitions, such
like below.

    #include <scsi/scsi_transport.h>
    enum fc_port_state

>  2. You cannot pollute the global namespace with unqualified names. All your
> names, macros, etc should have a vscsiif_ or VSCSIIF_ prefix. Names like
> DEFAULT_CAN_QUEUE and SG_TABLESIZE pollute the namespace and it's not
> immediately obvious which header they come from (which it would be if they
> were prefixed).

I would like to add the prefix to all the definitions.

>  3. Do you really mean for vscsiif_btf_response to be empty? And neither
> response structure has an identifier to match the request it responds to.
> Can requests not be handled out of order by your drivers? That would seem
> very odd.

The pvSCSI driver uses two rings for frontend-to-backend (ftb) and
backend-to-frontend (btf) communication. The reason why the
architecture is taken is as follows.

- The ftb_request packet was relatively big, so we could not allocate 
  enough entries on the ring. It caused a performance problem.
- By dividing into the two rings ("ftb" ring and "btf" ring), and using
  original queueing mechanism on both frontend and backend, the entries
  on the ring is immediately freed after queued. It contributes a good
  performance.

Therefore, the ftb_response and btf_response carries no information.
It is only used as a trigger of freeing the entry on the ring.

We also would like to discuss about design of the ring architecture
described above. Any comment is welcome.

> Also CONFIG_XEN_FC is ifdef'ed all over scsifront and scsiback. You'll have
> to find some way to clean that up, perhaps by providing null stub
> implementations of fc functions in a header file if !CONFIG_XEN_FC.

I think that one of the way to reduce CONFIG_XEN_FC is to ifdef'ing
intialization of FC transport functionality. (However, object modules
still include FC modules.) How do you think about this modification?
Could you tell me more detail about your advice?

Please note why the CONFIG_XEN_FC is needed are,

- Old Linux kernel has different definition of
  "struct fc_function_template". 
- In order to support the old Linux kernel on HVM guest domain, simple
  way is disabling FC functionality.

> FIXME comments in the connection setup code, accompanied by hard-coded
> constants, don't bode well. Is the connection setup protocol fully baked
> yet?

FIXME comment indicates tunable point of the parameter for performance
and so on. I understood the word "FIXME' was not appropriate. I will
remove the word.

> All this, coupled with the fact that you can only export whole HBAs, which
> doesn't seem likely to be a very popular usage scenario, makes me think this
> driver should wait until after 3.2.0.

We consider that SCSI support on guest domain is mandatory for bussiness
use. Using a NPIV technology on host domain, each guest domain can have
own virtual HBA. We have already evaluated on our environment the
conbination of NPIV and pvSCSI work well.
On the other hand, we undestand the usefullness of LUN assignment to
guest domain. We would like to develop it on next step in our current
plan.

Best regards,

-----
Jun Kamada