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

Re: [libvirt] [libvirt-glib 3/4] Allow unsetting of string & object properties



On Fri, May 04, 2012 at 03:07:44AM +0300, Zeeshan Ali (Khattak) wrote:
> From: "Zeeshan Ali (Khattak)" <zeeshanak gnome org>
> 
> Passing a 'NULL' value now deletes the corresponding node/tree.
> ---
>  .../libvirt-gconfig-domain-chardev-source-pty.c    |    6 ++
>  .../libvirt-gconfig-domain-controller.c            |   14 ++++-
>  libvirt-gconfig/libvirt-gconfig-domain-os.c        |   20 +++++++
>  libvirt-gconfig/libvirt-gconfig-domain-redirdev.c  |   14 ++++-
>  libvirt-gconfig/libvirt-gconfig-domain-seclabel.c  |    8 +++
>  libvirt-gconfig/libvirt-gconfig-domain.c           |   54 ++++++++++++++++---
>  libvirt-gconfig/libvirt-gconfig-object.c           |    7 ++-
>  .../libvirt-gconfig-storage-permissions.c          |    4 ++
>  .../libvirt-gconfig-storage-pool-source.c          |    4 ++
>  .../libvirt-gconfig-storage-pool-target.c          |   18 ++++++-
>  libvirt-gconfig/libvirt-gconfig-storage-pool.c     |   36 +++++++++++--
>  .../libvirt-gconfig-storage-vol-backing-store.c    |    4 ++
>  .../libvirt-gconfig-storage-vol-target.c           |   14 ++++-
>  libvirt-gconfig/libvirt-gconfig-storage-vol.c      |   32 +++++++++--
>  14 files changed, 201 insertions(+), 34 deletions(-)
> 
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c b/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c
> index ad47bc4..fd08584 100644
> --- a/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c
> @@ -85,6 +85,12 @@ void gvir_config_domain_source_pty_set_path(GVirConfigDomainChardevSourcePty *pt
>      GVirConfigObject *source;
>      g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CHARDEV_SOURCE_PTY(pty));
>  
> +    if (path == NULL) {
> +        gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(pty), "source", NULL);
> +
> +        return;
> +    }
> +
>      source = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(pty),
>                                                "source");

_set_path looks buggy before your changes: we want to generate
<source path="foo">, and this code would generate
<source><path>foo</path></source> if I'm not mistaken.

>      g_return_if_fail(GVIR_CONFIG_IS_OBJECT(source));
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-controller.c b/libvirt-gconfig/libvirt-gconfig-domain-controller.c
> index 2024b54..25de002 100644
> --- a/libvirt-gconfig/libvirt-gconfig-domain-controller.c
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-controller.c
> @@ -115,12 +115,20 @@ guint gvir_config_domain_controller_get_index(GVirConfigDomainController *contro
>      return index;
>  }
>  
> +/**
> + * gvir_config_domain_controller_set_address:
> + * @address: (allow-none):
> + */
>  void gvir_config_domain_controller_set_address(GVirConfigDomainController *controller,
>                                                 GVirConfigDomainAddress *address)
>  {
>      g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CONTROLLER(controller));
> -    g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_ADDRESS(address));
>  
> -    gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(controller),
> -                                      GVIR_CONFIG_OBJECT(address));
> +    if (address == NULL)
> +        gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(controller),
> +                                        "address",
> +                                        NULL);
> +    else
> +        gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(controller),
> +                                          GVIR_CONFIG_OBJECT(address));

Maybe gvir_config_object_attach_replace could get a (somewhat redundant)
const char *child_name argument? Then passing NULL as the GVirConfigObject
*child would mean we want to remove the children? This would turn the
functions using this into one liners as the ones using set_node_content.

I'll need to look more carefully into the rest of the code, but this looks
quite mechanical and good.

Christophe

Attachment: pgp2kOUAFDy27.pgp
Description: PGP signature


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