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

[Anthony Liguori <aliguori@us.ibm.com>]

On 07/17/2012 10:05 AM, Michael S. Tsirkin wrote:
> On Wed, Jul 11, 2012 at 09:15:00PM +0000, Nicholas A. Bellinger wrote:
>> From: Nicholas Bellinger<nab@linux-iscsi.org>
>>
>> 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.

> 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

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