On 05/02/2012 06:31 AM, Paolo Bonzini wrote:
In qdev, each bus in practice identified an abstract superclass, but
this was mostly hidden.  In QOM, instead, these abstract classes are
explicit so we can move bus properties there.

All bus property walks are removed, and all device property walks
are changed to look along the class hierarchy instead.

This breaks global bus properties, an obscure feature when used
with the command-line which is actually useful and used when used by
backwards-compatible machine types.  So this patch also adjust the
global bus properties in hw/pc_piix.c to refer to the abstract class.

Globals and other properties must be modified in the same patch to
avoid complications related to initialization ordering.

Signed-off-by: Paolo Bonzini<pbonz...@redhat.com>
---
  hw/i2c.c                      |    2 +-
  hw/ide/qdev.c                 |    2 +-
  hw/intel-hda.c                |    2 +-
  hw/pc_piix.c                  |    5 +++--
  hw/pci.c                      |    2 +-
  hw/qdev-monitor.c             |   41 ++++++++++++++++++-----------------------
  hw/qdev-properties.c          |   40 ++++++++++++++++++++++------------------
  hw/qdev.c                     |   36 ++++++++++--------------------------
  hw/qdev.h                     |    5 -----
  hw/scsi-bus.c                 |    2 +-
  hw/spapr_vio.c                |    2 +-
  hw/usb/bus.c                  |    2 +-
  hw/usb/dev-smartcard-reader.c |    2 +-
  hw/virtio-serial-bus.c        |    2 +-
  14 files changed, 62 insertions(+), 83 deletions(-)

diff --git a/hw/i2c.c b/hw/i2c.c
index cb10b1d..af5979e 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -25,7 +25,6 @@ static Property i2c_props[] = {
  static struct BusInfo i2c_bus_info = {
      .name = "I2C",
      .size = sizeof(i2c_bus),
-    .props = i2c_props,
  };

  static void i2c_bus_pre_save(void *opaque)
@@ -221,6 +220,7 @@ static void i2c_slave_class_init(ObjectClass *klass, void 
*data)
      DeviceClass *k = DEVICE_CLASS(klass);
      k->init = i2c_slave_qdev_init;
      k->bus_info =&i2c_bus_info;
+    k->props = i2c_props;
  }

  static TypeInfo i2c_slave_type_info = {
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index b67df3d..a91e878 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -36,7 +36,6 @@ static struct BusInfo ide_bus_info = {
      .name  = "IDE",
      .size  = sizeof(IDEBus),
      .get_fw_dev_path = idebus_get_fw_dev_path,
-    .props = ide_props,
  };

  void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id)
@@ -251,6 +250,7 @@ static void ide_device_class_init(ObjectClass *klass, void 
*data)
      DeviceClass *k = DEVICE_CLASS(klass);
      k->init = ide_qdev_init;
      k->bus_info =&ide_bus_info;
