On Fri, Jul 07, 2017 at 02:00:38AM +0000, Liu, Changpeng wrote:
>
>
> > -----Original Message-----
> > From: Yuanhan Liu [mailto:y...@fridaylinux.org]
> > Sent: Friday, July 7, 2017 8:51 AM
> > To: Liu, Changpeng <changpeng....@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
