[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH V4] Update pvSCSI protocol description



On 08/26/2014 12:06 PM, David Vrabel wrote:
On 26/08/14 05:15, Juergen Gross wrote:
Update the protocol description of the pvSCSI framework used to pass through
SCSI devices to a guest (pv or hvm).

4 versions in 24 hours?!  Please allow a few days for people to review
before posting updated versions.

Jan requested some substantial changes, so I replied quickly to
make further reviews easier.


--- a/xen/include/public/io/vscsiif.h
+++ b/xen/include/public/io/vscsiif.h
@@ -1,8 +1,11 @@
  
/******************************************************************************
   * vscsiif.h
- *
+ *
   * Based on the blkif.h code.
- *
+ *
+ * This interface is to be regarded as a stable API between XEN domains
+ * running potentially different Linux kernel versions.

There shouldn't be anything Linux-specific about this ABI.  I would drop
this paragraph.

Yeah, you are right.


+/*
+ * Request a SCSI operation specified via a CDB in vscsiif_request.cmnd.
+ * The target is specified via channel, id and lun.
+ * The operation to be performed is specified via a CDB in cmnd[], the length
+ * of the CDB is in cmd_len. sc_data_direction specifies the direction of data
+ * (to the device, from the device, or none at all).
+ * If data is to be transferred to or from the device the buffer(s) in the

Blank lines between paragraphs. please.

Okay.


+ * If "feature-sg-grant" in the Xenstore is set it is possible to specify more
+ * than VSCSIIF_SG_TABLESIZE scsiif_request_segment elements via indirection.
+ * The maximum number of allowed scsiif_request_segment elements is the value
+ * of the "feature-sg-grant" entry from Xenstore. When using indirection the
+ * seg[] array doesn't contain specifications of the data buffers, but
+ * references to scsiif_request_segment arrays, which in turn reference the
+ * data buffers. While nr_segments holds the number of populated seg[] entries
+ * (plus the set VSCSIIF_SG_GRANT bit), the number of scsiif_request_segment
+ * elements referencing the target data buffers is calculated from the lengths
+ * of the seg[] elements (the sum of all valid seg[].length divided by the
+ * size of one scsiif_request_segment structure).

Add a sentence such as "The frontend may use a mix of direct and
indirect requests."

A #define for the number of scsiif_request_segments per page might be
useful.

I don't mind adding it.


  /*
- * based on Linux kernel 2.6.18
+ * based on Linux kernel 2.6.18, still valid
+ * Changing these values requires support of multiple protocols via the rings
+ * as "old clients" will blindly use these values and the resulting structure
+ * sizes.

What does this comment about being "based on Linux kernel" mean?  Is it
useful?

It describes the origin of the "magic" numbers. I'd like to keep it.


With the minor typographical corrections made:

Reviewed-by: David Vrabel <david.vrabel@xxxxxxxxxx>

Thanks,

Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.