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

On Fri, Jul 07, 2017 at 02:00:38AM +0000, Liu, Changpeng wrote:

quoted

-----Original Message-----
From: Yuanhan Liu [mailto:yliu@fridaylinux.org]
Sent: Friday, July 7, 2017 8:51 AM
To: Liu, Changpeng <redacted>
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:

quoted

+.. 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.

quoted

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.

quoted

quoted

+	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.

quoted

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