|
From: | Anthony Liguori |
Subject: | Re: [Qemu-devel] [PATCH 4/5] virtio-scsi: Add start/stop functionality for vhost-scsi |
Date: | Tue, 11 Sep 2012 08:46:34 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1 |
On 09/10/2012 01:24 AM, Michael S. Tsirkin wrote:
On Mon, Sep 10, 2012 at 08:16:54AM +0200, Paolo Bonzini wrote:Il 09/09/2012 00:40, Michael S. Tsirkin ha scritto:On Fri, Sep 07, 2012 at 06:00:50PM +0200, Paolo Bonzini wrote:Il 07/09/2012 08:48, Nicholas A. Bellinger ha scritto:Cc: Stefan Hajnoczi<address@hidden> Cc: Zhi Yong Wu<address@hidden> Cc: Michael S. Tsirkin<address@hidden> Cc: Paolo Bonzini<address@hidden> Signed-off-by: Nicholas Bellinger<address@hidden> --- hw/virtio-pci.c | 2 ++ hw/virtio-scsi.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ hw/virtio-scsi.h | 1 + 3 files changed, 52 insertions(+), 0 deletions(-)Please create a completely separate device vhost-scsi-pci instead (or virtio-scsi-tcm-pci, or something like that). It is used completely differently from virtio-scsi-pci, it does not make sense to conflate the two.Ideally the name would say how it is different, not what backend it uses. Any good suggestions?I chose the backend name because, ideally, there would be no other difference. QEMU _could_ implement all the goodies in vhost-scsi (such as reservations or ALUA), it just doesn't do that yet. PaoloThen why do you say "It is used completely differently from virtio-scsi-pci"? Isn't it just a different backend? If yes then it should be a backend option, like it is for virtio-net.
I don't mean to bike shed here so don't take this as a nack on making it a backend option, but in retrospect, the way we did vhost-net was a mistake even though I strongly advocated for it to be a backend option.
The code to do it is really, really ugly. I think it would have made a lot more sense to just make it a device and then have it not use a netdev backend or any other kind of backend split.
For instance: qemu -device vhost-net-pci,tapfd=XI know this breaks the model of separate backends and frontends but since vhost-net absolutely requires a tap fd, I think it's better in the long run to not abuse the netdev backend to prevent user confusion. Having a dedicated backend type that only has one possible option and can only be used by one device is a bit silly too.
So I would be in favor of dropping/squashing 3/5 and radically simplifying how this was exposed to the user.
I would just take qemu_vhost_scsi_opts and make them device properties. Regards, Anthony Liguori
[Prev in Thread] | Current Thread | [Next in Thread] |