Patchwork [11/14] qbus: move get_fw_dev_path to DeviceClass

login
register
mail settings
Submitter Anthony Liguori
Date May 1, 2012, 6:18 p.m.
Message ID <1335896294-9530-12-git-send-email-aliguori@us.ibm.com>
Download mbox | patch
Permalink /patch/156183/
State New
Headers show

Comments

Anthony Liguori - May 1, 2012, 6:18 p.m.
It should have never been a bus method.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/ide/qdev.c |   33 +++++++++++++--------------------
 hw/isa-bus.c  |   31 +++++++++++++++----------------
 hw/pci.c      |   31 +++++++++++++++----------------
 hw/qdev.c     |   10 +++++-----
 hw/qdev.h     |    2 +-
 hw/scsi-bus.c |   33 ++++++++++++---------------------
 hw/sysbus.c   |   39 +++++++++++++++++++--------------------
 hw/usb/bus.c  |   55 +++++++++++++++++++++++++++----------------------------
 8 files changed, 107 insertions(+), 127 deletions(-)
Andreas Färber - May 1, 2012, 7:34 p.m.
Am 01.05.2012 20:18, schrieb Anthony Liguori:
> It should have never been a bus method.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
[...]
> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> index 4a468f8..5044018 100644
> --- a/hw/ide/qdev.c
> +++ b/hw/ide/qdev.c
> @@ -25,22 +25,13 @@
>  
>  /* --------------------------------- */
>  
> -static char *idebus_get_fw_dev_path(DeviceState *dev);
> -
>  #define TYPE_IDE_BUS "IDE"
> -
> -static void ide_bus_class_init(ObjectClass *klass, void *data)
> -{
> -    BusClass *k = BUS_CLASS(klass);
> -
> -    k->get_fw_dev_path = idebus_get_fw_dev_path;
> -}
> +#define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)

Move macro to preceding patch?

Otherwise looks good.

/-F
Anthony Liguori - May 1, 2012, 10:24 p.m.
On 05/01/2012 02:34 PM, Andreas Färber wrote:
> Am 01.05.2012 20:18, schrieb Anthony Liguori:
>> It should have never been a bus method.
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>> ---
> [...]
>> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
>> index 4a468f8..5044018 100644
>> --- a/hw/ide/qdev.c
>> +++ b/hw/ide/qdev.c
>> @@ -25,22 +25,13 @@
>>
>>   /* --------------------------------- */
>>
>> -static char *idebus_get_fw_dev_path(DeviceState *dev);
>> -
>>   #define TYPE_IDE_BUS "IDE"
>> -
>> -static void ide_bus_class_init(ObjectClass *klass, void *data)
>> -{
>> -    BusClass *k = BUS_CLASS(klass);
>> -
>> -    k->get_fw_dev_path = idebus_get_fw_dev_path;
>> -}
>> +#define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
>
> Move macro to preceding patch?
>
> Otherwise looks good.

Do you mean an independent patch?

Regards,

Anthony Liguori

>
> /-F
>
Andreas Färber - May 1, 2012, 10:36 p.m.
Am 02.05.2012 00:24, schrieb Anthony Liguori:
> On 05/01/2012 02:34 PM, Andreas Färber wrote:
>> Am 01.05.2012 20:18, schrieb Anthony Liguori:
>>> +#define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
>>
>> Move macro to preceding patch?
> 
> Do you mean an independent patch?

Sorry, I reviewed the patches in mail reception order. ;)

I meant 08/14 (qdev: convert busses to QEMU object model) where macros
for other bus types were introduced. Seemed like an oversight.

Andreas
Paolo Bonzini - May 2, 2012, 7:22 a.m.
Il 02/05/2012 00:36, Andreas Färber ha scritto:
> Sorry, I reviewed the patches in mail reception order. ;)
> 
> I meant 08/14 (qdev: convert busses to QEMU object model) where macros
> for other bus types were introduced. Seemed like an oversight.

A lot of these were missing, and furthermore they should be in a header
file.

Paolo

Patch

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 4a468f8..5044018 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -25,22 +25,13 @@ 
 
 /* --------------------------------- */
 
-static char *idebus_get_fw_dev_path(DeviceState *dev);
-
 #define TYPE_IDE_BUS "IDE"
