From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH V4] Update pvSCSI protocol description Date: Wed, 27 Aug 2014 16:21:41 -0400 Message-ID: <20140827202141.GB10321@laptop.dumpdata.com> References: <1409026507-4909-1-git-send-email-jgross@suse.com> <53FC5C21.6090301@citrix.com> <53FC5E01.6080908@suse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <53FC5E01.6080908@suse.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: =?iso-8859-1?Q?J=FCrgen_Gro=DF?= Cc: keir@xen.org, ian.campbell@citrix.com, tim@xen.org, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, David Vrabel , jbeulich@suse.com List-Id: xen-devel@lists.xenproject.org On Tue, Aug 26, 2014 at 12:14:25PM +0200, J=FCrgen Gro=DF wrote: > 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 th= rough > >>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.cmn= d. > >>+ * 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 spec= ify more > >>+ * than VSCSIIF_SG_TABLESIZE scsiif_request_segment elements via indir= ection. > >>+ * The maximum number of allowed scsiif_request_segment elements is th= e value > >>+ * of the "feature-sg-grant" entry from Xenstore. When using indirecti= on the > >>+ * seg[] array doesn't contain specifications of the data buffers, but > >>+ * references to scsiif_request_segment arrays, which in turn referenc= e 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_s= egment > >>+ * 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 th= e rings > >>+ * as "old clients" will blindly use these values and the resulting st= ructure > >>+ * 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 > = > Thanks, Please tack on: Reviewed-by: Konrad Rzeszutek Wilk Thank you. > = > Juergen > = > = > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel