Re: [Qemu-devel] [PATCH 4/5] virtio-scsi: Add start/stop functionality for vhost-scsi

[Nicholas A. Bellinger]

On Tue, 2012-09-11 at 18:07 +0300, Michael S. Tsirkin wrote:
> On Tue, Sep 11, 2012 at 08:46:34AM -0500, Anthony Liguori wrote:
> > 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:

<SNIP>

> > >>>>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.
> > >>
> > >>Paolo
> > >
> > >Then 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=X
> > 
> > I 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
> 
> I'd like to clarify that I'm fine with either approach.
> Even a separate device is OK if this is what others want
> though I like it the least.
> 

Hi MST, Paolo & Co,

I've been out the better part of the week with the flu, and am just now
catching up on emails from the last days..

So to better understand the reasoning for adding an separate PCI device
for vhost-scsi ahead of implementing the code changes, here are main
points from folk's comments:

*) Convert vhost-scsi into a separate standalone vhost-scsi-pci device

  - Lets userspace know that virtio-scsi + QEMU block and virtio-scsi + 
    tcm_vhost do not track SCSI state (such as reservations + ALUA), and
    hence are not interchangeable during live-migration.
  
  - Reduces complexity of adding vhost-scsi related logic into existing
    virtio-scsi-pci code path.

  - Having backends with one possible option doesn’t make much sense.

*) Keep vhost-scsi as a backend to virtio-scsi-pci

  - Reduces duplicated code amongst multiple virtio-scsi backends.
  
  - Follows the split for what existing vhost-net code already does.

So that said, two quick questions for Paolo & Co..

For the standalone vhost-scsi-pci device case, can you give a brief idea
as to what extent you'd like to see virtio-scsi.c code/defs duplicated
and/or shared amongst a new vhost-scsi-pci device..?

Also to help me along, can you give an example based on the current
usage below how the QEMU command line arguments would change with a
standalone vhost-scsi-pci device..?

./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -smp 4 -m 2048 \
    -hda /usr/src/qemu-vhost.git/debian_squeeze_amd64_standard-old.qcow2 \
    -vhost-scsi id=vhost-scsi0,wwpn=naa.600140579ad21088,tpgt=1 \
    -device virtio-scsi-pci,vhost-scsi=vhost-scsi0,event_idx=off

Thank you!

--nab