Re: [libvirt] [libvirt-glib 5/5] Add gvir_config_domain_os_get_boot_devices()

On Wed, May 09, 2012 at 03:54:38AM +0300, Zeeshan Ali (Khattak) wrote: > From: "Zeeshan Ali (Khattak)" <zeeshanak(a)gnome.org&gt; > > --- >  libvirt-gconfig/libvirt-gconfig-domain-os.c |   45 +++++++++++++++++++++++++++ >  libvirt-gconfig/libvirt-gconfig-domain-os.h |    1 + >  libvirt-gconfig/libvirt-gconfig.sym         |    1 + >  3 files changed, 47 insertions(+), 0 deletions(-) > > diff --git a/libvirt-gconfig/libvirt-gconfig-domain-os.c b/libvirt-gconfig/libvirt-gconfig-domain-os.c > index 74cdd4d..6e3cabd 100644 > --- a/libvirt-gconfig/libvirt-gconfig-domain-os.c > +++ b/libvirt-gconfig/libvirt-gconfig-domain-os.c > @@ -221,6 +221,51 @@ void gvir_config_domain_os_set_boot_devices(GVirConfigDomainOs *os, GList *boot_ >      } >  } > > +static gboolean add_boot_device(xmlNodePtr node, gpointer opaque) > +{ > +    GList **devices = (GList **)opaque; > +    const gchar *value; > + > +    if (g_strcmp0((const gchar *)node->name, "boot") != 0) > +        return TRUE; > + > +    value = gvir_config_xml_get_attribute_content(node, "dev"); > +    if (value != NULL) { > +        GVirConfigDomainOsBootDevice device; > + > +        device = gvir_config_genum_get_value > +                        (GVIR_CONFIG_TYPE_DOMAIN_OS_BOOT_DEVICE, > +                         value, > +                         GVIR_CONFIG_DOMAIN_OS_BOOT_DEVICE_HD); I had never thought of this, but the way gvir_config_genum_get_value works could cause issues: if a new member is added to the enum without libvirt-glib being updated, when we'll try to parse XML using the new member, we'll warn in the console but we will wrongly return the default value. I'm not sure if the right behaviour would be to ignore the unknown value, to return NULL, or something else. Anyway, just another thing to think about 'later' ;) > +        *devices = g_list_append(*devices, GINT_TO_POINTER(device)); > +    } else > +        g_debug("Failed to parse attribute 'dev' of node 'boot'"); > + > +    return TRUE; > +} > + > +/** > + * gvir_config_domain_os_get_boot_devices: > + * > + * Gets the list of devices attached to @os Can you add something like "The returned list should be freed with g_list_free(), after its elements have been unreffed with g_object_unref()." there so that memory management is 100% explicit?