Patchwork [v2,4/4] qdev: put all devices under /machine

login
register
mail settings
Submitter Paolo Bonzini
Date March 28, 2012, 2:34 p.m.
Message ID <1332945252-19025-1-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/149236/
State New
Headers show

Comments

Paolo Bonzini - March 28, 2012, 2:34 p.m.
Avoid cluttering too much the QOM root.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
	v1->v2: add qdev_get_machine() and use it.

 hw/piix_pci.c     |    2 +-
 hw/ppc_prep.c     |    2 +-
 hw/qdev-monitor.c |    4 ++--
 hw/qdev.c         |   13 ++++++++++++-
 hw/qdev.h         |    2 ++
 5 files changed, 18 insertions(+), 5 deletions(-)
Andreas Färber - March 28, 2012, 3:10 p.m.
Am 28.03.2012 16:34, schrieb Paolo Bonzini:
> Avoid cluttering too much the QOM root.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 	v1->v2: add qdev_get_machine() and use it.

Thanks,

> 
>  hw/piix_pci.c     |    2 +-
>  hw/ppc_prep.c     |    2 +-
>  hw/qdev-monitor.c |    4 ++--
>  hw/qdev.c         |   13 ++++++++++++-
>  hw/qdev.h         |    2 ++
>  5 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 9017565..179d9a6 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -276,7 +276,7 @@ static PCIBus *i440fx_common_init(const char *device_name,
>      b = pci_bus_new(&s->busdev.qdev, NULL, pci_address_space,
>                      address_space_io, 0);
>      s->bus = b;
> -    object_property_add_child(object_get_root(), "i440fx", OBJECT(dev), NULL);
> +    object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
>      qdev_init_nofail(dev);
>  
>      d = pci_create_simple(b, 0, device_name);
> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
> index 86c9336..9d8e659 100644
> --- a/hw/ppc_prep.c
> +++ b/hw/ppc_prep.c
> @@ -615,7 +615,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
>      sys = sysbus_from_qdev(dev);
>      pcihost = DO_UPCAST(PCIHostState, busdev, sys);
>      pcihost->address_space = get_system_memory();
> -    object_property_add_child(object_get_root(), "raven", OBJECT(dev), NULL);
> +    object_property_add_child(qdev_get_machine(), "raven", OBJECT(dev), NULL);
>      qdev_init_nofail(dev);
>      pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
>      if (pci_bus == NULL) {
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 031cb83..4783366 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -180,7 +180,7 @@ static Object *qdev_get_peripheral(void)
>      static Object *dev;
>  
>      if (dev == NULL) {
> -        dev = container_get("/peripheral");
> +        dev = container_get("/machine/peripheral");

I was kinda hoping we could even do something like this in 1/4:
container_get_relative(qdev_get_machine(), "peripheral");
w/ container_get(bla) -> container_get_relative(object_get_root(), bla).

Andreas
Paolo Bonzini - March 28, 2012, 3:19 p.m.
Il 28/03/2012 17:10, Andreas Färber ha scritto:
>> >      if (dev == NULL) {
>> > -        dev = container_get("/peripheral");
>> > +        dev = container_get("/machine/peripheral");
> I was kinda hoping we could even do something like this in 1/4:
> container_get_relative(qdev_get_machine(), "peripheral");
> w/ container_get(bla) -> container_get_relative(object_get_root(), bla).

Follow-up? O:-)

Paolo
Paolo Bonzini - April 5, 2012, 11:30 a.m.
Il 05/04/2012 13:21, Andreas Färber ha scritto:
> Specify the root to search from as argument. This avoids hardcoding
> "/machine" in some places and makes it more flexible.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Anthony Liguori <anthony@codemonkey.ws>

Looks good, thanks.

Paolo

> ---
>  hw/qdev-monitor.c     |    4 ++--
>  hw/qdev.c             |    7 ++++---
>  include/qemu/object.h |    3 ++-
>  qom/container.c       |    4 ++--
>  4 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index 4783366..67f296b 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
> @@ -180,7 +180,7 @@ static Object *qdev_get_peripheral(void)
>      static Object *dev;
>  
>      if (dev == NULL) {
> -        dev = container_get("/machine/peripheral");
> +        dev = container_get(qdev_get_machine(), "/peripheral");
>      }
>  
>      return dev;
> @@ -191,7 +191,7 @@ static Object *qdev_get_peripheral_anon(void)
>      static Object *dev;
>  
>      if (dev == NULL) {
> -        dev = container_get("/machine/peripheral-anon");
> +        dev = container_get(qdev_get_machine(), "/peripheral-anon");
>      }
>  
>      return dev;
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 0d3c0fc..efa4c5d 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -157,8 +157,9 @@ int qdev_init(DeviceState *dev)
>          static int unattached_count = 0;
>          gchar *name = g_strdup_printf("device[%d]", unattached_count++);
>  
> -        object_property_add_child(container_get("/machine/unattached"), name,
> -                                  OBJECT(dev), NULL);
> +        object_property_add_child(container_get(qdev_get_machine(),
> +                                                "/unattached"),
> +                                  name, OBJECT(dev), NULL);
>          g_free(name);
>      }
>  
> @@ -673,7 +674,7 @@ Object *qdev_get_machine(void)
>      static Object *dev;
>  
>      if (dev == NULL) {
> -        dev = container_get("/machine");
> +        dev = container_get(object_get_root(), "/machine");
>      }
>  
>      return dev;
> diff --git a/include/qemu/object.h b/include/qemu/object.h
> index a675937..ca1649c 100644
> --- a/include/qemu/object.h
> +++ b/include/qemu/object.h
> @@ -905,6 +905,7 @@ void object_property_add_str(Object *obj, const char *name,
>  
>  /**
>   * container_get:
> + * @root: root of the #path, e.g., object_get_root()
>   * @path: path to the container
>   *
>   * Return a container object whose path is @path.  Create more containers
> @@ -912,7 +913,7 @@ void object_property_add_str(Object *obj, const char *name,
>   *
>   * Returns: the container object.
>   */
> -Object *container_get(const char *path);
> +Object *container_get(Object *root, const char *path);
>  
>  
>  #endif
> diff --git a/qom/container.c b/qom/container.c
> index 67e9e8a..c9940ab 100644
> --- a/qom/container.c
> +++ b/qom/container.c
> @@ -25,7 +25,7 @@ static void container_register_types(void)
>      type_register_static(&container_info);
>  }
>  
> -Object *container_get(const char *path)
> +Object *container_get(Object *root, const char *path)
>  {
>      Object *obj, *child;
>      gchar **parts;
> @@ -33,7 +33,7 @@ Object *container_get(const char *path)
>  
>      parts = g_strsplit(path, "/", 0);
>      assert(parts != NULL && parts[0] != NULL && !parts[0][0]);
> -    obj = object_get_root();
> +    obj = root;
>  
>      for (i = 1; parts[i] != NULL; i++, obj = child) {
>          child = object_resolve_path_component(obj, parts[i]);
Andreas Färber - April 19, 2012, 3:19 p.m.
Am 05.04.2012 13:30, schrieb Paolo Bonzini:
> Il 05/04/2012 13:21, Andreas Färber ha scritto:
>> Specify the root to search from as argument. This avoids hardcoding
>> "/machine" in some places and makes it more flexible.
>>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Anthony Liguori <anthony@codemonkey.ws>
> 
> Looks good, thanks.

Ping! Anthony, can you apply or are you waiting for explicit *-bys?

Andreas

> 
> Paolo
> 
>> ---
>>  hw/qdev-monitor.c     |    4 ++--
>>  hw/qdev.c             |    7 ++++---
>>  include/qemu/object.h |    3 ++-
>>  qom/container.c       |    4 ++--
>>  4 files changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
>> index 4783366..67f296b 100644
>> --- a/hw/qdev-monitor.c
>> +++ b/hw/qdev-monitor.c
>> @@ -180,7 +180,7 @@ static Object *qdev_get_peripheral(void)
>>      static Object *dev;
>>  
>>      if (dev == NULL) {
>> -        dev = container_get("/machine/peripheral");
>> +        dev = container_get(qdev_get_machine(), "/peripheral");
>>      }
>>  
>>      return dev;
>> @@ -191,7 +191,7 @@ static Object *qdev_get_peripheral_anon(void)
>>      static Object *dev;
>>  
>>      if (dev == NULL) {
>> -        dev = container_get("/machine/peripheral-anon");
>> +        dev = container_get(qdev_get_machine(), "/peripheral-anon");
>>      }
>>  
>>      return dev;
>> diff --git a/hw/qdev.c b/hw/qdev.c
>> index 0d3c0fc..efa4c5d 100644
>> --- a/hw/qdev.c
>> +++ b/hw/qdev.c
>> @@ -157,8 +157,9 @@ int qdev_init(DeviceState *dev)
>>          static int unattached_count = 0;
>>          gchar *name = g_strdup_printf("device[%d]", unattached_count++);
>>  
>> -        object_property_add_child(container_get("/machine/unattached"), name,
>> -                                  OBJECT(dev), NULL);
>> +        object_property_add_child(container_get(qdev_get_machine(),
>> +                                                "/unattached"),
>> +                                  name, OBJECT(dev), NULL);
>>          g_free(name);
>>      }
>>  
>> @@ -673,7 +674,7 @@ Object *qdev_get_machine(void)
>>      static Object *dev;
>>  
>>      if (dev == NULL) {
>> -        dev = container_get("/machine");
>> +        dev = container_get(object_get_root(), "/machine");
>>      }
>>  
>>      return dev;
>> diff --git a/include/qemu/object.h b/include/qemu/object.h
>> index a675937..ca1649c 100644
>> --- a/include/qemu/object.h
>> +++ b/include/qemu/object.h
>> @@ -905,6 +905,7 @@ void object_property_add_str(Object *obj, const char *name,
>>  
>>  /**
>>   * container_get:
>> + * @root: root of the #path, e.g., object_get_root()
>>   * @path: path to the container
>>   *
>>   * Return a container object whose path is @path.  Create more containers
>> @@ -912,7 +913,7 @@ void object_property_add_str(Object *obj, const char *name,
>>   *
>>   * Returns: the container object.
>>   */
>> -Object *container_get(const char *path);
>> +Object *container_get(Object *root, const char *path);
>>  
>>  
>>  #endif
>> diff --git a/qom/container.c b/qom/container.c
>> index 67e9e8a..c9940ab 100644
>> --- a/qom/container.c
>> +++ b/qom/container.c
>> @@ -25,7 +25,7 @@ static void container_register_types(void)
>>      type_register_static(&container_info);
>>  }
>>  
>> -Object *container_get(const char *path)
>> +Object *container_get(Object *root, const char *path)
>>  {
>>      Object *obj, *child;
>>      gchar **parts;
>> @@ -33,7 +33,7 @@ Object *container_get(const char *path)
>>  
>>      parts = g_strsplit(path, "/", 0);
>>      assert(parts != NULL && parts[0] != NULL && !parts[0][0]);
>> -    obj = object_get_root();
>> +    obj = root;
>>  
>>      for (i = 1; parts[i] != NULL; i++, obj = child) {
>>          child = object_resolve_path_component(obj, parts[i]);
Anthony Liguori - April 19, 2012, 3:43 p.m.
On 04/19/2012 10:19 AM, Andreas Färber wrote:
> Am 05.04.2012 13:30, schrieb Paolo Bonzini:
>> Il 05/04/2012 13:21, Andreas Färber ha scritto:
>>> Specify the root to search from as argument. This avoids hardcoding
>>> "/machine" in some places and makes it more flexible.
>>>
>>> Signed-off-by: Andreas Färber<afaerber@suse.de>
>>> Cc: Paolo Bonzini<pbonzini@redhat.com>
>>> Cc: Anthony Liguori<anthony@codemonkey.ws>
>>
>> Looks good, thanks.
>
> Ping! Anthony, can you apply or are you waiting for explicit *-bys?

I've requeued it.  Please don't send patches in reply to other people's patches 
though.  New patches should always be a new top-level post or they're likely to 
get lost.

Regards,

Anthony Liguori

>
> Andreas
>
>>
>> Paolo
>>
>>> ---
>>>   hw/qdev-monitor.c     |    4 ++--
>>>   hw/qdev.c             |    7 ++++---
>>>   include/qemu/object.h |    3 ++-
>>>   qom/container.c       |    4 ++--
>>>   4 files changed, 10 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
>>> index 4783366..67f296b 100644
>>> --- a/hw/qdev-monitor.c
>>> +++ b/hw/qdev-monitor.c
>>> @@ -180,7 +180,7 @@ static Object *qdev_get_peripheral(void)
>>>       static Object *dev;
>>>
>>>       if (dev == NULL) {
>>> -        dev = container_get("/machine/peripheral");
>>> +        dev = container_get(qdev_get_machine(), "/peripheral");
>>>       }
>>>
>>>       return dev;
>>> @@ -191,7 +191,7 @@ static Object *qdev_get_peripheral_anon(void)
>>>       static Object *dev;
>>>
>>>       if (dev == NULL) {
>>> -        dev = container_get("/machine/peripheral-anon");
>>> +        dev = container_get(qdev_get_machine(), "/peripheral-anon");
>>>       }
>>>
>>>       return dev;
>>> diff --git a/hw/qdev.c b/hw/qdev.c
>>> index 0d3c0fc..efa4c5d 100644
>>> --- a/hw/qdev.c
>>> +++ b/hw/qdev.c
>>> @@ -157,8 +157,9 @@ int qdev_init(DeviceState *dev)
>>>           static int unattached_count = 0;
>>>           gchar *name = g_strdup_printf("device[%d]", unattached_count++);
>>>
>>> -        object_property_add_child(container_get("/machine/unattached"), name,
>>> -                                  OBJECT(dev), NULL);
>>> +        object_property_add_child(container_get(qdev_get_machine(),
>>> +                                                "/unattached"),
>>> +                                  name, OBJECT(dev), NULL);
>>>           g_free(name);
>>>       }
>>>
>>> @@ -673,7 +674,7 @@ Object *qdev_get_machine(void)
>>>       static Object *dev;
>>>
>>>       if (dev == NULL) {
>>> -        dev = container_get("/machine");
>>> +        dev = container_get(object_get_root(), "/machine");
>>>       }
>>>
>>>       return dev;
>>> diff --git a/include/qemu/object.h b/include/qemu/object.h
>>> index a675937..ca1649c 100644
>>> --- a/include/qemu/object.h
>>> +++ b/include/qemu/object.h
>>> @@ -905,6 +905,7 @@ void object_property_add_str(Object *obj, const char *name,
>>>
>>>   /**
>>>    * container_get:
>>> + * @root: root of the #path, e.g., object_get_root()
>>>    * @path: path to the container
>>>    *
>>>    * Return a container object whose path is @path.  Create more containers
>>> @@ -912,7 +913,7 @@ void object_property_add_str(Object *obj, const char *name,
>>>    *
>>>    * Returns: the container object.
>>>    */
>>> -Object *container_get(const char *path);
>>> +Object *container_get(Object *root, const char *path);
>>>
>>>
>>>   #endif
>>> diff --git a/qom/container.c b/qom/container.c
>>> index 67e9e8a..c9940ab 100644
>>> --- a/qom/container.c
>>> +++ b/qom/container.c
>>> @@ -25,7 +25,7 @@ static void container_register_types(void)
>>>       type_register_static(&container_info);
>>>   }
>>>
>>> -Object *container_get(const char *path)
>>> +Object *container_get(Object *root, const char *path)
>>>   {
>>>       Object *obj, *child;
>>>       gchar **parts;
>>> @@ -33,7 +33,7 @@ Object *container_get(const char *path)
>>>
>>>       parts = g_strsplit(path, "/", 0);
>>>       assert(parts != NULL&&  parts[0] != NULL&&  !parts[0][0]);
>>> -    obj = object_get_root();
>>> +    obj = root;
>>>
>>>       for (i = 1; parts[i] != NULL; i++, obj = child) {
>>>           child = object_resolve_path_component(obj, parts[i]);
>
Andreas Färber - April 19, 2012, 3:50 p.m.
Am 19.04.2012 17:43, schrieb Anthony Liguori:
> On 04/19/2012 10:19 AM, Andreas Färber wrote:
>> Am 05.04.2012 13:30, schrieb Paolo Bonzini:
>>> Il 05/04/2012 13:21, Andreas Färber ha scritto:
>>>> Specify the root to search from as argument. This avoids hardcoding
>>>> "/machine" in some places and makes it more flexible.
>>>>
>>>> Signed-off-by: Andreas Färber<afaerber@suse.de>
>>>> Cc: Paolo Bonzini<pbonzini@redhat.com>
>>>> Cc: Anthony Liguori<anthony@codemonkey.ws>
>>>
>>> Looks good, thanks.
>>
>> Ping! Anthony, can you apply or are you waiting for explicit *-bys?
> 
> I've requeued it.  Please don't send patches in reply to other people's
> patches though.  New patches should always be a new top-level post or
> they're likely to get lost.

Generally yes. In this case this was the change that I asked Paolo to do
in his series and that he suggested as a followup - me interpreting
followup as in his series before it gets applied, you applying the
series as is unaware of our IRC conversation.

Regards,
Andreas

Patch

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 9017565..179d9a6 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -276,7 +276,7 @@  static PCIBus *i440fx_common_init(const char *device_name,
     b = pci_bus_new(&s->busdev.qdev, NULL, pci_address_space,
                     address_space_io, 0);
     s->bus = b;
-    object_property_add_child(object_get_root(), "i440fx", OBJECT(dev), NULL);
+    object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
     qdev_init_nofail(dev);
 
     d = pci_create_simple(b, 0, device_name);
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 86c9336..9d8e659 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -615,7 +615,7 @@  static void ppc_prep_init (ram_addr_t ram_size,
     sys = sysbus_from_qdev(dev);
     pcihost = DO_UPCAST(PCIHostState, busdev, sys);
     pcihost->address_space = get_system_memory();
-    object_property_add_child(object_get_root(), "raven", OBJECT(dev), NULL);
+    object_property_add_child(qdev_get_machine(), "raven", OBJECT(dev), NULL);
     qdev_init_nofail(dev);
     pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
     if (pci_bus == NULL) {
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 031cb83..4783366 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -180,7 +180,7 @@  static Object *qdev_get_peripheral(void)
     static Object *dev;
 
     if (dev == NULL) {
-        dev = container_get("/peripheral");
+        dev = container_get("/machine/peripheral");
     }
 
     return dev;
@@ -191,7 +191,7 @@  static Object *qdev_get_peripheral_anon(void)
     static Object *dev;
 
     if (dev == NULL) {
-        dev = container_get("/peripheral-anon");
+        dev = container_get("/machine/peripheral-anon");
     }
 
     return dev;
diff --git a/hw/qdev.c b/hw/qdev.c
index f5c716e..0d3c0fc 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -157,7 +157,7 @@  int qdev_init(DeviceState *dev)
         static int unattached_count = 0;
         gchar *name = g_strdup_printf("device[%d]", unattached_count++);
 
-        object_property_add_child(container_get("/unattached"), name,
+        object_property_add_child(container_get("/machine/unattached"), name,
                                   OBJECT(dev), NULL);
         g_free(name);
     }
@@ -668,6 +668,17 @@  void device_reset(DeviceState *dev)
     }
 }
 
+Object *qdev_get_machine(void)
+{
+    static Object *dev;
+
+    if (dev == NULL) {
+        dev = container_get("/machine");
+    }
+
+    return dev;
+}
+
 static TypeInfo device_type_info = {
     .name = TYPE_DEVICE,
     .parent = TYPE_OBJECT,
diff --git a/hw/qdev.h b/hw/qdev.h
index 9cc3f98..a8df42f 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -349,6 +349,8 @@  BusInfo *qdev_get_bus_info(DeviceState *dev);
 
 Property *qdev_get_props(DeviceState *dev);
 
+Object *qdev_get_machine(void);
+
 /* FIXME: make this a link<> */
 void qdev_set_parent_bus(DeviceState *dev, BusState *bus);