-
-static void ide_bus_class_init(ObjectClass *klass, void *data)
-{
-    BusClass *k = BUS_CLASS(klass);
-
-    k->get_fw_dev_path = idebus_get_fw_dev_path;
-}
+#define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
 
 static TypeInfo ide_bus_info = {
     .name = TYPE_IDE_BUS,
     .parent = TYPE_BUS,
     .instance_size = sizeof(IDEBus),
-    .class_init = ide_bus_class_init,
 };
 
 void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id)
@@ -49,16 +40,6 @@  void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id)
     idebus->bus_id = bus_id;
 }
 
-static char *idebus_get_fw_dev_path(DeviceState *dev)
-{
-    char path[30];
-
-    snprintf(path, sizeof(path), "%s@%d", qdev_fw_name(dev),
-             ((IDEBus*)dev->parent_bus)->bus_id);
-
-    return strdup(path);
-}
-
 static int ide_qdev_init(DeviceState *qdev)
 {
     IDEDevice *dev = IDE_DEVICE(qdev);
@@ -250,11 +231,23 @@  static TypeInfo ide_drive_info = {
     .class_init    = ide_drive_class_init,
 };
 
+static char *ide_device_get_fw_dev_path(DeviceState *dev)
+{
+    IDEBus *parent_bus = IDE_BUS(dev->parent_bus);
+    char path[30];
+
+    snprintf(path, sizeof(path), "%s@%d", qdev_fw_name(dev),
+             parent_bus->bus_id);
+
+    return g_strdup(path);
+}
+
 static void ide_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = ide_qdev_init;
     k->bus_type = TYPE_IDE_BUS;
+    k->get_fw_dev_path = ide_device_get_fw_dev_path;
 }
 
 static Property ide_bus_properties[] = {
diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 2616f22..6141515 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -26,7 +26,6 @@  static ISABus *isabus;
 target_phys_addr_t isa_mem_base = 0;
 
 static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent);
-static char *isabus_get_fw_dev_path(DeviceState *dev);
 
 #define TYPE_ISA_BUS "ISA"
 
@@ -35,7 +34,6 @@  static void isa_bus_class_init(ObjectClass *klass, void *data)
     BusClass *k = BUS_CLASS(klass);
 
     k->print_dev = isabus_dev_print;
-    k->get_fw_dev_path = isabus_get_fw_dev_path;
 }
 
 static TypeInfo isa_bus_info = {
@@ -204,11 +202,26 @@  static TypeInfo isabus_bridge_info = {
     .class_init    = isabus_bridge_class_init,
 };
 
+static char *isa_device_get_fw_dev_path(DeviceState *dev)
+{
+    ISADevice *d = (ISADevice*)dev;
+    char path[40];
+    int off;
+
+    off = snprintf(path, sizeof(path), "%s", qdev_fw_name(dev));
+    if (d->ioport_id) {
+        snprintf(path + off, sizeof(path) - off, "@%04x", d->ioport_id);
+    }
+
+    return strdup(path);
+}
+
 static void isa_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = isa_qdev_init;
     k->bus_type = TYPE_ISA_BUS;
+    k->get_fw_dev_path = isa_device_get_fw_dev_path;
 }
 
 static TypeInfo isa_device_type_info = {
@@ -227,20 +240,6 @@  static void isabus_register_types(void)
     type_register_static(&isa_device_type_info);
 }
 
-static char *isabus_get_fw_dev_path(DeviceState *dev)
-{
-    ISADevice *d = (ISADevice*)dev;
-    char path[40];
-    int off;
-
-    off = snprintf(path, sizeof(path), "%s", qdev_fw_name(dev));
-    if (d->ioport_id) {
-        snprintf(path + off, sizeof(path) - off, "@%04x", d->ioport_id);
-    }
-
-    return strdup(path);
-}
-
 MemoryRegion *isa_address_space(ISADevice *dev)
 {
     return get_system_memory();
diff --git a/hw/pci.c b/hw/pci.c
index 291181e..425ceaa 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -40,7 +40,6 @@ 
 #endif
 
 static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
-static char *pcibus_get_fw_dev_path(DeviceState *dev);
 static int pcibus_reset(BusState *qbus);
 
 static void pci_bus_class_init(ObjectClass *klass, void *data)
@@ -48,7 +47,6 @@  static void pci_bus_class_init(ObjectClass *klass, void *data)
     BusClass *k = BUS_CLASS(klass);
 
     k->print_dev = pcibus_dev_print;
-    k->get_fw_dev_path = pcibus_get_fw_dev_path;
     k->reset = pcibus_reset;
 }
 
