All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Anthony Liguori <aliguori@us.ibm.com>
Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>,
	kvm-devel <kvm@vger.kernel.org>,
	Jan Kiszka <jan.kiszka@siemens.com>,
	qemu-devel <qemu-devel@nongnu.org>,
	lf-virt <virtualization@lists.linux-foundation.org>,
	Anthony Liguori <aliguori@linux.vnet.ibm.com>,
	target-devel <target-devel@vger.kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Zhi Yong Wu <wuzhy@cn.ibm.com>, Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH 4/5] virtio-scsi: Add start/stop functionality for vhost-scsi
Date: Tue, 11 Sep 2012 18:07:57 +0300	[thread overview]
Message-ID: <20120911150757.GA26666@redhat.com> (raw)
In-Reply-To: <504F40BA.7020800@us.ibm.com>

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:
> >>>>Il 07/09/2012 08:48, Nicholas A. Bellinger ha scritto:
> >>>>>Cc: Stefan Hajnoczi<stefanha@linux.vnet.ibm.com>
> >>>>>Cc: Zhi Yong Wu<wuzhy@linux.vnet.ibm.com>
> >>>>>Cc: Michael S. Tsirkin<mst@redhat.com>
> >>>>>Cc: Paolo Bonzini<pbonzini@redhat.com>
> >>>>>Signed-off-by: Nicholas Bellinger<nab@linux-iscsi.org>
> >>>>>---
> >>>>>  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.
> >>
> >>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.

> >

  reply	other threads:[~2012-09-11 15:07 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-07  6:48 [PATCH 0/5] vhost-scsi: Add support for host virtualized target Nicholas A. Bellinger
2012-09-07  6:48 ` [PATCH 1/5] monitor: Rename+move net_handle_fd_param -> monitor_handle_fd_param Nicholas A. Bellinger
2012-09-07  6:48 ` Nicholas A. Bellinger
2012-09-07  6:48 ` [PATCH 2/5] vhost: Pass device path to vhost_dev_init() Nicholas A. Bellinger
2012-09-07  6:48 ` Nicholas A. Bellinger
2012-09-07  6:48 ` [PATCH 3/5] vhost-scsi: add -vhost-scsi host device for use with tcm-vhost Nicholas A. Bellinger
2012-09-07  6:48 ` Nicholas A. Bellinger
2012-09-07  6:48 ` [PATCH 4/5] virtio-scsi: Add start/stop functionality for vhost-scsi Nicholas A. Bellinger
2012-09-07 16:00   ` Paolo Bonzini
2012-09-07 19:23     ` Nicholas A. Bellinger
2012-09-07 20:30       ` Paolo Bonzini
2012-09-07 20:30       ` Paolo Bonzini
2012-09-08 22:40     ` Michael S. Tsirkin
2012-09-10  6:16       ` Paolo Bonzini
2012-09-10  6:24         ` Michael S. Tsirkin
2012-09-10  6:42           ` Paolo Bonzini
2012-09-10  6:42           ` Paolo Bonzini
2012-09-10  6:49             ` Michael S. Tsirkin
2012-09-11 13:46           ` Anthony Liguori
2012-09-11 13:46           ` Anthony Liguori
2012-09-11 15:07             ` Michael S. Tsirkin [this message]
2012-09-13 22:27               ` Nicholas A. Bellinger
2012-09-14  6:45                 ` Paolo Bonzini
2012-09-14 12:34                   ` Michael S. Tsirkin
2012-09-13 22:27               ` Nicholas A. Bellinger
2012-09-14 12:50             ` Michael S. Tsirkin
2012-09-07  6:48 ` Nicholas A. Bellinger
2012-09-07  6:48 ` [PATCH 5/5] virtio-scsi: Set max_target=0 during vhost-scsi operation Nicholas A. Bellinger
2012-09-07  6:48 ` Nicholas A. Bellinger
2012-09-11  4:36 ` [PATCH 0/5] vhost-scsi: Add support for host virtualized target Asias He
2012-09-13 20:12   ` Nicholas A. Bellinger
2012-09-14  8:21     ` Paolo Bonzini
2012-09-13 20:12   ` Nicholas A. Bellinger
2013-01-17 16:43 ` Michael S. Tsirkin
2013-01-17 21:07   ` Nicholas A. Bellinger
2013-01-17 21:07   ` Nicholas A. Bellinger
2013-01-21  8:50     ` Michael S. Tsirkin
2013-01-28 13:01       ` Paolo Bonzini
2013-01-28 13:11         ` Michael S. Tsirkin
2013-01-28 13:29           ` Paolo Bonzini
2013-01-28 13:36             ` Michael S. Tsirkin
2013-01-28 13:33               ` Paolo Bonzini
2013-01-28 13:39                 ` Michael S. Tsirkin
2013-01-29 19:19                   ` Nicholas A. Bellinger
2013-01-29 19:19                   ` Nicholas A. Bellinger
2013-01-29 21:03                     ` Paolo Bonzini
2013-01-29 21:03                     ` Paolo Bonzini
2013-01-29 22:38                       ` Nicholas A. Bellinger
2013-01-29 22:38                       ` Nicholas A. Bellinger
2013-01-21 13:28     ` Anthony Liguori
2013-01-17 16:43 ` Michael S. Tsirkin
2013-04-02  3:28 ` [Qemu-devel] " Wenchao Xia
2013-04-02  5:07   ` Nicholas A. Bellinger
2013-04-02  5:07   ` Nicholas A. Bellinger
2013-04-02  3:28 ` Wenchao Xia

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120911150757.GA26666@redhat.com \
    --to=mst@redhat.com \
    --cc=aliguori@linux.vnet.ibm.com \
    --cc=aliguori@us.ibm.com \
    --cc=hch@lst.de \
    --cc=jan.kiszka@siemens.com \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@linux.vnet.ibm.com \
    --cc=target-devel@vger.kernel.org \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=wuzhy@cn.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.