Re: [dpdk-dev] [PATCH] examples/vhost: introduce a new vhost-user-scsi sample application

[Yuanhan Liu <yliu@fridaylinux.org>]

On Fri, Jul 07, 2017 at 02:00:38AM +0000, Liu, Changpeng wrote:
> 
> 
> > -----Original Message-----
> > From: Yuanhan Liu [mailto:yliu@fridaylinux.org]
> > Sent: Friday, July 7, 2017 8:51 AM
> > To: Liu, Changpeng <changpeng.liu@intel.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [PATCH] examples/vhost: introduce a new vhost-user-scsi sample
> > application
> > 
> > On Thu, Jun 29, 2017 at 05:28:32PM +0800, Changpeng Liu wrote:
> > > +.. note::
> > > +    You must check whether you Qemu can support "vhost-user-scsi" or not,
> > > +    the latest Qemu code is recommended.
> > 
> > You should specify the least QEMU version that supports vhost-user scsi here.
> 
> Okay, will add Qemu version and  commit ID.

Just the QEMU version has is enough.

> > They are common pitfalls of vhost-user examples. You don't have to repeat
> > it again here. Instead, you could add a link to the vhost-example doc and
> > let this section only cover issues belong to vhost-user scsi.

Make sure you will handle this comment in the next version.

> > > +	resp_code = 0x70; /* Current + Fixed format */
> > 
> > Could you add macros for those magic numbers? I saw quite a lot of them.
> All the numbers are defined by SCSI specifications, I think this is okay, if you looked into kernel scsi module or
> kernel LIO scsi target, they also used the specification numbers directly.

Okay.

> > 
> > BTW, you might want to build it on a 32bit system. I think this patch
> > won't build successfully on it.
> I didn't build it with 32bit system before, I don't know the result.

Nah, the build should be fine. I found it passes the build test:
    http://dpdk.org/ml/archives/test-report/2017-June/023091.html

	--yliu