@@ -1883,20 +1881,6 @@  static char *pci_dev_fw_name(DeviceState *dev, char *buf, int len)
     return buf;
 }
 
-static char *pcibus_get_fw_dev_path(DeviceState *dev)
-{
-    PCIDevice *d = (PCIDevice *)dev;
-    char path[50], name[33];
-    int off;
-
-    off = snprintf(path, sizeof(path), "%s@%x",
-                   pci_dev_fw_name(dev, name, sizeof name),
-                   PCI_SLOT(d->devfn));
-    if (PCI_FUNC(d->devfn))
-        snprintf(path + off, sizeof(path) + off, ",%x", PCI_FUNC(d->devfn));
-    return strdup(path);
-}
-
 static int pci_qdev_find_recursive(PCIBus *bus,
                                    const char *id, PCIDevice **pdev)
 {
@@ -1991,6 +1975,20 @@  MemoryRegion *pci_address_space_io(PCIDevice *dev)
     return dev->bus->address_space_io;
 }
 
+static char *pci_qdev_get_fw_dev_path(DeviceState *dev)
+{
+    PCIDevice *d = PCI_DEVICE(dev);
+    char path[50], name[33];
+    int off;
+
+    off = snprintf(path, sizeof(path), "%s@%x",
+                   pci_dev_fw_name(dev, name, sizeof name),
+                   PCI_SLOT(d->devfn));
+    if (PCI_FUNC(d->devfn))
+        snprintf(path + off, sizeof(path) + off, ",%x", PCI_FUNC(d->devfn));
+    return strdup(path);
+}
+
 static void pci_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
@@ -1999,6 +1997,7 @@  static void pci_device_class_init(ObjectClass *klass, void *data)
     k->exit = pci_unregister_device;
     k->bus_type = TYPE_PCI_BUS;
     k->get_dev_path = pci_qdev_get_dev_path;
+    k->get_fw_dev_path = pci_qdev_get_fw_dev_path;
 }
 
 static Property pci_bus_properties[] = {
diff --git a/hw/qdev.c b/hw/qdev.c
index eaa3e12..bee0cd6 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -483,12 +483,12 @@  void qbus_free(BusState *bus)
     }
 }
 
