[libvirt] [libvirt-glib] Add API to redefine an existing domain
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> --- libvirt-gobject/libvirt-gobject-connection.c | 37 ++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-connection.h | 4 +++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 42 insertions(+), 0 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index 6c8de11..471c795 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -1201,6 +1201,43 @@ GVirDomain *gvir_connection_create_domain(GVirConnection *conn, } /** + * gvir_connection_redefine_domain: + * @conn: the connection on which the dmain exists + * @conf: the new configuration for the domain + * + * Redefines an existing domain. + */ +void gvir_connection_redefine_domain(GVirConnection *conn, + GVirDomain *domain, + GVirConfigDomain *conf, + GError **err) +{ + const gchar *xml; + virDomainPtr handle; + GVirDomain *dom; + virDomainPtr dom_handle; + GVirConnectionPrivate *priv = conn->priv; + + xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf)); + + g_return_if_fail(xml != NULL); + + g_mutex_lock(priv->lock); + dom = g_hash_table_lookup (priv->domains, + (gpointer)gvir_domain_get_uuid(domain)); + g_mutex_unlock(priv->lock); + g_return_if_fail(dom != NULL); + /* FIXME: Check if config's domain ID is the same as domain passed */ + + if (!(handle = virDomainDefineXML(priv->conn, xml))) { + *err = gvir_error_new_literal(GVIR_CONNECTION_ERROR, + 0, + "Failed to redefine domain"); + return NULL; + } +} + +/** * gvir_connection_create_storage_pool: * @conn: the connection on which to create the pool * @conf: the configuration for the new storage pool diff --git a/libvirt-gobject/libvirt-gobject-connection.h b/libvirt-gobject/libvirt-gobject-connection.h index 9f4bdc3..c23d948 100644 --- a/libvirt-gobject/libvirt-gobject-connection.h +++ b/libvirt-gobject/libvirt-gobject-connection.h @@ -110,6 +110,10 @@ GVirDomain *gvir_connection_find_domain_by_name(GVirConnection *conn, GVirDomain *gvir_connection_create_domain(GVirConnection *conn, GVirConfigDomain *conf, GError **err); +void gvir_connection_redefine_domain(GVirConnection *conn, + GVirDomain *domain, + GVirConfigDomain *conf, + GError **err); #if 0 GList *gvir_connection_get_interfaces(GVirConnection *conn); diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 164b6b8..b5cc347 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -27,6 +27,7 @@ LIBVIRT_GOBJECT_0.0.1 { gvir_connection_find_domain_by_name; gvir_connection_find_storage_pool_by_name; gvir_connection_create_domain; + gvir_connection_redefine_domain; gvir_connection_create_storage_pool; gvir_domain_device_get_type; -- 1.7.7.1
Hey, Reading this patch, I'm wondering if gvir_domain_set_config might achieve what you are after (it's not implemented either) On Fri, Nov 18, 2011 at 09:24:32PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gobject/libvirt-gobject-connection.c | 37 ++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-connection.h | 4 +++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-connection.c b/libvirt-gobject/libvirt-gobject-connection.c index 6c8de11..471c795 100644 --- a/libvirt-gobject/libvirt-gobject-connection.c +++ b/libvirt-gobject/libvirt-gobject-connection.c @@ -1201,6 +1201,43 @@ GVirDomain *gvir_connection_create_domain(GVirConnection *conn, }
/** + * gvir_connection_redefine_domain: + * @conn: the connection on which the dmain exists + * @conf: the new configuration for the domain + * + * Redefines an existing domain. + */ +void gvir_connection_redefine_domain(GVirConnection *conn, + GVirDomain *domain, + GVirConfigDomain *conf, + GError **err)
For the record, I'm not sure I like the naming here.
+{ + const gchar *xml; + virDomainPtr handle; + GVirDomain *dom; + virDomainPtr dom_handle;
This is unused
+ GVirConnectionPrivate *priv = conn->priv; + + xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf)); + + g_return_if_fail(xml != NULL); + + g_mutex_lock(priv->lock); + dom = g_hash_table_lookup (priv->domains, + (gpointer)gvir_domain_get_uuid(domain)); + g_mutex_unlock(priv->lock); + g_return_if_fail(dom != NULL); + /* FIXME: Check if config's domain ID is the same as domain passed */ + + if (!(handle = virDomainDefineXML(priv->conn, xml))) {
Shouldn't handle be assigned to some persistent state? My guess is domain->priv->handle. Christophe
On Sat, Nov 19, 2011 at 12:56 AM, Christophe Fergeau <cfergeau@redhat.com> wrote:
Hey,
Reading this patch, I'm wondering if gvir_domain_set_config might achieve what you are after (it's not implemented either)
Yeah, that might actually be better.
+ GVirConnectionPrivate *priv = conn->priv; + + xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf)); + + g_return_if_fail(xml != NULL); + + g_mutex_lock(priv->lock); + dom = g_hash_table_lookup (priv->domains, + (gpointer)gvir_domain_get_uuid(domain)); + g_mutex_unlock(priv->lock); + g_return_if_fail(dom != NULL); + /* FIXME: Check if config's domain ID is the same as domain passed */ + + if (!(handle = virDomainDefineXML(priv->conn, xml))) {
Shouldn't handle be assigned to some persistent state? My guess is domain->priv->handle.
No, there is supposed to be already a corresponding domain with the handle in hash table. Don't let the 'DomainDefineXML' name fool you. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
On Sat, Nov 19, 2011 at 01:02:28AM +0200, Zeeshan Ali (Khattak) wrote:
On Sat, Nov 19, 2011 at 12:56 AM, Christophe Fergeau <cfergeau@redhat.com> wrote:
Hey,
Reading this patch, I'm wondering if gvir_domain_set_config might achieve what you are after (it's not implemented either)
Yeah, that might actually be better.
+ GVirConnectionPrivate *priv = conn->priv; + + xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf)); + + g_return_if_fail(xml != NULL); + + g_mutex_lock(priv->lock); + dom = g_hash_table_lookup (priv->domains, + (gpointer)gvir_domain_get_uuid(domain)); + g_mutex_unlock(priv->lock); + g_return_if_fail(dom != NULL); + /* FIXME: Check if config's domain ID is the same as domain passed */ + + if (!(handle = virDomainDefineXML(priv->conn, xml))) {
Shouldn't handle be assigned to some persistent state? My guess is domain->priv->handle.
No, there is supposed to be already a corresponding domain with the handle in hash table. Don't let the 'DomainDefineXML' name fool you.
No need for the handle variable then. This is one of the things that fooled me :) Christophe
Hi Hi Agreed with the comments from teuf + On Fri, Nov 18, 2011 at 8:24 PM, Zeeshan Ali (Khattak) <zeeshanak@gnome.org> wrote:
+void gvir_connection_redefine_domain(GVirConnection *conn, + GVirDomain *domain, + GVirConfigDomain *conf, + GError **err) +{
I would check that the given arguments are correct, != NULL or GVIR_IS_FOO. I would return TRUE on success, and error can be NULL (regular glib convention)
+ const gchar *xml; + virDomainPtr handle; + GVirDomain *dom; + virDomainPtr dom_handle; + GVirConnectionPrivate *priv = conn->priv; + + xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf)); + + g_return_if_fail(xml != NULL); + + g_mutex_lock(priv->lock); + dom = g_hash_table_lookup (priv->domains, + (gpointer)gvir_domain_get_uuid(domain)); + g_mutex_unlock(priv->lock); + g_return_if_fail(dom != NULL); + /* FIXME: Check if config's domain ID is the same as domain passed */
I suppose this is missing Christophe gconfig patches.
+ if (!(handle = virDomainDefineXML(priv->conn, xml))) { + *err = gvir_error_new_literal(GVIR_CONNECTION_ERROR, + 0, + "Failed to redefine domain"); + return NULL; + } +}
And I would verify that handle is == to current domain handle. If not, we probably need to replace it or we need to throw an error. At the minimum it would be nice to leave a comment after verifying that it is safe to ignore return value. (then send an updated patch for ack) regards -- Marc-André Lureau
participants (3)
-
Christophe Fergeau -
Marc-André Lureau -
Zeeshan Ali (Khattak)