Re: [RFC-v2 0/4] tcm_vhost+cmwq fabric driver code for-3.6

On Tue, Jul 17, 2012 at 01:55:42PM -0500, Anthony Liguori wrote:

On 07/17/2012 10:05 AM, Michael S. Tsirkin wrote:

quoted

On Wed, Jul 11, 2012 at 09:15:00PM +0000, Nicholas A. Bellinger wrote:

quoted

From: Nicholas Bellinger<redacted>

Hi folks,

The following is a RFC-v2 series of tcm_vhost target fabric driver code
currently in-flight for-3.6 mainline code.

After last week's developments along with the help of some new folks, the
changelog v1 ->  v2 so far looks like:

*) Fix drivers/vhost/test.c to use VHOST_NET_FEATURES in patch #1 (Asias He)
*) Fix tv_cmd completion ->  release SGL memory leak (nab)
*) Fix sparse warnings for static variable usage (Fengguang Wu)
*) Fix sparse warnings for min() typing + printk format specs (Fengguang Wu)
*) Convert to cmwq submission for I/O dispatch (nab + hch)

Also following Paolo's request, a patch for hw/virtio-scsi.c that sets
scsi_host->max_target=0 that removes the need for virtio-scsi LLD to hardcode
VirtIOSCSIConfig->max_id=1 in order to function with tcm_vhost.

Note this series has been pushed into target-pending.git/for-next-merge, and
should be getting picked up for tomorrow's linux-next build.

Please let us know if you have any concerns and/or additional review feedback.

Thank you!

It still seems not 100% clear whether this driver will have major
userspace using it. And if not, it would be very hard to support a driver
when recent userspace does not use it in the end.

I don't think this is a good reason to exclude something from the
kernel. However, there are good reasons why this doesn't make sense
for something like QEMU--specifically because we have a large number
of features in our block layer that tcm_vhost would bypass.

But perhaps it makes sense for something like native kvm tool.  And
if it did go into the kernel, we would certainly support it in QEMU.

But I do think the kernel should carefully consider whether it wants
to support an interface like this.  This an extremely complicated
ABI with a lot of subtle details around state and compatibility.

Are you absolutely confident that you can support a userspace
application that expects to get exactly the same response from all
possible commands in 20 kernel versions from now?  Virtualization
requires absolutely precise compatibility in terms of bugs and
features.  This is probably not something the TCM stack has had to
consider yet.

quoted

I think a good idea for 3.6 would be to make it depend on CONFIG_STAGING.
Then we don't commit to an ABI.

I think this is a good idea.  Even if it goes in, a really clear
policy would be needed wrt the userspace ABI.

While tcm_vhost is probably more useful than vhost_blk, it's a much
more complex ABI to maintain.

Regards,

Anthony Liguori

Maybe something like a whitelist of features will help?
Might even be a good idea to make it user controllable.

quoted

For this, you can add a separate Kconfig and source it from drivers/staging/Kconfig.
Maybe it needs to be in a separate directory drivers/vhost/staging/Kconfig.

quoted

Nicholas Bellinger (2):
  vhost: Add vhost_scsi specific defines
  tcm_vhost: Initial merge for vhost level target fabric driver

Stefan Hajnoczi (2):
  vhost: Separate vhost-net features from vhost features
  vhost: make vhost work queue visible

 drivers/vhost/Kconfig     |    6 +
 drivers/vhost/Makefile    |    1 +
 drivers/vhost/net.c       |    4 +-
 drivers/vhost/tcm_vhost.c | 1609 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/vhost/tcm_vhost.h |   74 ++
 drivers/vhost/test.c      |    4 +-
 drivers/vhost/vhost.c     |    5 +-
 drivers/vhost/vhost.h     |    6 +-
 include/linux/vhost.h     |    9 +
 9 files changed, 1710 insertions(+), 8 deletions(-)
 create mode 100644 drivers/vhost/tcm_vhost.c
 create mode 100644 drivers/vhost/tcm_vhost.h

--
1.7.2.5