-static char *bus_get_fw_dev_path(BusState *bus, DeviceState *dev)
+static char *do_qdev_get_fw_dev_path(DeviceState *dev)
 {
-    BusClass *bc = BUS_GET_CLASS(bus);
+    DeviceClass *dc = DEVICE_GET_CLASS(dev);
 
-    if (bc->get_fw_dev_path) {
-        return bc->get_fw_dev_path(dev);
+    if (dc->get_fw_dev_path) {
+        return dc->get_fw_dev_path(dev);
     }
 
     return NULL;
@@ -501,7 +501,7 @@  static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
     if (dev && dev->parent_bus) {
         char *d;
         l = qdev_get_fw_dev_path_helper(dev->parent_bus->parent, p, size);
-        d = bus_get_fw_dev_path(dev->parent_bus, dev);
+        d = do_qdev_get_fw_dev_path(dev);
         if (d) {
             l += snprintf(p + l, size - l, "%s", d);
             g_free(d);
diff --git a/hw/qdev.h b/hw/qdev.h
index 30bfbef..b09a07b 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -48,6 +48,7 @@  typedef struct DeviceClass {
     /* callbacks */
     void (*reset)(DeviceState *dev);
     char *(*get_dev_path)(DeviceState *dev);
+    char *(*get_fw_dev_path)(DeviceState *dev);
 
     /* device state */
     const VMStateDescription *vmsd;
@@ -96,7 +97,6 @@  struct BusClass {
 
     /* FIXME first arg should be BusState */
     void (*print_dev)(Monitor *mon, DeviceState *dev, int indent);
-    char *(*get_fw_dev_path)(DeviceState *dev);
     int (*reset)(BusState *bus);
 };
 
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 38189f3..98bb217 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -7,23 +7,13 @@ 
 #include "trace.h"
 #include "dma.h"
 
-static char *scsibus_get_dev_path(DeviceState *dev);
-static char *scsibus_get_fw_dev_path(DeviceState *dev);
 static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf);
 static void scsi_req_dequeue(SCSIRequest *req);
 
-static void scsi_bus_class_init(ObjectClass *klass, void *data)
-{
-    BusClass *k = BUS_CLASS(klass);
-
-    k->get_fw_dev_path = scsibus_get_fw_dev_path;
-}
-
 static TypeInfo scsi_bus_info = {
     .name = TYPE_SCSI_BUS,
     .parent = TYPE_BUS,
     .instance_size = sizeof(SCSIBus),
-    .class_init = scsi_bus_class_init,
 };
 static int next_scsi_bus;
 
@@ -1446,17 +1436,6 @@  static char *scsi_qdev_get_dev_path(DeviceState *dev)
     return path;
 }
 
-static char *scsibus_get_fw_dev_path(DeviceState *dev)
-{
-    SCSIDevice *d = SCSI_DEVICE(dev);
-    char path[100];
-
-    snprintf(path, sizeof(path), "channel@%x/%s@%x,%x", d->channel,
-             qdev_fw_name(dev), d->id, d->lun);
-
-    return strdup(path);
-}
-
 SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
 {
     DeviceState *qdev;
@@ -1571,6 +1550,17 @@  const VMStateDescription vmstate_scsi_device = {
     }
 };
 
+static char *scsi_qdev_get_fw_dev_path(DeviceState *dev)
+{
+    SCSIDevice *d = SCSI_DEVICE(dev);
+    char path[100];
+
+    snprintf(path, sizeof(path), "channel@%x/%s@%x,%x", d->channel,
+             qdev_fw_name(dev), d->id, d->lun);
+
+    return strdup(path);
+}
+
 static void scsi_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
@@ -1579,6 +1569,7 @@  static void scsi_device_class_init(ObjectClass *klass, void *data)
     k->unplug   = qdev_simple_unplug_cb;
     k->exit     = scsi_qdev_exit;
     k->get_dev_path = scsi_qdev_get_dev_path;
+    k->get_fw_dev_path = scsi_qdev_get_fw_dev_path;
 }
 
 static Property scsi_bus_properties[] = {
diff --git a/hw/sysbus.c b/hw/sysbus.c
index bad1bdc..92b86ba 100644
--- a/hw/sysbus.c
+++ b/hw/sysbus.c
@@ -22,14 +22,12 @@ 
 #include "exec-memory.h"
 
 static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent);
-static char *sysbus_get_fw_dev_path(DeviceState *dev);
 
 static void system_bus_class_init(ObjectClass *klass, void *data)
 {
     BusClass *k = BUS_CLASS(klass);
 
     k->print_dev = sysbus_dev_print;
-    k->get_fw_dev_path = sysbus_get_fw_dev_path;
 }
 
 static TypeInfo system_bus_info = {
@@ -196,24 +194,6 @@  static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent)
     }
 }
 
-static char *sysbus_get_fw_dev_path(DeviceState *dev)
-{
-    SysBusDevice *s = sysbus_from_qdev(dev);
-    char path[40];
-    int off;
-
-    off = snprintf(path, sizeof(path), "%s", qdev_fw_name(dev));
-
-    if (s->num_mmio) {
-        snprintf(path + off, sizeof(path) - off, "@"TARGET_FMT_plx,
-                 s->mmio[0].addr);
-    } else if (s->num_pio) {
-        snprintf(path + off, sizeof(path) - off, "@i%04x", s->pio[0]);
-    }
-
-    return strdup(path);
-}
-
 void sysbus_add_memory(SysBusDevice *dev, target_phys_addr_t addr,
                        MemoryRegion *mem)
 {
@@ -248,11 +228,30 @@  MemoryRegion *sysbus_address_space(SysBusDevice *dev)
     return get_system_memory();
 }
 
