12:25 p.m.
From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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
12:25 p.m.
New subject: [libvirt] [libvirt-glib 2/6] Add gvir_domain_disk_resize()
From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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
7 a.m.
New subject: [libvirt] [libvirt-glib 2/6] Add gvir_domain_disk_resize()
ACK
On Tue, Feb 28, 2012 at 08:25:03PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
12:25 p.m.
New subject: [libvirt] [libvirt-glib 3/6] Add gvir_domain_device_get_domain()
From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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
7:02 a.m.
New subject: [libvirt] [libvirt-glib 3/6] Add gvir_domain_device_get_domain()
On Tue, Feb 28, 2012 at 08:25:04PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
7:05 a.m.
New subject: [libvirt] [libvirt-glib 3/6] Add gvir_domain_device_get_domain()
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(a)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(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
12:25 p.m.
New subject: [libvirt] [libvirt-glib 4/6] Add 'config' property to GVirDomainDevice
From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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
7:11 a.m.
New subject: [libvirt] [libvirt-glib 4/6] Add 'config' property to GVirDomainDevice
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
12:25 p.m.
New subject: [libvirt] [libvirt-glib 5/6] Remove now redundant 'path' property
From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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
7:18 a.m.
New subject: [libvirt] [libvirt-glib 5/6] Remove now redundant 'path' property
On Tue, Feb 28, 2012 at 08:25:06PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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
7:26 a.m.
New subject: [libvirt] [libvirt-glib 5/6] Remove now redundant 'path' property
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(a)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 :|
7:51 a.m.
New subject: [libvirt] [libvirt-glib 5/6] Remove now redundant 'path' property
On Wed, Feb 29, 2012 at 3:18 PM, Christophe Fergeau <cfergeau(a)redhat.com> wrote:
On Tue, Feb 28, 2012 at 08:25:06PM +0200, Zeeshan Ali (Khattak)
wrote:
> From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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
12:25 p.m.
New subject: [libvirt] [libvirt-glib 6/6] Add gvir_domain_get_devices()
From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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
7:30 a.m.
New subject: [libvirt] [libvirt-glib 6/6] Add gvir_domain_get_devices()
On Tue, Feb 28, 2012 at 08:25:07PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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
8:01 a.m.
New subject: [libvirt] [libvirt-glib 6/6] Add gvir_domain_get_devices()
On Wed, Feb 29, 2012 at 3:30 PM, Christophe Fergeau <cfergeau(a)redhat.com> wrote:
On Tue, Feb 28, 2012 at 08:25:07PM +0200, Zeeshan Ali (Khattak)
wrote:
> From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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
8:24 a.m.
New subject: [libvirt] [libvirt-glib 6/6] Add gvir_domain_get_devices()
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(a)redhat.com> wrote:
> On Tue, Feb 28, 2012 at 08:25:07PM +0200, Zeeshan Ali (Khattak) wrote:
>> From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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
9:37 a.m.
New subject: [libvirt] [libvirt-glib 6/6] Add gvir_domain_get_devices()
On Wed, Feb 29, 2012 at 4:24 PM, Christophe Fergeau <cfergeau(a)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(a)redhat.com>
wrote:
> > On Tue, Feb 28, 2012 at 08:25:07PM +0200, Zeeshan Ali (Khattak) wrote:
> >> From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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
6:57 a.m.
On Tue, Feb 28, 2012 at 08:25:02PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
7:44 a.m.
On Wed, Feb 29, 2012 at 2:57 PM, Christophe Fergeau <cfergeau(a)redhat.com> wrote:
On Tue, Feb 28, 2012 at 08:25:02PM +0200, Zeeshan Ali (Khattak)
wrote:
> From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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-disp...
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
7:49 a.m.
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(a)redhat.com> wrote:
> On Tue, Feb 28, 2012 at 08:25:02PM +0200, Zeeshan Ali (Khattak) wrote:
>> From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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
8:10 a.m.
On Wed, Feb 29, 2012 at 3:49 PM, Christophe Fergeau <cfergeau(a)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(a)redhat.com>
wrote:
> > On Tue, Feb 28, 2012 at 08:25:02PM +0200, Zeeshan Ali (Khattak) wrote:
> >> From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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
8:22 a.m.
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
9:26 a.m.
On Wed, Feb 29, 2012 at 4:22 PM, Christophe Fergeau <cfergeau(a)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
9:41 a.m.
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
3:22 p.m.
On Wed, Feb 29, 2012 at 2:57 PM, Christophe Fergeau <cfergeau(a)redhat.com> wrote:
On Tue, Feb 28, 2012 at 08:25:02PM +0200, Zeeshan Ali (Khattak)
wrote:
> From: "Zeeshan Ali (Khattak)" <zeeshanak(a)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
2:40 a.m.
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
4680
days inactive
4682
days old
25 comments
3 participants
participants (3)
-
Christophe Fergeau -
Daniel P. Berrange -
Zeeshan Ali (Khattak)