[Qemu-devel] [PATCH for-1.5 0/4] virtio: fix bus command line compatibility.

[Qemu-devel] [PATCH for-1.5 0/4] virtio: fix bus command line compatibility.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

Cc: Libaiqing <libaiqing@huawei.com>
Cc: Michal Privoznik <mprivozn@redhat.com>

This series fix the bus name for virtio-serial-device and virtio-scsi-device.

The bus name for virtio-serial-device and virtio-scsi-device is not "id.0" as it
was the case before the refactoring.

This trigger the error:

qemu-system-xxx -device virtio-serial-pci,id=virtio-serial0
                -device virtserialport,bus=virtio-serial0.0

Bus 'virtio-serial0.0' is full
Bus 'virtio-serial0.0' not found

So this series set the name of those bus before the virtio-device's init.

KONRAD Frederic (4):
  virtio-x-bus: force bus name to virtio-bus.
  virtio-serial: fix command line compatibility.
  scsi: add scsi_named_bus_new().
  virtio-scsi: fix the command line compatibility.

 hw/char/virtio-serial-bus.c       | 16 +++++++++++++++-
 hw/s390x/s390-virtio-bus.c        | 22 +++++++++++++++++++++-
 hw/s390x/virtio-ccw.c             | 22 +++++++++++++++++++++-
 hw/scsi/scsi-bus.c                | 12 +++++++++---
 hw/scsi/virtio-scsi.c             | 14 +++++++++++++-
 hw/virtio/virtio-pci.c            | 23 ++++++++++++++++++++++-
 include/hw/scsi/scsi.h            |  2 ++
 include/hw/virtio/virtio-scsi.h   |  7 +++++++
 include/hw/virtio/virtio-serial.h |  8 ++++++++
 9 files changed, 118 insertions(+), 8 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH for-1.5 1/4] virtio-x-bus: force bus name to virtio-bus.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

When the proxy id is set, this bus take the name "id.0" which is expected
to be the virtio-device's first bus.

So force this name to "virtio-bus" as it is an internal bus.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/s390x/s390-virtio-bus.c | 5 ++++-
 hw/s390x/virtio-ccw.c      | 4 +++-
 hw/virtio/virtio-pci.c     | 5 ++++-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 2cff6b7..d52a547 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -669,7 +669,10 @@ static void virtio_s390_bus_new(VirtioBusState *bus, VirtIOS390Device *dev)
 {
     DeviceState *qdev = DEVICE(dev);
     BusState *qbus;
-    qbus_create_inplace((BusState *)bus, TYPE_VIRTIO_S390_BUS, qdev, NULL);
+    char virtio_bus_name[] = "virtio-bus";
+
+    qbus_create_inplace((BusState *)bus, TYPE_VIRTIO_S390_BUS, qdev,
+                        virtio_bus_name);
     qbus = BUS(bus);
     qbus->allow_hotplug = 1;
 }
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index c0f2646..ef99080 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1062,8 +1062,10 @@ static void virtio_ccw_bus_new(VirtioBusState *bus, VirtioCcwDevice *dev)
 {
     DeviceState *qdev = DEVICE(dev);
     BusState *qbus;
+    char virtio_bus_name[] = "virtio-bus";
 
-    qbus_create_inplace((BusState *)bus, TYPE_VIRTIO_CCW_BUS, qdev, NULL);
+    qbus_create_inplace((BusState *)bus, TYPE_VIRTIO_CCW_BUS, qdev,
+                        virtio_bus_name);
     qbus = BUS(bus);
     qbus->allow_hotplug = 1;
 }
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 41bb41e..070df44 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1474,7 +1474,10 @@ static void virtio_pci_bus_new(VirtioBusState *bus, VirtIOPCIProxy *dev)
 {
     DeviceState *qdev = DEVICE(dev);
     BusState *qbus;
-    qbus_create_inplace((BusState *)bus, TYPE_VIRTIO_PCI_BUS, qdev, NULL);
+    char virtio_bus_name[] = "virtio-bus";
+
+    qbus_create_inplace((BusState *)bus, TYPE_VIRTIO_PCI_BUS, qdev,
+                        virtio_bus_name);
     qbus = BUS(bus);
     qbus->allow_hotplug = 1;
 }
-- 
1.8.1.4

[Qemu-devel] [PATCH for-1.5 2/4] virtio-serial: fix command line compatibility.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

The bus name is wrong since the refactoring.

This keep the behaviour of the command line.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/char/virtio-serial-bus.c       | 16 +++++++++++++++-
 hw/s390x/s390-virtio-bus.c        |  8 ++++++++
 hw/s390x/virtio-ccw.c             |  9 +++++++++
 hw/virtio/virtio-pci.c            |  9 +++++++++
 include/hw/virtio/virtio-serial.h |  8 ++++++++
 5 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 3787ad2..0817041 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -889,6 +889,15 @@ static int virtser_port_qdev_exit(DeviceState *qdev)
     return 0;
 }
 
+void set_virtio_serial_bus_name(DeviceState *dev, const char *bus_name)
+{
+    VirtIOSerial *vser = VIRTIO_SERIAL(dev);
+    if (bus_name) {
+        vser->bus_name = g_malloc(strlen(bus_name) + 3);
+        snprintf(vser->bus_name, strlen(bus_name) + 3, "%s.0", bus_name);
+    }
+}
+
 static int virtio_serial_device_init(VirtIODevice *vdev)
 {
     DeviceState *qdev = DEVICE(vdev);
@@ -911,7 +920,8 @@ static int virtio_serial_device_init(VirtIODevice *vdev)
                 sizeof(struct virtio_console_config));
 
     /* Spawn a new virtio-serial bus on which the ports will ride as devices */
-    qbus_create_inplace(&vser->bus.qbus, TYPE_VIRTIO_SERIAL_BUS, qdev, NULL);
+    qbus_create_inplace(&vser->bus.qbus, TYPE_VIRTIO_SERIAL_BUS, qdev,
+                        vser->bus_name);
     vser->bus.qbus.allow_hotplug = 1;
     vser->bus.vser = vser;
     QTAILQ_INIT(&vser->ports);
@@ -992,6 +1002,10 @@ static int virtio_serial_device_exit(DeviceState *dev)
 
     unregister_savevm(dev, "virtio-console", vser);
 
+    if (vser->bus_name) {
+        g_free(vser->bus_name);
+    }
+
     g_free(vser->ivqs);
     g_free(vser->ovqs);
     g_free(vser->ports_map);
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index d52a547..6620d29 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -200,6 +200,14 @@ static int s390_virtio_serial_init(VirtIOS390Device *s390_dev)
 
     bus = DO_UPCAST(VirtIOS390Bus, bus, qdev->parent_bus);
 
