All of lore.kernel.org
 help / color / mirror / Atom feed
* [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 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.