[libvirt] [libvirt-glib] Add API to redefine an existing domain

From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> --- libvirt-gobject/libvirt-gobject-domain.c | 48 ++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 3 ++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 52 insertions(+), 0 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 1fa27bd..01ae1f4 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -449,6 +449,54 @@ GVirConfigDomain *gvir_domain_get_config(GVirDomain *dom, return conf; } +/** + * gvir_domain_set_config: + * @domain: the domain + * @conf: the new configuration for the domain + * @error: (allow-none): Place-holder for error or NULL + * + * Resets configuration of an existing domain. + * + * Note: If domain is already running, the new configuration will not take + * affect until domain reboots. + * + * Returns: TRUE on success, FALSE if an error occurred. + */ +gboolean gvir_domain_set_config(GVirDomain *domain, + GVirConfigDomain *conf, + GError **error) +{ + const gchar *xml; + virConnectPtr conn; + GVirDomainPrivate *priv = domain->priv; + + g_return_val_if_fail(GVIR_IS_DOMAIN (domain), FALSE); + g_return_val_if_fail(GVIR_IS_CONFIG_DOMAIN (conf), FALSE); + g_return_val_if_fail(error == NULL || *error == NULL, FALSE); + + xml = gvir_config_object_to_xml(GVIR_CONFIG_OBJECT(conf)); + + g_return_val_if_fail(xml != NULL, FALSE); + + if ((conn = virDomainGetConnect(priv->handle)) == NULL) { + if (error != NULL) + *error = gvir_error_new_literal(GVIR_DOMAIN_ERROR, + 0, + "Failed to get domain connection"); + return FALSE; + } + + if (virDomainDefineXML(conn, xml) == NULL) { + if (error != NULL) + *error = gvir_error_new_literal(GVIR_DOMAIN_ERROR, + 0, + "Failed to set " + "domain configuration"); + return FALSE; + } + + return TRUE; +} /** * gvir_domain_get_info: diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 94bd53e..0479de8 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -123,6 +123,9 @@ GVirDomainInfo *gvir_domain_get_info(GVirDomain *dom, GVirConfigDomain *gvir_domain_get_config(GVirDomain *dom, guint64 flags, GError **err); +gboolean gvir_domain_set_config(GVirDomain *domain, + GVirConfigDomain *conf, + GError **err); gchar *gvir_domain_screenshot(GVirDomain *dom, GVirStream *stream, diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 164b6b8..46c53f9 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -53,6 +53,7 @@ LIBVIRT_GOBJECT_0.0.1 { gvir_domain_shutdown; gvir_domain_reboot; gvir_domain_get_config; + gvir_domain_set_config; gvir_domain_get_info; gvir_domain_screenshot; -- 1.7.7.1

Hi On Mon, Nov 21, 2011 at 6:53 PM, Zeeshan Ali (Khattak) <zeeshanak@gnome.org> wrote:
+ g_return_val_if_fail(error == NULL || *error == NULL, FALSE);
This is wrong, it should be error != NULL && *error == NULL.
+ if (virDomainDefineXML(conn, xml) == NULL) { + if (error != NULL) + *error = gvir_error_new_literal(GVIR_DOMAIN_ERROR, + 0, + "Failed to set " + "domain configuration"); + return FALSE; + }
Can you please verify that the return value is safe to ignore? I would really prefer we add a runtime check that verifiy the handle is the same as the one currently associated with the domain. -- Marc-André Lureau

On Mon, Nov 21, 2011 at 9:27 PM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
Hi
On Mon, Nov 21, 2011 at 6:53 PM, Zeeshan Ali (Khattak) <zeeshanak@gnome.org> wrote:
+ g_return_val_if_fail(error == NULL || *error == NULL, FALSE);
This is wrong, it should be error != NULL && *error == NULL.
+ if (virDomainDefineXML(conn, xml) == NULL) { + if (error != NULL) + *error = gvir_error_new_literal(GVIR_DOMAIN_ERROR, + 0, + "Failed to set " + "domain configuration"); + return FALSE; + }
Can you please verify that the return value is safe to ignore?
Sorry, i don't get it. Verify how?
I would really prefer we add a runtime check that verifiy the handle is the same as the one currently associated with the domain.
Yeah, i'll add that! Forgot that it still applies. -- Regards, Zeeshan Ali (Khattak) FSF member#5124

On Mon, Nov 21, 2011 at 8:45 PM, Zeeshan Ali (Khattak) <zeeshanak@gnome.org> wrote:
Sorry, i don't get it. Verify how?
By doing what I proposed bellow, reading libvirt code, ask on libvirt list/maintainers, test etc..
I would really prefer we add a runtime check that verifiy the handle is the same as the one currently associated with the domain.
Yeah, i'll add that! Forgot that it still applies.
thanks -- Marc-André Lureau

On Mon, Nov 21, 2011 at 9:58 PM, Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
On Mon, Nov 21, 2011 at 8:45 PM, Zeeshan Ali (Khattak) <zeeshanak@gnome.org> wrote:
Sorry, i don't get it. Verify how?
By doing what I proposed bellow, reading libvirt code, ask on libvirt list/maintainers, test etc..
Ah that, yeah i already did that. Patching coming.. -- Regards, Zeeshan Ali (Khattak) FSF member#5124

On Mon, Nov 21, 2011 at 08:27:17PM +0100, Marc-André Lureau wrote:
Hi
On Mon, Nov 21, 2011 at 6:53 PM, Zeeshan Ali (Khattak) <zeeshanak@gnome.org> wrote:
+ g_return_val_if_fail(error == NULL || *error == NULL, FALSE);
This is wrong, it should be error != NULL && *error == NULL.
+ if (virDomainDefineXML(conn, xml) == NULL) { + if (error != NULL) + *error = gvir_error_new_literal(GVIR_DOMAIN_ERROR, + 0, + "Failed to set " + "domain configuration"); + return FALSE; + }
Can you please verify that the return value is safe to ignore?
I would really prefer we add a runtime check that verifiy the handle is the same as the one currently associated with the domain.
The actual pointer returned will *not* be the same as the same one you currently have, but assuming the XML has matching UUID and name, it will point to the same domain. So I guess what you want todo is verify the UUID and name in the XML, before defining it. Also, you need to call virDomainFree on the return value of virDomainDefineXML to avoid memleak. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Tue, Nov 22, 2011 at 11:20 AM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Mon, Nov 21, 2011 at 08:27:17PM +0100, Marc-André Lureau wrote:
Hi
On Mon, Nov 21, 2011 at 6:53 PM, Zeeshan Ali (Khattak) <zeeshanak@gnome.org> wrote:
+ g_return_val_if_fail(error == NULL || *error == NULL, FALSE);
This is wrong, it should be error != NULL && *error == NULL.
+ if (virDomainDefineXML(conn, xml) == NULL) { + if (error != NULL) + *error = gvir_error_new_literal(GVIR_DOMAIN_ERROR, + 0, + "Failed to set " + "domain configuration"); + return FALSE; + }
Can you please verify that the return value is safe to ignore?
I would really prefer we add a runtime check that verifiy the handle is the same as the one currently associated with the domain.
The actual pointer returned will *not* be the same as the same one you currently have, but assuming the XML has matching UUID and name, it will point to the same domain.
So I guess what you want todo is verify the UUID and name in the XML, before defining it.
Also, you need to call virDomainFree on the return value of virDomainDefineXML to avoid memleak.
Thanks! I already pushed the patch that does this and Marc-Andre ack'ed. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
participants (3)
-
Daniel P. Berrange
-
Marc-André Lureau
-
Zeeshan Ali (Khattak)