+    /*
+     * For command line compatibility, this set the virtio-serial-device bus
+     * name as before.
+     */
+    if (qdev->id) {
+        set_virtio_serial_bus_name(vdev, qdev->id);
+    }
+
     qdev_set_parent_bus(vdev, BUS(&s390_dev->bus));
     if (qdev_init(vdev) < 0) {
         return -1;
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index ef99080..daa94d6 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -593,6 +593,15 @@ static int virtio_ccw_serial_init(VirtioCcwDevice *ccw_dev)
 {
     VirtioSerialCcw *dev = VIRTIO_SERIAL_CCW(ccw_dev);
     DeviceState *vdev = DEVICE(&dev->vdev);
+    DeviceState *proxy = DEVICE(ccw_dev);
+
+    /*
+     * For command line compatibility, this set the virtio-serial-device bus
+     * name as before.
+     */
+    if (proxy->id) {
+        set_virtio_serial_bus_name(vdev, proxy->id);
+    }
 
     qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
     if (qdev_init(vdev) < 0) {
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 070df44..598876f 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1297,6 +1297,7 @@ static int virtio_serial_pci_init(VirtIOPCIProxy *vpci_dev)
 {
     VirtIOSerialPCI *dev = VIRTIO_SERIAL_PCI(vpci_dev);
     DeviceState *vdev = DEVICE(&dev->vdev);
+    DeviceState *proxy = DEVICE(vpci_dev);
 
     if (vpci_dev->class_code != PCI_CLASS_COMMUNICATION_OTHER &&
         vpci_dev->class_code != PCI_CLASS_DISPLAY_OTHER && /* qemu 0.10 */
@@ -1310,6 +1311,14 @@ static int virtio_serial_pci_init(VirtIOPCIProxy *vpci_dev)
         vpci_dev->nvectors = dev->vdev.serial.max_virtserial_ports + 1;
     }
 
+    /*
+     * For command line compatibility, this set the virtio-serial-device bus
+     * name as before.
+     */
+    if (proxy->id) {
+        set_virtio_serial_bus_name(vdev, proxy->id);
+    }
+
     qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
     if (qdev_init(vdev) < 0) {
         return -1;
diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
index 1d2040b..786e76b 100644
--- a/include/hw/virtio/virtio-serial.h
+++ b/include/hw/virtio/virtio-serial.h
@@ -194,6 +194,9 @@ typedef struct VirtIOSerialPostLoad {
 struct VirtIOSerial {
     VirtIODevice parent_obj;
 
+    /* bus_name, for command line compatibility */
+    char *bus_name;
+
     VirtQueue *c_ivq, *c_ovq;
     /* Arrays of ivqs and ovqs: one per port */
     VirtQueue **ivqs, **ovqs;
@@ -244,6 +247,11 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port);
  */
 void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle);
 
+/*
+ * For command line back compatibility, set the bus name before initialisation.
+ */
+void set_virtio_serial_bus_name(DeviceState *dev, const char *bus_name);
+
 #define TYPE_VIRTIO_SERIAL "virtio-serial-device"
 #define VIRTIO_SERIAL(obj) \
         OBJECT_CHECK(VirtIOSerial, (obj), TYPE_VIRTIO_SERIAL)
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH for-1.5 2/4] virtio-serial: fix command line compatibility.

[Paolo Bonzini]

Il 29/04/2013 17:12, fred.konrad@greensocs.com ha scritto:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> The bus name is wrong since the refactoring.
> 
> This keep the behaviour of the command line.
> 
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> ---
>  hw/char/virtio-serial-bus.c       | 16 +++++++++++++++-
>  hw/s390x/s390-virtio-bus.c        |  8 ++++++++
>  hw/s390x/virtio-ccw.c             |  9 +++++++++
>  hw/virtio/virtio-pci.c            |  9 +++++++++
>  include/hw/virtio/virtio-serial.h |  8 ++++++++
>  5 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index 3787ad2..0817041 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -889,6 +889,15 @@ static int virtser_port_qdev_exit(DeviceState *qdev)
>      return 0;
>  }
>  
> +void set_virtio_serial_bus_name(DeviceState *dev, const char *bus_name)
> +{
> +    VirtIOSerial *vser = VIRTIO_SERIAL(dev);
> +    if (bus_name) {
> +        vser->bus_name = g_malloc(strlen(bus_name) + 3);
> +        snprintf(vser->bus_name, strlen(bus_name) + 3, "%s.0", bus_name);
> +    }
> +}

Same comment as for virtio-scsi.

Thanks,

Paolo

>  static int virtio_serial_device_init(VirtIODevice *vdev)
>  {
>      DeviceState *qdev = DEVICE(vdev);
> @@ -911,7 +920,8 @@ static int virtio_serial_device_init(VirtIODevice *vdev)
>                  sizeof(struct virtio_console_config));
>  
>      /* Spawn a new virtio-serial bus on which the ports will ride as devices */
> -    qbus_create_inplace(&vser->bus.qbus, TYPE_VIRTIO_SERIAL_BUS, qdev, NULL);
> +    qbus_create_inplace(&vser->bus.qbus, TYPE_VIRTIO_SERIAL_BUS, qdev,
> +                        vser->bus_name);
>      vser->bus.qbus.allow_hotplug = 1;
>      vser->bus.vser = vser;
>      QTAILQ_INIT(&vser->ports);
> @@ -992,6 +1002,10 @@ static int virtio_serial_device_exit(DeviceState *dev)
>  
>      unregister_savevm(dev, "virtio-console", vser);
>  
> +    if (vser->bus_name) {
> +        g_free(vser->bus_name);
> +    }
> +
>      g_free(vser->ivqs);
>      g_free(vser->ovqs);
>      g_free(vser->ports_map);
> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> index d52a547..6620d29 100644
> --- a/hw/s390x/s390-virtio-bus.c
> +++ b/hw/s390x/s390-virtio-bus.c
> @@ -200,6 +200,14 @@ static int s390_virtio_serial_init(VirtIOS390Device *s390_dev)
>  
>      bus = DO_UPCAST(VirtIOS390Bus, bus, qdev->parent_bus);
>  
> +    /*
> +     * For command line compatibility, this set the virtio-serial-device bus
> +     * name as before.
> +     */
> +    if (qdev->id) {
> +        set_virtio_serial_bus_name(vdev, qdev->id);
> +    }
> +
>      qdev_set_parent_bus(vdev, BUS(&s390_dev->bus));
>      if (qdev_init(vdev) < 0) {
>          return -1;
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index ef99080..daa94d6 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -593,6 +593,15 @@ static int virtio_ccw_serial_init(VirtioCcwDevice *ccw_dev)
>  {
>      VirtioSerialCcw *dev = VIRTIO_SERIAL_CCW(ccw_dev);
>      DeviceState *vdev = DEVICE(&dev->vdev);
> +    DeviceState *proxy = DEVICE(ccw_dev);
> +
> +    /*
> +     * For command line compatibility, this set the virtio-serial-device bus
> +     * name as before.
> +     */
> +    if (proxy->id) {
> +        set_virtio_serial_bus_name(vdev, proxy->id);
> +    }
>  
>      qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
>      if (qdev_init(vdev) < 0) {
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 070df44..598876f 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1297,6 +1297,7 @@ static int virtio_serial_pci_init(VirtIOPCIProxy *vpci_dev)
>  {
>      VirtIOSerialPCI *dev = VIRTIO_SERIAL_PCI(vpci_dev);
>      DeviceState *vdev = DEVICE(&dev->vdev);
> +    DeviceState *proxy = DEVICE(vpci_dev);
>  
>      if (vpci_dev->class_code != PCI_CLASS_COMMUNICATION_OTHER &&
>          vpci_dev->class_code != PCI_CLASS_DISPLAY_OTHER && /* qemu 0.10 */
> @@ -1310,6 +1311,14 @@ static int virtio_serial_pci_init(VirtIOPCIProxy *vpci_dev)
>          vpci_dev->nvectors = dev->vdev.serial.max_virtserial_ports + 1;
>      }
>  
> +    /*
> +     * For command line compatibility, this set the virtio-serial-device bus
> +     * name as before.
> +     */
> +    if (proxy->id) {
> +        set_virtio_serial_bus_name(vdev, proxy->id);
> +    }
> +
>      qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
>      if (qdev_init(vdev) < 0) {
>          return -1;
> diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
> index 1d2040b..786e76b 100644
> --- a/include/hw/virtio/virtio-serial.h
> +++ b/include/hw/virtio/virtio-serial.h
> @@ -194,6 +194,9 @@ typedef struct VirtIOSerialPostLoad {
>  struct VirtIOSerial {
>      VirtIODevice parent_obj;
>  
> +    /* bus_name, for command line compatibility */
> +    char *bus_name;
> +
>      VirtQueue *c_ivq, *c_ovq;
>      /* Arrays of ivqs and ovqs: one per port */
>      VirtQueue **ivqs, **ovqs;
> @@ -244,6 +247,11 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port);
>   */
>  void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle);
>  
> +/*
> + * For command line back compatibility, set the bus name before initialisation.
> + */
> +void set_virtio_serial_bus_name(DeviceState *dev, const char *bus_name);
> +
>  #define TYPE_VIRTIO_SERIAL "virtio-serial-device"
>  #define VIRTIO_SERIAL(obj) \
>          OBJECT_CHECK(VirtIOSerial, (obj), TYPE_VIRTIO_SERIAL)
> 

[Qemu-devel] [PATCH for-1.5 3/4] scsi: add scsi_named_bus_new().

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

This add the possibility to create a scsi-bus with a specified name.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/scsi/scsi-bus.c     | 12 +++++++++---
 include/hw/scsi/scsi.h |  2 ++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 6239ee1..0364749 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -71,15 +71,21 @@ static void scsi_device_unit_attention_reported(SCSIDevice *s)
     }
 }
 
-/* Create a scsi bus, and attach devices to it.  */
-void scsi_bus_new(SCSIBus *bus, DeviceState *host, const SCSIBusInfo *info)
+void scsi_named_bus_new(SCSIBus *bus, DeviceState *host,
+                        const SCSIBusInfo *info, const char *bus_name)
 {
-    qbus_create_inplace(&bus->qbus, TYPE_SCSI_BUS, host, NULL);
+    qbus_create_inplace(&bus->qbus, TYPE_SCSI_BUS, host, bus_name);
     bus->busnr = next_scsi_bus++;
     bus->info = info;
     bus->qbus.allow_hotplug = 1;
 }
 
+/* Create a scsi bus, and attach devices to it.  */
+void scsi_bus_new(SCSIBus *bus, DeviceState *host, const SCSIBusInfo *info)
+{
+    scsi_named_bus_new(bus, host, info, NULL);
+}
+
 static void scsi_dma_restart_bh(void *opaque)
 {
     SCSIDevice *s = opaque;
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 3bda1c4..ddcb07c 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -153,6 +153,8 @@ struct SCSIBus {
 };
 
 void scsi_bus_new(SCSIBus *bus, DeviceState *host, const SCSIBusInfo *info);
+void scsi_named_bus_new(SCSIBus *bus, DeviceState *host,
+                        const SCSIBusInfo *info, const char *bus_name);
 
 static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
 {
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH for-1.5 3/4] scsi: add scsi_named_bus_new().

[Paolo Bonzini]

Il 29/04/2013 17:12, fred.konrad@greensocs.com ha scritto:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> This add the possibility to create a scsi-bus with a specified name.
> 
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> ---
>  hw/scsi/scsi-bus.c     | 12 +++++++++---
>  include/hw/scsi/scsi.h |  2 ++
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
> index 6239ee1..0364749 100644
> --- a/hw/scsi/scsi-bus.c
> +++ b/hw/scsi/scsi-bus.c
> @@ -71,15 +71,21 @@ static void scsi_device_unit_attention_reported(SCSIDevice *s)
>      }
>  }
>  
> -/* Create a scsi bus, and attach devices to it.  */
> -void scsi_bus_new(SCSIBus *bus, DeviceState *host, const SCSIBusInfo *info)
> +void scsi_named_bus_new(SCSIBus *bus, DeviceState *host,
> +                        const SCSIBusInfo *info, const char *bus_name)
>  {
> -    qbus_create_inplace(&bus->qbus, TYPE_SCSI_BUS, host, NULL);
> +    qbus_create_inplace(&bus->qbus, TYPE_SCSI_BUS, host, bus_name);
>      bus->busnr = next_scsi_bus++;
>      bus->info = info;
>      bus->qbus.allow_hotplug = 1;
>  }

I'd prefer that you just add the argument to scsi_bus_new and change all
callers.

Paolo

> +/* Create a scsi bus, and attach devices to it.  */
> +void scsi_bus_new(SCSIBus *bus, DeviceState *host, const SCSIBusInfo *info)
> +{
> +    scsi_named_bus_new(bus, host, info, NULL);
> +}
> +
>  static void scsi_dma_restart_bh(void *opaque)
>  {
>      SCSIDevice *s = opaque;
> diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
> index 3bda1c4..ddcb07c 100644
> --- a/include/hw/scsi/scsi.h
> +++ b/include/hw/scsi/scsi.h
> @@ -153,6 +153,8 @@ struct SCSIBus {
>  };
>  
>  void scsi_bus_new(SCSIBus *bus, DeviceState *host, const SCSIBusInfo *info);
> +void scsi_named_bus_new(SCSIBus *bus, DeviceState *host,
> +                        const SCSIBusInfo *info, const char *bus_name);
>  
>  static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
>  {
> 

Re: [Qemu-devel] [PATCH for-1.5 3/4] scsi: add scsi_named_bus_new().

[KONRAD Frédéric]

On 29/04/2013 17:40, Paolo Bonzini wrote:
> Il 29/04/2013 17:12, fred.konrad@greensocs.com ha scritto:
>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>
>> This add the possibility to create a scsi-bus with a specified name.
>>
>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>> ---
>>   hw/scsi/scsi-bus.c     | 12 +++++++++---
>>   include/hw/scsi/scsi.h |  2 ++
>>   2 files changed, 11 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
>> index 6239ee1..0364749 100644
>> --- a/hw/scsi/scsi-bus.c
>> +++ b/hw/scsi/scsi-bus.c
>> @@ -71,15 +71,21 @@ static void scsi_device_unit_attention_reported(SCSIDevice *s)
>>       }
>>   }
>>   
>> -/* Create a scsi bus, and attach devices to it.  */
>> -void scsi_bus_new(SCSIBus *bus, DeviceState *host, const SCSIBusInfo *info)
>> +void scsi_named_bus_new(SCSIBus *bus, DeviceState *host,
>> +                        const SCSIBusInfo *info, const char *bus_name)
>>   {
>> -    qbus_create_inplace(&bus->qbus, TYPE_SCSI_BUS, host, NULL);
>> +    qbus_create_inplace(&bus->qbus, TYPE_SCSI_BUS, host, bus_name);
>>       bus->busnr = next_scsi_bus++;
>>       bus->info = info;
>>       bus->qbus.allow_hotplug = 1;
>>   }
> I'd prefer that you just add the argument to scsi_bus_new and change all
> callers.
>
> Paolo

Ok, will do.

>> +/* Create a scsi bus, and attach devices to it.  */
>> +void scsi_bus_new(SCSIBus *bus, DeviceState *host, const SCSIBusInfo *info)
>> +{
>> +    scsi_named_bus_new(bus, host, info, NULL);
>> +}
>> +
>>   static void scsi_dma_restart_bh(void *opaque)
>>   {
>>       SCSIDevice *s = opaque;
>> diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
>> index 3bda1c4..ddcb07c 100644
>> --- a/include/hw/scsi/scsi.h
>> +++ b/include/hw/scsi/scsi.h
>> @@ -153,6 +153,8 @@ struct SCSIBus {
>>   };
>>   
>>   void scsi_bus_new(SCSIBus *bus, DeviceState *host, const SCSIBusInfo *info);
>> +void scsi_named_bus_new(SCSIBus *bus, DeviceState *host,
>> +                        const SCSIBusInfo *info, const char *bus_name);
>>   
>>   static inline SCSIBus *scsi_bus_from_device(SCSIDevice *d)
>>   {
>>

[Qemu-devel] [PATCH for-1.5 4/4] virtio-scsi: fix the command line compatibility.

[fred.konrad]

From: KONRAD Frederic <fred.konrad@greensocs.com>

The bus name is wrong since the refactoring.

This keep the behaviour of the command line.

Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
---
 hw/s390x/s390-virtio-bus.c      |  9 +++++++++
 hw/s390x/virtio-ccw.c           |  9 +++++++++
 hw/scsi/virtio-scsi.c           | 14 +++++++++++++-
 hw/virtio/virtio-pci.c          |  9 +++++++++
 include/hw/virtio/virtio-scsi.h |  7 +++++++
 5 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 6620d29..e1fd937 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -232,6 +232,15 @@ static int s390_virtio_scsi_init(VirtIOS390Device *s390_dev)
 {
     VirtIOSCSIS390 *dev = VIRTIO_SCSI_S390(s390_dev);
     DeviceState *vdev = DEVICE(&dev->vdev);
+    DeviceState *qdev = DEVICE(s390_dev);
+
+    /*
+     * For command line compatibility, this set the virtio-scsi-device bus
+     * name as before.
+     */
+    if (qdev->id) {
+        set_virtio_scsi_bus_name(vdev, qdev->id);
+    }
 
     qdev_set_parent_bus(vdev, BUS(&s390_dev->bus));
     if (qdev_init(vdev) < 0) {
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index daa94d6..9d69df5 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -677,6 +677,15 @@ static int virtio_ccw_scsi_init(VirtioCcwDevice *ccw_dev)
 {
     VirtIOSCSICcw *dev = VIRTIO_SCSI_CCW(ccw_dev);
     DeviceState *vdev = DEVICE(&dev->vdev);
+    DeviceState *qdev = DEVICE(ccw_dev);
+
+    /*
+     * For command line compatibility, this set the virtio-scsi-device bus
+     * name as before.
+     */
+    if (qdev->id) {
+        set_virtio_scsi_bus_name(vdev, qdev->id);
+    }
 
     qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
     if (qdev_init(vdev) < 0) {
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 84b3ac7..fc27131 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -587,6 +587,15 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
     .load_request = virtio_scsi_load_request,
 };
 
+void set_virtio_scsi_bus_name(DeviceState *dev, const char *bus_name)
+{
+    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
+    if (bus_name) {
+        vs->bus_name = g_malloc(strlen(bus_name) + 3);
+        snprintf(vs->bus_name, strlen(bus_name) + 3, "%s.0", bus_name);
+    }
+}
+
 int virtio_scsi_common_init(VirtIOSCSICommon *s)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(s);
@@ -624,7 +633,7 @@ static int virtio_scsi_device_init(VirtIODevice *vdev)
         return ret;
     }
 
-    scsi_bus_new(&s->bus, qdev, &virtio_scsi_scsi_info);
+    scsi_named_bus_new(&s->bus, qdev, &virtio_scsi_scsi_info, vs->bus_name);
     if (!qdev->hotplugged) {
         scsi_bus_legacy_handle_cmdline(&s->bus);
     }
@@ -639,6 +648,9 @@ int virtio_scsi_common_exit(VirtIOSCSICommon *vs)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(vs);
 
+    if (vs->bus_name) {
+        g_free(vs->bus_name);
+    }
     g_free(vs->cmd_vqs);
     virtio_cleanup(vdev);
     return 0;
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 598876f..14fb8dd 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1106,11 +1106,20 @@ static int virtio_scsi_pci_init_pci(VirtIOPCIProxy *vpci_dev)
     VirtIOSCSIPCI *dev = VIRTIO_SCSI_PCI(vpci_dev);
     DeviceState *vdev = DEVICE(&dev->vdev);
     VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
+    DeviceState *proxy = DEVICE(vpci_dev);
 
     if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
         vpci_dev->nvectors = vs->conf.num_queues + 3;
     }
 
+    /*
+     * For command line compatibility, this set the virtio-scsi-device bus
+     * name as before.
+     */
+    if (proxy->id) {
+        set_virtio_scsi_bus_name(vdev, proxy->id);
+    }
+
     qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
     if (qdev_init(vdev) < 0) {
         return -1;
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 4db346b..c356d54 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -164,6 +164,9 @@ typedef struct VirtIOSCSICommon {
     VirtQueue *ctrl_vq;
     VirtQueue *event_vq;
     VirtQueue **cmd_vqs;
+
+    /* bus_name, for command line compatibility */
+    char *bus_name;
 } VirtIOSCSICommon;
 
 typedef struct {
@@ -189,5 +192,9 @@ typedef struct {
 int virtio_scsi_common_init(VirtIOSCSICommon *vs);
 int virtio_scsi_common_exit(VirtIOSCSICommon *vs);
 
+/*
+ * For command line back compatibility, set the bus name before initialisation.
+ */
+void set_virtio_scsi_bus_name(DeviceState *dev, const char *bus_name);
 
 #endif /* _QEMU_VIRTIO_SCSI_H */
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH for-1.5 4/4] virtio-scsi: fix the command line compatibility.

[Paolo Bonzini]

Il 29/04/2013 17:12, fred.konrad@greensocs.com ha scritto:
> From: KONRAD Frederic <fred.konrad@greensocs.com>
> 
> The bus name is wrong since the refactoring.
> 
> This keep the behaviour of the command line.
> 
> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
> ---
>  hw/s390x/s390-virtio-bus.c      |  9 +++++++++
>  hw/s390x/virtio-ccw.c           |  9 +++++++++
>  hw/scsi/virtio-scsi.c           | 14 +++++++++++++-
>  hw/virtio/virtio-pci.c          |  9 +++++++++
>  include/hw/virtio/virtio-scsi.h |  7 +++++++
>  5 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> index 6620d29..e1fd937 100644
> --- a/hw/s390x/s390-virtio-bus.c
> +++ b/hw/s390x/s390-virtio-bus.c
> @@ -232,6 +232,15 @@ static int s390_virtio_scsi_init(VirtIOS390Device *s390_dev)
>  {
>      VirtIOSCSIS390 *dev = VIRTIO_SCSI_S390(s390_dev);
>      DeviceState *vdev = DEVICE(&dev->vdev);
> +    DeviceState *qdev = DEVICE(s390_dev);
> +
> +    /*
> +     * For command line compatibility, this set the virtio-scsi-device bus
> +     * name as before.
> +     */
> +    if (qdev->id) {
> +        set_virtio_scsi_bus_name(vdev, qdev->id);
> +    }

Could this be simply a qdev property?

Consider that qdev will strdup any bus name you pass, so it is perfectly
ok to do:

   bus_name = g_strdup_printf("%s.0", vs->bus_name);
   scsi_bus_new(..., bus_name);
   g_free(bus_name);

> +void set_virtio_scsi_bus_name(DeviceState *dev, const char *bus_name)
> +{
> +    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
> +    if (bus_name) {
> +        vs->bus_name = g_malloc(strlen(bus_name) + 3);
> +        snprintf(vs->bus_name, strlen(bus_name) + 3, "%s.0", bus_name);

This would use g_strdup_printf, as above.

Paolo

> +    }
> +}
> +
>  int virtio_scsi_common_init(VirtIOSCSICommon *s)
>  {
>      VirtIODevice *vdev = VIRTIO_DEVICE(s);
> @@ -624,7 +633,7 @@ static int virtio_scsi_device_init(VirtIODevice *vdev)
>          return ret;
>      }
>  
> -    scsi_bus_new(&s->bus, qdev, &virtio_scsi_scsi_info);
> +    scsi_named_bus_new(&s->bus, qdev, &virtio_scsi_scsi_info, vs->bus_name);
>      if (!qdev->hotplugged) {
>          scsi_bus_legacy_handle_cmdline(&s->bus);
>      }
> @@ -639,6 +648,9 @@ int virtio_scsi_common_exit(VirtIOSCSICommon *vs)
>  {
>      VirtIODevice *vdev = VIRTIO_DEVICE(vs);
>  
> +    if (vs->bus_name) {
> +        g_free(vs->bus_name);
> +    }
>      g_free(vs->cmd_vqs);
>      virtio_cleanup(vdev);
>      return 0;
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 598876f..14fb8dd 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1106,11 +1106,20 @@ static int virtio_scsi_pci_init_pci(VirtIOPCIProxy *vpci_dev)
>      VirtIOSCSIPCI *dev = VIRTIO_SCSI_PCI(vpci_dev);
>      DeviceState *vdev = DEVICE(&dev->vdev);
>      VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
> +    DeviceState *proxy = DEVICE(vpci_dev);
>  
>      if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
>          vpci_dev->nvectors = vs->conf.num_queues + 3;
>      }
>  
> +    /*
> +     * For command line compatibility, this set the virtio-scsi-device bus
> +     * name as before.
> +     */
> +    if (proxy->id) {
> +        set_virtio_scsi_bus_name(vdev, proxy->id);
> +    }
> +
>      qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
>      if (qdev_init(vdev) < 0) {
>          return -1;
> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> index 4db346b..c356d54 100644
> --- a/include/hw/virtio/virtio-scsi.h
> +++ b/include/hw/virtio/virtio-scsi.h
> @@ -164,6 +164,9 @@ typedef struct VirtIOSCSICommon {
>      VirtQueue *ctrl_vq;
>      VirtQueue *event_vq;
>      VirtQueue **cmd_vqs;
> +
> +    /* bus_name, for command line compatibility */
> +    char *bus_name;
>  } VirtIOSCSICommon;
>  
>  typedef struct {
> @@ -189,5 +192,9 @@ typedef struct {
>  int virtio_scsi_common_init(VirtIOSCSICommon *vs);
>  int virtio_scsi_common_exit(VirtIOSCSICommon *vs);
>  
> +/*
> + * For command line back compatibility, set the bus name before initialisation.
> + */
> +void set_virtio_scsi_bus_name(DeviceState *dev, const char *bus_name);
>  
>  #endif /* _QEMU_VIRTIO_SCSI_H */
> 

Re: [Qemu-devel] [PATCH for-1.5 4/4] virtio-scsi: fix the command line compatibility.

[KONRAD Frédéric]

On 29/04/2013 17:44, Paolo Bonzini wrote:
> Il 29/04/2013 17:12, fred.konrad@greensocs.com ha scritto:
>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>
>> The bus name is wrong since the refactoring.
>>
>> This keep the behaviour of the command line.
>>
>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>> ---
>>   hw/s390x/s390-virtio-bus.c      |  9 +++++++++
>>   hw/s390x/virtio-ccw.c           |  9 +++++++++
>>   hw/scsi/virtio-scsi.c           | 14 +++++++++++++-
>>   hw/virtio/virtio-pci.c          |  9 +++++++++
>>   include/hw/virtio/virtio-scsi.h |  7 +++++++
>>   5 files changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
>> index 6620d29..e1fd937 100644
>> --- a/hw/s390x/s390-virtio-bus.c
>> +++ b/hw/s390x/s390-virtio-bus.c
>> @@ -232,6 +232,15 @@ static int s390_virtio_scsi_init(VirtIOS390Device *s390_dev)
>>   {
>>       VirtIOSCSIS390 *dev = VIRTIO_SCSI_S390(s390_dev);
>>       DeviceState *vdev = DEVICE(&dev->vdev);
>> +    DeviceState *qdev = DEVICE(s390_dev);
>> +
>> +    /*
>> +     * For command line compatibility, this set the virtio-scsi-device bus
>> +     * name as before.
>> +     */
>> +    if (qdev->id) {
>> +        set_virtio_scsi_bus_name(vdev, qdev->id);
>> +    }
> Could this be simply a qdev property?

Yes, that can be a good idea.

What about adding a qdev property bus_name and using it in qbus_realize?

Like this:

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 4eb0134..c5d5407 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -421,6 +421,13 @@ static void qbus_realize(BusState *bus, DeviceState 
*parent, const char *name)

      if (name) {
          bus->name = g_strdup(name);
+    } else if (bus->parent && bus->parent->bus_name) {
+        /* parent device has bus_name -> use it for bus name */
+        len = strlen(bus->parent->bus_name) + 16;
+        buf = g_malloc(len);
+        snprintf(buf, len, "%s.%d", bus->parent->bus_name,
+                 bus->parent->num_child_bus);
+        bus->name = buf;
      } else if (bus->parent && bus->parent->id) {
          /* parent device has id -> use it for bus name */
          len = strlen(bus->parent->id) + 16;

If so, change to scsi_bus_new is not needed and the two new functions are
not needed.

Is that making sense?

Fred
> Consider that qdev will strdup any bus name you pass, so it is perfectly
> ok to do:
>
>     bus_name = g_strdup_printf("%s.0", vs->bus_name);
>     scsi_bus_new(..., bus_name);
>     g_free(bus_name);
>
>> +void set_virtio_scsi_bus_name(DeviceState *dev, const char *bus_name)
>> +{
>> +    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
>> +    if (bus_name) {
>> +        vs->bus_name = g_malloc(strlen(bus_name) + 3);
>> +        snprintf(vs->bus_name, strlen(bus_name) + 3, "%s.0", bus_name);
> This would use g_strdup_printf, as above.
>
> Paolo
>
>> +    }
>> +}
>> +
>>   int virtio_scsi_common_init(VirtIOSCSICommon *s)
>>   {
>>       VirtIODevice *vdev = VIRTIO_DEVICE(s);
>> @@ -624,7 +633,7 @@ static int virtio_scsi_device_init(VirtIODevice *vdev)
>>           return ret;
>>       }
>>   
>> -    scsi_bus_new(&s->bus, qdev, &virtio_scsi_scsi_info);
>> +    scsi_named_bus_new(&s->bus, qdev, &virtio_scsi_scsi_info, vs->bus_name);
>>       if (!qdev->hotplugged) {
>>           scsi_bus_legacy_handle_cmdline(&s->bus);
>>       }
>> @@ -639,6 +648,9 @@ int virtio_scsi_common_exit(VirtIOSCSICommon *vs)
>>   {
>>       VirtIODevice *vdev = VIRTIO_DEVICE(vs);
>>   
>> +    if (vs->bus_name) {
>> +        g_free(vs->bus_name);
>> +    }
>>       g_free(vs->cmd_vqs);
>>       virtio_cleanup(vdev);
>>       return 0;
>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>> index 598876f..14fb8dd 100644
>> --- a/hw/virtio/virtio-pci.c
>> +++ b/hw/virtio/virtio-pci.c
>> @@ -1106,11 +1106,20 @@ static int virtio_scsi_pci_init_pci(VirtIOPCIProxy *vpci_dev)
>>       VirtIOSCSIPCI *dev = VIRTIO_SCSI_PCI(vpci_dev);
>>       DeviceState *vdev = DEVICE(&dev->vdev);
>>       VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
>> +    DeviceState *proxy = DEVICE(vpci_dev);
>>   
>>       if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
>>           vpci_dev->nvectors = vs->conf.num_queues + 3;
>>       }
>>   
>> +    /*
>> +     * For command line compatibility, this set the virtio-scsi-device bus
>> +     * name as before.
>> +     */
>> +    if (proxy->id) {
>> +        set_virtio_scsi_bus_name(vdev, proxy->id);
>> +    }
>> +
>>       qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
>>       if (qdev_init(vdev) < 0) {
>>           return -1;
>> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
>> index 4db346b..c356d54 100644
>> --- a/include/hw/virtio/virtio-scsi.h
>> +++ b/include/hw/virtio/virtio-scsi.h
>> @@ -164,6 +164,9 @@ typedef struct VirtIOSCSICommon {
>>       VirtQueue *ctrl_vq;
>>       VirtQueue *event_vq;
>>       VirtQueue **cmd_vqs;
>> +
>> +    /* bus_name, for command line compatibility */
>> +    char *bus_name;
>>   } VirtIOSCSICommon;
>>   
>>   typedef struct {
>> @@ -189,5 +192,9 @@ typedef struct {
>>   int virtio_scsi_common_init(VirtIOSCSICommon *vs);
>>   int virtio_scsi_common_exit(VirtIOSCSICommon *vs);
>>   
>> +/*
>> + * For command line back compatibility, set the bus name before initialisation.
>> + */
>> +void set_virtio_scsi_bus_name(DeviceState *dev, const char *bus_name);
>>   
>>   #endif /* _QEMU_VIRTIO_SCSI_H */
>>

Re: [Qemu-devel] [PATCH for-1.5 4/4] virtio-scsi: fix the command line compatibility.

[Paolo Bonzini]

Il 29/04/2013 18:28, KONRAD Frédéric ha scritto:
>>>
>> Could this be simply a qdev property?
> 
> Yes, that can be a good idea.
> 
> What about adding a qdev property bus_name and using it in qbus_realize?
> 
> Like this:
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 4eb0134..c5d5407 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -421,6 +421,13 @@ static void qbus_realize(BusState *bus, DeviceState
> *parent, const char *name)
> 
>      if (name) {
>          bus->name = g_strdup(name);
> +    } else if (bus->parent && bus->parent->bus_name) {
> +        /* parent device has bus_name -> use it for bus name */
> +        len = strlen(bus->parent->bus_name) + 16;
> +        buf = g_malloc(len);
> +        snprintf(buf, len, "%s.%d", bus->parent->bus_name,
> +                 bus->parent->num_child_bus);
> +        bus->name = buf;
>      } else if (bus->parent && bus->parent->id) {
>          /* parent device has id -> use it for bus name */
>          len = strlen(bus->parent->id) + 16;
> 
> If so, change to scsi_bus_new is not needed and the two new functions are
> not needed.
> 
> Is that making sense?

Ah, that's a bit more extreme. :)

I think I like it, but I need more input.

Paolo

Re: [Qemu-devel] [PATCH for-1.5 4/4] virtio-scsi: fix the command line compatibility.

[KONRAD Frédéric]

On 29/04/2013 18:32, Paolo Bonzini wrote:
> Il 29/04/2013 18:28, KONRAD Frédéric ha scritto:
>>> Could this be simply a qdev property?
>> Yes, that can be a good idea.
>>
>> What about adding a qdev property bus_name and using it in qbus_realize?
>>
>> Like this:
>>
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index 4eb0134..c5d5407 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -421,6 +421,13 @@ static void qbus_realize(BusState *bus, DeviceState
>> *parent, const char *name)
>>
>>       if (name) {
>>           bus->name = g_strdup(name);
>> +    } else if (bus->parent && bus->parent->bus_name) {
>> +        /* parent device has bus_name -> use it for bus name */
>> +        len = strlen(bus->parent->bus_name) + 16;
>> +        buf = g_malloc(len);
>> +        snprintf(buf, len, "%s.%d", bus->parent->bus_name,
>> +                 bus->parent->num_child_bus);
>> +        bus->name = buf;
>>       } else if (bus->parent && bus->parent->id) {
>>           /* parent device has id -> use it for bus name */
>>           len = strlen(bus->parent->id) + 16;
>>
>> If so, change to scsi_bus_new is not needed and the two new functions are
>> not needed.
>>
>> Is that making sense?
> Ah, that's a bit more extreme. :)
>
> I think I like it, but I need more input.
>
> Paolo
Well, just for virtio-scsi-pci, something like that:

---
  hw/core/qdev.c         | 14 ++++++++++++++
  hw/virtio/virtio-pci.c |  9 +++++++++
  include/hw/qdev-core.h |  4 ++++
  3 files changed, 27 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 4eb0134..3aa0082 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -342,6 +342,13 @@ BusState *qdev_get_child_bus(DeviceState *dev, 
const char *name)
      return NULL;
  }

+void qdev_set_bus_name(DeviceState *dev, const char *bus_name)
+{
+    if (bus_name) {
+        dev->bus_name = g_strdup(bus_name);
+    }
+}
+
  int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn,
                         qbus_walkerfn *busfn, void *opaque)
  {
@@ -421,6 +428,13 @@ static void qbus_realize(BusState *bus, DeviceState 
*parent, const char *name)

      if (name) {
          bus->name = g_strdup(name);
+    } else if (bus->parent && bus->parent->bus_name) {
+        /* parent device has bus_name -> use it for bus name */
+        len = strlen(bus->parent->bus_name) + 16;
+        buf = g_malloc(len);
+        snprintf(buf, len, "%s.%d", bus->parent->bus_name,
+                 bus->parent->num_child_bus);
+        bus->name = buf;
      } else if (bus->parent && bus->parent->id) {
          /* parent device has id -> use it for bus name */
          len = strlen(bus->parent->id) + 16;
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 070df44..8d5d146 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1106,11 +1106,20 @@ static int 
virtio_scsi_pci_init_pci(VirtIOPCIProxy *vpci_dev)
      VirtIOSCSIPCI *dev = VIRTIO_SCSI_PCI(vpci_dev);
      DeviceState *vdev = DEVICE(&dev->vdev);
      VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
+    DeviceState *proxy = DEVICE(dev);

      if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
          vpci_dev->nvectors = vs->conf.num_queues + 3;
      }

+    /*
+     * For command line compatibility, this set the virtio-scsi-device bus
+     * name as before.
+     */
+    if (proxy->id) {
+        qdev_set_bus_name(vdev, proxy->id);
+    }
+
      qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
      if (qdev_init(vdev) < 0) {
          return -1;
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index cf83d54..332e11f 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -125,6 +125,7 @@ struct DeviceState {
      int num_child_bus;
      int instance_id_alias;
      int alias_required_for_version;
+    const char *bus_name;
  };

  #define TYPE_BUS "bus"
@@ -224,6 +225,9 @@ BusState *qdev_get_child_bus(DeviceState *dev, const 
char *name);
  void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n);
  void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);

+/* Set the bus_name property. */
+void qdev_set_bus_name(DeviceState *dev, const char *bus_name);
+
  BusState *qdev_get_parent_bus(DeviceState *dev);

  /*** BUS API. ***/
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH for-1.5 4/4] virtio-scsi: fix the command line compatibility.

[Andreas Färber]

Am 29.04.2013 19:39, schrieb KONRAD Frédéric:
> On 29/04/2013 18:32, Paolo Bonzini wrote:
>> Il 29/04/2013 18:28, KONRAD Frédéric ha scritto:
>>>> Could this be simply a qdev property?
>>> Yes, that can be a good idea.
>>>
>>> What about adding a qdev property bus_name and using it in qbus_realize?
>>>
>>> Like this:
>>>
>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>> index 4eb0134..c5d5407 100644
>>> --- a/hw/core/qdev.c
>>> +++ b/hw/core/qdev.c
>>> @@ -421,6 +421,13 @@ static void qbus_realize(BusState *bus, DeviceState
>>> *parent, const char *name)
>>>
>>>       if (name) {
>>>           bus->name = g_strdup(name);
>>> +    } else if (bus->parent && bus->parent->bus_name) {
>>> +        /* parent device has bus_name -> use it for bus name */
>>> +        len = strlen(bus->parent->bus_name) + 16;
>>> +        buf = g_malloc(len);
>>> +        snprintf(buf, len, "%s.%d", bus->parent->bus_name,
>>> +                 bus->parent->num_child_bus);
>>> +        bus->name = buf;
>>>       } else if (bus->parent && bus->parent->id) {
>>>           /* parent device has id -> use it for bus name */
>>>           len = strlen(bus->parent->id) + 16;
>>>
>>> If so, change to scsi_bus_new is not needed and the two new functions
>>> are
>>> not needed.
>>>
>>> Is that making sense?
>> Ah, that's a bit more extreme. :)
>>
>> I think I like it, but I need more input.
>>
>> Paolo
> Well, just for virtio-scsi-pci, something like that:
> 
> ---
>  hw/core/qdev.c         | 14 ++++++++++++++
>  hw/virtio/virtio-pci.c |  9 +++++++++
>  include/hw/qdev-core.h |  4 ++++
>  3 files changed, 27 insertions(+)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 4eb0134..3aa0082 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -342,6 +342,13 @@ BusState *qdev_get_child_bus(DeviceState *dev,
> const char *name)
>      return NULL;
>  }
> 
> +void qdev_set_bus_name(DeviceState *dev, const char *bus_name)

device_... for new functions please. :)

> +{


Might be called multiple times, so better insert:

if (dev->bus_name) {
    g_free(dev->bus_name);
    dev->bus_name = NULL;
}

> +    if (bus_name) {
> +        dev->bus_name = g_strdup(bus_name);
> +    }
> +}
> +
>  int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn,
>                         qbus_walkerfn *busfn, void *opaque)
>  {
> @@ -421,6 +428,13 @@ static void qbus_realize(BusState *bus, DeviceState
> *parent, const char *name)
> 
>      if (name) {
>          bus->name = g_strdup(name);
> +    } else if (bus->parent && bus->parent->bus_name) {
> +        /* parent device has bus_name -> use it for bus name */

This seems backwards to me. qdev had made sure to have a 1:n
relationship between device and bus, whereas you are making the name
template 1:1 here. Don't you rather want a qbus_set_name() mechanism
operating on the bus?

> +        len = strlen(bus->parent->bus_name) + 16;

Why 15 decimal digits? Is there any constant we could reuse?

> +        buf = g_malloc(len);
> +        snprintf(buf, len, "%s.%d", bus->parent->bus_name,
> +                 bus->parent->num_child_bus);
> +        bus->name = buf;
>      } else if (bus->parent && bus->parent->id) {
>          /* parent device has id -> use it for bus name */
>          len = strlen(bus->parent->id) + 16;
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 070df44..8d5d146 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1106,11 +1106,20 @@ static int
> virtio_scsi_pci_init_pci(VirtIOPCIProxy *vpci_dev)
>      VirtIOSCSIPCI *dev = VIRTIO_SCSI_PCI(vpci_dev);
>      DeviceState *vdev = DEVICE(&dev->vdev);
>      VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
> +    DeviceState *proxy = DEVICE(dev);
> 
>      if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
>          vpci_dev->nvectors = vs->conf.num_queues + 3;
>      }
> 
> +    /*
> +     * For command line compatibility, this set the virtio-scsi-device bus

"sets"

> +     * name as before.
> +     */
> +    if (proxy->id) {
> +        qdev_set_bus_name(vdev, proxy->id);
> +    }
> +
>      qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
>      if (qdev_init(vdev) < 0) {
>          return -1;
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index cf83d54..332e11f 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -125,6 +125,7 @@ struct DeviceState {
>      int num_child_bus;
>      int instance_id_alias;
>      int alias_required_for_version;
> +    const char *bus_name;
>  };
> 
>  #define TYPE_BUS "bus"
> @@ -224,6 +225,9 @@ BusState *qdev_get_child_bus(DeviceState *dev, const
> char *name);
>  void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n);
>  void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
> 
> +/* Set the bus_name property. */
> +void qdev_set_bus_name(DeviceState *dev, const char *bus_name);
> +
>  BusState *qdev_get_parent_bus(DeviceState *dev);
> 
>  /*** BUS API. ***/

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH for-1.5 4/4] virtio-scsi: fix the command line compatibility.

[KONRAD Frédéric]

On 29/04/2013 19:55, Andreas Färber wrote:
> Am 29.04.2013 19:39, schrieb KONRAD Frédéric:
>> On 29/04/2013 18:32, Paolo Bonzini wrote:
>>> Il 29/04/2013 18:28, KONRAD Frédéric ha scritto:
>>>>> Could this be simply a qdev property?
>>>> Yes, that can be a good idea.
>>>>
>>>> What about adding a qdev property bus_name and using it in qbus_realize?
>>>>
>>>> Like this:
>>>>
>>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>>> index 4eb0134..c5d5407 100644
>>>> --- a/hw/core/qdev.c
>>>> +++ b/hw/core/qdev.c
>>>> @@ -421,6 +421,13 @@ static void qbus_realize(BusState *bus, DeviceState
>>>> *parent, const char *name)
>>>>
>>>>        if (name) {
>>>>            bus->name = g_strdup(name);
>>>> +    } else if (bus->parent && bus->parent->bus_name) {
>>>> +        /* parent device has bus_name -> use it for bus name */
>>>> +        len = strlen(bus->parent->bus_name) + 16;
>>>> +        buf = g_malloc(len);
>>>> +        snprintf(buf, len, "%s.%d", bus->parent->bus_name,
>>>> +                 bus->parent->num_child_bus);
>>>> +        bus->name = buf;
>>>>        } else if (bus->parent && bus->parent->id) {
>>>>            /* parent device has id -> use it for bus name */
>>>>            len = strlen(bus->parent->id) + 16;
>>>>
>>>> If so, change to scsi_bus_new is not needed and the two new functions
>>>> are
>>>> not needed.
>>>>
>>>> Is that making sense?
>>> Ah, that's a bit more extreme. :)
>>>
>>> I think I like it, but I need more input.
>>>
>>> Paolo
>> Well, just for virtio-scsi-pci, something like that:
>>
>> ---
>>   hw/core/qdev.c         | 14 ++++++++++++++
>>   hw/virtio/virtio-pci.c |  9 +++++++++
>>   include/hw/qdev-core.h |  4 ++++
>>   3 files changed, 27 insertions(+)
>>
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index 4eb0134..3aa0082 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -342,6 +342,13 @@ BusState *qdev_get_child_bus(DeviceState *dev,
>> const char *name)
>>       return NULL;
>>   }
>>
>> +void qdev_set_bus_name(DeviceState *dev, const char *bus_name)
> device_... for new functions please. :)
>
>> +{
>
> Might be called multiple times, so better insert:
>
> if (dev->bus_name) {
>      g_free(dev->bus_name);
>      dev->bus_name = NULL;
> }
>
>> +    if (bus_name) {
>> +        dev->bus_name = g_strdup(bus_name);
>> +    }
>> +}
>> +
>>   int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn,
>>                          qbus_walkerfn *busfn, void *opaque)
>>   {
>> @@ -421,6 +428,13 @@ static void qbus_realize(BusState *bus, DeviceState
>> *parent, const char *name)
>>
>>       if (name) {
>>           bus->name = g_strdup(name);
>> +    } else if (bus->parent && bus->parent->bus_name) {
>> +        /* parent device has bus_name -> use it for bus name */
> This seems backwards to me. qdev had made sure to have a 1:n
> relationship between device and bus, whereas you are making the name
> template 1:1 here. Don't you rather want a qbus_set_name() mechanism
> operating on the bus?

What do you mean with the relationship 1:n / 1:1 I don't understand?

Note: I did that quickly to ask Paolo if it was worth doing like that or 
like the original
          series.
>
>> +        len = strlen(bus->parent->bus_name) + 16;
> Why 15 decimal digits? Is there any constant we could reuse?
>
>> +        buf = g_malloc(len);
>> +        snprintf(buf, len, "%s.%d", bus->parent->bus_name,
>> +                 bus->parent->num_child_bus);
>> +        bus->name = buf;
>>       } else if (bus->parent && bus->parent->id) {
>>           /* parent device has id -> use it for bus name */
>>           len = strlen(bus->parent->id) + 16;
>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>> index 070df44..8d5d146 100644
>> --- a/hw/virtio/virtio-pci.c
>> +++ b/hw/virtio/virtio-pci.c
>> @@ -1106,11 +1106,20 @@ static int
>> virtio_scsi_pci_init_pci(VirtIOPCIProxy *vpci_dev)
>>       VirtIOSCSIPCI *dev = VIRTIO_SCSI_PCI(vpci_dev);
>>       DeviceState *vdev = DEVICE(&dev->vdev);
>>       VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
>> +    DeviceState *proxy = DEVICE(dev);
>>
>>       if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
>>           vpci_dev->nvectors = vs->conf.num_queues + 3;
>>       }
>>
>> +    /*
>> +     * For command line compatibility, this set the virtio-scsi-device bus
> "sets"
>
>> +     * name as before.
>> +     */
>> +    if (proxy->id) {
>> +        qdev_set_bus_name(vdev, proxy->id);
>> +    }
>> +
>>       qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
>>       if (qdev_init(vdev) < 0) {
>>           return -1;
>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>> index cf83d54..332e11f 100644
>> --- a/include/hw/qdev-core.h
>> +++ b/include/hw/qdev-core.h
>> @@ -125,6 +125,7 @@ struct DeviceState {
>>       int num_child_bus;
>>       int instance_id_alias;
>>       int alias_required_for_version;
>> +    const char *bus_name;
>>   };
>>
>>   #define TYPE_BUS "bus"
>> @@ -224,6 +225,9 @@ BusState *qdev_get_child_bus(DeviceState *dev, const
>> char *name);
>>   void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n);
>>   void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
>>
>> +/* Set the bus_name property. */
>> +void qdev_set_bus_name(DeviceState *dev, const char *bus_name);
>> +
>>   BusState *qdev_get_parent_bus(DeviceState *dev);
>>
>>   /*** BUS API. ***/
> Andreas
>

Re: [Qemu-devel] [PATCH for-1.5 4/4] virtio-scsi: fix the command line compatibility.

[KONRAD Frédéric]

On 29/04/2013 17:44, Paolo Bonzini wrote:
> Il 29/04/2013 17:12, fred.konrad@greensocs.com ha scritto:
>> From: KONRAD Frederic <fred.konrad@greensocs.com>
>>
>> The bus name is wrong since the refactoring.
>>
>> This keep the behaviour of the command line.
>>
>> Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
>> ---
>>   hw/s390x/s390-virtio-bus.c      |  9 +++++++++
>>   hw/s390x/virtio-ccw.c           |  9 +++++++++
>>   hw/scsi/virtio-scsi.c           | 14 +++++++++++++-
>>   hw/virtio/virtio-pci.c          |  9 +++++++++
>>   include/hw/virtio/virtio-scsi.h |  7 +++++++
>>   5 files changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
>> index 6620d29..e1fd937 100644
>> --- a/hw/s390x/s390-virtio-bus.c
>> +++ b/hw/s390x/s390-virtio-bus.c
>> @@ -232,6 +232,15 @@ static int s390_virtio_scsi_init(VirtIOS390Device *s390_dev)
>>   {
>>       VirtIOSCSIS390 *dev = VIRTIO_SCSI_S390(s390_dev);
>>       DeviceState *vdev = DEVICE(&dev->vdev);
>> +    DeviceState *qdev = DEVICE(s390_dev);
>> +
>> +    /*
>> +     * For command line compatibility, this set the virtio-scsi-device bus
>> +     * name as before.
>> +     */
>> +    if (qdev->id) {
>> +        set_virtio_scsi_bus_name(vdev, qdev->id);
>> +    }
> Could this be simply a qdev property?

Well, maybe my solution is too extreme.

Maybe I can put bus_name in struct VirtIODevice instead to avoid the
set_virtio_x_bus_name code duplication?
>
> Consider that qdev will strdup any bus name you pass, so it is perfectly
> ok to do:
>
>     bus_name = g_strdup_printf("%s.0", vs->bus_name);
>     scsi_bus_new(..., bus_name);
>     g_free(bus_name);
>
>> +void set_virtio_scsi_bus_name(DeviceState *dev, const char *bus_name)
>> +{
>> +    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
>> +    if (bus_name) {
>> +        vs->bus_name = g_malloc(strlen(bus_name) + 3);
>> +        snprintf(vs->bus_name, strlen(bus_name) + 3, "%s.0", bus_name);
> This would use g_strdup_printf, as above.
>
> Paolo
>
>> +    }
>> +}
>> +
>>   int virtio_scsi_common_init(VirtIOSCSICommon *s)
>>   {
>>       VirtIODevice *vdev = VIRTIO_DEVICE(s);
>> @@ -624,7 +633,7 @@ static int virtio_scsi_device_init(VirtIODevice *vdev)
>>           return ret;
>>       }
>>   
>> -    scsi_bus_new(&s->bus, qdev, &virtio_scsi_scsi_info);
>> +    scsi_named_bus_new(&s->bus, qdev, &virtio_scsi_scsi_info, vs->bus_name);
>>       if (!qdev->hotplugged) {
>>           scsi_bus_legacy_handle_cmdline(&s->bus);
>>       }
>> @@ -639,6 +648,9 @@ int virtio_scsi_common_exit(VirtIOSCSICommon *vs)
>>   {
>>       VirtIODevice *vdev = VIRTIO_DEVICE(vs);
>>   
>> +    if (vs->bus_name) {
>> +        g_free(vs->bus_name);
>> +    }
>>       g_free(vs->cmd_vqs);
>>       virtio_cleanup(vdev);
>>       return 0;
>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>> index 598876f..14fb8dd 100644
>> --- a/hw/virtio/virtio-pci.c
>> +++ b/hw/virtio/virtio-pci.c
>> @@ -1106,11 +1106,20 @@ static int virtio_scsi_pci_init_pci(VirtIOPCIProxy *vpci_dev)
>>       VirtIOSCSIPCI *dev = VIRTIO_SCSI_PCI(vpci_dev);
>>       DeviceState *vdev = DEVICE(&dev->vdev);
>>       VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
>> +    DeviceState *proxy = DEVICE(vpci_dev);
>>   
>>       if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
>>           vpci_dev->nvectors = vs->conf.num_queues + 3;
>>       }
>>   
>> +    /*
>> +     * For command line compatibility, this set the virtio-scsi-device bus
>> +     * name as before.
>> +     */
>> +    if (proxy->id) {
>> +        set_virtio_scsi_bus_name(vdev, proxy->id);
>> +    }
>> +
>>       qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
>>       if (qdev_init(vdev) < 0) {
>>           return -1;
>> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
>> index 4db346b..c356d54 100644
>> --- a/include/hw/virtio/virtio-scsi.h
>> +++ b/include/hw/virtio/virtio-scsi.h
>> @@ -164,6 +164,9 @@ typedef struct VirtIOSCSICommon {
>>       VirtQueue *ctrl_vq;
>>       VirtQueue *event_vq;
>>       VirtQueue **cmd_vqs;
>> +
>> +    /* bus_name, for command line compatibility */
>> +    char *bus_name;
>>   } VirtIOSCSICommon;
>>   
>>   typedef struct {
>> @@ -189,5 +192,9 @@ typedef struct {
>>   int virtio_scsi_common_init(VirtIOSCSICommon *vs);
>>   int virtio_scsi_common_exit(VirtIOSCSICommon *vs);
>>   
>> +/*
>> + * For command line back compatibility, set the bus name before initialisation.
>> + */
>> +void set_virtio_scsi_bus_name(DeviceState *dev, const char *bus_name);
>>   
>>   #endif /* _QEMU_VIRTIO_SCSI_H */
>>

Re: [Qemu-devel] [PATCH for-1.5 4/4] virtio-scsi: fix the command line compatibility.

[Paolo Bonzini]

Il 30/04/2013 15:06, KONRAD Frédéric ha scritto:
>>>
>> Could this be simply a qdev property?
> 
> Well, maybe my solution is too extreme.
> 
> Maybe I can put bus_name in struct VirtIODevice instead to avoid the
> set_virtio_x_bus_name code duplication?

No, I think it should be either DeviceState or single devices.

Paolo

Re: [Qemu-devel] [PATCH for-1.5 4/4] virtio-scsi: fix the command line compatibility.

[KONRAD Frédéric]

On 30/04/2013 16:01, Paolo Bonzini wrote:
> Il 30/04/2013 15:06, KONRAD Frédéric ha scritto:
>>> Could this be simply a qdev property?
>> Well, maybe my solution is too extreme.
>>
>> Maybe I can put bus_name in struct VirtIODevice instead to avoid the
>> set_virtio_x_bus_name code duplication?
> No, I think it should be either DeviceState or single devices.
>
> Paolo
>
oops, just sent the fix with bus_name in VirtIODevice..

I think it's not necessary to put bus_name in DeviceState as it is only 
needed for these two devices.

But it may be better for it to be in VirtIODevice no?

Re: [Qemu-devel] [PATCH for-1.5 4/4] virtio-scsi: fix the command line compatibility.

[Paolo Bonzini]

Il 30/04/2013 16:15, KONRAD Frédéric ha scritto:
> On 30/04/2013 16:01, Paolo Bonzini wrote:
>> Il 30/04/2013 15:06, KONRAD Frédéric ha scritto:
>>>> Could this be simply a qdev property?
>>> Well, maybe my solution is too extreme.
>>>
>>> Maybe I can put bus_name in struct VirtIODevice instead to avoid the
>>> set_virtio_x_bus_name code duplication?
>> No, I think it should be either DeviceState or single devices.
>>
>> Paolo
>>
> oops, just sent the fix with bus_name in VirtIODevice..
> 
> I think it's not necessary to put bus_name in DeviceState as it is only
> needed for these two devices.
> 
> But it may be better for it to be in VirtIODevice no?

Yeah, in the end it doesn't look bad... I should let other people who
followed the refactoring more closely decide.

Paolo