+static char *sysbus_get_fw_dev_path(DeviceState *dev)
+{
+    SysBusDevice *s = sysbus_from_qdev(dev);
+    char path[40];
+    int off;
+
+    off = snprintf(path, sizeof(path), "%s", qdev_fw_name(dev));
+
+    if (s->num_mmio) {
+        snprintf(path + off, sizeof(path) - off, "@"TARGET_FMT_plx,
+                 s->mmio[0].addr);
+    } else if (s->num_pio) {
+        snprintf(path + off, sizeof(path) - off, "@i%04x", s->pio[0]);
+    }
+
+    return strdup(path);
+}
+
 static void sysbus_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
     k->init = sysbus_device_init;
     k->bus_type = TYPE_SYSTEM_BUS;
+    k->get_fw_dev_path = sysbus_get_fw_dev_path;
 }
 
 static TypeInfo sysbus_device_type_info = {
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 9b57d1c..da39282 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -7,7 +7,6 @@ 
 
 static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent);
 
-static char *usb_get_fw_dev_path(DeviceState *qdev);
 static int usb_qdev_exit(DeviceState *qdev);
 
 #define TYPE_USB_BUS "usb-bus"
@@ -17,7 +16,6 @@  static void usb_bus_class_init(ObjectClass *klass, void *data)
     BusClass *k = BUS_CLASS(klass);
 
     k->print_dev = usb_bus_dev_print;
-    k->get_fw_dev_path = usb_get_fw_dev_path;
 }
 
 static TypeInfo usb_bus_info = {
@@ -462,32 +460,6 @@  static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent)
                    dev->attached ? ", attached" : "");
 }
 
-static char *usb_get_fw_dev_path(DeviceState *qdev)
-{
-    USBDevice *dev = USB_DEVICE(qdev);
-    char *fw_path, *in;
-    ssize_t pos = 0, fw_len;
-    long nr;
-
-    fw_len = 32 + strlen(dev->port->path) * 6;
-    fw_path = g_malloc(fw_len);
-    in = dev->port->path;
-    while (fw_len - pos > 0) {
-        nr = strtol(in, &in, 10);
-        if (in[0] == '.') {
-            /* some hub between root port and device */
-            pos += snprintf(fw_path + pos, fw_len - pos, "hub@%ld/", nr);
-            in++;
-        } else {
-            /* the device itself */
-            pos += snprintf(fw_path + pos, fw_len - pos, "%s@%ld",
-                            qdev_fw_name(qdev), nr);
-            break;
-        }
-    }
-    return fw_path;
-}
-
 void usb_info(Monitor *mon)
 {
     USBBus *bus;
@@ -575,6 +547,32 @@  static char *usb_qdev_get_dev_path(DeviceState *qdev)
     }
 }
 
+static char *usb_qdev_get_fw_dev_path(DeviceState *qdev)
+{
+    USBDevice *dev = USB_DEVICE(qdev);
+    char *fw_path, *in;
+    ssize_t pos = 0, fw_len;
+    long nr;
+
+    fw_len = 32 + strlen(dev->port->path) * 6;
+    fw_path = g_malloc(fw_len);
+    in = dev->port->path;
+    while (fw_len - pos > 0) {
+        nr = strtol(in, &in, 10);
+        if (in[0] == '.') {
+            /* some hub between root port and device */
+            pos += snprintf(fw_path + pos, fw_len - pos, "hub@%ld/", nr);
+            in++;
+        } else {
+            /* the device itself */
+            pos += snprintf(fw_path + pos, fw_len - pos, "%s@%ld",
+                            qdev_fw_name(qdev), nr);
+            break;
+        }
+    }
+    return fw_path;
+}
+
 static void usb_device_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *k = DEVICE_CLASS(klass);
@@ -584,6 +582,7 @@  static void usb_device_class_init(ObjectClass *klass, void *data)
     k->unplug   = qdev_simple_unplug_cb;
     k->exit     = usb_qdev_exit;
     k->get_dev_path = usb_qdev_get_dev_path;
+    k->get_fw_dev_path = usb_qdev_get_fw_dev_path;
 }
 
 static Property usb_bus_properties[] = {