* [Qemu-devel] [PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci. @ 2013-06-10 9:53 fred.konrad 2013-06-10 11:58 ` Andreas Färber 2013-06-10 17:00 ` Michael S. Tsirkin 0 siblings, 2 replies; 8+ messages in thread From: fred.konrad @ 2013-06-10 9:53 UTC (permalink / raw) To: aliguori, qemu-devel, qemu-stable Cc: aik, pbonzini, mst, mark.burton, fred.konrad From: KONRAD Frederic <fred.konrad@greensocs.com> 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 <aik@ozlabs.ru> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> --- 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); } /* -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci. 2013-06-10 9:53 [Qemu-devel] [PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci fred.konrad @ 2013-06-10 11:58 ` Andreas Färber 2013-06-10 12:06 ` Frederic Konrad 2013-06-10 17:00 ` Michael S. Tsirkin 1 sibling, 1 reply; 8+ messages in thread From: Andreas Färber @ 2013-06-10 11:58 UTC (permalink / raw) To: fred.konrad Cc: aliguori, mst, aik, mark.burton, qemu-devel, qemu-stable, pbonzini Am 10.06.2013 11:53, schrieb fred.konrad@greensocs.com: > From: KONRAD Frederic <fred.konrad@greensocs.com> > > 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 <aik@ozlabs.ru> Missing Cc: qemu-stable@nongnu.org > Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> Otherwise, Reviewed-by: Andreas Färber <afaerber@suse.de> Can you add a simple qtest for this hot-add scenario? Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci. 2013-06-10 11:58 ` Andreas Färber @ 2013-06-10 12:06 ` Frederic Konrad 0 siblings, 0 replies; 8+ messages in thread From: Frederic Konrad @ 2013-06-10 12:06 UTC (permalink / raw) To: Andreas Färber Cc: aliguori, mst, aik, mark.burton, qemu-devel, qemu-stable, pbonzini On 10/06/2013 13:58, Andreas Färber wrote: > Am 10.06.2013 11:53, schrieb fred.konrad@greensocs.com: >> From: KONRAD Frederic <fred.konrad@greensocs.com> >> >> 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 <aik@ozlabs.ru> > Missing > > Cc: qemu-stable@nongnu.org > oops, I CC'ed it by hand. >> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> > Otherwise, > > Reviewed-by: Andreas Färber <afaerber@suse.de> > > Can you add a simple qtest for this hot-add scenario? Yes but I'm not familiar with this, can you point me at this qtest mechanism? Thanks, Fred > > Andreas > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci. 2013-06-10 9:53 [Qemu-devel] [PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci fred.konrad 2013-06-10 11:58 ` Andreas Färber @ 2013-06-10 17:00 ` Michael S. Tsirkin 2013-06-11 6:43 ` Frederic Konrad 1 sibling, 1 reply; 8+ messages in thread From: Michael S. Tsirkin @ 2013-06-10 17:00 UTC (permalink / raw) To: fred.konrad Cc: aliguori, aik, mark.burton, qemu-devel, qemu-stable, pbonzini, david On Mon, Jun 10, 2013 at 11:53:04AM +0200, fred.konrad@greensocs.com wrote: > From: KONRAD Frederic <fred.konrad@greensocs.com> > > 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 <aik@ozlabs.ru> > Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> > --- > 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? > -- > 1.8.1.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci. 2013-06-10 17:00 ` Michael S. Tsirkin @ 2013-06-11 6:43 ` Frederic Konrad 2013-06-11 7:21 ` Michael S. Tsirkin 0 siblings, 1 reply; 8+ messages in thread From: Frederic Konrad @ 2013-06-11 6:43 UTC (permalink / raw) To: Michael S. Tsirkin Cc: aliguori, aik, mark.burton, qemu-devel, qemu-stable, pbonzini, david 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 <fred.konrad@greensocs.com> >> >> 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 <aik@ozlabs.ru> >> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> >> --- >> 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 > >> -- >> 1.8.1.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci. 2013-06-11 6:43 ` Frederic Konrad @ 2013-06-11 7:21 ` Michael S. Tsirkin 2013-06-11 7:32 ` Frederic Konrad 0 siblings, 1 reply; 8+ messages in thread From: Michael S. Tsirkin @ 2013-06-11 7:21 UTC (permalink / raw) To: Frederic Konrad Cc: aliguori, aik, mark.burton, qemu-devel, qemu-stable, pbonzini, david 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 <fred.konrad@greensocs.com> > >> > >>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 <aik@ozlabs.ru> > >>Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> > >>--- > >> 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci. 2013-06-11 7:21 ` Michael S. Tsirkin @ 2013-06-11 7:32 ` Frederic Konrad 2013-06-11 7:49 ` Michael S. Tsirkin 0 siblings, 1 reply; 8+ messages in thread From: Frederic Konrad @ 2013-06-11 7:32 UTC (permalink / raw) To: Michael S. Tsirkin Cc: aliguori, aik, mark.burton, qemu-devel, qemu-stable, pbonzini, Andreas Färber, david On 11/06/2013 09:21, Michael S. Tsirkin wrote: > 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 <fred.konrad@greensocs.com> >>>> >>>> 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 <aik@ozlabs.ru> >>>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> >>>> --- >>>> 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. Ok, what do you propose to fix that properly? If we want to keep the behaviour of drive_add command, I think we have two solutions: A/ Change the scsi_hot_add to check if it is a virtio-scsi-pci device and take virtio-scsi-device's scsi bus. B/ Make somehow virtio-scsi-pci having a scsi-bus (and the same in virtio-scsi-device). I chose A because it seems a lot easier. For the moment B seems not possible, but would it be a cleaner solution? What do you think? Fred > >>>> -- >>>> 1.8.1.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci. 2013-06-11 7:32 ` Frederic Konrad @ 2013-06-11 7:49 ` Michael S. Tsirkin 0 siblings, 0 replies; 8+ messages in thread From: Michael S. Tsirkin @ 2013-06-11 7:49 UTC (permalink / raw) To: Frederic Konrad Cc: aliguori, aik, mark.burton, qemu-devel, qemu-stable, pbonzini, Andreas Färber, david On Tue, Jun 11, 2013 at 09:32:05AM +0200, Frederic Konrad wrote: > On 11/06/2013 09:21, Michael S. Tsirkin wrote: > >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 <fred.konrad@greensocs.com> > >>>> > >>>>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 <aik@ozlabs.ru> > >>>>Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com> > >>>>--- > >>>> 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. > > Ok, what do you propose to fix that properly? The problem is not your patch, it's that generally we don't appear have a supported way to add drives to devices. I think drive_hot_add should first of all stop calling pci_drive_hot_add unless pci_addr is specified. Teach it to get device id instead. Add a QMP interface, not just HMP. Rename the file from device-hotplug.c > > If we want to keep the behaviour of drive_add command, > I > think we have two solutions: > A/ Change the scsi_hot_add to check if it is a virtio-scsi-pci > device and take > virtio-scsi-device's scsi bus. > B/ Make somehow virtio-scsi-pci having a scsi-bus > (and the same in virtio-scsi-device). > > I chose A because it seems a lot easier. For the moment B seems not > possible, > but would it be a cleaner solution? What do you think? > > Fred It sounds cleaner though I'm not sure how to implement this. > > > >>>>-- > >>>>1.8.1.4 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-06-11 7:48 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-06-10 9:53 [Qemu-devel] [PATCH] virtio-scsi: forward scsibus for virtio-scsi-pci fred.konrad 2013-06-10 11:58 ` Andreas Färber 2013-06-10 12:06 ` Frederic Konrad 2013-06-10 17:00 ` Michael S. Tsirkin 2013-06-11 6:43 ` Frederic Konrad 2013-06-11 7:21 ` Michael S. Tsirkin 2013-06-11 7:32 ` Frederic Konrad 2013-06-11 7:49 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).