From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52154) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UmIsu-0005tu-N1 for qemu-devel@nongnu.org; Tue, 11 Jun 2013 03:20:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UmIso-0006Zb-42 for qemu-devel@nongnu.org; Tue, 11 Jun 2013 03:20:52 -0400 Date: Tue, 11 Jun 2013 10:21:12 +0300 From: "Michael S. Tsirkin" Message-ID: <20130611072112.GD31474@redhat.com> References: <1370857984-17584-1-git-send-email-fred.konrad@greensocs.com> <20130610170040.GA31383@redhat.com> <51B6C727.9050604@greensocs.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <51B6C727.9050604@greensocs.com> Subject: Re: [Qemu-devel] [PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Frederic Konrad Cc: aliguori@us.ibm.com, aik@ozlabs.ru, mark.burton@greensocs.com, qemu-devel@nongnu.org, qemu-stable@nongnu.org, pbonzini@redhat.com, david@gibson.dropbear.id.au On Tue, Jun 11, 2013 at 08:43:51AM +0200, Frederic Konrad wrote: > On 10/06/2013 19:00, Michael S. Tsirkin wrote: > >On Mon, Jun 10, 2013 at 11:53:04AM +0200, fred.konrad@greensocs.com wrote: > >>From: KONRAD Frederic > >> > >>This fix a bug with scsi hotplug on virtio-scsi-pci: > >> > >>As virtio-scsi-pci doesn't have any scsi bus, we need to forward scsi-hot-add > >>to the virtio-scsi-device plugged on the virtio-bus. > >> > >>Reported-by: Alexey Kardashevskiy > >>Signed-off-by: KONRAD Frederic > >>--- > >> hw/pci/pci-hotplug.c | 19 +++++++++++++++++-- > >> 1 file changed, 17 insertions(+), 2 deletions(-) > >> > >>diff --git a/hw/pci/pci-hotplug.c b/hw/pci/pci-hotplug.c > >>index 12287d1..c708752 100644 > >>--- a/hw/pci/pci-hotplug.c > >>+++ b/hw/pci/pci-hotplug.c > >>@@ -30,6 +30,8 @@ > >> #include "monitor/monitor.h" > >> #include "hw/scsi/scsi.h" > >> #include "hw/virtio/virtio-blk.h" > >>+#include "hw/virtio/virtio-scsi.h" > >>+#include "hw/virtio/virtio-pci.h" > >> #include "qemu/config-file.h" > >> #include "sysemu/blockdev.h" > >> #include "qapi/error.h" > >>@@ -79,13 +81,26 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter, > >> { > >> SCSIBus *scsibus; > >> SCSIDevice *scsidev; > >>+ VirtIOPCIProxy *virtio_proxy; > >> scsibus = (SCSIBus *) > >> object_dynamic_cast(OBJECT(QLIST_FIRST(&adapter->child_bus)), > >> TYPE_SCSI_BUS); > >> if (!scsibus) { > >>- error_report("Device is not a SCSI adapter"); > >>- return -1; > >>+ /* > >>+ * Check if the adapter is a virtio-scsi-pci, and forward scsi_hot_add > >>+ * to the virtio-scsi-device. > >>+ */ > >>+ if (!object_dynamic_cast(OBJECT(adapter), TYPE_VIRTIO_SCSI_PCI)) { > >>+ error_report("Device is not a SCSI adapter"); > >>+ return -1; > >>+ } > >>+ virtio_proxy = VIRTIO_PCI(adapter); > >>+ adapter = DEVICE(virtio_proxy->bus.vdev); > >>+ scsibus = (SCSIBus *) > >>+ object_dynamic_cast(OBJECT(QLIST_FIRST(&adapter->child_bus)), > >>+ TYPE_SCSI_BUS); > >>+ assert(scsibus); > >> } > >> /* > >By the way I really wonder. pci-hotplug.c was supposed to > >be legacy interface. > >Is this the only way to add scsi disks? > >And are other ways broken, too? > > Here you can add scsi disks to a given device. > I think the other ways add scsi disks to a given bus? > > Do you see any other? > > Thanks, > Fred I don't know, that's why I'm asking. If it is, that's crazy. There's no reason for it to be tied to pci at all. The commands in pci-hotplug.c are legacy - they don't support things like multi root systems and there's no sane way to make them do it. Your patch probably makes sense for stable, but for 1.6, let's fix this properly. > > > >>-- > >>1.8.1.4