[libvirt] [libvirt-glib 1/3] API to get/set 'description' node in domain config

From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> --- libvirt-gconfig/libvirt-gconfig-domain.c | 28 ++++++++++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain.h | 2 ++ libvirt-gconfig/libvirt-gconfig.sym | 2 ++ 3 files changed, 32 insertions(+), 0 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index fba1ee2..c4027a3 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -39,6 +39,7 @@ G_DEFINE_TYPE(GVirConfigDomain, gvir_config_domain, GVIR_CONFIG_TYPE_OBJECT); enum { PROP_0, PROP_NAME, + PROP_DESCRIPTION, PROP_MEMORY, PROP_VCPU, PROP_FEATURES @@ -55,6 +56,9 @@ static void gvir_config_domain_get_property(GObject *object, case PROP_NAME: g_value_take_string(value, gvir_config_domain_get_name(domain)); break; + case PROP_DESCRIPTION: + g_value_take_string(value, gvir_config_domain_get_description(domain)); + break; case PROP_MEMORY: g_value_set_uint64(value, gvir_config_domain_get_memory(domain)); break; @@ -81,6 +85,9 @@ static void gvir_config_domain_set_property(GObject *object, case PROP_NAME: gvir_config_domain_set_name(domain, g_value_get_string(value)); break; + case PROP_DESCRIPTION: + gvir_config_domain_set_description(domain, g_value_get_string(value)); + break; case PROP_MEMORY: gvir_config_domain_set_memory(domain, g_value_get_uint64(value)); break; @@ -114,6 +121,14 @@ static void gvir_config_domain_class_init(GVirConfigDomainClass *klass) G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)); g_object_class_install_property(object_class, + PROP_DESCRIPTION, + g_param_spec_string("description", + "Description", + "Description (could be anything).", + NULL, + G_PARAM_READWRITE | + G_PARAM_STATIC_STRINGS)); + g_object_class_install_property(object_class, PROP_MEMORY, g_param_spec_uint64("memory", "Memory", @@ -196,6 +211,19 @@ void gvir_config_domain_set_name(GVirConfigDomain *domain, const char *name) g_object_notify(G_OBJECT(domain), "name"); } +char *gvir_config_domain_get_description(GVirConfigDomain *domain) +{ + return gvir_config_object_get_node_content(GVIR_CONFIG_OBJECT(domain), + "description"); +} + +void gvir_config_domain_set_description(GVirConfigDomain *domain, const char *description) +{ + gvir_config_object_set_node_content(GVIR_CONFIG_OBJECT(domain), + "description", description); + g_object_notify(G_OBJECT(domain), "description"); +} + /** * gvir_config_domain_get_memory: * @domain: A domain configuration object. diff --git a/libvirt-gconfig/libvirt-gconfig-domain.h b/libvirt-gconfig/libvirt-gconfig-domain.h index e68f1ac..6cdaec9 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.h +++ b/libvirt-gconfig/libvirt-gconfig-domain.h @@ -102,6 +102,8 @@ GVirConfigDomain *gvir_config_domain_new(void); void gvir_config_domain_set_virt_type(GVirConfigDomain *domain, GVirConfigDomainVirtType type); char *gvir_config_domain_get_name(GVirConfigDomain *domain); void gvir_config_domain_set_name(GVirConfigDomain *domain, const char *name); +char *gvir_config_domain_get_description(GVirConfigDomain *domain); +void gvir_config_domain_set_description(GVirConfigDomain *domain, const char *description); guint64 gvir_config_domain_get_memory(GVirConfigDomain *domain); void gvir_config_domain_set_memory(GVirConfigDomain *domain, guint64 memory); guint64 gvir_config_domain_get_vcpus(GVirConfigDomain *domain); diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index 9bfe1d9..383acca 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -23,6 +23,8 @@ LIBVIRT_GCONFIG_0.0.4 { gvir_config_domain_set_memory; gvir_config_domain_get_name; gvir_config_domain_set_name; + gvir_config_domain_get_description; + gvir_config_domain_set_description; gvir_config_domain_set_os; gvir_config_domain_set_seclabel; gvir_config_domain_get_vcpus; -- 1.7.7.5

From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> virDomainIsPersistent() failing most probably means that domain is no longer available so flaging it as transient isn't exactly wrong. --- libvirt-gobject/libvirt-gobject-domain.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 2bc12d9..c1a67a5 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -844,5 +844,5 @@ gboolean gvir_domain_get_persistent(GVirDomain *dom) { g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE); - return virDomainIsPersistent(dom->priv->handle); + return virDomainIsPersistent(dom->priv->handle) == 1; } -- 1.7.7.5

From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> --- libvirt-gobject/libvirt-gobject-storage-pool.c | 6 +++--- libvirt-gobject/libvirt-gobject-storage-pool.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 5bd3f0a..bf25641 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -499,11 +499,11 @@ GList *gvir_storage_pool_get_volumes(GVirStoragePool *pool) * * Return value: (transfer full): the #GVirStorageVol, or NULL */ -GVirStoragePool *gvir_storage_pool_get_volume(GVirStoragePool *pool, - const gchar *name) +GVirStorageVol *gvir_storage_pool_get_volume(GVirStoragePool *pool, + const gchar *name) { GVirStoragePoolPrivate *priv = pool->priv; - GVirStoragePool *volume; + GVirStorageVol *volume; g_mutex_lock(priv->lock); volume = g_hash_table_lookup(priv->volumes, name); diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.h b/libvirt-gobject/libvirt-gobject-storage-pool.h index 778844b..4589844 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.h +++ b/libvirt-gobject/libvirt-gobject-storage-pool.h @@ -102,8 +102,8 @@ gboolean gvir_storage_pool_refresh_finish(GVirStoragePool *pool, GError **err); GList *gvir_storage_pool_get_volumes(GVirStoragePool *pool); -GVirStoragePool *gvir_storage_pool_get_volume(GVirStoragePool *pool, - const gchar *name); +GVirStorageVol *gvir_storage_pool_get_volume(GVirStoragePool *pool, + const gchar *name); GVirStorageVol *gvir_storage_pool_create_volume (GVirStoragePool *pool, GVirConfigStorageVol *conf, -- 1.7.7.5

On Wed, Jan 18, 2012 at 04:53:29AM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gobject/libvirt-gobject-storage-pool.c | 6 +++--- libvirt-gobject/libvirt-gobject-storage-pool.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 5bd3f0a..bf25641 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -499,11 +499,11 @@ GList *gvir_storage_pool_get_volumes(GVirStoragePool *pool) * * Return value: (transfer full): the #GVirStorageVol, or NULL */ -GVirStoragePool *gvir_storage_pool_get_volume(GVirStoragePool *pool, - const gchar *name) +GVirStorageVol *gvir_storage_pool_get_volume(GVirStoragePool *pool, + const gchar *name) { GVirStoragePoolPrivate *priv = pool->priv; - GVirStoragePool *volume; + GVirStorageVol *volume;
g_mutex_lock(priv->lock); volume = g_hash_table_lookup(priv->volumes, name);
Might be worth adding a g_return_val_if_fail(GVIR_IS_STORAGE_VOL(volume)) here when volume is non NULL ? Christophe

On Wed, Jan 18, 2012 at 12:11 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Wed, Jan 18, 2012 at 04:53:29AM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gobject/libvirt-gobject-storage-pool.c | 6 +++--- libvirt-gobject/libvirt-gobject-storage-pool.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 5bd3f0a..bf25641 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -499,11 +499,11 @@ GList *gvir_storage_pool_get_volumes(GVirStoragePool *pool) * * Return value: (transfer full): the #GVirStorageVol, or NULL */ -GVirStoragePool *gvir_storage_pool_get_volume(GVirStoragePool *pool, - const gchar *name) +GVirStorageVol *gvir_storage_pool_get_volume(GVirStoragePool *pool, + const gchar *name) { GVirStoragePoolPrivate *priv = pool->priv; - GVirStoragePool *volume; + GVirStorageVol *volume;
g_mutex_lock(priv->lock); volume = g_hash_table_lookup(priv->volumes, name);
Might be worth adding a g_return_val_if_fail(GVIR_IS_STORAGE_VOL(volume)) here when volume is non NULL ?
We don't put anything other than volumes to this hashtable so there is no need AFAICT. -- Regards, Zeeshan Ali (Khattak) FSF member#5124

On Wed, Jan 18, 2012 at 03:49:28PM +0200, Zeeshan Ali (Khattak) wrote:
On Wed, Jan 18, 2012 at 12:11 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Wed, Jan 18, 2012 at 04:53:29AM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gobject/libvirt-gobject-storage-pool.c | 6 +++--- libvirt-gobject/libvirt-gobject-storage-pool.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 5bd3f0a..bf25641 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -499,11 +499,11 @@ GList *gvir_storage_pool_get_volumes(GVirStoragePool *pool) * * Return value: (transfer full): the #GVirStorageVol, or NULL */ -GVirStoragePool *gvir_storage_pool_get_volume(GVirStoragePool *pool, - const gchar *name) +GVirStorageVol *gvir_storage_pool_get_volume(GVirStoragePool *pool, + const gchar *name) { GVirStoragePoolPrivate *priv = pool->priv; - GVirStoragePool *volume; + GVirStorageVol *volume;
g_mutex_lock(priv->lock); volume = g_hash_table_lookup(priv->volumes, name);
Might be worth adding a g_return_val_if_fail(GVIR_IS_STORAGE_VOL(volume)) here when volume is non NULL ?
We don't put anything other than volumes to this hashtable so there is no need AFAICT.
Well, we were mistakenly casting its content to the wrong type, who knows which other mistakes we will be doing in the future? I was just asking if it's worth adding this check as a way to catch future bugs. Christophe

On Wed, Jan 18, 2012 at 4:07 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Wed, Jan 18, 2012 at 03:49:28PM +0200, Zeeshan Ali (Khattak) wrote:
On Wed, Jan 18, 2012 at 12:11 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Wed, Jan 18, 2012 at 04:53:29AM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gobject/libvirt-gobject-storage-pool.c | 6 +++--- libvirt-gobject/libvirt-gobject-storage-pool.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-storage-pool.c b/libvirt-gobject/libvirt-gobject-storage-pool.c index 5bd3f0a..bf25641 100644 --- a/libvirt-gobject/libvirt-gobject-storage-pool.c +++ b/libvirt-gobject/libvirt-gobject-storage-pool.c @@ -499,11 +499,11 @@ GList *gvir_storage_pool_get_volumes(GVirStoragePool *pool) * * Return value: (transfer full): the #GVirStorageVol, or NULL */ -GVirStoragePool *gvir_storage_pool_get_volume(GVirStoragePool *pool, - const gchar *name) +GVirStorageVol *gvir_storage_pool_get_volume(GVirStoragePool *pool, + const gchar *name) { GVirStoragePoolPrivate *priv = pool->priv; - GVirStoragePool *volume; + GVirStorageVol *volume;
g_mutex_lock(priv->lock); volume = g_hash_table_lookup(priv->volumes, name);
Might be worth adding a g_return_val_if_fail(GVIR_IS_STORAGE_VOL(volume)) here when volume is non NULL ?
We don't put anything other than volumes to this hashtable so there is no need AFAICT.
Well, we were mistakenly casting its content to the wrong type, who knows which other mistakes we will be doing in the future? I was just asking if it's worth adding this check as a way to catch future bugs.
That mistake was immediately caught as soon as I used this function in Boxes, so a warning on console would not have helped anything. -- Regards, Zeeshan Ali (Khattak) FSF member#5124

On Wed, Jan 18, 2012 at 04:53:27AM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gconfig/libvirt-gconfig-domain.c | 28 ++++++++++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain.h | 2 ++ libvirt-gconfig/libvirt-gconfig.sym | 2 ++ 3 files changed, 32 insertions(+), 0 deletions(-)
ACK, to all 3 patches. 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, Jan 18, 2012 at 04:53:27AM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gconfig/libvirt-gconfig-domain.c | 28 ++++++++++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain.h | 2 ++ libvirt-gconfig/libvirt-gconfig.sym | 2 ++ 3 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index fba1ee2..c4027a3 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -39,6 +39,7 @@ G_DEFINE_TYPE(GVirConfigDomain, gvir_config_domain, GVIR_CONFIG_TYPE_OBJECT); enum { PROP_0, PROP_NAME, + PROP_DESCRIPTION, PROP_MEMORY, PROP_VCPU, PROP_FEATURES @@ -55,6 +56,9 @@ static void gvir_config_domain_get_property(GObject *object, case PROP_NAME: g_value_take_string(value, gvir_config_domain_get_name(domain)); break; + case PROP_DESCRIPTION: + g_value_take_string(value, gvir_config_domain_get_description(domain)); + break; case PROP_MEMORY: g_value_set_uint64(value, gvir_config_domain_get_memory(domain)); break; @@ -81,6 +85,9 @@ static void gvir_config_domain_set_property(GObject *object, case PROP_NAME: gvir_config_domain_set_name(domain, g_value_get_string(value)); break; + case PROP_DESCRIPTION: + gvir_config_domain_set_description(domain, g_value_get_string(value)); + break; case PROP_MEMORY: gvir_config_domain_set_memory(domain, g_value_get_uint64(value)); break; @@ -114,6 +121,14 @@ static void gvir_config_domain_class_init(GVirConfigDomainClass *klass) G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)); g_object_class_install_property(object_class, + PROP_DESCRIPTION, + g_param_spec_string("description", + "Description", + "Description (could be anything).",
maybe "arbitrary human-readable description" would be better than "could be anything" ?
+ NULL, + G_PARAM_READWRITE | + G_PARAM_STATIC_STRINGS)); + g_object_class_install_property(object_class, PROP_MEMORY, g_param_spec_uint64("memory", "Memory", @@ -196,6 +211,19 @@ void gvir_config_domain_set_name(GVirConfigDomain *domain, const char *name) g_object_notify(G_OBJECT(domain), "name"); }
+char *gvir_config_domain_get_description(GVirConfigDomain *domain) +{ + return gvir_config_object_get_node_content(GVIR_CONFIG_OBJECT(domain), + "description"); +} + +void gvir_config_domain_set_description(GVirConfigDomain *domain, const char *description) +{ + gvir_config_object_set_node_content(GVIR_CONFIG_OBJECT(domain), + "description", description); + g_object_notify(G_OBJECT(domain), "description"); +} + /** * gvir_config_domain_get_memory: * @domain: A domain configuration object. diff --git a/libvirt-gconfig/libvirt-gconfig-domain.h b/libvirt-gconfig/libvirt-gconfig-domain.h index e68f1ac..6cdaec9 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.h +++ b/libvirt-gconfig/libvirt-gconfig-domain.h @@ -102,6 +102,8 @@ GVirConfigDomain *gvir_config_domain_new(void); void gvir_config_domain_set_virt_type(GVirConfigDomain *domain, GVirConfigDomainVirtType type); char *gvir_config_domain_get_name(GVirConfigDomain *domain); void gvir_config_domain_set_name(GVirConfigDomain *domain, const char *name); +char *gvir_config_domain_get_description(GVirConfigDomain *domain); +void gvir_config_domain_set_description(GVirConfigDomain *domain, const char *description); guint64 gvir_config_domain_get_memory(GVirConfigDomain *domain); void gvir_config_domain_set_memory(GVirConfigDomain *domain, guint64 memory); guint64 gvir_config_domain_get_vcpus(GVirConfigDomain *domain); diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index 9bfe1d9..383acca 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -23,6 +23,8 @@ LIBVIRT_GCONFIG_0.0.4 { gvir_config_domain_set_memory; gvir_config_domain_get_name; gvir_config_domain_set_name; + gvir_config_domain_get_description; + gvir_config_domain_set_description; gvir_config_domain_set_os; gvir_config_domain_set_seclabel; gvir_config_domain_get_vcpus;
Can you keep the symbol list in the .sym file alphabetically ordered ? ACK otherwise Christophe
participants (3)
-
Christophe Fergeau
-
Daniel P. Berrange
-
Zeeshan Ali (Khattak)