[libvirt] [libvirt-glib 1/4] More generic gvir_config_object_add_child()
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> --- libvirt-gconfig/libvirt-gconfig-domain-channel.c | 6 ++++-- libvirt-gconfig/libvirt-gconfig-domain-console.c | 3 ++- libvirt-gconfig/libvirt-gconfig-domain-disk.c | 18 ++++++++++++------ libvirt-gconfig/libvirt-gconfig-domain-filesys.c | 6 ++++-- libvirt-gconfig/libvirt-gconfig-domain.c | 6 ++++-- libvirt-gconfig/libvirt-gconfig-object-private.h | 5 ++++- libvirt-gconfig/libvirt-gconfig-object.c | 13 ++++++++----- .../libvirt-gconfig-storage-pool-source.c | 3 ++- 8 files changed, 40 insertions(+), 20 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-channel.c b/libvirt-gconfig/libvirt-gconfig-domain-channel.c index a4f9527..f81cea2 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-channel.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-channel.c @@ -77,7 +77,8 @@ void gvir_config_domain_channel_set_target_type(GVirConfigDomainChannel *channel g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CHANNEL(channel)); gvir_config_object_add_child_with_attribute_enum(GVIR_CONFIG_OBJECT(channel), - "target", "type", + "target", GVIR_CONFIG_TYPE_OBJECT, + "type", GVIR_CONFIG_TYPE_DOMAIN_CHANNEL_TARGET_TYPE, type); } @@ -89,5 +90,6 @@ void gvir_config_domain_channel_set_target_name(GVirConfigDomainChannel *channel g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CHANNEL(channel)); gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(channel), - "target", "name", name); + "target", GVIR_CONFIG_TYPE_OBJECT, + "name", name); } diff --git a/libvirt-gconfig/libvirt-gconfig-domain-console.c b/libvirt-gconfig/libvirt-gconfig-domain-console.c index db97322..954f9d2 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-console.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-console.c @@ -76,7 +76,8 @@ void gvir_config_domain_console_set_target_type(GVirConfigDomainConsole *console g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CONSOLE(console)); gvir_config_object_add_child_with_attribute_enum(GVIR_CONFIG_OBJECT(console), - "target", "type", + "target", GVIR_CONFIG_TYPE_OBJECT, + "type", GVIR_CONFIG_TYPE_DOMAIN_CONSOLE_TARGET_TYPE, type); } diff --git a/libvirt-gconfig/libvirt-gconfig-domain-disk.c b/libvirt-gconfig/libvirt-gconfig-domain-disk.c index a29ea47..d5a5b70 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-disk.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-disk.c @@ -136,7 +136,8 @@ void gvir_config_domain_disk_set_startup_policy(GVirConfigDomainDisk *disk, str = gvir_config_genum_get_nick(GVIR_CONFIG_TYPE_DOMAIN_DISK_STARTUP_POLICY, policy); g_return_if_fail(str != NULL); gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(disk), - "source", "startupPolicy", str); + "source", GVIR_CONFIG_TYPE_OBJECT, + "startupPolicy", str); } void gvir_config_domain_disk_set_source(GVirConfigDomainDisk *disk, @@ -172,7 +173,8 @@ void gvir_config_domain_disk_set_driver_name(GVirConfigDomainDisk *disk, { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_DISK(disk)); gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(disk), - "driver", "name", driver_name); + "driver", GVIR_CONFIG_TYPE_OBJECT, + "name", driver_name); } void gvir_config_domain_disk_set_driver_type(GVirConfigDomainDisk *disk, @@ -180,7 +182,8 @@ void gvir_config_domain_disk_set_driver_type(GVirConfigDomainDisk *disk, { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_DISK(disk)); gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(disk), - "driver", "type", driver_type); + "driver", GVIR_CONFIG_TYPE_OBJECT, + "type", driver_type); } void gvir_config_domain_disk_set_driver_cache(GVirConfigDomainDisk *disk, @@ -192,7 +195,8 @@ void gvir_config_domain_disk_set_driver_cache(GVirConfigDomainDisk *disk, cache_str = gvir_config_genum_get_nick(GVIR_CONFIG_TYPE_DOMAIN_DISK_CACHE_TYPE, cache_type); g_return_if_fail(cache_str != NULL); gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(disk), - "driver", "cache", cache_str); + "driver", GVIR_CONFIG_TYPE_OBJECT, + "cache", cache_str); } void gvir_config_domain_disk_set_target_bus(GVirConfigDomainDisk *disk, @@ -204,7 +208,8 @@ void gvir_config_domain_disk_set_target_bus(GVirConfigDomainDisk *disk, bus_str = gvir_config_genum_get_nick(GVIR_CONFIG_TYPE_DOMAIN_DISK_BUS, bus); g_return_if_fail(bus_str != NULL); gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(disk), - "target", "bus", bus_str); + "target", GVIR_CONFIG_TYPE_OBJECT, + "bus", bus_str); } void gvir_config_domain_disk_set_target_dev(GVirConfigDomainDisk *disk, @@ -212,7 +217,8 @@ void gvir_config_domain_disk_set_target_dev(GVirConfigDomainDisk *disk, { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_DISK(disk)); gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(disk), - "target", "dev", dev); + "target", GVIR_CONFIG_TYPE_OBJECT, + "dev", dev); } GVirConfigDomainDiskType diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c index 904a7a3..8dcdff2 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c @@ -99,7 +99,8 @@ void gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys *filesys GVirConfigObject *node; g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys)); - node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), + "driver", GVIR_CONFIG_TYPE_OBJECT); g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node)); if (type != GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT) gvir_config_object_set_attribute_with_type( @@ -146,7 +147,8 @@ void gvir_config_domain_filesys_set_target(GVirConfigDomainFilesys *filesys, { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys)); gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(filesys), - "target", "dir", path); + "target", GVIR_CONFIG_TYPE_OBJECT, + "dir", path); } void gvir_config_domain_filesys_set_readonly(GVirConfigDomainFilesys *filesys, diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index c8cd1c5..04915ab 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -401,7 +401,8 @@ void gvir_config_domain_add_device(GVirConfigDomain *domain, g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(device)); devices_node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(domain), - "devices"); + "devices", + GVIR_CONFIG_TYPE_OBJECT); gvir_config_object_attach_add(devices_node, GVIR_CONFIG_OBJECT(device)); g_object_unref(G_OBJECT(devices_node)); @@ -465,7 +466,8 @@ gboolean gvir_config_domain_set_custom_xml(GVirConfigDomain *domain, g_return_val_if_fail(error == NULL || *error == NULL, FALSE); metadata = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(domain), - "metadata"); + "metadata", + GVIR_CONFIG_TYPE_OBJECT); custom_xml = gvir_config_object_new_from_xml(GVIR_CONFIG_TYPE_OBJECT, NULL, NULL, xml, error); diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h index a6b7395..ba9c676 100644 --- a/libvirt-gconfig/libvirt-gconfig-object-private.h +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h @@ -54,13 +54,16 @@ void gvir_config_object_set_node_content_uint64(GVirConfigObject *object, const char *node_name, guint64 value); GVirConfigObject *gvir_config_object_add_child(GVirConfigObject *object, - const char *child_name); + const char *child_name, + GType child_type); void gvir_config_object_add_child_with_attribute(GVirConfigObject *object, const char *child_name, + GType child_type, const char *attr_name, const char *attr_value); void gvir_config_object_add_child_with_attribute_enum(GVirConfigObject *object, const char *child_name, + GType child_type, const char *attr_name, GType attr_type, unsigned int attr_value); diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index ee3584a..df4836d 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -367,7 +367,8 @@ gvir_config_object_foreach_child(GVirConfigObject *object, G_GNUC_INTERNAL GVirConfigObject * gvir_config_object_add_child(GVirConfigObject *object, - const char *child_name) + const char *child_name, + GType child_type) { xmlNodePtr new_node; xmlNodePtr old_node; @@ -380,13 +381,13 @@ gvir_config_object_add_child(GVirConfigObject *object, FALSE); if (old_node != NULL) { xmlFreeNode(new_node); - return GVIR_CONFIG_OBJECT(g_object_new(GVIR_CONFIG_TYPE_OBJECT, + return GVIR_CONFIG_OBJECT(g_object_new(child_type, "doc", object->priv->doc, "node", old_node, NULL)); } - return GVIR_CONFIG_OBJECT(g_object_new(GVIR_CONFIG_TYPE_OBJECT, + return GVIR_CONFIG_OBJECT(g_object_new(child_type, "doc", object->priv->doc, "node", new_node, NULL)); @@ -395,12 +396,13 @@ gvir_config_object_add_child(GVirConfigObject *object, G_GNUC_INTERNAL void gvir_config_object_add_child_with_attribute(GVirConfigObject *object, const char *child_name, + GType child_type, const char *attr_name, const char *attr_value) { GVirConfigObject *child; - child = gvir_config_object_add_child(object, child_name); + child = gvir_config_object_add_child(object, child_name, child_type); gvir_config_object_set_attribute(child, attr_name, attr_value, NULL); g_object_unref(G_OBJECT(child)); } @@ -408,13 +410,14 @@ gvir_config_object_add_child_with_attribute(GVirConfigObject *object, void gvir_config_object_add_child_with_attribute_enum(GVirConfigObject *object, const char *child_name, + GType child_type, const char *attr_name, GType attr_type, unsigned int attr_value) { GVirConfigObject *child; - child = gvir_config_object_add_child(object, child_name); + child = gvir_config_object_add_child(object, child_name, child_type); gvir_config_object_set_attribute_with_type(child, attr_name, attr_type, attr_value, NULL); g_object_unref(G_OBJECT(child)); } diff --git a/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c b/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c index d92c692..e78989c 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c @@ -91,7 +91,8 @@ void gvir_config_storage_pool_source_set_device_path(GVirConfigStoragePoolSource g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL_SOURCE(source)); - node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(source), "device"); + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(source), + "device", GVIR_CONFIG_TYPE_OBJECT); g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node)); gvir_config_object_set_attribute(node, "path", device_path, NULL); g_object_unref(G_OBJECT(node)); -- 1.7.7.6
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> --- libvirt-gconfig/libvirt-gconfig-domain.c | 18 ++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain.h | 1 + libvirt-gconfig/libvirt-gconfig.sym | 5 +++++ 3 files changed, 24 insertions(+), 0 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index 04915ab..b19ac4c 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -323,6 +323,24 @@ void gvir_config_domain_set_clock(GVirConfigDomain *domain, GVIR_CONFIG_OBJECT(klock)); } +/** + * gvir_config_domain_get_os: + * + * Returns: (transfer full): + */ +GVirConfigDomainOs *gvir_config_domain_get_os(GVirConfigDomain *domain) +{ + GVirConfigObject *object; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN(domain), NULL); + + object = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(domain), + "os", + GVIR_CONFIG_TYPE_DOMAIN_OS); + + return GVIR_CONFIG_DOMAIN_OS(object); +} + void gvir_config_domain_set_os(GVirConfigDomain *domain, GVirConfigDomainOs *os) { diff --git a/libvirt-gconfig/libvirt-gconfig-domain.h b/libvirt-gconfig/libvirt-gconfig-domain.h index 1dbfd95..bdb842b 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.h +++ b/libvirt-gconfig/libvirt-gconfig-domain.h @@ -114,6 +114,7 @@ void gvir_config_domain_set_features(GVirConfigDomain *domain, const GStrv features); void gvir_config_domain_set_clock(GVirConfigDomain *domain, GVirConfigDomainClock *klock); +GVirConfigDomainOs *gvir_config_domain_get_os(GVirConfigDomain *domain); void gvir_config_domain_set_os(GVirConfigDomain *domain, GVirConfigDomainOs *os); void gvir_config_domain_set_seclabel(GVirConfigDomain *domain, diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index ffeb16b..67e9c3f 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -374,4 +374,9 @@ LIBVIRT_GCONFIG_0.0.8 { *; }; +LIBVIRT_GCONFIG_0.0.9 { + global: + gvir_config_domain_get_os; +} LIBVIRT_GCONFIG_0.0.8; + # .... define new API here using predicted next version number .... -- 1.7.7.6
On Fri, May 04, 2012 at 03:07:43AM +0300, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gconfig/libvirt-gconfig-domain.c | 18 ++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain.h | 1 + libvirt-gconfig/libvirt-gconfig.sym | 5 +++++ 3 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index 04915ab..b19ac4c 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -323,6 +323,24 @@ void gvir_config_domain_set_clock(GVirConfigDomain *domain, GVIR_CONFIG_OBJECT(klock)); }
+/** + * gvir_config_domain_get_os: + * + * Returns: (transfer full): + */ +GVirConfigDomainOs *gvir_config_domain_get_os(GVirConfigDomain *domain) +{ + GVirConfigObject *object; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN(domain), NULL); + + object = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(domain), + "os", + GVIR_CONFIG_TYPE_DOMAIN_OS); + + return GVIR_CONFIG_DOMAIN_OS(object); +}
We want to return a NULL object when there is no "os" node in the configuration I think, or to create a default node with the default values if the presence of this node is mandatory and if libvirt doesn't create it for us. Christophe
+ void gvir_config_domain_set_os(GVirConfigDomain *domain, GVirConfigDomainOs *os) { diff --git a/libvirt-gconfig/libvirt-gconfig-domain.h b/libvirt-gconfig/libvirt-gconfig-domain.h index 1dbfd95..bdb842b 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.h +++ b/libvirt-gconfig/libvirt-gconfig-domain.h @@ -114,6 +114,7 @@ void gvir_config_domain_set_features(GVirConfigDomain *domain, const GStrv features); void gvir_config_domain_set_clock(GVirConfigDomain *domain, GVirConfigDomainClock *klock); +GVirConfigDomainOs *gvir_config_domain_get_os(GVirConfigDomain *domain); void gvir_config_domain_set_os(GVirConfigDomain *domain, GVirConfigDomainOs *os); void gvir_config_domain_set_seclabel(GVirConfigDomain *domain, diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index ffeb16b..67e9c3f 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -374,4 +374,9 @@ LIBVIRT_GCONFIG_0.0.8 { *; };
+LIBVIRT_GCONFIG_0.0.9 { + global: + gvir_config_domain_get_os; +} 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
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"); 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)); } diff --git a/libvirt-gconfig/libvirt-gconfig-domain-os.c b/libvirt-gconfig/libvirt-gconfig-domain-os.c index 86f90a2..74cdd4d 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-os.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-os.c @@ -81,6 +81,10 @@ void gvir_config_domain_os_set_os_type(GVirConfigDomainOs *os, "type", type_str); } +/** + * gvir_config_domain_os_set_kernel: + * @kernel: (allow-none): The kernel path + */ void gvir_config_domain_os_set_kernel(GVirConfigDomainOs *os, const char * kernel) { @@ -88,6 +92,10 @@ void gvir_config_domain_os_set_kernel(GVirConfigDomainOs *os, "kernel", kernel); } +/** + * gvir_config_domain_os_set_ramdisk: + * @ramdisk: (allow-none): The ramdisk path + */ void gvir_config_domain_os_set_ramdisk(GVirConfigDomainOs *os, const char * ramdisk) { @@ -95,6 +103,10 @@ void gvir_config_domain_os_set_ramdisk(GVirConfigDomainOs *os, "initrd", ramdisk); } +/** + * gvir_config_domain_os_set_cmdline: + * @cmdline: (allow-none): The direct boot commandline + */ void gvir_config_domain_os_set_cmdline(GVirConfigDomainOs *os, const char * cmdline) { @@ -102,6 +114,10 @@ void gvir_config_domain_os_set_cmdline(GVirConfigDomainOs *os, "cmdline", cmdline); } +/** + * gvir_config_domain_os_set_init: + * @init: (allow-none): + */ void gvir_config_domain_os_set_init(GVirConfigDomainOs *os, const char * init) { @@ -109,6 +125,10 @@ void gvir_config_domain_os_set_init(GVirConfigDomainOs *os, "init", init); } +/** + * gvir_config_domain_os_set_loader: + * @loader: (allow-none): + */ void gvir_config_domain_os_set_loader(GVirConfigDomainOs *os, const char * loader) { diff --git a/libvirt-gconfig/libvirt-gconfig-domain-redirdev.c b/libvirt-gconfig/libvirt-gconfig-domain-redirdev.c index efecb5a..2aae03d 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-redirdev.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-redirdev.c @@ -81,12 +81,20 @@ void gvir_config_domain_redirdev_set_bus(GVirConfigDomainRedirdev *redirdev, NULL); } +/** + * gvir_config_domain_redirdev_set_address: + * @address: (allow-none): + */ void gvir_config_domain_redirdev_set_address(GVirConfigDomainRedirdev *redirdev, GVirConfigDomainAddress *address) { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_REDIRDEV(redirdev)); - g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_ADDRESS(address)); - gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(redirdev), - GVIR_CONFIG_OBJECT(address)); + if (address == NULL) + gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(redirdev), + "address", + NULL); + else + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(redirdev), + GVIR_CONFIG_OBJECT(address)); } diff --git a/libvirt-gconfig/libvirt-gconfig-domain-seclabel.c b/libvirt-gconfig/libvirt-gconfig-domain-seclabel.c index cc83797..9d9ec33 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-seclabel.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-seclabel.c @@ -89,6 +89,10 @@ void gvir_config_domain_seclabel_set_model(GVirConfigDomainSeclabel *seclabel, } +/** + * gvir_config_domain_seclabel_set_baselabel: + * @label: (allow-none): + */ void gvir_config_domain_seclabel_set_baselabel(GVirConfigDomainSeclabel *seclabel, const char *label) { @@ -98,6 +102,10 @@ void gvir_config_domain_seclabel_set_baselabel(GVirConfigDomainSeclabel *seclabe "baselabel", label); } +/** + * gvir_config_domain_seclabel_set_label: + * @label: (allow-none): + */ void gvir_config_domain_seclabel_set_label(GVirConfigDomainSeclabel *seclabel, const char *label) { diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index b19ac4c..aeeb659 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -204,6 +204,10 @@ const char *gvir_config_domain_get_name(GVirConfigDomain *domain) "name"); } +/** + * gvir_config_domain_set_name: + * @name: (allow-none): + */ void gvir_config_domain_set_name(GVirConfigDomain *domain, const char *name) { gvir_config_object_set_node_content(GVIR_CONFIG_OBJECT(domain), @@ -217,6 +221,10 @@ const char *gvir_config_domain_get_description(GVirConfigDomain *domain) "description"); } +/** + * gvir_config_domain_set_description: + * @description: (allow-none): + */ void gvir_config_domain_set_description(GVirConfigDomain *domain, const char *description) { gvir_config_object_set_node_content(GVIR_CONFIG_OBJECT(domain), @@ -313,14 +321,20 @@ void gvir_config_domain_set_features(GVirConfigDomain *domain, g_object_notify(G_OBJECT(domain), "features"); } +/** + * gvir_config_domain_set_clock: + * @klock: (allow-none): + */ void gvir_config_domain_set_clock(GVirConfigDomain *domain, GVirConfigDomainClock *klock) { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN(domain)); - g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CLOCK(klock)); - gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), - GVIR_CONFIG_OBJECT(klock)); + if (klock == NULL) + gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(domain), "clock", NULL); + else + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), + GVIR_CONFIG_OBJECT(klock)); } /** @@ -341,24 +355,38 @@ GVirConfigDomainOs *gvir_config_domain_get_os(GVirConfigDomain *domain) return GVIR_CONFIG_DOMAIN_OS(object); } +/** + * gvir_config_domain_set_os: + * @os: (allow-none): + */ void gvir_config_domain_set_os(GVirConfigDomain *domain, GVirConfigDomainOs *os) { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN(domain)); - g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_OS(os)); - gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), - GVIR_CONFIG_OBJECT(os)); + if (os == NULL) + gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(domain), "os", NULL); + else + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), + GVIR_CONFIG_OBJECT(os)); } +/** + * gvir_config_domain_set_seclabel: + * @seclabel: (allow-none): + */ void gvir_config_domain_set_seclabel(GVirConfigDomain *domain, GVirConfigDomainSeclabel *seclabel) { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN(domain)); - g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_SECLABEL(seclabel)); - gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), - GVIR_CONFIG_OBJECT(seclabel)); + if (seclabel == NULL) + gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(domain), + "seclabel", + NULL); + else + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), + GVIR_CONFIG_OBJECT(seclabel)); } void gvir_config_domain_set_lifecycle(GVirConfigDomain *domain, @@ -394,6 +422,14 @@ void gvir_config_domain_set_devices(GVirConfigDomain *domain, g_return_if_fail(GVIR_CONFIG_IS_DOMAIN(domain)); + if (devices == NULL) { + gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(domain), + "devices", + NULL); + + return; + } + devices_node = gvir_config_object_new(GVIR_CONFIG_TYPE_OBJECT, "devices", NULL); for (it = devices; it != NULL; it = it->next) { diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index df4836d..4115367 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -530,7 +530,12 @@ gvir_config_object_set_node_content(GVirConfigObject *object, g_return_if_fail(GVIR_CONFIG_IS_OBJECT(object)); g_return_if_fail(node_name != NULL); - g_return_if_fail(value != NULL); + + if (value == NULL) { + gvir_config_object_delete_child(object, node_name, NULL); + + return; + } node = gvir_config_object_replace_child(object, node_name); g_return_if_fail(node != NULL); diff --git a/libvirt-gconfig/libvirt-gconfig-storage-permissions.c b/libvirt-gconfig/libvirt-gconfig-storage-permissions.c index 57e50c4..5c0d40f 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-permissions.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-permissions.c @@ -79,6 +79,10 @@ void gvir_config_storage_permissions_set_group(GVirConfigStoragePermissions *per "group", group); } +/** + * gvir_config_storage_permissions_set_label: + * @label: (allow-none): + */ void gvir_config_storage_permissions_set_label(GVirConfigStoragePermissions *perms, const char *label) { diff --git a/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c b/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c index e78989c..2f5af57 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c @@ -137,6 +137,10 @@ void gvir_config_storage_pool_source_set_host(GVirConfigStoragePoolSource *sourc g_object_unref(G_OBJECT(node)); } +/** + * gvir_config_storage_pool_source_set_name: + * @name: (allow-none): + */ void gvir_config_storage_pool_source_set_name(GVirConfigStoragePoolSource *source, const char *name) { diff --git a/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c b/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c index 0d7f164..3ed1680 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c @@ -71,6 +71,10 @@ GVirConfigStoragePoolTarget *gvir_config_storage_pool_target_new_from_xml(const return GVIR_CONFIG_STORAGE_POOL_TARGET(object); } +/** + * gvir_config_storage_pool_target_set_path: + * @path: (allow-none): + */ void gvir_config_storage_pool_target_set_path(GVirConfigStoragePoolTarget *target, const char *path) { @@ -80,12 +84,20 @@ void gvir_config_storage_pool_target_set_path(GVirConfigStoragePoolTarget *targe "path", path); } +/** + * gvir_config_storage_pool_perms_set_permissions: + * @perms: (allow-none): + */ void gvir_config_storage_pool_target_set_permissions(GVirConfigStoragePoolTarget *target, GVirConfigStoragePermissions *perms) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL_TARGET(target)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_PERMISSIONS(perms)); - gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(target), - GVIR_CONFIG_OBJECT(perms)); + if (perms == NULL) + gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(target), + "permissions", + NULL); + else + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(target), + GVIR_CONFIG_OBJECT(perms)); } diff --git a/libvirt-gconfig/libvirt-gconfig-storage-pool.c b/libvirt-gconfig/libvirt-gconfig-storage-pool.c index c524c17..d98eb5a 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-pool.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-pool.c @@ -86,6 +86,10 @@ void gvir_config_storage_pool_set_pool_type(GVirConfigStoragePool *pool, NULL); } +/** + * gvir_config_storage_pool_set_name: + * @name: (allow-none): + */ void gvir_config_storage_pool_set_name(GVirConfigStoragePool *pool, const char *name) { @@ -95,6 +99,10 @@ void gvir_config_storage_pool_set_name(GVirConfigStoragePool *pool, "name", name); } +/** + * gvir_config_storage_pool_set_uuid: + * @uuid: (allow-none): + */ void gvir_config_storage_pool_set_uuid(GVirConfigStoragePool *pool, const char *uuid) { @@ -131,22 +139,38 @@ void gvir_config_storage_pool_set_available(GVirConfigStoragePool *pool, "available", available); } +/** + * gvir_config_storage_pool_set_source: + * @source: (allow-none): + */ void gvir_config_storage_pool_set_source(GVirConfigStoragePool *pool, GVirConfigStoragePoolSource *source) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL(pool)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL_SOURCE(source)); - gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(pool), - GVIR_CONFIG_OBJECT(source)); + if (source == NULL) + gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(pool), + "source", + NULL); + else + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(pool), + GVIR_CONFIG_OBJECT(source)); } +/** + * gvir_config_storage_pool_set_target: + * @target: (allow-none): + */ void gvir_config_storage_pool_set_target(GVirConfigStoragePool *pool, GVirConfigStoragePoolTarget *target) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL(pool)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL_TARGET(target)); - gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(pool), - GVIR_CONFIG_OBJECT(target)); + if (target == NULL) + gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(pool), + "target", + NULL); + else + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(pool), + GVIR_CONFIG_OBJECT(target)); } diff --git a/libvirt-gconfig/libvirt-gconfig-storage-vol-backing-store.c b/libvirt-gconfig/libvirt-gconfig-storage-vol-backing-store.c index 2e8aa22..620c8e6 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-vol-backing-store.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-vol-backing-store.c @@ -84,6 +84,10 @@ void gvir_config_storage_vol_backing_store_set_format(GVirConfigStorageVolBackin g_object_unref(G_OBJECT(node)); } +/** + * gvir_config_storage_vol_backing_store_set_path: + * @path: (allow-none): + */ void gvir_config_storage_vol_backing_store_set_path(GVirConfigStorageVolBackingStore *backing_store, const char *path) { diff --git a/libvirt-gconfig/libvirt-gconfig-storage-vol-target.c b/libvirt-gconfig/libvirt-gconfig-storage-vol-target.c index 3786e2b..1de545e 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-vol-target.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-vol-target.c @@ -84,12 +84,20 @@ void gvir_config_storage_vol_target_set_format(GVirConfigStorageVolTarget *targe g_object_unref(G_OBJECT(node)); } +/** + * gvir_config_storage_vol_target_set_permissions: + * @perms: (allow-none): + */ void gvir_config_storage_vol_target_set_permissions(GVirConfigStorageVolTarget *target, GVirConfigStoragePermissions *perms) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL_TARGET(target)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_PERMISSIONS(perms)); - gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(target), - GVIR_CONFIG_OBJECT(perms)); + if (perms == NULL) + gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(target), + "permissions", + NULL); + else + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(target), + GVIR_CONFIG_OBJECT(perms)); } diff --git a/libvirt-gconfig/libvirt-gconfig-storage-vol.c b/libvirt-gconfig/libvirt-gconfig-storage-vol.c index 6281c02..a030a5c 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-vol.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-vol.c @@ -74,6 +74,10 @@ GVirConfigStorageVol *gvir_config_storage_vol_new_from_xml(const gchar *xml, return GVIR_CONFIG_STORAGE_VOL(object); } +/** + * gvir_config_storage_vol_set_name: + * @name: (allow-none): + */ void gvir_config_storage_vol_set_name(GVirConfigStorageVol *vol, const char *name) { @@ -101,22 +105,38 @@ void gvir_config_storage_vol_set_allocation(GVirConfigStorageVol *vol, "allocation", allocation); } +/** + * gvir_config_storage_vol_set_target: + * @target: (allow-none): + */ void gvir_config_storage_vol_set_target(GVirConfigStorageVol *vol, GVirConfigStorageVolTarget *target) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL(vol)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL_TARGET(target)); - gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(vol), - GVIR_CONFIG_OBJECT(target)); + if (target == NULL) + gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(vol), + "target", + NULL); + else + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(vol), + GVIR_CONFIG_OBJECT(target)); } +/** + * gvir_config_storage_vol_set_backing_store: + * @backing_store: (allow-none): + */ void gvir_config_storage_vol_set_backing_store(GVirConfigStorageVol *vol, GVirConfigStorageVolBackingStore *backing_store) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL(vol)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL_BACKING_STORE(backing_store)); - gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(vol), - GVIR_CONFIG_OBJECT(backing_store)); + if (backing_store == NULL) + gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(vol), + "backingStore", + NULL); + else + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(vol), + GVIR_CONFIG_OBJECT(backing_store)); } -- 1.7.7.6
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
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> --- libvirt-gconfig/libvirt-gconfig-domain-os.c | 44 +++++++++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain-os.h | 1 + libvirt-gconfig/libvirt-gconfig.sym | 1 + 3 files changed, 46 insertions(+), 0 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-os.c b/libvirt-gconfig/libvirt-gconfig-domain-os.c index 74cdd4d..f1a75dd 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-os.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-os.c @@ -221,6 +221,50 @@ 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; + GVirConfigDomainOsBootDevice device; + 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) { + device = gvir_config_genum_get_value + (GVIR_CONFIG_TYPE_DOMAIN_OS_BOOT_DEVICE, + value, + GVIR_CONFIG_DOMAIN_OS_BOOT_DEVICE_HD); + *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 + * + * 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), + "devices", + 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
On Fri, May 04, 2012 at 03:07:45AM +0300, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gconfig/libvirt-gconfig-domain-os.c | 44 +++++++++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain-os.h | 1 + libvirt-gconfig/libvirt-gconfig.sym | 1 + 3 files changed, 46 insertions(+), 0 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-os.c b/libvirt-gconfig/libvirt-gconfig-domain-os.c index 74cdd4d..f1a75dd 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-os.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-os.c @@ -221,6 +221,50 @@ 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; + GVirConfigDomainOsBootDevice device; + 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 can be declared in this block.
+ device = gvir_config_genum_get_value + (GVIR_CONFIG_TYPE_DOMAIN_OS_BOOT_DEVICE, + value, + GVIR_CONFIG_DOMAIN_OS_BOOT_DEVICE_HD); + *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 + * + * 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), + "devices",
This should be NULL instead of "devices", we want to parse Looks good otherwise, Christophe
+ 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
I thought we had agreed to avoid the invasive patch complicating the common path, and to just go with a new gvir_config_object_add_child_with_type? This should probably even be get_child_with_type since when the needed node is not present, we don't want to create a new empty node but we want to return NULL. Christophe On Fri, May 04, 2012 at 03:07:42AM +0300, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gconfig/libvirt-gconfig-domain-channel.c | 6 ++++-- libvirt-gconfig/libvirt-gconfig-domain-console.c | 3 ++- libvirt-gconfig/libvirt-gconfig-domain-disk.c | 18 ++++++++++++------ libvirt-gconfig/libvirt-gconfig-domain-filesys.c | 6 ++++-- libvirt-gconfig/libvirt-gconfig-domain.c | 6 ++++-- libvirt-gconfig/libvirt-gconfig-object-private.h | 5 ++++- libvirt-gconfig/libvirt-gconfig-object.c | 13 ++++++++----- .../libvirt-gconfig-storage-pool-source.c | 3 ++- 8 files changed, 40 insertions(+), 20 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-channel.c b/libvirt-gconfig/libvirt-gconfig-domain-channel.c index a4f9527..f81cea2 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-channel.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-channel.c @@ -77,7 +77,8 @@ void gvir_config_domain_channel_set_target_type(GVirConfigDomainChannel *channel g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CHANNEL(channel));
gvir_config_object_add_child_with_attribute_enum(GVIR_CONFIG_OBJECT(channel), - "target", "type", + "target", GVIR_CONFIG_TYPE_OBJECT, + "type", GVIR_CONFIG_TYPE_DOMAIN_CHANNEL_TARGET_TYPE, type); } @@ -89,5 +90,6 @@ void gvir_config_domain_channel_set_target_name(GVirConfigDomainChannel *channel g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CHANNEL(channel));
gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(channel), - "target", "name", name); + "target", GVIR_CONFIG_TYPE_OBJECT, + "name", name); } diff --git a/libvirt-gconfig/libvirt-gconfig-domain-console.c b/libvirt-gconfig/libvirt-gconfig-domain-console.c index db97322..954f9d2 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-console.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-console.c @@ -76,7 +76,8 @@ void gvir_config_domain_console_set_target_type(GVirConfigDomainConsole *console g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CONSOLE(console));
gvir_config_object_add_child_with_attribute_enum(GVIR_CONFIG_OBJECT(console), - "target", "type", + "target", GVIR_CONFIG_TYPE_OBJECT, + "type", GVIR_CONFIG_TYPE_DOMAIN_CONSOLE_TARGET_TYPE, type); } diff --git a/libvirt-gconfig/libvirt-gconfig-domain-disk.c b/libvirt-gconfig/libvirt-gconfig-domain-disk.c index a29ea47..d5a5b70 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-disk.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-disk.c @@ -136,7 +136,8 @@ void gvir_config_domain_disk_set_startup_policy(GVirConfigDomainDisk *disk, str = gvir_config_genum_get_nick(GVIR_CONFIG_TYPE_DOMAIN_DISK_STARTUP_POLICY, policy); g_return_if_fail(str != NULL); gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(disk), - "source", "startupPolicy", str); + "source", GVIR_CONFIG_TYPE_OBJECT, + "startupPolicy", str); }
void gvir_config_domain_disk_set_source(GVirConfigDomainDisk *disk, @@ -172,7 +173,8 @@ void gvir_config_domain_disk_set_driver_name(GVirConfigDomainDisk *disk, { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_DISK(disk)); gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(disk), - "driver", "name", driver_name); + "driver", GVIR_CONFIG_TYPE_OBJECT, + "name", driver_name); }
void gvir_config_domain_disk_set_driver_type(GVirConfigDomainDisk *disk, @@ -180,7 +182,8 @@ void gvir_config_domain_disk_set_driver_type(GVirConfigDomainDisk *disk, { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_DISK(disk)); gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(disk), - "driver", "type", driver_type); + "driver", GVIR_CONFIG_TYPE_OBJECT, + "type", driver_type); }
void gvir_config_domain_disk_set_driver_cache(GVirConfigDomainDisk *disk, @@ -192,7 +195,8 @@ void gvir_config_domain_disk_set_driver_cache(GVirConfigDomainDisk *disk, cache_str = gvir_config_genum_get_nick(GVIR_CONFIG_TYPE_DOMAIN_DISK_CACHE_TYPE, cache_type); g_return_if_fail(cache_str != NULL); gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(disk), - "driver", "cache", cache_str); + "driver", GVIR_CONFIG_TYPE_OBJECT, + "cache", cache_str); }
void gvir_config_domain_disk_set_target_bus(GVirConfigDomainDisk *disk, @@ -204,7 +208,8 @@ void gvir_config_domain_disk_set_target_bus(GVirConfigDomainDisk *disk, bus_str = gvir_config_genum_get_nick(GVIR_CONFIG_TYPE_DOMAIN_DISK_BUS, bus); g_return_if_fail(bus_str != NULL); gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(disk), - "target", "bus", bus_str); + "target", GVIR_CONFIG_TYPE_OBJECT, + "bus", bus_str); }
void gvir_config_domain_disk_set_target_dev(GVirConfigDomainDisk *disk, @@ -212,7 +217,8 @@ void gvir_config_domain_disk_set_target_dev(GVirConfigDomainDisk *disk, { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_DISK(disk)); gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(disk), - "target", "dev", dev); + "target", GVIR_CONFIG_TYPE_OBJECT, + "dev", dev); }
GVirConfigDomainDiskType diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c index 904a7a3..8dcdff2 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c @@ -99,7 +99,8 @@ void gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys *filesys GVirConfigObject *node;
g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys)); - node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), + "driver", GVIR_CONFIG_TYPE_OBJECT); g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node)); if (type != GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT) gvir_config_object_set_attribute_with_type( @@ -146,7 +147,8 @@ void gvir_config_domain_filesys_set_target(GVirConfigDomainFilesys *filesys, { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys)); gvir_config_object_add_child_with_attribute(GVIR_CONFIG_OBJECT(filesys), - "target", "dir", path); + "target", GVIR_CONFIG_TYPE_OBJECT, + "dir", path); }
void gvir_config_domain_filesys_set_readonly(GVirConfigDomainFilesys *filesys, diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index c8cd1c5..04915ab 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -401,7 +401,8 @@ void gvir_config_domain_add_device(GVirConfigDomain *domain, g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(device));
devices_node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(domain), - "devices"); + "devices", + GVIR_CONFIG_TYPE_OBJECT);
gvir_config_object_attach_add(devices_node, GVIR_CONFIG_OBJECT(device)); g_object_unref(G_OBJECT(devices_node)); @@ -465,7 +466,8 @@ gboolean gvir_config_domain_set_custom_xml(GVirConfigDomain *domain, g_return_val_if_fail(error == NULL || *error == NULL, FALSE);
metadata = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(domain), - "metadata"); + "metadata", + GVIR_CONFIG_TYPE_OBJECT);
custom_xml = gvir_config_object_new_from_xml(GVIR_CONFIG_TYPE_OBJECT, NULL, NULL, xml, error); diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h index a6b7395..ba9c676 100644 --- a/libvirt-gconfig/libvirt-gconfig-object-private.h +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h @@ -54,13 +54,16 @@ void gvir_config_object_set_node_content_uint64(GVirConfigObject *object, const char *node_name, guint64 value); GVirConfigObject *gvir_config_object_add_child(GVirConfigObject *object, - const char *child_name); + const char *child_name, + GType child_type); void gvir_config_object_add_child_with_attribute(GVirConfigObject *object, const char *child_name, + GType child_type, const char *attr_name, const char *attr_value); void gvir_config_object_add_child_with_attribute_enum(GVirConfigObject *object, const char *child_name, + GType child_type, const char *attr_name, GType attr_type, unsigned int attr_value); diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index ee3584a..df4836d 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -367,7 +367,8 @@ gvir_config_object_foreach_child(GVirConfigObject *object,
G_GNUC_INTERNAL GVirConfigObject * gvir_config_object_add_child(GVirConfigObject *object, - const char *child_name) + const char *child_name, + GType child_type) { xmlNodePtr new_node; xmlNodePtr old_node; @@ -380,13 +381,13 @@ gvir_config_object_add_child(GVirConfigObject *object, FALSE); if (old_node != NULL) { xmlFreeNode(new_node); - return GVIR_CONFIG_OBJECT(g_object_new(GVIR_CONFIG_TYPE_OBJECT, + return GVIR_CONFIG_OBJECT(g_object_new(child_type, "doc", object->priv->doc, "node", old_node, NULL)); }
- return GVIR_CONFIG_OBJECT(g_object_new(GVIR_CONFIG_TYPE_OBJECT, + return GVIR_CONFIG_OBJECT(g_object_new(child_type, "doc", object->priv->doc, "node", new_node, NULL)); @@ -395,12 +396,13 @@ gvir_config_object_add_child(GVirConfigObject *object, G_GNUC_INTERNAL void gvir_config_object_add_child_with_attribute(GVirConfigObject *object, const char *child_name, + GType child_type, const char *attr_name, const char *attr_value) { GVirConfigObject *child;
- child = gvir_config_object_add_child(object, child_name); + child = gvir_config_object_add_child(object, child_name, child_type); gvir_config_object_set_attribute(child, attr_name, attr_value, NULL); g_object_unref(G_OBJECT(child)); } @@ -408,13 +410,14 @@ gvir_config_object_add_child_with_attribute(GVirConfigObject *object,
void gvir_config_object_add_child_with_attribute_enum(GVirConfigObject *object, const char *child_name, + GType child_type, const char *attr_name, GType attr_type, unsigned int attr_value) { GVirConfigObject *child;
- child = gvir_config_object_add_child(object, child_name); + child = gvir_config_object_add_child(object, child_name, child_type); gvir_config_object_set_attribute_with_type(child, attr_name, attr_type, attr_value, NULL); g_object_unref(G_OBJECT(child)); } diff --git a/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c b/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c index d92c692..e78989c 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c @@ -91,7 +91,8 @@ void gvir_config_storage_pool_source_set_device_path(GVirConfigStoragePoolSource
g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL_SOURCE(source));
- node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(source), "device"); + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(source), + "device", GVIR_CONFIG_TYPE_OBJECT); g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node)); gvir_config_object_set_attribute(node, "path", device_path, NULL); g_object_unref(G_OBJECT(node)); -- 1.7.7.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> This function should set 'path' as attribute of 'source' node rather than as child node. --- .../libvirt-gconfig-domain-chardev-source-pty.c | 11 ++++------- 1 files changed, 4 insertions(+), 7 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..201e123 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c @@ -82,13 +82,10 @@ GVirConfigDomainChardevSourcePty *gvir_config_domain_chardev_source_pty_new_from void gvir_config_domain_source_pty_set_path(GVirConfigDomainChardevSourcePty *pty, const char *path) { - GVirConfigObject *source; g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CHARDEV_SOURCE_PTY(pty)); - source = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(pty), - "source"); - g_return_if_fail(GVIR_CONFIG_IS_OBJECT(source)); - gvir_config_object_set_node_content(GVIR_CONFIG_OBJECT(source), - "path", path); - g_object_unref(G_OBJECT(source)); + gvir_config_object_replace_child_with_attribute(GVIR_CONFIG_OBJECT(pty), + "source", + "path", + path); } -- 1.7.7.6
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> - gvir_config_object_add_child_with_type() - gvir_config_object_get_child() - gvir_config_object_get_child_with_type() --- libvirt-gconfig/libvirt-gconfig-object-private.h | 8 ++++ libvirt-gconfig/libvirt-gconfig-object.c | 47 ++++++++++++++++++++-- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h index a6b7395..eb2cc09 100644 --- a/libvirt-gconfig/libvirt-gconfig-object-private.h +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h @@ -55,6 +55,9 @@ void gvir_config_object_set_node_content_uint64(GVirConfigObject *object, guint64 value); GVirConfigObject *gvir_config_object_add_child(GVirConfigObject *object, const char *child_name); +GVirConfigObject *gvir_config_object_add_child_with_type(GVirConfigObject *object, + const char *child_name, + GType child_type); void gvir_config_object_add_child_with_attribute(GVirConfigObject *object, const char *child_name, const char *attr_name, @@ -96,6 +99,11 @@ void gvir_config_object_foreach_child(GVirConfigObject *object, gboolean gvir_config_object_set_namespace(GVirConfigObject *object, const char *ns, const char *ns_uri); +GVirConfigObject *gvir_config_object_get_child(GVirConfigObject *object, + const gchar *child_name); +GVirConfigObject *gvir_config_object_get_child_with_type(GVirConfigObject *object, + const gchar *child_name, + GType child_type); G_END_DECLS diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index ee3584a..3dac59a 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -366,8 +366,9 @@ gvir_config_object_foreach_child(GVirConfigObject *object, } G_GNUC_INTERNAL GVirConfigObject * -gvir_config_object_add_child(GVirConfigObject *object, - const char *child_name) +gvir_config_object_add_child_with_type(GVirConfigObject *object, + const char *child_name, + GType child_type) { xmlNodePtr new_node; xmlNodePtr old_node; @@ -380,18 +381,27 @@ gvir_config_object_add_child(GVirConfigObject *object, FALSE); if (old_node != NULL) { xmlFreeNode(new_node); - return GVIR_CONFIG_OBJECT(g_object_new(GVIR_CONFIG_TYPE_OBJECT, + return GVIR_CONFIG_OBJECT(g_object_new(child_type, "doc", object->priv->doc, "node", old_node, NULL)); } - return GVIR_CONFIG_OBJECT(g_object_new(GVIR_CONFIG_TYPE_OBJECT, + return GVIR_CONFIG_OBJECT(g_object_new(child_type, "doc", object->priv->doc, "node", new_node, NULL)); } +G_GNUC_INTERNAL GVirConfigObject * +gvir_config_object_add_child(GVirConfigObject *object, + const char *child_name) +{ + return gvir_config_object_add_child_with_type(object, + child_name, + GVIR_CONFIG_TYPE_OBJECT); +} + G_GNUC_INTERNAL void gvir_config_object_add_child_with_attribute(GVirConfigObject *object, const char *child_name, @@ -865,3 +875,32 @@ gvir_config_object_set_namespace(GVirConfigObject *object, const char *ns, return TRUE; } + +G_GNUC_INTERNAL GVirConfigObject * +gvir_config_object_get_child_with_type(GVirConfigObject *object, + const gchar *child_name, + GType child_type) +{ + xmlNodePtr node; + + g_return_val_if_fail(GVIR_CONFIG_IS_OBJECT(object), NULL); + g_return_val_if_fail(child_name != NULL, NULL); + + node = gvir_config_xml_get_element(object->priv->node, child_name, NULL); + g_return_val_if_fail(node != NULL, NULL); + + return gvir_config_object_new_from_tree(child_type, + object->priv->doc, + object->priv->schema, + node); +} + +G_GNUC_INTERNAL GVirConfigObject * +gvir_config_object_get_child(GVirConfigObject *object, + const gchar *child_name) +{ + return gvir_config_object_get_child_with_type(object, + child_name, + GVIR_CONFIG_TYPE_OBJECT); +} + -- 1.7.7.6
On Wed, May 09, 2012 at 03:54:35AM +0300, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
- gvir_config_object_add_child_with_type() - gvir_config_object_get_child() - gvir_config_object_get_child_with_type() --- libvirt-gconfig/libvirt-gconfig-object-private.h | 8 ++++ libvirt-gconfig/libvirt-gconfig-object.c | 47 ++++++++++++++++++++-- 2 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h index a6b7395..eb2cc09 100644 --- a/libvirt-gconfig/libvirt-gconfig-object-private.h +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h @@ -55,6 +55,9 @@ void gvir_config_object_set_node_content_uint64(GVirConfigObject *object, guint64 value); GVirConfigObject *gvir_config_object_add_child(GVirConfigObject *object, const char *child_name); +GVirConfigObject *gvir_config_object_add_child_with_type(GVirConfigObject *object, + const char *child_name, + GType child_type);
This API is not used at all, just drop it
void gvir_config_object_add_child_with_attribute(GVirConfigObject *object, const char *child_name, const char *attr_name, @@ -96,6 +99,11 @@ void gvir_config_object_foreach_child(GVirConfigObject *object, gboolean gvir_config_object_set_namespace(GVirConfigObject *object, const char *ns, const char *ns_uri); +GVirConfigObject *gvir_config_object_get_child(GVirConfigObject *object, + const gchar *child_name); +GVirConfigObject *gvir_config_object_get_child_with_type(GVirConfigObject *object, + const gchar *child_name, + GType child_type);
G_END_DECLS
diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index ee3584a..3dac59a 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -366,8 +366,9 @@ gvir_config_object_foreach_child(GVirConfigObject *object, }
G_GNUC_INTERNAL GVirConfigObject * -gvir_config_object_add_child(GVirConfigObject *object, - const char *child_name) +gvir_config_object_add_child_with_type(GVirConfigObject *object, + const char *child_name, + GType child_type) { xmlNodePtr new_node; xmlNodePtr old_node; @@ -380,18 +381,27 @@ gvir_config_object_add_child(GVirConfigObject *object, FALSE); if (old_node != NULL) { xmlFreeNode(new_node); - return GVIR_CONFIG_OBJECT(g_object_new(GVIR_CONFIG_TYPE_OBJECT, + return GVIR_CONFIG_OBJECT(g_object_new(child_type, "doc", object->priv->doc, "node", old_node, NULL)); }
- return GVIR_CONFIG_OBJECT(g_object_new(GVIR_CONFIG_TYPE_OBJECT, + return GVIR_CONFIG_OBJECT(g_object_new(child_type, "doc", object->priv->doc, "node", new_node, NULL)); }
+G_GNUC_INTERNAL GVirConfigObject * +gvir_config_object_add_child(GVirConfigObject *object, + const char *child_name) +{ + return gvir_config_object_add_child_with_type(object, + child_name, + GVIR_CONFIG_TYPE_OBJECT); +} + G_GNUC_INTERNAL void gvir_config_object_add_child_with_attribute(GVirConfigObject *object, const char *child_name, @@ -865,3 +875,32 @@ gvir_config_object_set_namespace(GVirConfigObject *object, const char *ns,
return TRUE; } + +G_GNUC_INTERNAL GVirConfigObject * +gvir_config_object_get_child_with_type(GVirConfigObject *object, + const gchar *child_name, + GType child_type) +{ + xmlNodePtr node; + + g_return_val_if_fail(GVIR_CONFIG_IS_OBJECT(object), NULL); + g_return_val_if_fail(child_name != NULL, NULL); + + node = gvir_config_xml_get_element(object->priv->node, child_name, NULL); + g_return_val_if_fail(node != NULL, NULL);
Do we want to be extra paranoid and check that node->name is child_name?
+ + return gvir_config_object_new_from_tree(child_type, + object->priv->doc, + object->priv->schema, + node);
So I think this API is a bit too limited, and a bit dangerous, but I'm not sure what the best forward is. What I'm worried about is that it's basically casting a random xml node to an arbitrary type, and we don't really have a way to let the class it's being cast to make some sanity checks first. Maybe we could have a new_from_tree function pointer on GVirObjectClass and use that to create new instances of GVirObject. Child classes would be able to override it. Maybe we could use a GInitable. Maybe we should do something totally different, I don't really know, just random thinking ;) ACK with the add_child_with_type changes removed. Christophe
+} + +G_GNUC_INTERNAL GVirConfigObject * +gvir_config_object_get_child(GVirConfigObject *object, + const gchar *child_name) +{ + return gvir_config_object_get_child_with_type(object, + child_name, + GVIR_CONFIG_TYPE_OBJECT); +} + -- 1.7.7.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> --- libvirt-gconfig/libvirt-gconfig-domain.c | 18 ++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain.h | 1 + libvirt-gconfig/libvirt-gconfig.sym | 5 +++++ 3 files changed, 24 insertions(+), 0 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index c8cd1c5..b95515e 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -323,6 +323,24 @@ void gvir_config_domain_set_clock(GVirConfigDomain *domain, GVIR_CONFIG_OBJECT(klock)); } +/** + * gvir_config_domain_get_os: + * + * Returns: (transfer full): + */ +GVirConfigDomainOs *gvir_config_domain_get_os(GVirConfigDomain *domain) +{ + GVirConfigObject *object; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN(domain), NULL); + + object = gvir_config_object_get_child_with_type(GVIR_CONFIG_OBJECT(domain), + "os", + GVIR_CONFIG_TYPE_DOMAIN_OS); + + return GVIR_CONFIG_DOMAIN_OS(object); +} + void gvir_config_domain_set_os(GVirConfigDomain *domain, GVirConfigDomainOs *os) { diff --git a/libvirt-gconfig/libvirt-gconfig-domain.h b/libvirt-gconfig/libvirt-gconfig-domain.h index 1dbfd95..bdb842b 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.h +++ b/libvirt-gconfig/libvirt-gconfig-domain.h @@ -114,6 +114,7 @@ void gvir_config_domain_set_features(GVirConfigDomain *domain, const GStrv features); void gvir_config_domain_set_clock(GVirConfigDomain *domain, GVirConfigDomainClock *klock); +GVirConfigDomainOs *gvir_config_domain_get_os(GVirConfigDomain *domain); void gvir_config_domain_set_os(GVirConfigDomain *domain, GVirConfigDomainOs *os); void gvir_config_domain_set_seclabel(GVirConfigDomain *domain, diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index ffeb16b..67e9c3f 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -374,4 +374,9 @@ LIBVIRT_GCONFIG_0.0.8 { *; }; +LIBVIRT_GCONFIG_0.0.9 { + global: + gvir_config_domain_get_os; +} LIBVIRT_GCONFIG_0.0.8; + # .... define new API here using predicted next version number .... -- 1.7.7.6
ACK On Wed, May 09, 2012 at 03:54:36AM +0300, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gconfig/libvirt-gconfig-domain.c | 18 ++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain.h | 1 + libvirt-gconfig/libvirt-gconfig.sym | 5 +++++ 3 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index c8cd1c5..b95515e 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -323,6 +323,24 @@ void gvir_config_domain_set_clock(GVirConfigDomain *domain, GVIR_CONFIG_OBJECT(klock)); }
+/** + * gvir_config_domain_get_os: + * + * Returns: (transfer full): + */
Some documentation for this function while you are adding this comment ? ;)
+GVirConfigDomainOs *gvir_config_domain_get_os(GVirConfigDomain *domain) +{ + GVirConfigObject *object; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN(domain), NULL); + + object = gvir_config_object_get_child_with_type(GVIR_CONFIG_OBJECT(domain), + "os", + GVIR_CONFIG_TYPE_DOMAIN_OS); + + return GVIR_CONFIG_DOMAIN_OS(object); +} + void gvir_config_domain_set_os(GVirConfigDomain *domain, GVirConfigDomainOs *os) { diff --git a/libvirt-gconfig/libvirt-gconfig-domain.h b/libvirt-gconfig/libvirt-gconfig-domain.h index 1dbfd95..bdb842b 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.h +++ b/libvirt-gconfig/libvirt-gconfig-domain.h @@ -114,6 +114,7 @@ void gvir_config_domain_set_features(GVirConfigDomain *domain, const GStrv features); void gvir_config_domain_set_clock(GVirConfigDomain *domain, GVirConfigDomainClock *klock); +GVirConfigDomainOs *gvir_config_domain_get_os(GVirConfigDomain *domain); void gvir_config_domain_set_os(GVirConfigDomain *domain, GVirConfigDomainOs *os); void gvir_config_domain_set_seclabel(GVirConfigDomain *domain, diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index ffeb16b..67e9c3f 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -374,4 +374,9 @@ LIBVIRT_GCONFIG_0.0.8 { *; };
+LIBVIRT_GCONFIG_0.0.9 { + global: + gvir_config_domain_get_os; +} 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
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 | 9 ++- libvirt-gconfig/libvirt-gconfig-domain-os.c | 20 +++++++ libvirt-gconfig/libvirt-gconfig-domain-redirdev.c | 9 ++- libvirt-gconfig/libvirt-gconfig-domain-seclabel.c | 8 +++ libvirt-gconfig/libvirt-gconfig-domain.c | 59 +++++++++++++++----- libvirt-gconfig/libvirt-gconfig-object-private.h | 3 +- libvirt-gconfig/libvirt-gconfig-object.c | 18 +++++- .../libvirt-gconfig-storage-permissions.c | 4 + .../libvirt-gconfig-storage-pool-source.c | 4 + .../libvirt-gconfig-storage-pool-target.c | 14 ++++- libvirt-gconfig/libvirt-gconfig-storage-pool.c | 28 ++++++++- .../libvirt-gconfig-storage-vol-backing-store.c | 4 + .../libvirt-gconfig-storage-vol-target.c | 10 +++- libvirt-gconfig/libvirt-gconfig-storage-vol.c | 24 +++++++- 15 files changed, 185 insertions(+), 35 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c b/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c index 201e123..e9b37ea 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c @@ -84,6 +84,12 @@ void gvir_config_domain_source_pty_set_path(GVirConfigDomainChardevSourcePty *pt { 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; + } + gvir_config_object_replace_child_with_attribute(GVIR_CONFIG_OBJECT(pty), "source", "path", diff --git a/libvirt-gconfig/libvirt-gconfig-domain-controller.c b/libvirt-gconfig/libvirt-gconfig-domain-controller.c index 2024b54..dddb965 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-controller.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-controller.c @@ -115,12 +115,17 @@ 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)); + g_return_if_fail(address == NULL || GVIR_CONFIG_IS_DOMAIN_ADDRESS(address)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(controller), - GVIR_CONFIG_OBJECT(address)); + GVIR_CONFIG_OBJECT(address), + "address"); } diff --git a/libvirt-gconfig/libvirt-gconfig-domain-os.c b/libvirt-gconfig/libvirt-gconfig-domain-os.c index 86f90a2..74cdd4d 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-os.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-os.c @@ -81,6 +81,10 @@ void gvir_config_domain_os_set_os_type(GVirConfigDomainOs *os, "type", type_str); } +/** + * gvir_config_domain_os_set_kernel: + * @kernel: (allow-none): The kernel path + */ void gvir_config_domain_os_set_kernel(GVirConfigDomainOs *os, const char * kernel) { @@ -88,6 +92,10 @@ void gvir_config_domain_os_set_kernel(GVirConfigDomainOs *os, "kernel", kernel); } +/** + * gvir_config_domain_os_set_ramdisk: + * @ramdisk: (allow-none): The ramdisk path + */ void gvir_config_domain_os_set_ramdisk(GVirConfigDomainOs *os, const char * ramdisk) { @@ -95,6 +103,10 @@ void gvir_config_domain_os_set_ramdisk(GVirConfigDomainOs *os, "initrd", ramdisk); } +/** + * gvir_config_domain_os_set_cmdline: + * @cmdline: (allow-none): The direct boot commandline + */ void gvir_config_domain_os_set_cmdline(GVirConfigDomainOs *os, const char * cmdline) { @@ -102,6 +114,10 @@ void gvir_config_domain_os_set_cmdline(GVirConfigDomainOs *os, "cmdline", cmdline); } +/** + * gvir_config_domain_os_set_init: + * @init: (allow-none): + */ void gvir_config_domain_os_set_init(GVirConfigDomainOs *os, const char * init) { @@ -109,6 +125,10 @@ void gvir_config_domain_os_set_init(GVirConfigDomainOs *os, "init", init); } +/** + * gvir_config_domain_os_set_loader: + * @loader: (allow-none): + */ void gvir_config_domain_os_set_loader(GVirConfigDomainOs *os, const char * loader) { diff --git a/libvirt-gconfig/libvirt-gconfig-domain-redirdev.c b/libvirt-gconfig/libvirt-gconfig-domain-redirdev.c index efecb5a..79bcab5 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-redirdev.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-redirdev.c @@ -81,12 +81,17 @@ void gvir_config_domain_redirdev_set_bus(GVirConfigDomainRedirdev *redirdev, NULL); } +/** + * gvir_config_domain_redirdev_set_address: + * @address: (allow-none): + */ void gvir_config_domain_redirdev_set_address(GVirConfigDomainRedirdev *redirdev, GVirConfigDomainAddress *address) { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_REDIRDEV(redirdev)); - g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_ADDRESS(address)); + g_return_if_fail(address == NULL || GVIR_CONFIG_IS_DOMAIN_ADDRESS(address)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(redirdev), - GVIR_CONFIG_OBJECT(address)); + GVIR_CONFIG_OBJECT(address), + "address"); } diff --git a/libvirt-gconfig/libvirt-gconfig-domain-seclabel.c b/libvirt-gconfig/libvirt-gconfig-domain-seclabel.c index cc83797..9d9ec33 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-seclabel.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-seclabel.c @@ -89,6 +89,10 @@ void gvir_config_domain_seclabel_set_model(GVirConfigDomainSeclabel *seclabel, } +/** + * gvir_config_domain_seclabel_set_baselabel: + * @label: (allow-none): + */ void gvir_config_domain_seclabel_set_baselabel(GVirConfigDomainSeclabel *seclabel, const char *label) { @@ -98,6 +102,10 @@ void gvir_config_domain_seclabel_set_baselabel(GVirConfigDomainSeclabel *seclabe "baselabel", label); } +/** + * gvir_config_domain_seclabel_set_label: + * @label: (allow-none): + */ void gvir_config_domain_seclabel_set_label(GVirConfigDomainSeclabel *seclabel, const char *label) { diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index b95515e..281387a 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -204,6 +204,10 @@ const char *gvir_config_domain_get_name(GVirConfigDomain *domain) "name"); } +/** + * gvir_config_domain_set_name: + * @name: (allow-none): + */ void gvir_config_domain_set_name(GVirConfigDomain *domain, const char *name) { gvir_config_object_set_node_content(GVIR_CONFIG_OBJECT(domain), @@ -217,6 +221,10 @@ const char *gvir_config_domain_get_description(GVirConfigDomain *domain) "description"); } +/** + * gvir_config_domain_set_description: + * @description: (allow-none): + */ void gvir_config_domain_set_description(GVirConfigDomain *domain, const char *description) { gvir_config_object_set_node_content(GVIR_CONFIG_OBJECT(domain), @@ -313,6 +321,10 @@ void gvir_config_domain_set_features(GVirConfigDomain *domain, g_object_notify(G_OBJECT(domain), "features"); } +/** + * gvir_config_domain_set_clock: + * @klock: (allow-none): + */ void gvir_config_domain_set_clock(GVirConfigDomain *domain, GVirConfigDomainClock *klock) { @@ -320,7 +332,8 @@ void gvir_config_domain_set_clock(GVirConfigDomain *domain, g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CLOCK(klock)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), - GVIR_CONFIG_OBJECT(klock)); + GVIR_CONFIG_OBJECT(klock), + "clock"); } /** @@ -341,24 +354,35 @@ GVirConfigDomainOs *gvir_config_domain_get_os(GVirConfigDomain *domain) return GVIR_CONFIG_DOMAIN_OS(object); } +/** + * gvir_config_domain_set_os: + * @os: (allow-none): + */ void gvir_config_domain_set_os(GVirConfigDomain *domain, GVirConfigDomainOs *os) { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN(domain)); - g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_OS(os)); + g_return_if_fail(os == NULL || GVIR_CONFIG_IS_DOMAIN_OS(os)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), - GVIR_CONFIG_OBJECT(os)); + GVIR_CONFIG_OBJECT(os), + "os"); } +/** + * gvir_config_domain_set_seclabel: + * @seclabel: (allow-none): + */ void gvir_config_domain_set_seclabel(GVirConfigDomain *domain, GVirConfigDomainSeclabel *seclabel) { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN(domain)); - g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_SECLABEL(seclabel)); + g_return_if_fail(seclabel == NULL || + GVIR_CONFIG_IS_DOMAIN_SECLABEL(seclabel)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), - GVIR_CONFIG_OBJECT(seclabel)); + GVIR_CONFIG_OBJECT(seclabel), + "seclabel"); } void gvir_config_domain_set_lifecycle(GVirConfigDomain *domain, @@ -389,24 +413,29 @@ void gvir_config_domain_set_lifecycle(GVirConfigDomain *domain, void gvir_config_domain_set_devices(GVirConfigDomain *domain, GList *devices) { - GVirConfigObject *devices_node; + GVirConfigObject *devices_node = NULL; GList *it; g_return_if_fail(GVIR_CONFIG_IS_DOMAIN(domain)); - devices_node = gvir_config_object_new(GVIR_CONFIG_TYPE_OBJECT, - "devices", NULL); - for (it = devices; it != NULL; it = it->next) { - if (!GVIR_CONFIG_IS_DOMAIN_DEVICE(it->data)) { - g_warn_if_reached(); - continue; + if (devices != NULL) { + devices_node = gvir_config_object_new(GVIR_CONFIG_TYPE_OBJECT, + "devices", + NULL); + + for (it = devices; it != NULL; it = it->next) { + if (!GVIR_CONFIG_IS_DOMAIN_DEVICE(it->data)) { + g_warn_if_reached(); + continue; + } + gvir_config_object_attach_add(devices_node, + GVIR_CONFIG_OBJECT(it->data)); } - gvir_config_object_attach_add(devices_node, - GVIR_CONFIG_OBJECT(it->data)); } gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), - devices_node); + devices_node, + "devices"); g_object_unref(G_OBJECT(devices_node)); } diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h index eb2cc09..7cfcda6 100644 --- a/libvirt-gconfig/libvirt-gconfig-object-private.h +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h @@ -85,7 +85,8 @@ void gvir_config_object_set_child(GVirConfigObject *object, void gvir_config_object_attach_add(GVirConfigObject *parent, GVirConfigObject *child); void gvir_config_object_attach_replace(GVirConfigObject *parent, - GVirConfigObject *child); + GVirConfigObject *child, + const char *child_name); void gvir_config_object_set_attribute(GVirConfigObject *object, ...) G_GNUC_NULL_TERMINATED; void gvir_config_object_set_attribute_with_type(GVirConfigObject *object, diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index 3dac59a..0495a3f 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -537,7 +537,12 @@ gvir_config_object_set_node_content(GVirConfigObject *object, g_return_if_fail(GVIR_CONFIG_IS_OBJECT(object)); g_return_if_fail(node_name != NULL); - g_return_if_fail(value != NULL); + + if (value == NULL) { + gvir_config_object_delete_child(object, node_name, NULL); + + return; + } node = gvir_config_object_replace_child(object, node_name); g_return_if_fail(node != NULL); @@ -834,9 +839,16 @@ gvir_config_object_attach(GVirConfigObject *parent, GVirConfigObject *child, gbo } G_GNUC_INTERNAL void -gvir_config_object_attach_replace(GVirConfigObject *parent, GVirConfigObject *child) +gvir_config_object_attach_replace(GVirConfigObject *parent, + GVirConfigObject *child, + const char *child_name) { - gvir_config_object_attach(parent, child, TRUE); + g_return_if_fail(child_name != NULL); + + if (child == NULL) + gvir_config_object_delete_children(parent, child_name, NULL); + else + gvir_config_object_attach(parent, child, TRUE); } G_GNUC_INTERNAL void diff --git a/libvirt-gconfig/libvirt-gconfig-storage-permissions.c b/libvirt-gconfig/libvirt-gconfig-storage-permissions.c index 57e50c4..5c0d40f 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-permissions.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-permissions.c @@ -79,6 +79,10 @@ void gvir_config_storage_permissions_set_group(GVirConfigStoragePermissions *per "group", group); } +/** + * gvir_config_storage_permissions_set_label: + * @label: (allow-none): + */ void gvir_config_storage_permissions_set_label(GVirConfigStoragePermissions *perms, const char *label) { diff --git a/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c b/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c index d92c692..c9a14da 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c @@ -136,6 +136,10 @@ void gvir_config_storage_pool_source_set_host(GVirConfigStoragePoolSource *sourc g_object_unref(G_OBJECT(node)); } +/** + * gvir_config_storage_pool_source_set_name: + * @name: (allow-none): + */ void gvir_config_storage_pool_source_set_name(GVirConfigStoragePoolSource *source, const char *name) { diff --git a/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c b/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c index 0d7f164..f90f0f1 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c @@ -71,6 +71,10 @@ GVirConfigStoragePoolTarget *gvir_config_storage_pool_target_new_from_xml(const return GVIR_CONFIG_STORAGE_POOL_TARGET(object); } +/** + * gvir_config_storage_pool_target_set_path: + * @path: (allow-none): + */ void gvir_config_storage_pool_target_set_path(GVirConfigStoragePoolTarget *target, const char *path) { @@ -80,12 +84,18 @@ void gvir_config_storage_pool_target_set_path(GVirConfigStoragePoolTarget *targe "path", path); } +/** + * gvir_config_storage_pool_perms_set_permissions: + * @perms: (allow-none): + */ void gvir_config_storage_pool_target_set_permissions(GVirConfigStoragePoolTarget *target, GVirConfigStoragePermissions *perms) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL_TARGET(target)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_PERMISSIONS(perms)); + g_return_if_fail(perms == NULL || + GVIR_CONFIG_IS_STORAGE_PERMISSIONS(perms)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(target), - GVIR_CONFIG_OBJECT(perms)); + GVIR_CONFIG_OBJECT(perms), + "permissions"); } diff --git a/libvirt-gconfig/libvirt-gconfig-storage-pool.c b/libvirt-gconfig/libvirt-gconfig-storage-pool.c index c524c17..ac58c4e 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-pool.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-pool.c @@ -86,6 +86,10 @@ void gvir_config_storage_pool_set_pool_type(GVirConfigStoragePool *pool, NULL); } +/** + * gvir_config_storage_pool_set_name: + * @name: (allow-none): + */ void gvir_config_storage_pool_set_name(GVirConfigStoragePool *pool, const char *name) { @@ -95,6 +99,10 @@ void gvir_config_storage_pool_set_name(GVirConfigStoragePool *pool, "name", name); } +/** + * gvir_config_storage_pool_set_uuid: + * @uuid: (allow-none): + */ void gvir_config_storage_pool_set_uuid(GVirConfigStoragePool *pool, const char *uuid) { @@ -131,22 +139,34 @@ void gvir_config_storage_pool_set_available(GVirConfigStoragePool *pool, "available", available); } +/** + * gvir_config_storage_pool_set_source: + * @source: (allow-none): + */ void gvir_config_storage_pool_set_source(GVirConfigStoragePool *pool, GVirConfigStoragePoolSource *source) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL(pool)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL_SOURCE(source)); + g_return_if_fail(source == NULL || + GVIR_CONFIG_IS_STORAGE_POOL_SOURCE(source)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(pool), - GVIR_CONFIG_OBJECT(source)); + GVIR_CONFIG_OBJECT(source), + "source"); } +/** + * gvir_config_storage_pool_set_target: + * @target: (allow-none): + */ void gvir_config_storage_pool_set_target(GVirConfigStoragePool *pool, GVirConfigStoragePoolTarget *target) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL(pool)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL_TARGET(target)); + g_return_if_fail(target == NULL || + GVIR_CONFIG_IS_STORAGE_POOL_TARGET(target)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(pool), - GVIR_CONFIG_OBJECT(target)); + GVIR_CONFIG_OBJECT(target), + "target"); } diff --git a/libvirt-gconfig/libvirt-gconfig-storage-vol-backing-store.c b/libvirt-gconfig/libvirt-gconfig-storage-vol-backing-store.c index 2e8aa22..620c8e6 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-vol-backing-store.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-vol-backing-store.c @@ -84,6 +84,10 @@ void gvir_config_storage_vol_backing_store_set_format(GVirConfigStorageVolBackin g_object_unref(G_OBJECT(node)); } +/** + * gvir_config_storage_vol_backing_store_set_path: + * @path: (allow-none): + */ void gvir_config_storage_vol_backing_store_set_path(GVirConfigStorageVolBackingStore *backing_store, const char *path) { diff --git a/libvirt-gconfig/libvirt-gconfig-storage-vol-target.c b/libvirt-gconfig/libvirt-gconfig-storage-vol-target.c index 3786e2b..f242219 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-vol-target.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-vol-target.c @@ -84,12 +84,18 @@ void gvir_config_storage_vol_target_set_format(GVirConfigStorageVolTarget *targe g_object_unref(G_OBJECT(node)); } +/** + * gvir_config_storage_vol_target_set_permissions: + * @perms: (allow-none): + */ void gvir_config_storage_vol_target_set_permissions(GVirConfigStorageVolTarget *target, GVirConfigStoragePermissions *perms) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL_TARGET(target)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_PERMISSIONS(perms)); + g_return_if_fail(perms == NULL || + GVIR_CONFIG_IS_STORAGE_PERMISSIONS(perms)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(target), - GVIR_CONFIG_OBJECT(perms)); + GVIR_CONFIG_OBJECT(perms), + "permissions"); } diff --git a/libvirt-gconfig/libvirt-gconfig-storage-vol.c b/libvirt-gconfig/libvirt-gconfig-storage-vol.c index 6281c02..0bd0805 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-vol.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-vol.c @@ -74,6 +74,10 @@ GVirConfigStorageVol *gvir_config_storage_vol_new_from_xml(const gchar *xml, return GVIR_CONFIG_STORAGE_VOL(object); } +/** + * gvir_config_storage_vol_set_name: + * @name: (allow-none): + */ void gvir_config_storage_vol_set_name(GVirConfigStorageVol *vol, const char *name) { @@ -101,22 +105,34 @@ void gvir_config_storage_vol_set_allocation(GVirConfigStorageVol *vol, "allocation", allocation); } +/** + * gvir_config_storage_vol_set_target: + * @target: (allow-none): + */ void gvir_config_storage_vol_set_target(GVirConfigStorageVol *vol, GVirConfigStorageVolTarget *target) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL(vol)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL_TARGET(target)); + g_return_if_fail(target == NULL || + GVIR_CONFIG_IS_STORAGE_VOL_TARGET(target)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(vol), - GVIR_CONFIG_OBJECT(target)); + GVIR_CONFIG_OBJECT(target), + "target"); } +/** + * gvir_config_storage_vol_set_backing_store: + * @backing_store: (allow-none): + */ void gvir_config_storage_vol_set_backing_store(GVirConfigStorageVol *vol, GVirConfigStorageVolBackingStore *backing_store) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL(vol)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL_BACKING_STORE(backing_store)); + g_return_if_fail(backing_store == NULL || + GVIR_CONFIG_IS_STORAGE_VOL_BACKING_STORE(backing_store)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(vol), - GVIR_CONFIG_OBJECT(backing_store)); + GVIR_CONFIG_OBJECT(backing_store), + "backingStore"); } -- 1.7.7.6
Hey, I've split this patch in 2 to make it more digestable. First one is unchanged and only contains the gvir_config_object_set_node_content changes The 2nd one only has the gvir_config_object_attach_replace changes, and it has some changes: I reordered the parameter to _attach_replace to make the order similar to most other functions, and I changed the GVIR_CONFIG_OBJECT() casts to (GVirConfigObject *) otherwise we'd get a runtime check when passing a NULL pointer. gvir_config_domain_set_devices could try to unref a NULL pointer, I've changed that, and gvir_config_domain_set_clock was still rejecting NULL klock parameters through a g_return_val_if_fail. I've dropped the gvir_config_domain_source_pty_set_path change for now since there are other places where _replace_child_with_attribute is used and could handle NULL pointers as well. ACK on these 2 patches, but you should review the 2nd one for obvious mistakes. Christophe
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> This changes gvir_config_object_set_node_content to accept a NULL content and to remove the corresponding node when this happens --- libvirt-gconfig/libvirt-gconfig-domain-os.c | 20 ++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain-seclabel.c | 8 ++++++++ libvirt-gconfig/libvirt-gconfig-domain.c | 8 ++++++++ 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 | 4 ++++ libvirt-gconfig/libvirt-gconfig-storage-pool.c | 8 ++++++++ .../libvirt-gconfig-storage-vol-backing-store.c | 4 ++++ libvirt-gconfig/libvirt-gconfig-storage-vol.c | 4 ++++ 10 files changed, 70 insertions(+), 1 deletion(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-os.c b/libvirt-gconfig/libvirt-gconfig-domain-os.c index 86f90a2..74cdd4d 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-os.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-os.c @@ -81,6 +81,10 @@ void gvir_config_domain_os_set_os_type(GVirConfigDomainOs *os, "type", type_str); } +/** + * gvir_config_domain_os_set_kernel: + * @kernel: (allow-none): The kernel path + */ void gvir_config_domain_os_set_kernel(GVirConfigDomainOs *os, const char * kernel) { @@ -88,6 +92,10 @@ void gvir_config_domain_os_set_kernel(GVirConfigDomainOs *os, "kernel", kernel); } +/** + * gvir_config_domain_os_set_ramdisk: + * @ramdisk: (allow-none): The ramdisk path + */ void gvir_config_domain_os_set_ramdisk(GVirConfigDomainOs *os, const char * ramdisk) { @@ -95,6 +103,10 @@ void gvir_config_domain_os_set_ramdisk(GVirConfigDomainOs *os, "initrd", ramdisk); } +/** + * gvir_config_domain_os_set_cmdline: + * @cmdline: (allow-none): The direct boot commandline + */ void gvir_config_domain_os_set_cmdline(GVirConfigDomainOs *os, const char * cmdline) { @@ -102,6 +114,10 @@ void gvir_config_domain_os_set_cmdline(GVirConfigDomainOs *os, "cmdline", cmdline); } +/** + * gvir_config_domain_os_set_init: + * @init: (allow-none): + */ void gvir_config_domain_os_set_init(GVirConfigDomainOs *os, const char * init) { @@ -109,6 +125,10 @@ void gvir_config_domain_os_set_init(GVirConfigDomainOs *os, "init", init); } +/** + * gvir_config_domain_os_set_loader: + * @loader: (allow-none): + */ void gvir_config_domain_os_set_loader(GVirConfigDomainOs *os, const char * loader) { diff --git a/libvirt-gconfig/libvirt-gconfig-domain-seclabel.c b/libvirt-gconfig/libvirt-gconfig-domain-seclabel.c index cc83797..9d9ec33 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-seclabel.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-seclabel.c @@ -89,6 +89,10 @@ void gvir_config_domain_seclabel_set_model(GVirConfigDomainSeclabel *seclabel, } +/** + * gvir_config_domain_seclabel_set_baselabel: + * @label: (allow-none): + */ void gvir_config_domain_seclabel_set_baselabel(GVirConfigDomainSeclabel *seclabel, const char *label) { @@ -98,6 +102,10 @@ void gvir_config_domain_seclabel_set_baselabel(GVirConfigDomainSeclabel *seclabe "baselabel", label); } +/** + * gvir_config_domain_seclabel_set_label: + * @label: (allow-none): + */ void gvir_config_domain_seclabel_set_label(GVirConfigDomainSeclabel *seclabel, const char *label) { diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index b95515e..d100009 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -204,6 +204,10 @@ const char *gvir_config_domain_get_name(GVirConfigDomain *domain) "name"); } +/** + * gvir_config_domain_set_name: + * @name: (allow-none): + */ void gvir_config_domain_set_name(GVirConfigDomain *domain, const char *name) { gvir_config_object_set_node_content(GVIR_CONFIG_OBJECT(domain), @@ -217,6 +221,10 @@ const char *gvir_config_domain_get_description(GVirConfigDomain *domain) "description"); } +/** + * gvir_config_domain_set_description: + * @description: (allow-none): + */ void gvir_config_domain_set_description(GVirConfigDomain *domain, const char *description) { gvir_config_object_set_node_content(GVIR_CONFIG_OBJECT(domain), diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index 3dac59a..288bbc4 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -537,7 +537,12 @@ gvir_config_object_set_node_content(GVirConfigObject *object, g_return_if_fail(GVIR_CONFIG_IS_OBJECT(object)); g_return_if_fail(node_name != NULL); - g_return_if_fail(value != NULL); + + if (value == NULL) { + gvir_config_object_delete_child(object, node_name, NULL); + + return; + } node = gvir_config_object_replace_child(object, node_name); g_return_if_fail(node != NULL); diff --git a/libvirt-gconfig/libvirt-gconfig-storage-permissions.c b/libvirt-gconfig/libvirt-gconfig-storage-permissions.c index 57e50c4..5c0d40f 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-permissions.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-permissions.c @@ -79,6 +79,10 @@ void gvir_config_storage_permissions_set_group(GVirConfigStoragePermissions *per "group", group); } +/** + * gvir_config_storage_permissions_set_label: + * @label: (allow-none): + */ void gvir_config_storage_permissions_set_label(GVirConfigStoragePermissions *perms, const char *label) { diff --git a/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c b/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c index d92c692..c9a14da 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-pool-source.c @@ -136,6 +136,10 @@ void gvir_config_storage_pool_source_set_host(GVirConfigStoragePoolSource *sourc g_object_unref(G_OBJECT(node)); } +/** + * gvir_config_storage_pool_source_set_name: + * @name: (allow-none): + */ void gvir_config_storage_pool_source_set_name(GVirConfigStoragePoolSource *source, const char *name) { diff --git a/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c b/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c index 0d7f164..bf97194 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c @@ -71,6 +71,10 @@ GVirConfigStoragePoolTarget *gvir_config_storage_pool_target_new_from_xml(const return GVIR_CONFIG_STORAGE_POOL_TARGET(object); } +/** + * gvir_config_storage_pool_target_set_path: + * @path: (allow-none): + */ void gvir_config_storage_pool_target_set_path(GVirConfigStoragePoolTarget *target, const char *path) { diff --git a/libvirt-gconfig/libvirt-gconfig-storage-pool.c b/libvirt-gconfig/libvirt-gconfig-storage-pool.c index c524c17..1bbcfe9 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-pool.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-pool.c @@ -86,6 +86,10 @@ void gvir_config_storage_pool_set_pool_type(GVirConfigStoragePool *pool, NULL); } +/** + * gvir_config_storage_pool_set_name: + * @name: (allow-none): + */ void gvir_config_storage_pool_set_name(GVirConfigStoragePool *pool, const char *name) { @@ -95,6 +99,10 @@ void gvir_config_storage_pool_set_name(GVirConfigStoragePool *pool, "name", name); } +/** + * gvir_config_storage_pool_set_uuid: + * @uuid: (allow-none): + */ void gvir_config_storage_pool_set_uuid(GVirConfigStoragePool *pool, const char *uuid) { diff --git a/libvirt-gconfig/libvirt-gconfig-storage-vol-backing-store.c b/libvirt-gconfig/libvirt-gconfig-storage-vol-backing-store.c index 2e8aa22..620c8e6 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-vol-backing-store.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-vol-backing-store.c @@ -84,6 +84,10 @@ void gvir_config_storage_vol_backing_store_set_format(GVirConfigStorageVolBackin g_object_unref(G_OBJECT(node)); } +/** + * gvir_config_storage_vol_backing_store_set_path: + * @path: (allow-none): + */ void gvir_config_storage_vol_backing_store_set_path(GVirConfigStorageVolBackingStore *backing_store, const char *path) { diff --git a/libvirt-gconfig/libvirt-gconfig-storage-vol.c b/libvirt-gconfig/libvirt-gconfig-storage-vol.c index 6281c02..e20dca3 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-vol.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-vol.c @@ -74,6 +74,10 @@ GVirConfigStorageVol *gvir_config_storage_vol_new_from_xml(const gchar *xml, return GVIR_CONFIG_STORAGE_VOL(object); } +/** + * gvir_config_storage_vol_set_name: + * @name: (allow-none): + */ void gvir_config_storage_vol_set_name(GVirConfigStorageVol *vol, const char *name) { -- 1.7.10.1
On Thu, May 10, 2012 at 9:58 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
This changes gvir_config_object_set_node_content to accept a NULL content and to remove the corresponding node when this happens
Wonder if i can ACK this change but if i can then ACK for both patches. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> This changes gvir_config_object_attach_replace to accept a NULL child object. Since we need to be able to find the node to remove when this happens, gvir_config_object_attach_replace gets a third argument with the name of the node to add/remove. --- .../libvirt-gconfig-domain-controller.c | 9 ++++-- libvirt-gconfig/libvirt-gconfig-domain-redirdev.c | 9 ++++-- libvirt-gconfig/libvirt-gconfig-domain.c | 34 ++++++++++++++++---- libvirt-gconfig/libvirt-gconfig-object-private.h | 1 + libvirt-gconfig/libvirt-gconfig-object.c | 11 +++++-- .../libvirt-gconfig-storage-pool-target.c | 10 ++++-- libvirt-gconfig/libvirt-gconfig-storage-pool.c | 20 +++++++++--- .../libvirt-gconfig-storage-vol-target.c | 10 ++++-- libvirt-gconfig/libvirt-gconfig-storage-vol.c | 20 +++++++++--- 9 files changed, 100 insertions(+), 24 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-controller.c b/libvirt-gconfig/libvirt-gconfig-domain-controller.c index 2024b54..4fed84c 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-controller.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-controller.c @@ -115,12 +115,17 @@ 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)); + g_return_if_fail(address == NULL || GVIR_CONFIG_IS_DOMAIN_ADDRESS(address)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(controller), - GVIR_CONFIG_OBJECT(address)); + "address", + (GVirConfigObject *)address); } diff --git a/libvirt-gconfig/libvirt-gconfig-domain-redirdev.c b/libvirt-gconfig/libvirt-gconfig-domain-redirdev.c index efecb5a..435abff 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-redirdev.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-redirdev.c @@ -81,12 +81,17 @@ void gvir_config_domain_redirdev_set_bus(GVirConfigDomainRedirdev *redirdev, NULL); } +/** + * gvir_config_domain_redirdev_set_address: + * @address: (allow-none): + */ void gvir_config_domain_redirdev_set_address(GVirConfigDomainRedirdev *redirdev, GVirConfigDomainAddress *address) { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_REDIRDEV(redirdev)); - g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_ADDRESS(address)); + g_return_if_fail(address == NULL || GVIR_CONFIG_IS_DOMAIN_ADDRESS(address)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(redirdev), - GVIR_CONFIG_OBJECT(address)); + "address", + (GVirConfigObject *)address); } diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index d100009..52ce0ef 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -321,14 +321,19 @@ void gvir_config_domain_set_features(GVirConfigDomain *domain, g_object_notify(G_OBJECT(domain), "features"); } +/** + * gvir_config_domain_set_clock: + * @klock: (allow-none): + */ void gvir_config_domain_set_clock(GVirConfigDomain *domain, GVirConfigDomainClock *klock) { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN(domain)); - g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CLOCK(klock)); + g_return_if_fail(klock != NULL || GVIR_CONFIG_IS_DOMAIN_CLOCK(klock)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), - GVIR_CONFIG_OBJECT(klock)); + "clock", + (GVirConfigObject *)klock); } /** @@ -349,24 +354,35 @@ GVirConfigDomainOs *gvir_config_domain_get_os(GVirConfigDomain *domain) return GVIR_CONFIG_DOMAIN_OS(object); } +/** + * gvir_config_domain_set_os: + * @os: (allow-none): + */ void gvir_config_domain_set_os(GVirConfigDomain *domain, GVirConfigDomainOs *os) { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN(domain)); - g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_OS(os)); + g_return_if_fail(os == NULL || GVIR_CONFIG_IS_DOMAIN_OS(os)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), - GVIR_CONFIG_OBJECT(os)); + "os", + (GVirConfigObject *)os); } +/** + * gvir_config_domain_set_seclabel: + * @seclabel: (allow-none): + */ void gvir_config_domain_set_seclabel(GVirConfigDomain *domain, GVirConfigDomainSeclabel *seclabel) { g_return_if_fail(GVIR_CONFIG_IS_DOMAIN(domain)); - g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_SECLABEL(seclabel)); + g_return_if_fail(seclabel == NULL || + GVIR_CONFIG_IS_DOMAIN_SECLABEL(seclabel)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), - GVIR_CONFIG_OBJECT(seclabel)); + "seclabel", + (GVirConfigObject *)seclabel); } void gvir_config_domain_set_lifecycle(GVirConfigDomain *domain, @@ -402,8 +418,13 @@ void gvir_config_domain_set_devices(GVirConfigDomain *domain, g_return_if_fail(GVIR_CONFIG_IS_DOMAIN(domain)); + if (devices == NULL) { + gvir_config_object_delete_children(GVIR_CONFIG_OBJECT(domain), "devices", NULL); + return; + } devices_node = gvir_config_object_new(GVIR_CONFIG_TYPE_OBJECT, "devices", NULL); + for (it = devices; it != NULL; it = it->next) { if (!GVIR_CONFIG_IS_DOMAIN_DEVICE(it->data)) { g_warn_if_reached(); @@ -414,6 +435,7 @@ void gvir_config_domain_set_devices(GVirConfigDomain *domain, } gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), + "devices", devices_node); g_object_unref(G_OBJECT(devices_node)); } diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h index eb2cc09..a22b945 100644 --- a/libvirt-gconfig/libvirt-gconfig-object-private.h +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h @@ -85,6 +85,7 @@ void gvir_config_object_set_child(GVirConfigObject *object, void gvir_config_object_attach_add(GVirConfigObject *parent, GVirConfigObject *child); void gvir_config_object_attach_replace(GVirConfigObject *parent, + const char *child_name, GVirConfigObject *child); void gvir_config_object_set_attribute(GVirConfigObject *object, ...) G_GNUC_NULL_TERMINATED; diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index 288bbc4..76e3134 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -839,9 +839,16 @@ gvir_config_object_attach(GVirConfigObject *parent, GVirConfigObject *child, gbo } G_GNUC_INTERNAL void -gvir_config_object_attach_replace(GVirConfigObject *parent, GVirConfigObject *child) +gvir_config_object_attach_replace(GVirConfigObject *parent, + const char *child_name, + GVirConfigObject *child) { - gvir_config_object_attach(parent, child, TRUE); + g_return_if_fail(child_name != NULL); + + if (child == NULL) + gvir_config_object_delete_children(parent, child_name, NULL); + else + gvir_config_object_attach(parent, child, TRUE); } G_GNUC_INTERNAL void diff --git a/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c b/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c index bf97194..cd8c9ee 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c @@ -84,12 +84,18 @@ void gvir_config_storage_pool_target_set_path(GVirConfigStoragePoolTarget *targe "path", path); } +/** + * gvir_config_storage_pool_perms_set_permissions: + * @perms: (allow-none): + */ void gvir_config_storage_pool_target_set_permissions(GVirConfigStoragePoolTarget *target, GVirConfigStoragePermissions *perms) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL_TARGET(target)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_PERMISSIONS(perms)); + g_return_if_fail(perms == NULL || + GVIR_CONFIG_IS_STORAGE_PERMISSIONS(perms)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(target), - GVIR_CONFIG_OBJECT(perms)); + "permissions", + (GVirConfigObject *)perms); } diff --git a/libvirt-gconfig/libvirt-gconfig-storage-pool.c b/libvirt-gconfig/libvirt-gconfig-storage-pool.c index 1bbcfe9..1ea410a 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-pool.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-pool.c @@ -139,22 +139,34 @@ void gvir_config_storage_pool_set_available(GVirConfigStoragePool *pool, "available", available); } +/** + * gvir_config_storage_pool_set_source: + * @source: (allow-none): + */ void gvir_config_storage_pool_set_source(GVirConfigStoragePool *pool, GVirConfigStoragePoolSource *source) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL(pool)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL_SOURCE(source)); + g_return_if_fail(source == NULL || + GVIR_CONFIG_IS_STORAGE_POOL_SOURCE(source)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(pool), - GVIR_CONFIG_OBJECT(source)); + "source", + (GVirConfigObject *)source); } +/** + * gvir_config_storage_pool_set_target: + * @target: (allow-none): + */ void gvir_config_storage_pool_set_target(GVirConfigStoragePool *pool, GVirConfigStoragePoolTarget *target) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL(pool)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL_TARGET(target)); + g_return_if_fail(target == NULL || + GVIR_CONFIG_IS_STORAGE_POOL_TARGET(target)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(pool), - GVIR_CONFIG_OBJECT(target)); + "target", + (GVirConfigObject *)target); } diff --git a/libvirt-gconfig/libvirt-gconfig-storage-vol-target.c b/libvirt-gconfig/libvirt-gconfig-storage-vol-target.c index 3786e2b..5ffcfe4 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-vol-target.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-vol-target.c @@ -84,12 +84,18 @@ void gvir_config_storage_vol_target_set_format(GVirConfigStorageVolTarget *targe g_object_unref(G_OBJECT(node)); } +/** + * gvir_config_storage_vol_target_set_permissions: + * @perms: (allow-none): + */ void gvir_config_storage_vol_target_set_permissions(GVirConfigStorageVolTarget *target, GVirConfigStoragePermissions *perms) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL_TARGET(target)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_PERMISSIONS(perms)); + g_return_if_fail(perms == NULL || + GVIR_CONFIG_IS_STORAGE_PERMISSIONS(perms)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(target), - GVIR_CONFIG_OBJECT(perms)); + "permissions", + (GVirConfigObject *)perms); } diff --git a/libvirt-gconfig/libvirt-gconfig-storage-vol.c b/libvirt-gconfig/libvirt-gconfig-storage-vol.c index e20dca3..316ea9e 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-vol.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-vol.c @@ -105,22 +105,34 @@ void gvir_config_storage_vol_set_allocation(GVirConfigStorageVol *vol, "allocation", allocation); } +/** + * gvir_config_storage_vol_set_target: + * @target: (allow-none): + */ void gvir_config_storage_vol_set_target(GVirConfigStorageVol *vol, GVirConfigStorageVolTarget *target) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL(vol)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL_TARGET(target)); + g_return_if_fail(target == NULL || + GVIR_CONFIG_IS_STORAGE_VOL_TARGET(target)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(vol), - GVIR_CONFIG_OBJECT(target)); + "target", + (GVirConfigObject *)target); } +/** + * gvir_config_storage_vol_set_backing_store: + * @backing_store: (allow-none): + */ void gvir_config_storage_vol_set_backing_store(GVirConfigStorageVol *vol, GVirConfigStorageVolBackingStore *backing_store) { g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL(vol)); - g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL_BACKING_STORE(backing_store)); + g_return_if_fail(backing_store == NULL || + GVIR_CONFIG_IS_STORAGE_VOL_BACKING_STORE(backing_store)); gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(vol), - GVIR_CONFIG_OBJECT(backing_store)); + "backingStore", + (GVirConfigObject *)backing_store); } -- 1.7.10.1
On Thu, May 10, 2012 at 9:58 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
Hey,
I've split this patch in 2 to make it more digestable. First one is unchanged and only contains the gvir_config_object_set_node_content changes The 2nd one only has the gvir_config_object_attach_replace changes, and it has some changes: I reordered the parameter to _attach_replace to make the order similar to most other functions, and I changed the GVIR_CONFIG_OBJECT() casts to (GVirConfigObject *) otherwise we'd get a runtime check when passing a NULL pointer.
Thats not true. I was also afraid of that and I wrote a test app to see if G_TYPE_CHECK_INSTANCE_CAST minds a null but it didn't.
gvir_config_domain_set_devices could try to unref a NULL pointer, I've changed that, and gvir_config_domain_set_clock was still rejecting NULL klock parameters through a g_return_val_if_fail.
I've dropped the gvir_config_domain_source_pty_set_path change for now since there are other places where _replace_child_with_attribute is used and could handle NULL pointers as well.
ACK on these 2 patches, but you should review the 2nd one for obvious mistakes.
Oh, I thought it was just that you divided them in 2. I'll have another look. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
On Thu, May 10, 2012 at 11:49:43PM +0300, Zeeshan Ali (Khattak) wrote:
On Thu, May 10, 2012 at 9:58 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
The 2nd one only has the gvir_config_object_attach_replace changes, and it has some changes: I reordered the parameter to _attach_replace to make the order similar to most other functions, and I changed the GVIR_CONFIG_OBJECT() casts to (GVirConfigObject *) otherwise we'd get a runtime check when passing a NULL pointer.
Thats not true. I was also afraid of that and I wrote a test app to see if G_TYPE_CHECK_INSTANCE_CAST minds a null but it didn't.
Indeed, I checked that myself too. For some reason I was convinced it would trigger a warning. Just change it back then ;) Christophe
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); + *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 + * + * 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
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
On Thu, May 10, 2012 at 8:28 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
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?
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
On Fri, May 11, 2012 at 06:10:24AM +0300, Zeeshan Ali (Khattak) wrote:
The annotations says it all.
You have to know exactly what it means, and it's written very small in the doc, so no, the annotation doesn't say much if you are not familiar with it, and you have to notice it.
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).
I'll fix the doc when this happens. Christophe
On Fri, May 11, 2012 at 11:30 AM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Fri, May 11, 2012 at 06:10:24AM +0300, Zeeshan Ali (Khattak) wrote:
The annotations says it all.
You have to know exactly what it means, and it's written very small in the doc, so no, the annotation doesn't say much if you are not familiar with it, and you have to notice it.
The annotation is not supposed to be directly meant for humans, thats why gtk-doc needs to do a better job. i-e the info is already there, just needs translation.
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).
I'll fix the doc when this happens.
I discussed this with Stefan like 3 years ago and IIRC he said "its a known issue" and that he has been looking into the matter so can't be sure somebody actually filed a bug about it. But if it hasn't been fixed for all these years, it'll take a competent hacker who is bothered by this to look to get this fixed. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
On Fri, May 11, 2012 at 03:08:06PM +0300, Zeeshan Ali (Khattak) wrote:
The annotation is not supposed to be directly meant for humans, thats why gtk-doc needs to do a better job. i-e the info is already there, just needs translation.
Yep, I don't know if gtk-doc uses the introspection information at all though (to be able to tell which functions should be used to free the container and which function should be used to free its content). In the mean time, I'd feel more comfortable if we explicitly say what should be done to free the returned data since that is one of the thing that I miss the most when using most C APIs Christophe
On Fri, May 11, 2012 at 02:27:37PM +0200, Christophe Fergeau wrote:
On Fri, May 11, 2012 at 03:08:06PM +0300, Zeeshan Ali (Khattak) wrote:
The annotation is not supposed to be directly meant for humans, thats why gtk-doc needs to do a better job. i-e the info is already there, just needs translation.
Yep, I don't know if gtk-doc uses the introspection information at all though (to be able to tell which functions should be used to free the container and which function should be used to free its content). In the mean time, I'd feel more comfortable if we explicitly say what should be done to free the returned data since that is one of the thing that I miss the most when using most C APIs
If I look at the docs for GTK, I see GTK-DOC is including details of the annotations: eg http://developer.gnome.org/gtk3/stable/GtkWindow.html#gtk-window-get-default... "Returns: copy of default icon list. [element-type GdkPixbuf][transfer container]" IMHO this is sufficient and we don't need to explicitly mention the transfer rules in the docs string. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Fri, May 11, 2012 at 01:32:34PM +0100, Daniel P. Berrange wrote:
If I look at the docs for GTK, I see GTK-DOC is including details of the annotations:
eg
http://developer.gnome.org/gtk3/stable/GtkWindow.html#gtk-window-get-default...
"Returns: copy of default icon list. [element-type GdkPixbuf][transfer container]"
IMHO this is sufficient and we don't need to explicitly mention the transfer rules in the docs string.
I know it's in there, but if you are not familiar with gtk conventions at all, you have to first figure out what that means exactly, and then you have to find the correct way of free-ing the container and what it contains (which is not necessarily g_object_unref, it can contain strings, boxed types, ...). That's why I really prefer having it mentioned explicitly, memory handling is boring enough/error-prone enough so anything that makes it simpler is a plus in my opinion. Christophe
On Fri, May 11, 2012 at 4:14 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Fri, May 11, 2012 at 01:32:34PM +0100, Daniel P. Berrange wrote:
If I look at the docs for GTK, I see GTK-DOC is including details of the annotations:
eg
http://developer.gnome.org/gtk3/stable/GtkWindow.html#gtk-window-get-default...
"Returns: copy of default icon list. [element-type GdkPixbuf][transfer container]"
IMHO this is sufficient and we don't need to explicitly mention the transfer rules in the docs string.
I know it's in there, but if you are not familiar with gtk conventions at all, you have to first figure out what that means exactly, and then you have to find the correct way of free-ing the container and what it contains (which is not necessarily g_object_unref, it can contain strings, boxed types, ...). That's why I really prefer having it mentioned explicitly, memory handling is boring enough/error-prone enough so anything that makes it simpler is a plus in my opinion.
I don't exactly disagree with your assertions here but what you are proposing is a local work-around. If this really is a big issue, we have a major issue in all our gtk-doc/gir-using libraries. The solution is to hack gtk-doc and there is no reason to have a work-around till that is fixed. Any ways, the patch is pushed so if you don't agree with me and Daniel, feel free to make corrections in master. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
On Fri, May 11, 2012 at 04:23:05PM +0300, Zeeshan Ali (Khattak) wrote:
I don't exactly disagree with your assertions here but what you are proposing is a local work-around. If this really is a big issue, we have a major issue in all our gtk-doc/gir-using libraries.
This is what GTK+ is doing in most of the code, this hasn't been fixed in years, so I don't see the point in calling that a "workaround" Christophe
On Fri, May 11, 2012 at 01:32:34PM +0100, Daniel P. Berrange wrote:
On Fri, May 11, 2012 at 02:27:37PM +0200, Christophe Fergeau wrote:
On Fri, May 11, 2012 at 03:08:06PM +0300, Zeeshan Ali (Khattak) wrote:
The annotation is not supposed to be directly meant for humans, thats why gtk-doc needs to do a better job. i-e the info is already there, just needs translation.
Yep, I don't know if gtk-doc uses the introspection information at all though (to be able to tell which functions should be used to free the container and which function should be used to free its content). In the mean time, I'd feel more comfortable if we explicitly say what should be done to free the returned data since that is one of the thing that I miss the most when using most C APIs
If I look at the docs for GTK, I see GTK-DOC is including details of the annotations:
eg
http://developer.gnome.org/gtk3/stable/GtkWindow.html#gtk-window-get-default...
"Returns: copy of default icon list. [element-type GdkPixbuf][transfer container]"
Forgot to mention that gtk+ also has a note saying how to free the return value, from your link: "The list is a copy and should be freed with g_list_free(), but the pixbufs in the list have not had their reference count incremented. " Christophe
On Fri, May 11, 2012 at 03:15:29PM +0200, Christophe Fergeau wrote:
On Fri, May 11, 2012 at 01:32:34PM +0100, Daniel P. Berrange wrote:
On Fri, May 11, 2012 at 02:27:37PM +0200, Christophe Fergeau wrote:
On Fri, May 11, 2012 at 03:08:06PM +0300, Zeeshan Ali (Khattak) wrote:
The annotation is not supposed to be directly meant for humans, thats why gtk-doc needs to do a better job. i-e the info is already there, just needs translation.
Yep, I don't know if gtk-doc uses the introspection information at all though (to be able to tell which functions should be used to free the container and which function should be used to free its content). In the mean time, I'd feel more comfortable if we explicitly say what should be done to free the returned data since that is one of the thing that I miss the most when using most C APIs
If I look at the docs for GTK, I see GTK-DOC is including details of the annotations:
eg
http://developer.gnome.org/gtk3/stable/GtkWindow.html#gtk-window-get-default...
"Returns: copy of default icon list. [element-type GdkPixbuf][transfer container]"
Forgot to mention that gtk+ also has a note saying how to free the return value, from your link: "The list is a copy and should be freed with g_list_free(), but the pixbufs in the list have not had their reference count incremented. "
Oh, I missed that. If anyone wishes to include such notes in the comments for libvirt-glib I have no objection to this, either way. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Fri, May 11, 2012 at 06:10:24AM +0300, Zeeshan Ali (Khattak) wrote:
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
I could not find such a bug report by the way. Christophe
ACK On Wed, May 09, 2012 at 03:54:34AM +0300, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
This function should set 'path' as attribute of 'source' node rather than as child node. --- .../libvirt-gconfig-domain-chardev-source-pty.c | 11 ++++------- 1 files changed, 4 insertions(+), 7 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..201e123 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-chardev-source-pty.c @@ -82,13 +82,10 @@ GVirConfigDomainChardevSourcePty *gvir_config_domain_chardev_source_pty_new_from void gvir_config_domain_source_pty_set_path(GVirConfigDomainChardevSourcePty *pty, const char *path) { - GVirConfigObject *source; g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CHARDEV_SOURCE_PTY(pty));
- source = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(pty), - "source"); - g_return_if_fail(GVIR_CONFIG_IS_OBJECT(source)); - gvir_config_object_set_node_content(GVIR_CONFIG_OBJECT(source), - "path", path); - g_object_unref(G_OBJECT(source)); + gvir_config_object_replace_child_with_attribute(GVIR_CONFIG_OBJECT(pty), + "source", + "path", + path); } -- 1.7.7.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
Christophe Fergeau -
Daniel P. Berrange -
Zeeshan Ali (Khattak)