| Message ID | 1332945252-19025-1-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
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]);
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]);
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]); >
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
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);
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(-)