+    k->props = ide_props;
  }

  static TypeInfo ide_device_type_info = {
diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index 0994f6b..e2bd41e 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -37,7 +37,6 @@ static Property hda_props[] = {
  static struct BusInfo hda_codec_bus_info = {
      .name      = "HDA",
      .size      = sizeof(HDACodecBus),
-    .props     = hda_props,
  };

  void hda_codec_bus_init(DeviceState *dev, HDACodecBus *bus,
@@ -1278,6 +1277,7 @@ static void hda_codec_device_class_init(ObjectClass 
*klass, void *data)
      k->init = hda_codec_dev_init;
      k->exit = hda_codec_dev_exit;
      k->bus_info =&hda_codec_bus_info;
+    k->props = hda_props;
  }

  static TypeInfo hda_codec_device_type_info = {
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 6a75718..ef6afb1 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -29,6 +29,7 @@
  #include "apic.h"
  #include "pci.h"
  #include "pci_ids.h"
+#include "usb.h"
  #include "net.h"
  #include "boards.h"
  #include "ide.h"
@@ -378,7 +379,7 @@ static QEMUMachine pc_machine_v1_1 = {
              .property = "vapic",\
              .value    = "off",\
          },{\
-            .driver   = "USB",\
+            .driver   = TYPE_USB_DEVICE,\
              .property = "full-path",\
              .value    = "no",\
          }
@@ -451,7 +452,7 @@ static QEMUMachine pc_machine_v0_14 = {
  #define PC_COMPAT_0_13 \
          PC_COMPAT_0_14,\
          {\
-            .driver   = "PCI",\
+            .driver   = TYPE_PCI_DEVICE,\
              .property = "command_serr_enable",\
              .value    = "off",\
          },{\
diff --git a/hw/pci.c b/hw/pci.c
index 403651f..6319f4d 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -62,7 +62,6 @@ struct BusInfo pci_bus_info = {
      .get_dev_path = pcibus_get_dev_path,
      .get_fw_dev_path = pcibus_get_fw_dev_path,
      .reset      = pcibus_reset,
-    .props      = pci_props,
  };

  static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
@@ -2004,6 +2003,7 @@ static void pci_device_class_init(ObjectClass *klass, 
void *data)
      k->unplug = pci_unplug_device;
      k->exit = pci_unregister_device;
      k->bus_info =&pci_bus_info;
+    k->props = pci_props;
  }

  static TypeInfo pci_device_type_info = {
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index eed781d..d9c6adc 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -123,7 +123,6 @@ int qdev_device_help(QemuOpts *opts)
      const char *driver;
      Property *prop;
      ObjectClass *klass;
-    DeviceClass *info;

      driver = qemu_opt_get(opts, "driver");
      if (driver&&  !strcmp(driver, "?")) {
@@ -149,30 +148,22 @@ int qdev_device_help(QemuOpts *opts)
      if (!klass) {
          return 0;
      }
-    info = DEVICE_CLASS(klass);
-
-    for (prop = info->props; prop&&  prop->name; prop++) {
-        /*
-         * TODO Properties without a parser are just for dirty hacks.
-         * qdev_prop_ptr is the only such PropertyInfo.  It's marked
-         * for removal.  This conditional should be removed along with
-         * it.
-         */
-        if (!prop->info->set) {
-            continue;           /* no way to set it, don't show */
-        }
-        error_printf("%s.%s=%s\n", driver, prop->name,
-                     prop->info->legacy_name ?: prop->info->name);
-    }
-    if (info->bus_info) {
-        for (prop = info->bus_info->props; prop&&  prop->name; prop++) {
+    do {
+        for (prop = DEVICE_CLASS(klass)->props; prop&&  prop->name; prop++) {
+            /*
+             * TODO Properties without a parser are just for dirty hacks.
+             * qdev_prop_ptr is the only such PropertyInfo.  It's marked
+             * for removal.  This conditional should be removed along with
+             * it.
+             */
              if (!prop->info->set) {
                  continue;           /* no way to set it, don't show */
              }
              error_printf("%s.%s=%s\n", driver, prop->name,
                           prop->info->legacy_name ?: prop->info->name);
          }
-    }
+        klass = object_class_get_parent(klass);
+    } while (klass != object_class_by_name(TYPE_DEVICE));
      return 1;
  }

@@ -482,7 +473,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
  static void qbus_print(Monitor *mon, BusState *bus, int indent);

  static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
-                             const char *prefix, int indent)
+                             int indent)
  {
      if (!props)
          return;
@@ -501,7 +492,7 @@ static void qdev_print_props(Monitor *mon, DeviceState 
*dev, Property *props,
              error_free(err);
              continue;
          }
-        qdev_printf("%s-prop: %s = %s\n", prefix, props->name,
+        qdev_printf("%s = %s\n", props->name,
                      value&&  *value ? value : "<null>");
          g_free(value);
      }
@@ -509,6 +500,7 @@ static void qdev_print_props(Monitor *mon, DeviceState 
*dev, Property *props,

  static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
  {
+    ObjectClass *class;
      BusState *child;
      qdev_printf("dev: %s, id \"%s\"\n", object_get_typename(OBJECT(dev)),
                  dev->id ? dev->id : "");
@@ -519,8 +511,11 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int 
indent)
      if (dev->num_gpio_out) {
          qdev_printf("gpio-out %d\n", dev->num_gpio_out);
      }
-    qdev_print_props(mon, dev, qdev_get_props(dev), "dev", indent);
-    qdev_print_props(mon, dev, dev->parent_bus->info->props, "bus", indent);
+    class = object_get_class(OBJECT(dev));
+    do {
+        qdev_print_props(mon, dev, DEVICE_CLASS(class)->props, indent);
+        class = object_class_get_parent(class);
+    } while (class != object_class_by_name(TYPE_DEVICE));
      if (dev->parent_bus->info->print_dev)
          dev->parent_bus->info->print_dev(mon, dev, indent);
      QLIST_FOREACH(child,&dev->child_bus, sibling) {
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 98dd06a..572b83c 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -939,17 +939,18 @@ static Property *qdev_prop_walk(Property *props, const 
char *name)

  static Property *qdev_prop_find(DeviceState *dev, const char *name)
  {
+    ObjectClass *class;
      Property *prop;

      /* device properties */
-    prop = qdev_prop_walk(qdev_get_props(dev), name);
-    if (prop)
-        return prop;
-
-    /* bus properties */
-    prop = qdev_prop_walk(dev->parent_bus->info->props, name);
-    if (prop)
-        return prop;
+    class = object_get_class(OBJECT(dev));
+    do {
+        prop = qdev_prop_walk(DEVICE_CLASS(class)->props, name);
+        if (prop) {
+            return prop;
+        }
+        class = object_class_get_parent(class);
+    } while (class != object_class_by_name(TYPE_DEVICE));

      return NULL;
  }
@@ -1169,17 +1170,20 @@ void qdev_prop_register_global_list(GlobalProperty 
*props)

  void qdev_prop_set_globals(DeviceState *dev)
  {
-    GlobalProperty *prop;
-
-    QTAILQ_FOREACH(prop,&global_props, next) {
-        if (strcmp(object_get_typename(OBJECT(dev)), prop->driver) != 0&&
-            strcmp(qdev_get_bus_info(dev)->name, prop->driver) != 0) {
-            continue;
-        }
-        if (qdev_prop_parse(dev, prop->property, prop->value) != 0) {
-            exit(1);
+    ObjectClass *class = object_get_class(OBJECT(dev));
+
+    do {
+        GlobalProperty *prop;
+        QTAILQ_FOREACH(prop,&global_props, next) {
+            if (strcmp(object_class_get_name(class), prop->driver) != 0) {
+                continue;
+            }
+            if (qdev_prop_parse(dev, prop->property, prop->value) != 0) {
+                exit(1);
+            }
          }
-    }
+        class = object_class_get_parent(class);
+    } while (class);
  }

  static int qdev_add_one_global(QemuOpts *opts, void *opaque)
diff --git a/hw/qdev.c b/hw/qdev.c
index 67d7770..98efc8b 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -45,18 +45,6 @@ const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
      return dc->vmsd;
  }

-BusInfo *qdev_get_bus_info(DeviceState *dev)
-{
-    DeviceClass *dc = DEVICE_GET_CLASS(dev);
-    return dc->bus_info;
-}
-
-Property *qdev_get_props(DeviceState *dev)
-{
-    DeviceClass *dc = DEVICE_GET_CLASS(dev);
-    return dc->props;
-}
-
  const char *qdev_fw_name(DeviceState *dev)
  {
      DeviceClass *dc = DEVICE_GET_CLASS(dev);
@@ -78,20 +66,12 @@ static void qdev_property_add_legacy(DeviceState *dev, 
Property *prop,

  void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
  {
-    Property *prop;
-
      if (qdev_hotplug) {
          assert(bus->allow_hotplug);
      }

      dev->parent_bus = bus;
      QTAILQ_INSERT_HEAD(&bus->children, dev, sibling);
-
-    for (prop = qdev_get_bus_info(dev)->props; prop&&  prop->name; prop++) {
-        qdev_property_add_legacy(dev, prop, NULL);
-        qdev_property_add_static(dev, prop, NULL);
-    }
-    qdev_prop_set_defaults(dev, dev->parent_bus->info->props);
  }

  /* Create a new device.  This only initializes the device state structure
@@ -615,6 +595,7 @@ void qdev_property_add_static(DeviceState *dev, Property 
*prop,
  static void device_initfn(Object *obj)
  {
      DeviceState *dev = DEVICE(obj);
+    ObjectClass *class;
      Property *prop;

      if (qdev_hotplug) {
@@ -625,12 +606,15 @@ static void device_initfn(Object *obj)
      dev->instance_id_alias = -1;
      dev->state = DEV_STATE_CREATED;

-    for (prop = qdev_get_props(dev); prop&&  prop->name; prop++) {
-        qdev_property_add_legacy(dev, prop, NULL);
-        qdev_property_add_static(dev, prop, NULL);
-    }
-
-    qdev_prop_set_defaults(dev, qdev_get_props(dev));
+    class = object_get_class(OBJECT(dev));
+    do {
+        for (prop = DEVICE_CLASS(class)->props; prop&&  prop->name; prop++) {
+            qdev_property_add_legacy(dev, prop, NULL);
+            qdev_property_add_static(dev, prop, NULL);
+        }
+        qdev_prop_set_defaults(dev, DEVICE_CLASS(class)->props);
+        class = object_class_get_parent(class);
+    } while (class != object_class_by_name(TYPE_DEVICE));
  }


This little bit of magic is a bit too magical for my taste.

Polymorphism relies on the idea that a subclass overloads base class members/methods. From the base classes perspective, it's unaware if a subclass has overloaded something (that's allowed to be overloaded).

This code doesn't get the current class *as* the base class but rather gets the non-overloaded form of the base class. This isn't really something that most OO systems allow and I think it could lead to major ugliness in the future.

I much prefer moving property installation to a function call that is invoked during base class init.

Regards,

Anthony Liguori


Reply via email to