[libvirt] [libvirt-glib 1/6] Getters for GVirConfigDomainInterface attributes
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> --- libvirt-gconfig/libvirt-gconfig-domain-interface.c | 35 ++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain-interface.h | 4 ++ libvirt-gconfig/libvirt-gconfig.sym | 4 ++ 3 files changed, 43 insertions(+), 0 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.c b/libvirt-gconfig/libvirt-gconfig-domain-interface.c index 85cc194..61d35bd 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-interface.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.c @@ -96,6 +96,41 @@ void gvir_config_domain_interface_set_model(GVirConfigDomainInterface *interface "model", "type", model); } +const char *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface *interface) +{ + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), NULL); + + return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(interface), + "target", "device"); +} + +GVirConfigDomainInterfaceLinkState gvir_config_domain_interface_get_link_state(GVirConfigDomainInterface *interface) +{ + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), + GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_DEFAULT); + + return gvir_config_object_get_attribute_genum(GVIR_CONFIG_OBJECT(interface), + "link", "state", + GVIR_CONFIG_TYPE_DOMAIN_INTERFACE_LINK_STATE, + GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_DEFAULT); +} + +const char *gvir_config_domain_interface_get_mac(GVirConfigDomainInterface *interface) +{ + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), NULL); + + return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(interface), + "mac", "address"); +} + +const char *gvir_config_domain_interface_get_model(GVirConfigDomainInterface *interface) +{ + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), NULL); + + return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(interface), + "model", "type"); +} + G_GNUC_INTERNAL GVirConfigDomainDevice * gvir_config_domain_interface_new_from_tree(GVirConfigXmlDoc *doc, xmlNodePtr tree) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.h b/libvirt-gconfig/libvirt-gconfig-domain-interface.h index 6e802fb..c8c4fb3 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-interface.h +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.h @@ -72,6 +72,10 @@ void gvir_config_domain_interface_set_mac(GVirConfigDomainInterface *interface, const char *mac_address); void gvir_config_domain_interface_set_model(GVirConfigDomainInterface *interface, const char *model); +const char *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface *interface); +GVirConfigDomainInterfaceLinkState gvir_config_domain_interface_get_link_state(GVirConfigDomainInterface *interface); +const char *gvir_config_domain_interface_get_mac(GVirConfigDomainInterface *interface); +const char *gvir_config_domain_interface_get_model(GVirConfigDomainInterface *interface); G_END_DECLS diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index 96ce58f..f91b8b0 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -145,6 +145,10 @@ LIBVIRT_GCONFIG_0.0.4 { gvir_config_domain_interface_set_link_state; gvir_config_domain_interface_set_mac; gvir_config_domain_interface_set_model; + gvir_config_domain_interface_get_ifname; + gvir_config_domain_interface_get_link_state; + gvir_config_domain_interface_get_mac; + gvir_config_domain_interface_get_model; gvir_config_domain_interface_bridge_get_type; gvir_config_domain_interface_bridge_new; -- 1.7.7.6
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> Binding for virDomainBlockResize(). --- libvirt-gobject/libvirt-gobject-domain-disk.c | 38 +++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain-disk.h | 4 ++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 43 insertions(+), 0 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.c b/libvirt-gobject/libvirt-gobject-domain-disk.c index f98d816..fb7672e 100644 --- a/libvirt-gobject/libvirt-gobject-domain-disk.c +++ b/libvirt-gobject/libvirt-gobject-domain-disk.c @@ -192,3 +192,41 @@ end: virDomainFree(handle); return ret; } + +/** + * gvir_domain_disk_resize: + * @self: the domain disk + * @size: new size of the block image in kilobytes + * @flags: flags, currently unused. Pass '0'. + * @err: placeholder for an error, or NULL + * + * This function resize the disk of a running domain. + * + * Returns: TRUE if size was successfully changed, FALSE otherwise. + **/ +gboolean gvir_domain_disk_resize(GVirDomainDisk *self, + guint64 size, + guint flags, + GError **err) +{ + gboolean ret = FALSE; + virDomainPtr handle; + + g_return_val_if_fail(GVIR_IS_DOMAIN_DISK(self), FALSE); + g_return_val_if_fail(err == NULL || *err != NULL, FALSE); + + handle = gvir_domain_device_get_domain_handle(GVIR_DOMAIN_DEVICE(self)); + + if (virDomainBlockResize(handle, self->priv->path, size, flags) < 0) { + gvir_set_error_literal(err, GVIR_DOMAIN_DISK_ERROR, + 0, + "Failed to resize domain disk"); + goto end; + } + + ret = TRUE; + +end: + virDomainFree(handle); + return ret; +} diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.h b/libvirt-gobject/libvirt-gobject-domain-disk.h index 21f2357..1788d63 100644 --- a/libvirt-gobject/libvirt-gobject-domain-disk.h +++ b/libvirt-gobject/libvirt-gobject-domain-disk.h @@ -72,6 +72,10 @@ GType gvir_domain_disk_get_type(void); GType gvir_domain_disk_stats_get_type(void); GVirDomainDiskStats *gvir_domain_disk_get_stats(GVirDomainDisk *self, GError **err); +gboolean gvir_domain_disk_resize(GVirDomainDisk *self, + guint64 size, + guint flags, + GError **err); G_END_DECLS diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 1ad6b53..5081f41 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -37,6 +37,7 @@ LIBVIRT_GOBJECT_0.0.4 { gvir_domain_disk_get_type; gvir_domain_disk_stats_get_type; gvir_domain_disk_get_stats; + gvir_domain_disk_resize; gvir_domain_interface_get_type; gvir_domain_interface_stats_get_type; -- 1.7.7.6
ACK On Tue, Feb 28, 2012 at 08:25:03PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
Binding for virDomainBlockResize(). --- libvirt-gobject/libvirt-gobject-domain-disk.c | 38 +++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain-disk.h | 4 ++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.c b/libvirt-gobject/libvirt-gobject-domain-disk.c index f98d816..fb7672e 100644 --- a/libvirt-gobject/libvirt-gobject-domain-disk.c +++ b/libvirt-gobject/libvirt-gobject-domain-disk.c @@ -192,3 +192,41 @@ end: virDomainFree(handle); return ret; } + +/** + * gvir_domain_disk_resize: + * @self: the domain disk + * @size: new size of the block image in kilobytes + * @flags: flags, currently unused. Pass '0'. + * @err: placeholder for an error, or NULL + * + * This function resize the disk of a running domain. + * + * Returns: TRUE if size was successfully changed, FALSE otherwise. + **/ +gboolean gvir_domain_disk_resize(GVirDomainDisk *self, + guint64 size, + guint flags, + GError **err) +{ + gboolean ret = FALSE; + virDomainPtr handle; + + g_return_val_if_fail(GVIR_IS_DOMAIN_DISK(self), FALSE); + g_return_val_if_fail(err == NULL || *err != NULL, FALSE); + + handle = gvir_domain_device_get_domain_handle(GVIR_DOMAIN_DEVICE(self)); + + if (virDomainBlockResize(handle, self->priv->path, size, flags) < 0) { + gvir_set_error_literal(err, GVIR_DOMAIN_DISK_ERROR, + 0, + "Failed to resize domain disk"); + goto end; + } + + ret = TRUE; + +end: + virDomainFree(handle); + return ret; +} diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.h b/libvirt-gobject/libvirt-gobject-domain-disk.h index 21f2357..1788d63 100644 --- a/libvirt-gobject/libvirt-gobject-domain-disk.h +++ b/libvirt-gobject/libvirt-gobject-domain-disk.h @@ -72,6 +72,10 @@ GType gvir_domain_disk_get_type(void); GType gvir_domain_disk_stats_get_type(void);
GVirDomainDiskStats *gvir_domain_disk_get_stats(GVirDomainDisk *self, GError **err); +gboolean gvir_domain_disk_resize(GVirDomainDisk *self, + guint64 size, + guint flags, + GError **err);
G_END_DECLS
diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 1ad6b53..5081f41 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -37,6 +37,7 @@ LIBVIRT_GOBJECT_0.0.4 { gvir_domain_disk_get_type; gvir_domain_disk_stats_get_type; gvir_domain_disk_get_stats; + gvir_domain_disk_resize;
gvir_domain_interface_get_type; gvir_domain_interface_stats_get_type; -- 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> Getter for the associated domain of a domain device. --- libvirt-gobject/libvirt-gobject-domain-device.c | 10 ++++++++++ libvirt-gobject/libvirt-gobject-domain-device.h | 3 +++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 14 insertions(+), 0 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c b/libvirt-gobject/libvirt-gobject-domain-device.c index 528b513..6282d8b 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device.c +++ b/libvirt-gobject/libvirt-gobject-domain-device.c @@ -134,3 +134,13 @@ virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self) return handle; } + +/** + * gvir_domain_device_get_domain: + * @device: the domain device + * + * Returns: (transfer full): the associate domain + */ +GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device) { + return g_object_ref (device->priv->domain); +} diff --git a/libvirt-gobject/libvirt-gobject-domain-device.h b/libvirt-gobject/libvirt-gobject-domain-device.h index 96c0433..98acc2d 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device.h +++ b/libvirt-gobject/libvirt-gobject-domain-device.h @@ -27,6 +27,8 @@ #ifndef __LIBVIRT_GOBJECT_DOMAIN_DEVICE_H__ #define __LIBVIRT_GOBJECT_DOMAIN_DEVICE_H__ +#include <libvirt-gobject/libvirt-gobject-domain.h> + G_BEGIN_DECLS #define GVIR_TYPE_DOMAIN_DEVICE (gvir_domain_device_get_type ()) @@ -58,6 +60,7 @@ struct _GVirDomainDeviceClass GType gvir_domain_device_get_type(void); +GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device); G_END_DECLS diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 5081f41..0097692 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -33,6 +33,7 @@ LIBVIRT_GOBJECT_0.0.4 { gvir_connection_get_node_info; gvir_domain_device_get_type; + gvir_domain_device_get_domain; gvir_domain_disk_get_type; gvir_domain_disk_stats_get_type; -- 1.7.7.6
On Tue, Feb 28, 2012 at 08:25:04PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
Getter for the associated domain of a domain device.
NB: it already exists as a gobject property
--- libvirt-gobject/libvirt-gobject-domain-device.c | 10 ++++++++++ libvirt-gobject/libvirt-gobject-domain-device.h | 3 +++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c b/libvirt-gobject/libvirt-gobject-domain-device.c index 528b513..6282d8b 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device.c +++ b/libvirt-gobject/libvirt-gobject-domain-device.c @@ -134,3 +134,13 @@ virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self)
return handle; } + +/** + * gvir_domain_device_get_domain: + * @device: the domain device + * + * Returns: (transfer full): the associate domain
associated ACK Christophe
+ */ +GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device) { + return g_object_ref (device->priv->domain); +} diff --git a/libvirt-gobject/libvirt-gobject-domain-device.h b/libvirt-gobject/libvirt-gobject-domain-device.h index 96c0433..98acc2d 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device.h +++ b/libvirt-gobject/libvirt-gobject-domain-device.h @@ -27,6 +27,8 @@ #ifndef __LIBVIRT_GOBJECT_DOMAIN_DEVICE_H__ #define __LIBVIRT_GOBJECT_DOMAIN_DEVICE_H__
+#include <libvirt-gobject/libvirt-gobject-domain.h> + G_BEGIN_DECLS
#define GVIR_TYPE_DOMAIN_DEVICE (gvir_domain_device_get_type ()) @@ -58,6 +60,7 @@ struct _GVirDomainDeviceClass
GType gvir_domain_device_get_type(void); +GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device);
G_END_DECLS
diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 5081f41..0097692 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -33,6 +33,7 @@ LIBVIRT_GOBJECT_0.0.4 { gvir_connection_get_node_info;
gvir_domain_device_get_type; + gvir_domain_device_get_domain;
gvir_domain_disk_get_type; gvir_domain_disk_stats_get_type; -- 1.7.7.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Feb 29, 2012 at 02:02:08PM +0100, Christophe Fergeau wrote:
On Tue, Feb 28, 2012 at 08:25:04PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
Getter for the associated domain of a domain device.
NB: it already exists as a gobject property
--- libvirt-gobject/libvirt-gobject-domain-device.c | 10 ++++++++++ libvirt-gobject/libvirt-gobject-domain-device.h | 3 +++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c b/libvirt-gobject/libvirt-gobject-domain-device.c index 528b513..6282d8b 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device.c +++ b/libvirt-gobject/libvirt-gobject-domain-device.c @@ -134,3 +134,13 @@ virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self)
return handle; } + +/** + * gvir_domain_device_get_domain: + * @device: the domain device + * + * Returns: (transfer full): the associate domain
associated
ACK
Oh, one more nit below
+ */ +GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device) {
opening brace on a new line
+ return g_object_ref (device->priv->domain); +} diff --git a/libvirt-gobject/libvirt-gobject-domain-device.h b/libvirt-gobject/libvirt-gobject-domain-device.h index 96c0433..98acc2d 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device.h +++ b/libvirt-gobject/libvirt-gobject-domain-device.h @@ -27,6 +27,8 @@ #ifndef __LIBVIRT_GOBJECT_DOMAIN_DEVICE_H__ #define __LIBVIRT_GOBJECT_DOMAIN_DEVICE_H__
+#include <libvirt-gobject/libvirt-gobject-domain.h> + G_BEGIN_DECLS
#define GVIR_TYPE_DOMAIN_DEVICE (gvir_domain_device_get_type ()) @@ -58,6 +60,7 @@ struct _GVirDomainDeviceClass
GType gvir_domain_device_get_type(void); +GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device);
G_END_DECLS
diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 5081f41..0097692 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -33,6 +33,7 @@ LIBVIRT_GOBJECT_0.0.4 { gvir_connection_get_node_info;
gvir_domain_device_get_type; + gvir_domain_device_get_domain;
gvir_domain_disk_get_type; gvir_domain_disk_stats_get_type; -- 1.7.7.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> GVirDomainDevice should have an associated GVirConfigDomainDevice. --- libvirt-gobject/libvirt-gobject-domain-device.c | 32 +++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain-device.h | 1 + libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 34 insertions(+), 0 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c b/libvirt-gobject/libvirt-gobject-domain-device.c index 6282d8b..85879d2 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device.c +++ b/libvirt-gobject/libvirt-gobject-domain-device.c @@ -37,6 +37,7 @@ struct _GVirDomainDevicePrivate { GVirDomain *domain; + GVirConfigDomainDevice *config; }; G_DEFINE_ABSTRACT_TYPE(GVirDomainDevice, gvir_domain_device, G_TYPE_OBJECT); @@ -44,6 +45,7 @@ G_DEFINE_ABSTRACT_TYPE(GVirDomainDevice, gvir_domain_device, G_TYPE_OBJECT); enum { PROP_0, PROP_DOMAIN, + PROP_CONFIG, }; static void gvir_domain_device_get_property(GObject *object, @@ -59,6 +61,10 @@ static void gvir_domain_device_get_property(GObject *object, g_value_set_object(value, priv->domain); break; + case PROP_CONFIG: + g_value_set_object(value, priv->config); + break; + default: G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); } @@ -79,6 +85,11 @@ static void gvir_domain_device_set_property(GObject *object, priv->domain = g_value_dup_object(value); break; + case PROP_CONFIG: + g_clear_object(&priv->config); + priv->config = g_value_dup_object(value); + break; + default: G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); } @@ -93,6 +104,7 @@ static void gvir_domain_device_finalize(GObject *object) g_debug("Finalize GVirDomainDevice=%p", self); g_clear_object(&priv->domain); + g_clear_object(&priv->config); G_OBJECT_CLASS(gvir_domain_device_parent_class)->finalize(object); } @@ -115,6 +127,16 @@ static void gvir_domain_device_class_init(GVirDomainDeviceClass *klass) G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS)); + g_object_class_install_property(object_class, + PROP_CONFIG, + g_param_spec_object("config", + "Config", + "The configuration", + GVIR_CONFIG_TYPE_DOMAIN_DEVICE, + G_PARAM_READWRITE | + G_PARAM_CONSTRUCT_ONLY | + G_PARAM_STATIC_STRINGS)); + g_type_class_add_private(klass, sizeof(GVirDomainDevicePrivate)); } @@ -144,3 +166,13 @@ virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self) GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device) { return g_object_ref (device->priv->domain); } + +/** + * gvir_domain_device_get_config: + * @device: the domain device + * + * Returns: (transfer full): the config + */ +GVirConfigDomainDevice *gvir_domain_device_get_config(GVirDomainDevice *device) { + return g_object_ref (device->priv->config); +} diff --git a/libvirt-gobject/libvirt-gobject-domain-device.h b/libvirt-gobject/libvirt-gobject-domain-device.h index 98acc2d..b308477 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device.h +++ b/libvirt-gobject/libvirt-gobject-domain-device.h @@ -61,6 +61,7 @@ struct _GVirDomainDeviceClass GType gvir_domain_device_get_type(void); GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device); +GVirConfigDomainDevice *gvir_domain_device_get_config(GVirDomainDevice *device); G_END_DECLS diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 0097692..d6999dc 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -34,6 +34,7 @@ LIBVIRT_GOBJECT_0.0.4 { gvir_domain_device_get_type; gvir_domain_device_get_domain; + gvir_domain_device_get_config; gvir_domain_disk_get_type; gvir_domain_disk_stats_get_type; -- 1.7.7.6
The code to add the property looks good to me except for one minor nit, see below. I'd rather to see patch 6/6 come right after it since I was wondering how config was getting set. On Tue, Feb 28, 2012 at 08:25:05PM +0200, Zeeshan Ali (Khattak) wrote:
@@ -144,3 +166,13 @@ virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self) GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device) { return g_object_ref (device->priv->domain); } + +/** + * gvir_domain_device_get_config: + * @device: the domain device + * + * Returns: (transfer full): the config + */ +GVirConfigDomainDevice *gvir_domain_device_get_config(GVirDomainDevice *device) {
{ on a new line Christophe
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> Remove now redundant 'path' property from GVirDomainDevice subclasses. --- libvirt-gobject/libvirt-gobject-domain-disk.c | 88 ++++---------------- libvirt-gobject/libvirt-gobject-domain-disk.h | 3 +- libvirt-gobject/libvirt-gobject-domain-interface.c | 89 +++----------------- libvirt-gobject/libvirt-gobject-domain-interface.h | 3 +- 4 files changed, 31 insertions(+), 152 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.c b/libvirt-gobject/libvirt-gobject-domain-disk.c index fb7672e..42e0e6c 100644 --- a/libvirt-gobject/libvirt-gobject-domain-disk.c +++ b/libvirt-gobject/libvirt-gobject-domain-disk.c @@ -34,75 +34,22 @@ #define GVIR_DOMAIN_DISK_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_DOMAIN_DISK, GVirDomainDiskPrivate)) -struct _GVirDomainDiskPrivate -{ - gchar *path; -}; - G_DEFINE_TYPE(GVirDomainDisk, gvir_domain_disk, GVIR_TYPE_DOMAIN_DEVICE); -enum { - PROP_0, - PROP_PATH, -}; - #define GVIR_DOMAIN_DISK_ERROR gvir_domain_disk_error_quark() - static GQuark gvir_domain_disk_error_quark(void) { return g_quark_from_static_string("gvir-domain-disk"); } -static void gvir_domain_disk_get_property(GObject *object, - guint prop_id, - GValue *value, - GParamSpec *pspec) -{ - GVirDomainDisk *self = GVIR_DOMAIN_DISK(object); - GVirDomainDiskPrivate *priv = self->priv; - - switch (prop_id) { - case PROP_PATH: - g_value_set_string(value, priv->path); - break; - - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); - } -} - - -static void gvir_domain_disk_set_property(GObject *object, - guint prop_id, - const GValue *value, - GParamSpec *pspec) -{ - GVirDomainDisk *self = GVIR_DOMAIN_DISK(object); - GVirDomainDiskPrivate *priv = self->priv; - - switch (prop_id) { - case PROP_PATH: - g_free(priv->path); - priv->path = g_value_dup_string(value); - break; - - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); - } -} - - static void gvir_domain_disk_finalize(GObject *object) { GVirDomainDisk *self = GVIR_DOMAIN_DISK(object); - GVirDomainDiskPrivate *priv = self->priv; g_debug("Finalize GVirDomainDisk=%p", self); - g_free(priv->path); - G_OBJECT_CLASS(gvir_domain_disk_parent_class)->finalize(object); } @@ -111,27 +58,11 @@ static void gvir_domain_disk_class_init(GVirDomainDiskClass *klass) GObjectClass *object_class = G_OBJECT_CLASS (klass); object_class->finalize = gvir_domain_disk_finalize; - object_class->get_property = gvir_domain_disk_get_property; - object_class->set_property = gvir_domain_disk_set_property; - - g_object_class_install_property(object_class, - PROP_PATH, - g_param_spec_string("path", - "Path", - "The disk path", - NULL, - G_PARAM_READWRITE | - G_PARAM_CONSTRUCT_ONLY | - G_PARAM_STATIC_STRINGS)); - - g_type_class_add_private(klass, sizeof(GVirDomainDiskPrivate)); } static void gvir_domain_disk_init(GVirDomainDisk *self) { g_debug("Init GVirDomainDisk=%p", self); - - self->priv = GVIR_DOMAIN_DISK_GET_PRIVATE(self); } static GVirDomainDiskStats * @@ -151,6 +82,15 @@ gvir_domain_disk_stats_free(GVirDomainDiskStats *stats) G_DEFINE_BOXED_TYPE(GVirDomainDiskStats, gvir_domain_disk_stats, gvir_domain_disk_stats_copy, gvir_domain_disk_stats_free) +static const gchar *gvir_domain_disk_get_path(GVirDomainDisk *self) +{ + GVirConfigDomainDevice *config; + + config = gvir_domain_device_get_config(GVIR_DOMAIN_DEVICE(self)); + + return gvir_config_domain_disk_get_target_dev (GVIR_CONFIG_DOMAIN_DISK (config)); +} + /** * gvir_domain_disk_get_stats: * @self: the domain disk @@ -166,15 +106,15 @@ GVirDomainDiskStats *gvir_domain_disk_get_stats(GVirDomainDisk *self, GError **e { GVirDomainDiskStats *ret = NULL; virDomainBlockStatsStruct stats; - GVirDomainDiskPrivate *priv; virDomainPtr handle; + const gchar *path; g_return_val_if_fail(GVIR_IS_DOMAIN_DISK(self), NULL); - priv = self->priv; handle = gvir_domain_device_get_domain_handle(GVIR_DOMAIN_DEVICE(self)); + path = gvir_domain_disk_get_path (self); - if (virDomainBlockStats(handle, priv->path, &stats, sizeof (stats)) < 0) { + if (virDomainBlockStats(handle, path, &stats, sizeof (stats)) < 0) { gvir_set_error_literal(err, GVIR_DOMAIN_DISK_ERROR, 0, "Unable to get domain disk stats"); @@ -211,13 +151,15 @@ gboolean gvir_domain_disk_resize(GVirDomainDisk *self, { gboolean ret = FALSE; virDomainPtr handle; + const gchar *path; g_return_val_if_fail(GVIR_IS_DOMAIN_DISK(self), FALSE); g_return_val_if_fail(err == NULL || *err != NULL, FALSE); handle = gvir_domain_device_get_domain_handle(GVIR_DOMAIN_DEVICE(self)); + path = gvir_domain_disk_get_path (self); - if (virDomainBlockResize(handle, self->priv->path, size, flags) < 0) { + if (virDomainBlockResize(handle, path, size, flags) < 0) { gvir_set_error_literal(err, GVIR_DOMAIN_DISK_ERROR, 0, "Failed to resize domain disk"); diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.h b/libvirt-gobject/libvirt-gobject-domain-disk.h index 1788d63..fb8b3dc 100644 --- a/libvirt-gobject/libvirt-gobject-domain-disk.h +++ b/libvirt-gobject/libvirt-gobject-domain-disk.h @@ -56,7 +56,8 @@ struct _GVirDomainDisk { GVirDomainDevice parent; - GVirDomainDiskPrivate *priv; + /* In case we need a private struct in future */ + gpointer padding[1]; /* Do not add fields to this struct */ }; diff --git a/libvirt-gobject/libvirt-gobject-domain-interface.c b/libvirt-gobject/libvirt-gobject-domain-interface.c index 0917e03..9ec3877 100644 --- a/libvirt-gobject/libvirt-gobject-domain-interface.c +++ b/libvirt-gobject/libvirt-gobject-domain-interface.c @@ -31,78 +31,22 @@ #include "libvirt-gobject/libvirt-gobject-domain-device-private.h" -#define GVIR_DOMAIN_INTERFACE_GET_PRIVATE(obj) \ - (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_DOMAIN_INTERFACE, GVirDomainInterfacePrivate)) - -struct _GVirDomainInterfacePrivate -{ - gchar *path; -}; - G_DEFINE_TYPE(GVirDomainInterface, gvir_domain_interface, GVIR_TYPE_DOMAIN_DEVICE); -enum { - PROP_0, - PROP_PATH, -}; - #define GVIR_DOMAIN_INTERFACE_ERROR gvir_domain_interface_error_quark() - static GQuark gvir_domain_interface_error_quark(void) { return g_quark_from_static_string("gvir-domain-interface"); } -static void gvir_domain_interface_get_property(GObject *object, - guint prop_id, - GValue *value, - GParamSpec *pspec) -{ - GVirDomainInterface *self = GVIR_DOMAIN_INTERFACE(object); - GVirDomainInterfacePrivate *priv = self->priv; - - switch (prop_id) { - case PROP_PATH: - g_value_set_string(value, priv->path); - break; - - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); - } -} - - -static void gvir_domain_interface_set_property(GObject *object, - guint prop_id, - const GValue *value, - GParamSpec *pspec) -{ - GVirDomainInterface *self = GVIR_DOMAIN_INTERFACE(object); - GVirDomainInterfacePrivate *priv = self->priv; - - switch (prop_id) { - case PROP_PATH: - g_free(priv->path); - priv->path = g_value_dup_string(value); - break; - - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); - } -} - - static void gvir_domain_interface_finalize(GObject *object) { GVirDomainInterface *self = GVIR_DOMAIN_INTERFACE(object); - GVirDomainInterfacePrivate *priv = self->priv; g_debug("Finalize GVirDomainInterface=%p", self); - g_free(priv->path); - G_OBJECT_CLASS(gvir_domain_interface_parent_class)->finalize(object); } @@ -111,27 +55,11 @@ static void gvir_domain_interface_class_init(GVirDomainInterfaceClass *klass) GObjectClass *object_class = G_OBJECT_CLASS (klass); object_class->finalize = gvir_domain_interface_finalize; - object_class->get_property = gvir_domain_interface_get_property; - object_class->set_property = gvir_domain_interface_set_property; - - g_object_class_install_property(object_class, - PROP_PATH, - g_param_spec_string("path", - "Path", - "The interface path", - NULL, - G_PARAM_READWRITE | - G_PARAM_CONSTRUCT_ONLY | - G_PARAM_STATIC_STRINGS)); - - g_type_class_add_private(klass, sizeof(GVirDomainInterfacePrivate)); } static void gvir_domain_interface_init(GVirDomainInterface *self) { g_debug("Init GVirDomainInterface=%p", self); - - self->priv = GVIR_DOMAIN_INTERFACE_GET_PRIVATE(self); } static GVirDomainInterfaceStats * @@ -140,17 +68,24 @@ gvir_domain_interface_stats_copy(GVirDomainInterfaceStats *stats) return g_slice_dup(GVirDomainInterfaceStats, stats); } - static void gvir_domain_interface_stats_free(GVirDomainInterfaceStats *stats) { g_slice_free(GVirDomainInterfaceStats, stats); } - G_DEFINE_BOXED_TYPE(GVirDomainInterfaceStats, gvir_domain_interface_stats, gvir_domain_interface_stats_copy, gvir_domain_interface_stats_free) +static const gchar *gvir_domain_interface_get_path(GVirDomainInterface *self) +{ + GVirConfigDomainDevice *config; + + config = gvir_domain_device_get_config(GVIR_DOMAIN_DEVICE(self)); + + return gvir_config_domain_interface_get_ifname (GVIR_CONFIG_DOMAIN_INTERFACE (config)); +} + /** * gvir_domain_interface_get_stats: * @self: the domain interface @@ -166,15 +101,15 @@ GVirDomainInterfaceStats *gvir_domain_interface_get_stats(GVirDomainInterface *s { GVirDomainInterfaceStats *ret = NULL; virDomainInterfaceStatsStruct stats; - GVirDomainInterfacePrivate *priv; virDomainPtr handle; + const gchar *path; g_return_val_if_fail(GVIR_IS_DOMAIN_INTERFACE(self), NULL); - priv = self->priv; handle = gvir_domain_device_get_domain_handle(GVIR_DOMAIN_DEVICE(self)); + path = gvir_domain_interface_get_path (self); - if (virDomainInterfaceStats(handle, priv->path, &stats, sizeof (stats)) < 0) { + if (virDomainInterfaceStats(handle, path, &stats, sizeof (stats)) < 0) { gvir_set_error_literal(err, GVIR_DOMAIN_INTERFACE_ERROR, 0, "Unable to get domain interface stats"); diff --git a/libvirt-gobject/libvirt-gobject-domain-interface.h b/libvirt-gobject/libvirt-gobject-domain-interface.h index 62848db..26b7d1c 100644 --- a/libvirt-gobject/libvirt-gobject-domain-interface.h +++ b/libvirt-gobject/libvirt-gobject-domain-interface.h @@ -59,7 +59,8 @@ struct _GVirDomainInterface { GVirDomainDevice parent; - GVirDomainInterfacePrivate *priv; + /* In case we need a private struct in future */ + gpointer padding[1]; /* Do not add fields to this struct */ }; -- 1.7.7.6
On Tue, Feb 28, 2012 at 08:25:06PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
Remove now redundant 'path' property from GVirDomainDevice subclasses. --- libvirt-gobject/libvirt-gobject-domain-disk.c | 88 ++++---------------- libvirt-gobject/libvirt-gobject-domain-disk.h | 3 +- libvirt-gobject/libvirt-gobject-domain-interface.c | 89 +++----------------- libvirt-gobject/libvirt-gobject-domain-interface.h | 3 +- 4 files changed, 31 insertions(+), 152 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.c b/libvirt-gobject/libvirt-gobject-domain-disk.c index fb7672e..42e0e6c 100644 --- a/libvirt-gobject/libvirt-gobject-domain-disk.c +++ b/libvirt-gobject/libvirt-gobject-domain-disk.c @@ -34,75 +34,22 @@ #define GVIR_DOMAIN_DISK_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_DOMAIN_DISK, GVirDomainDiskPrivate))
-struct _GVirDomainDiskPrivate -{ - gchar *path; -};
The rest of the code (GVirConfig*) always has a Private structure with a gboolean unused when it's empty, I'd rather we stayed consistent (though I keep thinking that we should remove these empty private structs everywhere :)
- G_DEFINE_TYPE(GVirDomainDisk, gvir_domain_disk, GVIR_TYPE_DOMAIN_DEVICE);
-enum { - PROP_0, - PROP_PATH, -}; - #define GVIR_DOMAIN_DISK_ERROR gvir_domain_disk_error_quark()
- static GQuark gvir_domain_disk_error_quark(void) { return g_quark_from_static_string("gvir-domain-disk"); }
-static void gvir_domain_disk_get_property(GObject *object, - guint prop_id, - GValue *value, - GParamSpec *pspec) -{ - GVirDomainDisk *self = GVIR_DOMAIN_DISK(object); - GVirDomainDiskPrivate *priv = self->priv; - - switch (prop_id) { - case PROP_PATH: - g_value_set_string(value, priv->path); - break; - - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); - } -} - - -static void gvir_domain_disk_set_property(GObject *object, - guint prop_id, - const GValue *value, - GParamSpec *pspec) -{ - GVirDomainDisk *self = GVIR_DOMAIN_DISK(object); - GVirDomainDiskPrivate *priv = self->priv; - - switch (prop_id) { - case PROP_PATH: - g_free(priv->path); - priv->path = g_value_dup_string(value); - break; - - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); - } -} - - static void gvir_domain_disk_finalize(GObject *object) { GVirDomainDisk *self = GVIR_DOMAIN_DISK(object); - GVirDomainDiskPrivate *priv = self->priv;
g_debug("Finalize GVirDomainDisk=%p", self);
- g_free(priv->path); - G_OBJECT_CLASS(gvir_domain_disk_parent_class)->finalize(object); }
@@ -111,27 +58,11 @@ static void gvir_domain_disk_class_init(GVirDomainDiskClass *klass) GObjectClass *object_class = G_OBJECT_CLASS (klass);
object_class->finalize = gvir_domain_disk_finalize; - object_class->get_property = gvir_domain_disk_get_property; - object_class->set_property = gvir_domain_disk_set_property; - - g_object_class_install_property(object_class, - PROP_PATH, - g_param_spec_string("path", - "Path", - "The disk path", - NULL, - G_PARAM_READWRITE | - G_PARAM_CONSTRUCT_ONLY | - G_PARAM_STATIC_STRINGS)); - - g_type_class_add_private(klass, sizeof(GVirDomainDiskPrivate));
This would need to stay if we keep the Private struct
}
static void gvir_domain_disk_init(GVirDomainDisk *self) { g_debug("Init GVirDomainDisk=%p", self); - - self->priv = GVIR_DOMAIN_DISK_GET_PRIVATE(self);
Ditto.
}
static GVirDomainDiskStats * @@ -151,6 +82,15 @@ gvir_domain_disk_stats_free(GVirDomainDiskStats *stats) G_DEFINE_BOXED_TYPE(GVirDomainDiskStats, gvir_domain_disk_stats, gvir_domain_disk_stats_copy, gvir_domain_disk_stats_free)
+static const gchar *gvir_domain_disk_get_path(GVirDomainDisk *self) +{ + GVirConfigDomainDevice *config; + + config = gvir_domain_device_get_config(GVIR_DOMAIN_DEVICE(self));
config needs to be unref'ed after use.
+ + return gvir_config_domain_disk_get_target_dev (GVIR_CONFIG_DOMAIN_DISK (config));
and the return value would be non-const after the changes I mentioned in my previous review.
+} + /** * gvir_domain_disk_get_stats: * @self: the domain disk @@ -166,15 +106,15 @@ GVirDomainDiskStats *gvir_domain_disk_get_stats(GVirDomainDisk *self, GError **e { GVirDomainDiskStats *ret = NULL; virDomainBlockStatsStruct stats; - GVirDomainDiskPrivate *priv; virDomainPtr handle; + const gchar *path;
g_return_val_if_fail(GVIR_IS_DOMAIN_DISK(self), NULL);
- priv = self->priv; handle = gvir_domain_device_get_domain_handle(GVIR_DOMAIN_DEVICE(self)); + path = gvir_domain_disk_get_path (self);
will need to be freed
- if (virDomainBlockStats(handle, priv->path, &stats, sizeof (stats)) < 0) { + if (virDomainBlockStats(handle, path, &stats, sizeof (stats)) < 0) { gvir_set_error_literal(err, GVIR_DOMAIN_DISK_ERROR, 0, "Unable to get domain disk stats"); @@ -211,13 +151,15 @@ gboolean gvir_domain_disk_resize(GVirDomainDisk *self, { gboolean ret = FALSE; virDomainPtr handle; + const gchar *path;
g_return_val_if_fail(GVIR_IS_DOMAIN_DISK(self), FALSE); g_return_val_if_fail(err == NULL || *err != NULL, FALSE);
handle = gvir_domain_device_get_domain_handle(GVIR_DOMAIN_DEVICE(self)); + path = gvir_domain_disk_get_path (self);
and same here
- if (virDomainBlockResize(handle, self->priv->path, size, flags) < 0) { + if (virDomainBlockResize(handle, path, size, flags) < 0) { gvir_set_error_literal(err, GVIR_DOMAIN_DISK_ERROR, 0, "Failed to resize domain disk"); diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.h b/libvirt-gobject/libvirt-gobject-domain-disk.h index 1788d63..fb8b3dc 100644 --- a/libvirt-gobject/libvirt-gobject-domain-disk.h +++ b/libvirt-gobject/libvirt-gobject-domain-disk.h @@ -56,7 +56,8 @@ struct _GVirDomainDisk { GVirDomainDevice parent;
- GVirDomainDiskPrivate *priv; + /* In case we need a private struct in future */ + gpointer padding[1];
Avoiding this seems like a good reason for not keeping the Private struct (at least name it gpointer priv);
/* Do not add fields to this struct */ }; diff --git a/libvirt-gobject/libvirt-gobject-domain-interface.c b/libvirt-gobject/libvirt-gobject-domain-interface.c index 0917e03..9ec3877 100644 --- a/libvirt-gobject/libvirt-gobject-domain-interface.c +++ b/libvirt-gobject/libvirt-gobject-domain-interface.c @@ -31,78 +31,22 @@
#include "libvirt-gobject/libvirt-gobject-domain-device-private.h"
-#define GVIR_DOMAIN_INTERFACE_GET_PRIVATE(obj) \ - (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_DOMAIN_INTERFACE, GVirDomainInterfacePrivate)) - -struct _GVirDomainInterfacePrivate -{ - gchar *path; -};
Same comment about the Private struct.
- G_DEFINE_TYPE(GVirDomainInterface, gvir_domain_interface, GVIR_TYPE_DOMAIN_DEVICE);
-enum { - PROP_0, - PROP_PATH, -}; - #define GVIR_DOMAIN_INTERFACE_ERROR gvir_domain_interface_error_quark()
- static GQuark gvir_domain_interface_error_quark(void) { return g_quark_from_static_string("gvir-domain-interface"); }
-static void gvir_domain_interface_get_property(GObject *object, - guint prop_id, - GValue *value, - GParamSpec *pspec) -{ - GVirDomainInterface *self = GVIR_DOMAIN_INTERFACE(object); - GVirDomainInterfacePrivate *priv = self->priv; - - switch (prop_id) { - case PROP_PATH: - g_value_set_string(value, priv->path); - break; - - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); - } -} - - -static void gvir_domain_interface_set_property(GObject *object, - guint prop_id, - const GValue *value, - GParamSpec *pspec) -{ - GVirDomainInterface *self = GVIR_DOMAIN_INTERFACE(object); - GVirDomainInterfacePrivate *priv = self->priv; - - switch (prop_id) { - case PROP_PATH: - g_free(priv->path); - priv->path = g_value_dup_string(value); - break; - - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); - } -} - - static void gvir_domain_interface_finalize(GObject *object) { GVirDomainInterface *self = GVIR_DOMAIN_INTERFACE(object); - GVirDomainInterfacePrivate *priv = self->priv;
g_debug("Finalize GVirDomainInterface=%p", self);
- g_free(priv->path); - G_OBJECT_CLASS(gvir_domain_interface_parent_class)->finalize(object); }
@@ -111,27 +55,11 @@ static void gvir_domain_interface_class_init(GVirDomainInterfaceClass *klass) GObjectClass *object_class = G_OBJECT_CLASS (klass);
object_class->finalize = gvir_domain_interface_finalize; - object_class->get_property = gvir_domain_interface_get_property; - object_class->set_property = gvir_domain_interface_set_property; - - g_object_class_install_property(object_class, - PROP_PATH, - g_param_spec_string("path", - "Path", - "The interface path", - NULL, - G_PARAM_READWRITE | - G_PARAM_CONSTRUCT_ONLY | - G_PARAM_STATIC_STRINGS)); - - g_type_class_add_private(klass, sizeof(GVirDomainInterfacePrivate)); }
static void gvir_domain_interface_init(GVirDomainInterface *self) { g_debug("Init GVirDomainInterface=%p", self); - - self->priv = GVIR_DOMAIN_INTERFACE_GET_PRIVATE(self); }
static GVirDomainInterfaceStats * @@ -140,17 +68,24 @@ gvir_domain_interface_stats_copy(GVirDomainInterfaceStats *stats) return g_slice_dup(GVirDomainInterfaceStats, stats); }
- static void gvir_domain_interface_stats_free(GVirDomainInterfaceStats *stats) { g_slice_free(GVirDomainInterfaceStats, stats); }
- G_DEFINE_BOXED_TYPE(GVirDomainInterfaceStats, gvir_domain_interface_stats, gvir_domain_interface_stats_copy, gvir_domain_interface_stats_free)
+static const gchar *gvir_domain_interface_get_path(GVirDomainInterface *self) +{ + GVirConfigDomainDevice *config; + + config = gvir_domain_device_get_config(GVIR_DOMAIN_DEVICE(self)); + + return gvir_config_domain_interface_get_ifname (GVIR_CONFIG_DOMAIN_INTERFACE (config));
Same leaks as earlier
+} + /** * gvir_domain_interface_get_stats: * @self: the domain interface @@ -166,15 +101,15 @@ GVirDomainInterfaceStats *gvir_domain_interface_get_stats(GVirDomainInterface *s { GVirDomainInterfaceStats *ret = NULL; virDomainInterfaceStatsStruct stats; - GVirDomainInterfacePrivate *priv; virDomainPtr handle; + const gchar *path;
g_return_val_if_fail(GVIR_IS_DOMAIN_INTERFACE(self), NULL);
- priv = self->priv; handle = gvir_domain_device_get_domain_handle(GVIR_DOMAIN_DEVICE(self)); + path = gvir_domain_interface_get_path (self);
Same here.
- if (virDomainInterfaceStats(handle, priv->path, &stats, sizeof (stats)) < 0) { + if (virDomainInterfaceStats(handle, path, &stats, sizeof (stats)) < 0) { gvir_set_error_literal(err, GVIR_DOMAIN_INTERFACE_ERROR, 0, "Unable to get domain interface stats"); diff --git a/libvirt-gobject/libvirt-gobject-domain-interface.h b/libvirt-gobject/libvirt-gobject-domain-interface.h index 62848db..26b7d1c 100644 --- a/libvirt-gobject/libvirt-gobject-domain-interface.h +++ b/libvirt-gobject/libvirt-gobject-domain-interface.h @@ -59,7 +59,8 @@ struct _GVirDomainInterface { GVirDomainDevice parent;
- GVirDomainInterfacePrivate *priv; + /* In case we need a private struct in future */ + gpointer padding[1];
Can you send an updated version of this patch once you have fixed all of these things? Thanks, Christophe
On Wed, Feb 29, 2012 at 02:18:31PM +0100, Christophe Fergeau wrote:
On Tue, Feb 28, 2012 at 08:25:06PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
Remove now redundant 'path' property from GVirDomainDevice subclasses. --- libvirt-gobject/libvirt-gobject-domain-disk.c | 88 ++++---------------- libvirt-gobject/libvirt-gobject-domain-disk.h | 3 +- libvirt-gobject/libvirt-gobject-domain-interface.c | 89 +++----------------- libvirt-gobject/libvirt-gobject-domain-interface.h | 3 +- 4 files changed, 31 insertions(+), 152 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.c b/libvirt-gobject/libvirt-gobject-domain-disk.c index fb7672e..42e0e6c 100644 --- a/libvirt-gobject/libvirt-gobject-domain-disk.c +++ b/libvirt-gobject/libvirt-gobject-domain-disk.c @@ -34,75 +34,22 @@ #define GVIR_DOMAIN_DISK_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_DOMAIN_DISK, GVirDomainDiskPrivate))
-struct _GVirDomainDiskPrivate -{ - gchar *path; -};
The rest of the code (GVirConfig*) always has a Private structure with a gboolean unused when it's empty, I'd rather we stayed consistent (though I keep thinking that we should remove these empty private structs everywhere :)
[snip]
diff --git a/libvirt-gobject/libvirt-gobject-domain-interface.h b/libvirt-gobject/libvirt-gobject-domain-interface.h index 62848db..26b7d1c 100644 --- a/libvirt-gobject/libvirt-gobject-domain-interface.h +++ b/libvirt-gobject/libvirt-gobject-domain-interface.h @@ -59,7 +59,8 @@ struct _GVirDomainInterface { GVirDomainDevice parent;
- GVirDomainInterfacePrivate *priv; + /* In case we need a private struct in future */ + gpointer padding[1];
I prefer that we keep the empty private struct (with a dummy gboolean unsed), rather than doing this. Regards, 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 Wed, Feb 29, 2012 at 3:18 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Tue, Feb 28, 2012 at 08:25:06PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
Remove now redundant 'path' property from GVirDomainDevice subclasses. --- libvirt-gobject/libvirt-gobject-domain-disk.c | 88 ++++---------------- libvirt-gobject/libvirt-gobject-domain-disk.h | 3 +- libvirt-gobject/libvirt-gobject-domain-interface.c | 89 +++----------------- libvirt-gobject/libvirt-gobject-domain-interface.h | 3 +- 4 files changed, 31 insertions(+), 152 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain-disk.c b/libvirt-gobject/libvirt-gobject-domain-disk.c index fb7672e..42e0e6c 100644 --- a/libvirt-gobject/libvirt-gobject-domain-disk.c +++ b/libvirt-gobject/libvirt-gobject-domain-disk.c @@ -34,75 +34,22 @@ #define GVIR_DOMAIN_DISK_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_DOMAIN_DISK, GVirDomainDiskPrivate))
-struct _GVirDomainDiskPrivate -{ - gchar *path; -};
The rest of the code (GVirConfig*) always has a Private structure with a gboolean unused when it's empty, I'd rather we stayed consistent (though I keep thinking that we should remove these empty private structs everywhere :)
Ah ok, I didn't notice it. Since Daniel wants to keeep this private struct too, I'll modify this patch accordingly.
static GVirDomainDiskStats * @@ -151,6 +82,15 @@ gvir_domain_disk_stats_free(GVirDomainDiskStats *stats) G_DEFINE_BOXED_TYPE(GVirDomainDiskStats, gvir_domain_disk_stats, gvir_domain_disk_stats_copy, gvir_domain_disk_stats_free)
+static const gchar *gvir_domain_disk_get_path(GVirDomainDisk *self) +{ + GVirConfigDomainDevice *config; + + config = gvir_domain_device_get_config(GVIR_DOMAIN_DEVICE(self));
config needs to be unref'ed after use.
Oops!
+ + return gvir_config_domain_disk_get_target_dev (GVIR_CONFIG_DOMAIN_DISK (config));
and the return value would be non-const after the changes I mentioned in my previous review.
But you are wrong (see reply to that mail). :)
diff --git a/libvirt-gobject/libvirt-gobject-domain-interface.h b/libvirt-gobject/libvirt-gobject-domain-interface.h index 62848db..26b7d1c 100644 --- a/libvirt-gobject/libvirt-gobject-domain-interface.h +++ b/libvirt-gobject/libvirt-gobject-domain-interface.h @@ -59,7 +59,8 @@ struct _GVirDomainInterface { GVirDomainDevice parent;
- GVirDomainInterfacePrivate *priv; + /* In case we need a private struct in future */ + gpointer padding[1];
Can you send an updated version of this patch once you have fixed all of these things?
Sure, once we agree on the const issue. :) -- Regards, Zeeshan Ali (Khattak) FSF member#5124
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> Currently we only support existing DomainDevice implementations: DomainDisk and DomainInterface. --- .../libvirt-gobject-domain-device-private.h | 2 + libvirt-gobject/libvirt-gobject-domain-device.c | 21 ++++++++++ libvirt-gobject/libvirt-gobject-domain.c | 42 ++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 3 + libvirt-gobject/libvirt-gobject.sym | 1 + 5 files changed, 69 insertions(+), 0 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-domain-device-private.h b/libvirt-gobject/libvirt-gobject-domain-device-private.h index 72c660e..292a2ac 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device-private.h +++ b/libvirt-gobject/libvirt-gobject-domain-device-private.h @@ -24,6 +24,8 @@ G_BEGIN_DECLS +G_GNUC_INTERNAL GVirDomainDevice *gvir_domain_device_new(GVirConfigDomainDevice *config, + GVirDomain *domain); virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self); G_END_DECLS diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c b/libvirt-gobject/libvirt-gobject-domain-device.c index 85879d2..0ec5dad 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device.c +++ b/libvirt-gobject/libvirt-gobject-domain-device.c @@ -176,3 +176,24 @@ GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device) { GVirConfigDomainDevice *gvir_domain_device_get_config(GVirDomainDevice *device) { return g_object_ref (device->priv->config); } + +G_GNUC_INTERNAL GVirDomainDevice *gvir_domain_device_new(GVirConfigDomainDevice *config, + GVirDomain *domain) +{ + GType type; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(config), NULL); + + if (GVIR_CONFIG_IS_DOMAIN_DISK(config)) + type = GVIR_TYPE_DOMAIN_DISK; + else if (GVIR_CONFIG_IS_DOMAIN_INTERFACE(config)) + type = GVIR_TYPE_DOMAIN_INTERFACE; + else { + g_debug("Unknown device type: %s", G_OBJECT_TYPE_NAME(config)); + return NULL; + } + + return GVIR_DOMAIN_DEVICE(g_object_new(type, + "config", config, + "domain", domain, NULL)); +} diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 23ad882..3c66c9c 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -29,6 +29,7 @@ #include "libvirt-glib/libvirt-glib.h" #include "libvirt-gobject/libvirt-gobject.h" #include "libvirt-gobject-compat.h" +#include "libvirt-gobject/libvirt-gobject-domain-device-private.h" #define GVIR_DOMAIN_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_DOMAIN, GVirDomainPrivate)) @@ -868,3 +869,44 @@ gboolean gvir_domain_get_saved(GVirDomain *dom) return virDomainHasManagedSaveImage(dom->priv->handle, 0) == 1; } + +/** + * gvir_domain_get_devices: + * @domain: the domain + * @err: place-holder for possible errors, or NULL + * + * Gets the list of devices attached to @domain + * + * Returns: (element-type LibvirtGObject.DomainDevice) (transfer full): a newly + * allocated #GList of #GVirDomainDevice. + */ +GList *gvir_domain_get_devices(GVirDomain *domain, + GError **err) +{ + GVirConfigDomain *config; + GList *config_devices; + GList *node; + GList *ret = NULL; + + g_return_val_if_fail(GVIR_IS_DOMAIN(domain), NULL); + + config = gvir_domain_get_config(domain, 0, err); + if (config == NULL) + return ret; + + config_devices = gvir_config_domain_get_devices(config); + for (node = config_devices; node != NULL; node = node->next) { + GVirConfigDomainDevice *device_config; + GVirDomainDevice *device; + + device_config = GVIR_CONFIG_DOMAIN_DEVICE(node->data); + device = gvir_domain_device_new(device_config, domain); + if (device != NULL) + ret = g_list_append(ret, device); + + g_object_unref (device_config); + } + g_list_free (config_devices); + + return ret; +} diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 56500a8..8a4836e 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -183,6 +183,9 @@ gboolean gvir_domain_save_finish (GVirDomain *dom, gboolean gvir_domain_get_persistent(GVirDomain *dom); gboolean gvir_domain_get_saved(GVirDomain *dom); +GList *gvir_domain_get_devices(GVirDomain *domain, + GError **err); + G_END_DECLS #endif /* __LIBVIRT_GOBJECT_DOMAIN_H__ */ diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index d6999dc..b7b95cb 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -72,6 +72,7 @@ LIBVIRT_GOBJECT_0.0.4 { gvir_domain_get_persistent; gvir_domain_get_saved; gvir_domain_screenshot; + gvir_domain_get_devices; gvir_domain_snapshot_get_type; gvir_domain_snapshot_handle_get_type; -- 1.7.7.6
On Tue, Feb 28, 2012 at 08:25:07PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
Currently we only support existing DomainDevice implementations: DomainDisk and DomainInterface. --- .../libvirt-gobject-domain-device-private.h | 2 + libvirt-gobject/libvirt-gobject-domain-device.c | 21 ++++++++++ libvirt-gobject/libvirt-gobject-domain.c | 42 ++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 3 + libvirt-gobject/libvirt-gobject.sym | 1 + 5 files changed, 69 insertions(+), 0 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain-device-private.h b/libvirt-gobject/libvirt-gobject-domain-device-private.h index 72c660e..292a2ac 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device-private.h +++ b/libvirt-gobject/libvirt-gobject-domain-device-private.h @@ -24,6 +24,8 @@
G_BEGIN_DECLS
+G_GNUC_INTERNAL GVirDomainDevice *gvir_domain_device_new(GVirConfigDomainDevice *config, + GVirDomain *domain); virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self);
G_END_DECLS diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c b/libvirt-gobject/libvirt-gobject-domain-device.c index 85879d2..0ec5dad 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device.c +++ b/libvirt-gobject/libvirt-gobject-domain-device.c @@ -176,3 +176,24 @@ GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device) { GVirConfigDomainDevice *gvir_domain_device_get_config(GVirDomainDevice *device) { return g_object_ref (device->priv->config); } + +G_GNUC_INTERNAL GVirDomainDevice *gvir_domain_device_new(GVirConfigDomainDevice *config, + GVirDomain *domain)
gvir_domain_new_device(GVirDomain *domain, GVirConfigDomainDevice *config) maybe ? Having the argument that is always non-NULL first feels better to me.
+{ + GType type; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(config), NULL); + + if (GVIR_CONFIG_IS_DOMAIN_DISK(config)) + type = GVIR_TYPE_DOMAIN_DISK; + else if (GVIR_CONFIG_IS_DOMAIN_INTERFACE(config)) + type = GVIR_TYPE_DOMAIN_INTERFACE; + else { + g_debug("Unknown device type: %s", G_OBJECT_TYPE_NAME(config)); + return NULL; + }
I'd have a slight preference for something like switch (G_OBJECT_TYPE(config)) { case GVIR_CONFIG_TYPE_DOMAIN_DISK: type = GVIR_TYPE_DOMAIN_DISK; break; ... but I'm fine with either version.
+ + return GVIR_DOMAIN_DEVICE(g_object_new(type, + "config", config, + "domain", domain, NULL)); +} diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 23ad882..3c66c9c 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -29,6 +29,7 @@ #include "libvirt-glib/libvirt-glib.h" #include "libvirt-gobject/libvirt-gobject.h" #include "libvirt-gobject-compat.h" +#include "libvirt-gobject/libvirt-gobject-domain-device-private.h"
#define GVIR_DOMAIN_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_DOMAIN, GVirDomainPrivate)) @@ -868,3 +869,44 @@ gboolean gvir_domain_get_saved(GVirDomain *dom)
return virDomainHasManagedSaveImage(dom->priv->handle, 0) == 1; } + +/** + * gvir_domain_get_devices: + * @domain: the domain + * @err: place-holder for possible errors, or NULL + * + * Gets the list of devices attached to @domain + * + * Returns: (element-type LibvirtGObject.DomainDevice) (transfer full): a newly + * allocated #GList of #GVirDomainDevice. + */ +GList *gvir_domain_get_devices(GVirDomain *domain, + GError **err) +{ + GVirConfigDomain *config; + GList *config_devices; + GList *node; + GList *ret = NULL; + + g_return_val_if_fail(GVIR_IS_DOMAIN(domain), NULL); + + config = gvir_domain_get_config(domain, 0, err); + if (config == NULL) + return ret; + + config_devices = gvir_config_domain_get_devices(config); + for (node = config_devices; node != NULL; node = node->next) { + GVirConfigDomainDevice *device_config; + GVirDomainDevice *device; + + device_config = GVIR_CONFIG_DOMAIN_DEVICE(node->data); + device = gvir_domain_device_new(device_config, domain); + if (device != NULL) + ret = g_list_append(ret, device);
slight preference for g_list_prepend (much more efficient when the list is big).
+ + g_object_unref (device_config); + } + g_list_free (config_devices);
The individual elements need to be unref'ed too as well as config.
+ + return ret; +} diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 56500a8..8a4836e 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -183,6 +183,9 @@ gboolean gvir_domain_save_finish (GVirDomain *dom, gboolean gvir_domain_get_persistent(GVirDomain *dom); gboolean gvir_domain_get_saved(GVirDomain *dom);
+GList *gvir_domain_get_devices(GVirDomain *domain, + GError **err); + G_END_DECLS
#endif /* __LIBVIRT_GOBJECT_DOMAIN_H__ */ diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index d6999dc..b7b95cb 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -72,6 +72,7 @@ LIBVIRT_GOBJECT_0.0.4 { gvir_domain_get_persistent; gvir_domain_get_saved; gvir_domain_screenshot; + gvir_domain_get_devices;
Try to get the list sorted/consistent with the other entries. Christophe
On Wed, Feb 29, 2012 at 3:30 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Tue, Feb 28, 2012 at 08:25:07PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
Currently we only support existing DomainDevice implementations: DomainDisk and DomainInterface. --- .../libvirt-gobject-domain-device-private.h | 2 + libvirt-gobject/libvirt-gobject-domain-device.c | 21 ++++++++++ libvirt-gobject/libvirt-gobject-domain.c | 42 ++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 3 + libvirt-gobject/libvirt-gobject.sym | 1 + 5 files changed, 69 insertions(+), 0 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain-device-private.h b/libvirt-gobject/libvirt-gobject-domain-device-private.h index 72c660e..292a2ac 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device-private.h +++ b/libvirt-gobject/libvirt-gobject-domain-device-private.h @@ -24,6 +24,8 @@
G_BEGIN_DECLS
+G_GNUC_INTERNAL GVirDomainDevice *gvir_domain_device_new(GVirConfigDomainDevice *config, + GVirDomain *domain); virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self);
G_END_DECLS diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c b/libvirt-gobject/libvirt-gobject-domain-device.c index 85879d2..0ec5dad 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device.c +++ b/libvirt-gobject/libvirt-gobject-domain-device.c @@ -176,3 +176,24 @@ GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device) { GVirConfigDomainDevice *gvir_domain_device_get_config(GVirDomainDevice *device) { return g_object_ref (device->priv->config); } + +G_GNUC_INTERNAL GVirDomainDevice *gvir_domain_device_new(GVirConfigDomainDevice *config, + GVirDomain *domain)
gvir_domain_new_device(GVirDomain *domain, GVirConfigDomainDevice *config) maybe ? Having the argument that is always non-NULL first feels better to me.
Who said config is nullable? :)
+{ + GType type; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(config), NULL); + + if (GVIR_CONFIG_IS_DOMAIN_DISK(config)) + type = GVIR_TYPE_DOMAIN_DISK; + else if (GVIR_CONFIG_IS_DOMAIN_INTERFACE(config)) + type = GVIR_TYPE_DOMAIN_INTERFACE; + else { + g_debug("Unknown device type: %s", G_OBJECT_TYPE_NAME(config)); + return NULL; + }
I'd have a slight preference for something like
switch (G_OBJECT_TYPE(config)) { case GVIR_CONFIG_TYPE_DOMAIN_DISK: type = GVIR_TYPE_DOMAIN_DISK; break; ...
but I'm fine with either version.
I would have prefered it too but if any of these classes are subclassed tomorrow and we forget to update this code, we'll start loosing config instances we can actually handle.
+ + device_config = GVIR_CONFIG_DOMAIN_DEVICE(node->data); + device = gvir_domain_device_new(device_config, domain); + if (device != NULL) + ret = g_list_append(ret, device);
slight preference for g_list_prepend (much more efficient when the list is big).
Good point but wouldn't you expect devices to be sorted in the order they are sorted in the XML?
+ + g_object_unref (device_config); + } + g_list_free (config_devices);
The individual elements need to be unref'ed too as well as config.
Thats what g_object_unref is doing just above? I think I should name this 'device_configs' to be consistent with 'device_config' and therefore clear..
diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index d6999dc..b7b95cb 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -72,6 +72,7 @@ LIBVIRT_GOBJECT_0.0.4 { gvir_domain_get_persistent; gvir_domain_get_saved; gvir_domain_screenshot; + gvir_domain_get_devices;
Try to get the list sorted/consistent with the other entries.
K. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
On Wed, Feb 29, 2012 at 04:01:40PM +0200, Zeeshan Ali (Khattak) wrote:
On Wed, Feb 29, 2012 at 3:30 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Tue, Feb 28, 2012 at 08:25:07PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
Currently we only support existing DomainDevice implementations: DomainDisk and DomainInterface. --- .../libvirt-gobject-domain-device-private.h | 2 + libvirt-gobject/libvirt-gobject-domain-device.c | 21 ++++++++++ libvirt-gobject/libvirt-gobject-domain.c | 42 ++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 3 + libvirt-gobject/libvirt-gobject.sym | 1 + 5 files changed, 69 insertions(+), 0 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain-device-private.h b/libvirt-gobject/libvirt-gobject-domain-device-private.h index 72c660e..292a2ac 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device-private.h +++ b/libvirt-gobject/libvirt-gobject-domain-device-private.h @@ -24,6 +24,8 @@
G_BEGIN_DECLS
+G_GNUC_INTERNAL GVirDomainDevice *gvir_domain_device_new(GVirConfigDomainDevice *config, + GVirDomain *domain); virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self);
G_END_DECLS diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c b/libvirt-gobject/libvirt-gobject-domain-device.c index 85879d2..0ec5dad 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device.c +++ b/libvirt-gobject/libvirt-gobject-domain-device.c @@ -176,3 +176,24 @@ GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device) { GVirConfigDomainDevice *gvir_domain_device_get_config(GVirDomainDevice *device) { return g_object_ref (device->priv->config); } + +G_GNUC_INTERNAL GVirDomainDevice *gvir_domain_device_new(GVirConfigDomainDevice *config, + GVirDomain *domain)
gvir_domain_new_device(GVirDomain *domain, GVirConfigDomainDevice *config) maybe ? Having the argument that is always non-NULL first feels better to me.
Who said config is nullable? :)
I assumed this because of the lack of g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(config), ...) It still kind of sounds better to me to say "let's create a device associated with this GVirDomain", ie to have this method operating on a GVirDomain.
+{ + GType type; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(config), NULL); + + if (GVIR_CONFIG_IS_DOMAIN_DISK(config)) + type = GVIR_TYPE_DOMAIN_DISK; + else if (GVIR_CONFIG_IS_DOMAIN_INTERFACE(config)) + type = GVIR_TYPE_DOMAIN_INTERFACE; + else { + g_debug("Unknown device type: %s", G_OBJECT_TYPE_NAME(config)); + return NULL; + }
I'd have a slight preference for something like
switch (G_OBJECT_TYPE(config)) { case GVIR_CONFIG_TYPE_DOMAIN_DISK: type = GVIR_TYPE_DOMAIN_DISK; break; ...
but I'm fine with either version.
I would have prefered it too but if any of these classes are subclassed tomorrow and we forget to update this code, we'll start loosing config instances we can actually handle.
Ah good point.
+ + device_config = GVIR_CONFIG_DOMAIN_DEVICE(node->data); + device = gvir_domain_device_new(device_config, domain); + if (device != NULL) + ret = g_list_append(ret, device);
slight preference for g_list_prepend (much more efficient when the list is big).
Good point but wouldn't you expect devices to be sorted in the order they are sorted in the XML?
You can always do a g_list_reverse after the fact, and I'd rather we avoid to make such guarantees anyway. If gvir_config_domain_get_devices used g_list_prepend (which I just noticed it's not doing), you'd need to prepend to return things in the order of the XML :) Not that important here, it's just a better practice to use g_list_prepend.
+ + g_object_unref (device_config); + } + g_list_free (config_devices);
The individual elements need to be unref'ed too as well as config.
Thats what g_object_unref is doing just above? I think I should name
True, I didn't pay enough attention.
this 'device_configs' to be consistent with 'device_config' and therefore clear..
I could pretend that's what confused me... :) This would be better indeed now that you say it. Christophe
On Wed, Feb 29, 2012 at 4:24 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Wed, Feb 29, 2012 at 04:01:40PM +0200, Zeeshan Ali (Khattak) wrote:
On Wed, Feb 29, 2012 at 3:30 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Tue, Feb 28, 2012 at 08:25:07PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
Currently we only support existing DomainDevice implementations: DomainDisk and DomainInterface. --- .../libvirt-gobject-domain-device-private.h | 2 + libvirt-gobject/libvirt-gobject-domain-device.c | 21 ++++++++++ libvirt-gobject/libvirt-gobject-domain.c | 42 ++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 3 + libvirt-gobject/libvirt-gobject.sym | 1 + 5 files changed, 69 insertions(+), 0 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain-device-private.h b/libvirt-gobject/libvirt-gobject-domain-device-private.h index 72c660e..292a2ac 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device-private.h +++ b/libvirt-gobject/libvirt-gobject-domain-device-private.h @@ -24,6 +24,8 @@
G_BEGIN_DECLS
+G_GNUC_INTERNAL GVirDomainDevice *gvir_domain_device_new(GVirConfigDomainDevice *config, + GVirDomain *domain); virDomainPtr gvir_domain_device_get_domain_handle(GVirDomainDevice *self);
G_END_DECLS diff --git a/libvirt-gobject/libvirt-gobject-domain-device.c b/libvirt-gobject/libvirt-gobject-domain-device.c index 85879d2..0ec5dad 100644 --- a/libvirt-gobject/libvirt-gobject-domain-device.c +++ b/libvirt-gobject/libvirt-gobject-domain-device.c @@ -176,3 +176,24 @@ GVirDomain *gvir_domain_device_get_domain(GVirDomainDevice *device) { GVirConfigDomainDevice *gvir_domain_device_get_config(GVirDomainDevice *device) { return g_object_ref (device->priv->config); } + +G_GNUC_INTERNAL GVirDomainDevice *gvir_domain_device_new(GVirConfigDomainDevice *config, + GVirDomain *domain)
gvir_domain_new_device(GVirDomain *domain, GVirConfigDomainDevice *config) maybe ? Having the argument that is always non-NULL first feels better to me.
Who said config is nullable? :)
I assumed this because of the lack of g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_DEVICE(config), ...)
Ah, should fix that.
It still kind of sounds better to me to say "let's create a device associated with this GVirDomain", ie to have this method operating on a GVirDomain.
Hmm.. OK, i'll change the order.
+ + device_config = GVIR_CONFIG_DOMAIN_DEVICE(node->data); + device = gvir_domain_device_new(device_config, domain); + if (device != NULL) + ret = g_list_append(ret, device);
slight preference for g_list_prepend (much more efficient when the list is big).
Good point but wouldn't you expect devices to be sorted in the order they are sorted in the XML?
You can always do a g_list_reverse after the fact, and I'd rather we avoid to make such guarantees anyway. If gvir_config_domain_get_devices used g_list_prepend (which I just noticed it's not doing), you'd need to prepend to return things in the order of the XML :) Not that important here, it's just a better practice to use g_list_prepend.
OK. Will change this too then.. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
On Tue, Feb 28, 2012 at 08:25:02PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gconfig/libvirt-gconfig-domain-interface.c | 35 ++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain-interface.h | 4 ++ libvirt-gconfig/libvirt-gconfig.sym | 4 ++ 3 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.c b/libvirt-gconfig/libvirt-gconfig-domain-interface.c index 85cc194..61d35bd 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-interface.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.c @@ -96,6 +96,41 @@ void gvir_config_domain_interface_set_model(GVirConfigDomainInterface *interface "model", "type", model); }
+const char *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface *interface)
Unless I'm missing something, this should not be const (caller needs to free the returned string).
+{ + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), NULL); + + return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(interface), + "target", "device");
This is "dev", not "device"
+} + +GVirConfigDomainInterfaceLinkState gvir_config_domain_interface_get_link_state(GVirConfigDomainInterface *interface) +{ + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), + GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_DEFAULT); + + return gvir_config_object_get_attribute_genum(GVIR_CONFIG_OBJECT(interface), + "link", "state", + GVIR_CONFIG_TYPE_DOMAIN_INTERFACE_LINK_STATE, + GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_DEFAULT); +}
Have you tested this? Is it available for reading after having been set? The reason I'm asking is that libvirt documentation only talk about modifying it.
+ +const char *gvir_config_domain_interface_get_mac(GVirConfigDomainInterface *interface)
Non-const here too
+{ + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), NULL); + + return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(interface), + "mac", "address"); +} + +const char *gvir_config_domain_interface_get_model(GVirConfigDomainInterface *interface)
And same for this getter.
+{ + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), NULL); + + return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(interface), + "model", "type"); +} + G_GNUC_INTERNAL GVirConfigDomainDevice * gvir_config_domain_interface_new_from_tree(GVirConfigXmlDoc *doc, xmlNodePtr tree) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.h b/libvirt-gconfig/libvirt-gconfig-domain-interface.h index 6e802fb..c8c4fb3 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-interface.h +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.h @@ -72,6 +72,10 @@ void gvir_config_domain_interface_set_mac(GVirConfigDomainInterface *interface, const char *mac_address); void gvir_config_domain_interface_set_model(GVirConfigDomainInterface *interface, const char *model); +const char *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface *interface); +GVirConfigDomainInterfaceLinkState gvir_config_domain_interface_get_link_state(GVirConfigDomainInterface *interface); +const char *gvir_config_domain_interface_get_mac(GVirConfigDomainInterface *interface); +const char *gvir_config_domain_interface_get_model(GVirConfigDomainInterface *interface);
G_END_DECLS
diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index 96ce58f..f91b8b0 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -145,6 +145,10 @@ LIBVIRT_GCONFIG_0.0.4 { gvir_config_domain_interface_set_link_state; gvir_config_domain_interface_set_mac; gvir_config_domain_interface_set_model; + gvir_config_domain_interface_get_ifname; + gvir_config_domain_interface_get_link_state; + gvir_config_domain_interface_get_mac; + gvir_config_domain_interface_get_model;
The other sections in this file put getter and setter for the same property next to each other, I'd prefer if we stayed consistant: + gvir_config_domain_interface_get_ifname; + gvir_config_domain_interface_set_ifname; + gvir_config_domain_interface_get_link_state; gvir_config_domain_interface_set_link_state; + gvir_config_domain_interface_get_mac; gvir_config_domain_interface_set_mac; + gvir_config_domain_interface_get_model; gvir_config_domain_interface_set_model;
gvir_config_domain_interface_bridge_get_type; gvir_config_domain_interface_bridge_new; -- 1.7.7.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Feb 29, 2012 at 2:57 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Tue, Feb 28, 2012 at 08:25:02PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gconfig/libvirt-gconfig-domain-interface.c | 35 ++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain-interface.h | 4 ++ libvirt-gconfig/libvirt-gconfig.sym | 4 ++ 3 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.c b/libvirt-gconfig/libvirt-gconfig-domain-interface.c index 85cc194..61d35bd 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-interface.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.c @@ -96,6 +96,41 @@ void gvir_config_domain_interface_set_model(GVirConfigDomainInterface *interface "model", "type", model); }
+const char *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface *interface)
Unless I'm missing something, this should not be const (caller needs to free the returned string).
String getters usually do return const in the gobject world[1] as opposed to object getters as one require allocation/de-allocation and the other only requires incrementing/decrementing a counter. Also is the fact that strings are readily usable so you can just make a call to the getter and thats it but objects are not usually readily usable in the sense that you have to pass it to to some other function to do something with it. Note that I'm just making educated guesses here as to why 'const' for strings makes more sense as to why this practice is followed.
+{ + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), NULL); + + return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(interface), + "target", "device");
This is "dev", not "device"
OK
+} + +GVirConfigDomainInterfaceLinkState gvir_config_domain_interface_get_link_state(GVirConfigDomainInterface *interface) +{ + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), + GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_DEFAULT); + + return gvir_config_object_get_attribute_genum(GVIR_CONFIG_OBJECT(interface), + "link", "state", + GVIR_CONFIG_TYPE_DOMAIN_INTERFACE_LINK_STATE, + GVIR_CONFIG_DOMAIN_INTERFACE_LINK_STATE_DEFAULT); +}
Have you tested this? Is it available for reading after having been set? The reason I'm asking is that libvirt documentation only talk about modifying it.
No, I haven't really tested this part. The only test I have done is with the Boxes patch[2].
diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index 96ce58f..f91b8b0 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -145,6 +145,10 @@ LIBVIRT_GCONFIG_0.0.4 { gvir_config_domain_interface_set_link_state; gvir_config_domain_interface_set_mac; gvir_config_domain_interface_set_model; + gvir_config_domain_interface_get_ifname; + gvir_config_domain_interface_get_link_state; + gvir_config_domain_interface_get_mac; + gvir_config_domain_interface_get_model;
The other sections in this file put getter and setter for the same property next to each other, I'd prefer if we stayed consistant:
Sure. -- Regards, Zeeshan Ali (Khattak) FSF member#5124 [1] http://developer.gnome.org/gtk/2.24/GtkWindow.html#gtk-window-get-icon-name http://developer.gnome.org/gtk/2.24/GtkWindow.html#gtk-window-get-title http://developer.gnome.org/gio/stable/GFileInfo.html#g-file-info-get-name http://developer.gnome.org/gio/stable/GFileInfo.html#g-file-info-get-display... I can provide many many more examples for core gnome libs if you like but I guess you get the point :) [2] https://bugzilla.gnome.org/show_bug.cgi?id=670994
On Wed, Feb 29, 2012 at 03:44:20PM +0200, Zeeshan Ali (Khattak) wrote:
On Wed, Feb 29, 2012 at 2:57 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Tue, Feb 28, 2012 at 08:25:02PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gconfig/libvirt-gconfig-domain-interface.c | 35 ++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain-interface.h | 4 ++ libvirt-gconfig/libvirt-gconfig.sym | 4 ++ 3 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.c b/libvirt-gconfig/libvirt-gconfig-domain-interface.c index 85cc194..61d35bd 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-interface.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.c @@ -96,6 +96,41 @@ void gvir_config_domain_interface_set_model(GVirConfigDomainInterface *interface "model", "type", model); }
+const char *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface *interface)
Unless I'm missing something, this should not be const (caller needs to free the returned string).
String getters usually do return const in the gobject world[1] as opposed to object getters as one require allocation/de-allocation and the other only requires incrementing/decrementing a counter. Also is the fact that strings are readily usable so you can just make a call to the getter and thats it but objects are not usually readily usable in the sense that you have to pass it to to some other function to do something with it. Note that I'm just making educated guesses here as to why 'const' for strings makes more sense as to why this practice is followed.
Strings are const in the gobject world when they are owned by the object and not by the caller of the getter (ie the caller doesn't need to free them). They are non-const when the caller has to free them. In this case, the strings need to be freed by the caller, thus "const" should not be used. Christophe
On Wed, Feb 29, 2012 at 3:49 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Wed, Feb 29, 2012 at 03:44:20PM +0200, Zeeshan Ali (Khattak) wrote:
On Wed, Feb 29, 2012 at 2:57 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Tue, Feb 28, 2012 at 08:25:02PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gconfig/libvirt-gconfig-domain-interface.c | 35 ++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain-interface.h | 4 ++ libvirt-gconfig/libvirt-gconfig.sym | 4 ++ 3 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.c b/libvirt-gconfig/libvirt-gconfig-domain-interface.c index 85cc194..61d35bd 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-interface.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.c @@ -96,6 +96,41 @@ void gvir_config_domain_interface_set_model(GVirConfigDomainInterface *interface "model", "type", model); }
+const char *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface *interface)
Unless I'm missing something, this should not be const (caller needs to free the returned string).
String getters usually do return const in the gobject world[1] as opposed to object getters as one require allocation/de-allocation and the other only requires incrementing/decrementing a counter. Also is the fact that strings are readily usable so you can just make a call to the getter and thats it but objects are not usually readily usable in the sense that you have to pass it to to some other function to do something with it. Note that I'm just making educated guesses here as to why 'const' for strings makes more sense as to why this practice is followed.
Strings are const in the gobject world when they are owned by the object and not by the caller of the getter (ie the caller doesn't need to free them). They are non-const when the caller has to free them. In this case, the strings need to be freed by the caller, thus "const" should not be used.
Ah, I just realized why that is so: static char *libxml_str_to_glib(xmlChar *str) { char *g_str; if (str == NULL) return NULL; g_str = g_strdup((char *)str); xmlFree(str); return g_str; } This function is not needed as all you needed was to cast the 'xmlChar *' to 'const gchar *' and return const from all users of this function. Since we still are not API/ABI stable, I propose we change this all over as there is no need to force apps to free strings all the time and waste processor/memory on all these string allocation/de-allocation. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
On Wed, Feb 29, 2012 at 04:10:29PM +0200, Zeeshan Ali (Khattak) wrote:
Ah, I just realized why that is so:
static char *libxml_str_to_glib(xmlChar *str) { char *g_str;
if (str == NULL) return NULL; g_str = g_strdup((char *)str); xmlFree(str);
return g_str; }
This function is not needed as all you needed was to cast the 'xmlChar *' to 'const gchar *' and return const from all users of this function. Since we still are not API/ABI stable, I propose we change this all over as there is no need to force apps to free strings all the time and waste processor/memory on all these string allocation/de-allocation.
You'll still need to free the input "str", and you have no guarantee that xmlFree and g_free will call the same function to free memory in the end, especially in a library. This function is sucky, but there is no clean way around it as far as I know. If you really insist on returning const from your getters, you'll need to cache their value in GVirDomainDevicePrivate Christophe
On Wed, Feb 29, 2012 at 4:22 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Wed, Feb 29, 2012 at 04:10:29PM +0200, Zeeshan Ali (Khattak) wrote:
Ah, I just realized why that is so:
static char *libxml_str_to_glib(xmlChar *str) { char *g_str;
if (str == NULL) return NULL; g_str = g_strdup((char *)str); xmlFree(str);
return g_str; }
This function is not needed as all you needed was to cast the 'xmlChar *' to 'const gchar *' and return const from all users of this function. Since we still are not API/ABI stable, I propose we change this all over as there is no need to force apps to free strings all the time and waste processor/memory on all these string allocation/de-allocation.
You'll still need to free the input "str", and you have no guarantee that xmlFree and g_free will call the same function to free memory in the end,
There wont' be any need for freeing if gvir_config_xml_get_child_element_content() and similar functions returned chid_node->content rather than xmlNodeGetContent (child_node). That way you never need to free any string. The original gupnp-av code that you based this xml utils code on, does exactly that. I wonder why you changed it?
If you really insist on returning const from your getters, you'll need to cache their value in GVirDomainDevicePrivate
Not if its const returned from all functions. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
On Wed, Feb 29, 2012 at 05:26:58PM +0200, Zeeshan Ali (Khattak) wrote:
There wont' be any need for freeing if gvir_config_xml_get_child_element_content() and similar functions returned chid_node->content rather than xmlNodeGetContent (child_node). That way you never need to free any string. The original gupnp-av code that you based this xml utils code on, does exactly that. I wonder why you changed it?
I didn't copy and paste the code nor look at it that closely. Since libxml2 has public getters which seem to be doing complicated things, it felt safer to use them :) Christophe
On Wed, Feb 29, 2012 at 2:57 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Tue, Feb 28, 2012 at 08:25:02PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gconfig/libvirt-gconfig-domain-interface.c | 35 ++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain-interface.h | 4 ++ libvirt-gconfig/libvirt-gconfig.sym | 4 ++ 3 files changed, 43 insertions(+), 0 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.c b/libvirt-gconfig/libvirt-gconfig-domain-interface.c index 85cc194..61d35bd 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-interface.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.c @@ -96,6 +96,41 @@ void gvir_config_domain_interface_set_model(GVirConfigDomainInterface *interface "model", "type", model); }
+const char *gvir_config_domain_interface_get_ifname(GVirConfigDomainInterface *interface)
Unless I'm missing something, this should not be const (caller needs to free the returned string).
+{ + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface), NULL); + + return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(interface), + "target", "device");
This is "dev", not "device"
Turns out that i copy&pasted the "device" from the corresponding setter. Wonder if this explains why I see a flat network graph in boxes for every domain.. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
On Wed, Feb 29, 2012 at 11:22:52PM +0200, Zeeshan Ali (Khattak) wrote:
Turns out that i copy&pasted the "device" from the corresponding setter.
I realized yesterday that you probably got "device" from the setter and that I should have checked if the attribute name was correct there... I just checked both libvirt documentation and its source code, and "dev" is the correct name. Christophe
participants (3)
-
Christophe Fergeau -
Daniel P. Berrange -
Zeeshan Ali (Khattak)