[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 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?

ACK

Christophe

> + *
> + * Returns: (element-type LibvirtGConfig.DomainOsBootDevice) (transfer full):
> + * a newly allocated #GList of #GVirConfigDomainOsBootDevice.
> + */
> +GList *gvir_config_domain_os_get_boot_devices(GVirConfigDomainOs *os)
> +{
> +    GList *devices = NULL;
> +
> +    g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_OS(os), NULL);
> +
> +    gvir_config_object_foreach_child(GVIR_CONFIG_OBJECT(os),
> +                                     NULL,
> +                                     add_boot_device,
> +                                     &devices);
> +
> +    return devices;
> +}
> +
>  void gvir_config_domain_os_set_arch(GVirConfigDomainOs *os, const char *arch)
>  {
>      xmlNodePtr os_node;
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-os.h b/libvirt-gconfig/libvirt-gconfig-domain-os.h
> index 55a162b..44b8bdd 100644
> --- a/libvirt-gconfig/libvirt-gconfig-domain-os.h
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-os.h
> @@ -82,6 +82,7 @@ GVirConfigDomainOs *gvir_config_domain_os_new_from_xml(const gchar *xml, GError
>  
>  void gvir_config_domain_os_set_os_type(GVirConfigDomainOs *os, GVirConfigDomainOsType type);
>  void gvir_config_domain_os_set_arch(GVirConfigDomainOs *os, const char *arch);
> +GList *gvir_config_domain_os_get_boot_devices(GVirConfigDomainOs *os);
>  void gvir_config_domain_os_set_boot_devices(GVirConfigDomainOs *os, GList *boot_devices);
>  void gvir_config_domain_os_set_kernel(GVirConfigDomainOs *os, const char *kernel);
>  void gvir_config_domain_os_set_ramdisk(GVirConfigDomainOs *os, const char *ramdisk);
> diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym
> index 67e9c3f..7bc9e2d 100644
> --- a/libvirt-gconfig/libvirt-gconfig.sym
> +++ b/libvirt-gconfig/libvirt-gconfig.sym
> @@ -377,6 +377,7 @@ LIBVIRT_GCONFIG_0.0.8 {
>  LIBVIRT_GCONFIG_0.0.9 {
>    global:
>  	gvir_config_domain_get_os;
> +	gvir_config_domain_os_get_boot_devices;
>  } LIBVIRT_GCONFIG_0.0.8;
>  
>  # .... define new API here using predicted next version number ....
> -- 
> 1.7.7.6
> 
> --
> libvir-list mailing list
> libvir-list redhat com
> https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: pgptxNUZHNL0u.pgp
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]