Hi,

As usual I forgot to attach a summary. The changes in v2 are:

* Keeping a debug() log for unknown devices.
* Adding the test case XML file I forgot to add last time.

On Thu, Nov 3, 2016 at 10:06 AM, Zeeshan Ali <zee...@gmail.com> 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 | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-device.c 
> b/libvirt-gconfig/libvirt-gconfig-domain-device.c
> index f78173a..2475519 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,22 @@ 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);
>
> +    if (type == GVIR_CONFIG_TYPE_DOMAIN_DEVICE)
> +        g_debug("Proper support for '%s' device nodes is not yet 
> implemented", tree->name);
> +
>      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);
> -    return NULL;
>  }
>
>
> --
> 2.9.3
>



-- 
Regards,

Zeeshan Ali

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to