Re: [libvirt] [glib 3/5] gconfig: Create objects for all domain device nodes

Hey, On Tue, Nov 01, 2016 at 05:46:05PM +0100, Zeeshan Ali wrote: > Currently we can and do get into serious trouble with this kind of code: > > devices = gvir_config_domain_get_devices(domain); > gvir_config_domain_set_devices(domain, domain); > > since the first call above won't return a complete list of objects present > in the domain but only the ones we have specific classes for and the > second call above overwrites all device nodes under the domain. This > lately made Boxes break against the latest libvirt, where a new device > node was made compulsory[1]. > > Although we should add support for all know domain devices ASAP, new > devices will be added in future and this can happen again. So let's first > ensure that gvir_config_domain_get_devices() always returns all devices > under the domain. All unknown/unimplemented devices will now be returned > as the very generic DomainDevice objects. Once we add support for a > particular device, there will be no API/ABI breakage since the new class > will inherit from DomainDevice class. > > [1] https://bugzilla.redhat.com/show_bug.cgi?id=1388091 > --- > libvirt-gconfig/libvirt-gconfig-domain-device.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/libvirt-gconfig/libvirt-gconfig-domain-device.c b/libvirt-gconfig/libvirt-gconfig-domain-device.c > index f78173a..3efca62 100644 > --- a/libvirt-gconfig/libvirt-gconfig-domain-device.c > +++ b/libvirt-gconfig/libvirt-gconfig-domain-device.c > @@ -64,7 +64,7 @@ gvir_config_domain_device_new_from_tree(GVirConfigXmlDoc *doc, > } else if (xmlStrEqual(tree->name, (xmlChar*)"controller")) { > return gvir_config_domain_controller_new_from_tree(doc, tree); > } else if (xmlStrEqual(tree->name, (xmlChar*)"lease")) { > - goto unimplemented; > + type = GVIR_CONFIG_TYPE_DOMAIN_DEVICE; > } else if (xmlStrEqual(tree->name, (xmlChar*)"hostdev")) { > return gvir_config_domain_hostdev_new_from_tree(doc, tree); > } else if (xmlStrEqual(tree->name, (xmlChar*)"redirdev")) { > @@ -76,7 +76,7 @@ gvir_config_domain_device_new_from_tree(GVirConfigXmlDoc *doc, > } else if (xmlStrEqual(tree->name, (xmlChar*)"input")) { > type = GVIR_CONFIG_TYPE_DOMAIN_INPUT; > } else if (xmlStrEqual(tree->name, (xmlChar*)"hub")) { > - goto unimplemented; > + type = GVIR_CONFIG_TYPE_DOMAIN_DEVICE; > } else if (xmlStrEqual(tree->name, (xmlChar*)"graphics")) { > return gvir_config_domain_graphics_new_from_tree(doc, tree); > } else if (xmlStrEqual(tree->name, (xmlChar*)"video")) { > @@ -90,22 +90,19 @@ gvir_config_domain_device_new_from_tree(GVirConfigXmlDoc *doc, > } else if (xmlStrEqual(tree->name, (xmlChar*)"channel")) { > type = GVIR_CONFIG_TYPE_DOMAIN_CHANNEL; > } else if (xmlStrEqual(tree->name, (xmlChar*)"watchdog")) { > - goto unimplemented; > + type = GVIR_CONFIG_TYPE_DOMAIN_DEVICE; > } else if (xmlStrEqual(tree->name, (xmlChar*)"sound")) { > type = GVIR_CONFIG_TYPE_DOMAIN_SOUND; > } else if (xmlStrEqual(tree->name, (xmlChar*)"memballoon")) { > type = GVIR_CONFIG_TYPE_DOMAIN_MEMBALLOON; > } else { > g_debug("Unknown device node: %s", tree->name); > - return NULL; > + type = GVIR_CONFIG_TYPE_DOMAIN_DEVICE; > } > > g_return_val_if_fail(g_type_is_a(type, GVIR_CONFIG_TYPE_DOMAIN_DEVICE), NULL); > > return GVIR_CONFIG_DOMAIN_DEVICE(gvir_config_object_new_from_tree(type, doc, NULL, tree)); > -unimplemented: > - g_debug("Parsing of '%s' device nodes is unimplemented", tree->name); Very minor, but I would have kept this g_debug().