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

[Zeeshan Ali (Khattak)]

On Thu, May 10, 2012 at 8:28 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> On Wed, May 09, 2012 at 03:54:38AM +0300, Zeeshan Ali (Khattak) wrote:
>> From: "Zeeshan Ali (Khattak)" <zeeshanak at gnome.org>
>>
>> ---
>>  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?

The annotations says it all. IIRC there was a bug on gtk-doc to
generate more helpful output based on these annotations. I don't know
if that has been fixed or not but when/if it is, we'll have very silly
looking duplication of docs in the same place in the output (there
will be duplication of info in source code comment any ways). Long
story short, this should be fixed in/by gtk-doc!

-- 
Regards,

Zeeshan Ali (Khattak)
FSF member#5124