[libvirt] [libvirt-glib 1/2] API to suspend a domain

From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> --- libvirt-gobject/libvirt-gobject-domain.c | 30 ++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 3 +++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 34 insertions(+), 0 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 4301b14..e4963ed 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -678,3 +678,33 @@ gboolean gvir_domain_open_graphics(GVirDomain *dom, cleanup: return ret; } + +/** + * gir_domain_suspend: + * @dom: the domain to suspend + * @err: Place-holder for possible errors + * + * Suspends an active domain, the process is frozen without further access to + * CPU resources and I/O but the memory used by the domain at the hypervisor + * level will stay allocated. Use gvir_domain_resume() to reactivate the domain. + * + * Returns: TRUE if domain was suspended successfully, FALSE otherwise. + */ +gboolean gvir_domain_suspend (GVirDomain *dom, + GError **err) +{ + gboolean ret = FALSE; + + g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE); + + if (virDomainSuspend(dom->priv->handle) < 0) { + gvir_set_error_literal(err, GVIR_DOMAIN_ERROR, + 0, + "Unable to suspend domain"); + goto cleanup; + } + + ret = TRUE; +cleanup: + return ret; +} diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 6fcec8d..a5923f4 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -153,6 +153,9 @@ gboolean gvir_domain_open_graphics(GVirDomain *dom, unsigned int flags, GError **err); +gboolean gvir_domain_suspend (GVirDomain *dom, + 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 a523adc..526098d 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -51,6 +51,7 @@ LIBVIRT_GOBJECT_0.0.3 { gvir_domain_start; gvir_domain_resume; gvir_domain_stop; + gvir_domain_suspend; gvir_domain_delete; gvir_domain_open_console; gvir_domain_open_graphics; -- 1.7.7.4

From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> Its just a set of synchronous and asynchronous wrappers around virDomainManagedSave. --- libvirt-gobject/libvirt-gobject-domain.c | 120 ++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 11 +++ libvirt-gobject/libvirt-gobject.sym | 3 + 3 files changed, 134 insertions(+), 0 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index e4963ed..8496257 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -708,3 +708,123 @@ gboolean gvir_domain_suspend (GVirDomain *dom, cleanup: return ret; } + +/** + * gvir_domain_saved_suspend: + * @dom: the domain to save and suspend + * @flags: extra flags, currently unused + * @err: Place-holder for possible errors + * + * Just like #gvir_domain_suspend but also saves the state of the domain on disk + * and therefore makes it possible to restore the domain to its previous state + * even after shutdown/reboot of host machine. + * + * Returns: TRUE if domain was saved and suspended successfully, FALSE otherwise. + */ +gboolean gvir_domain_saved_suspend (GVirDomain *dom, + unsigned int flags, + GError **err) +{ + gboolean ret = FALSE; + + g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE); + + if (virDomainManagedSave(dom->priv->handle, flags) < 0) { + gvir_set_error_literal(err, GVIR_DOMAIN_ERROR, + 0, + "Unable to saved and suspend domain"); + goto cleanup; + } + + ret = TRUE; +cleanup: + return ret; +} + +typedef struct { + guint flags; +} DomainSavedSuspendData; + +static void +gvir_domain_saved_suspend_helper(GSimpleAsyncResult *res, + GObject *object, + GCancellable *cancellable G_GNUC_UNUSED) +{ + GVirDomain *dom = GVIR_DOMAIN(object); + DomainSavedSuspendData *data; + GError *err = NULL; + + data = (DomainSavedSuspendData *) g_object_get_data(G_OBJECT(res), + "DomainSavedSuspendData"); + + if (!gvir_domain_saved_suspend(dom, data->flags, &err)) { + g_simple_async_result_set_from_error(res, err); + g_error_free(err); + } + + g_slice_free (DomainSavedSuspendData, data); +} + +/** + * gir_domain_saved_suspend_async: + * @dom: the domain to save and suspend + * @flags: extra flags, currently unused + * @cancellable: (allow-none)(transfer none): cancellation object + * @callback: (scope async): completion callback + * @user_data: (closure): opaque data for callback + * + * Asynchronous variant of #gvir_domain_saved_suspend. + */ +void gvir_domain_saved_suspend_async (GVirDomain *dom, + unsigned int flags, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data) +{ + GSimpleAsyncResult *res; + DomainSavedSuspendData *data; + + g_return_if_fail(GVIR_IS_DOMAIN(dom)); + + data = g_new0(DomainSavedSuspendData, 1); + data->flags = flags; + + res = g_simple_async_result_new(G_OBJECT(dom), + callback, + user_data, + gvir_domain_saved_suspend); + g_object_set_data(G_OBJECT(res), "DomainSavedSuspendData", data); + g_simple_async_result_run_in_thread(res, + gvir_domain_saved_suspend_helper, + G_PRIORITY_DEFAULT, + cancellable); + g_object_unref(res); +} + +/** + * gir_domain_saved_suspend_finish: + * @dom: the domain to save and suspend + * @result: (transfer none): async method result + * @err: Place-holder for possible errors + * + * Finishes the operation started by #gvir_domain_saved_suspend_async. + * + * Returns: TRUE if domain was saved and suspended successfully, FALSE otherwise. + */ +gboolean gvir_domain_saved_suspend_finish (GVirDomain *dom, + GAsyncResult *result, + GError **err) +{ + g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE); + g_return_val_if_fail(G_IS_ASYNC_RESULT(result), FALSE); + + if (G_IS_SIMPLE_ASYNC_RESULT(result)) { + GSimpleAsyncResult *simple = G_SIMPLE_ASYNC_RESULT(result); + g_warn_if_fail (g_simple_async_result_get_source_tag(simple) == + gvir_domain_saved_suspend); + if (g_simple_async_result_propagate_error(simple, err)) + return FALSE; + } + + return TRUE; +} diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index a5923f4..96a1560 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -155,6 +155,17 @@ gboolean gvir_domain_open_graphics(GVirDomain *dom, gboolean gvir_domain_suspend (GVirDomain *dom, GError **err); +gboolean gvir_domain_saved_suspend (GVirDomain *dom, + unsigned int flags, + GError **err); +void gvir_domain_saved_suspend_async (GVirDomain *dom, + unsigned int flags, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data); +gboolean gvir_domain_saved_suspend_finish (GVirDomain *dom, + GAsyncResult *result, + GError **err); G_END_DECLS diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 526098d..c04d67a 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -52,6 +52,9 @@ LIBVIRT_GOBJECT_0.0.3 { gvir_domain_resume; gvir_domain_stop; gvir_domain_suspend; + gvir_domain_saved_suspend; + gvir_domain_saved_suspend_async; + gvir_domain_saved_suspend_finish; gvir_domain_delete; gvir_domain_open_console; gvir_domain_open_graphics; -- 1.7.7.4

Hi On Fri, Dec 23, 2011 at 8:58 PM, Zeeshan Ali (Khattak) <zeeshanak@gnome.org>wrote:
+ data = g_new0(DomainSavedSuspendData, 1);
+ g_slice_free (DomainSavedSuspendData, data); You are mixing g_new and g_slice, I don't think that's a problem, but that's not consistant and worries me. g_object_{set,get}_data could probably use the _full variant, to make sure the data is released on object release. Even better would be to follow glib/gio pattern, which seems to use g_simple_async_result_set_op_res_gpointer() instead for passing arguments, although the name function suggest result use case only. (see gio/gfile.c). The existing code could be updated too. -- Marc-André Lureau

On Tue, Jan 03, 2012 at 03:06:09PM +0100, Marc-André Lureau wrote:
Hi
On Fri, Dec 23, 2011 at 8:58 PM, Zeeshan Ali (Khattak) <zeeshanak@gnome.org>wrote:
+ data = g_new0(DomainSavedSuspendData, 1);
+ g_slice_free (DomainSavedSuspendData, data);
You are mixing g_new and g_slice, I don't think that's a problem, but that's not consistant and worries me.
Actually, I'm fairly sure that is a serious problem. At the very least it will be a memory leak, but it could well cause other nasty effects http://developer.gnome.org/glib/stable/glib-Memory-Slices.html#g-slice-free1 "The memory must have been allocated via g_slice_alloc() or g_slice_alloc0() and the block_size has to match the size specified upon allocation." 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, Jan 3, 2012 at 4:11 PM, Daniel P. Berrange <berrange@redhat.com> wrote:
On Tue, Jan 03, 2012 at 03:06:09PM +0100, Marc-André Lureau wrote:
Hi
On Fri, Dec 23, 2011 at 8:58 PM, Zeeshan Ali (Khattak) <zeeshanak@gnome.org>wrote:
+ data = g_new0(DomainSavedSuspendData, 1);
+ g_slice_free (DomainSavedSuspendData, data);
You are mixing g_new and g_slice, I don't think that's a problem, but that's not consistant and worries me.
Actually, I'm fairly sure that is a serious problem. At the very least it will be a memory leak, but it could well cause other nasty effects
It *is* a serious problem, elmarco is just being polite to me. :) I was supposed to use g_slice_new0() there. -- Regards, Zeeshan Ali (Khattak) FSF member#5124

Hey, On Fri, Dec 23, 2011 at 09:58:23PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
Its just a set of synchronous and asynchronous wrappers around virDomainManagedSave. --- libvirt-gobject/libvirt-gobject-domain.c | 120 ++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 11 +++ libvirt-gobject/libvirt-gobject.sym | 3 + 3 files changed, 134 insertions(+), 0 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index e4963ed..8496257 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -708,3 +708,123 @@ gboolean gvir_domain_suspend (GVirDomain *dom, cleanup: return ret; } + +/** + * gvir_domain_saved_suspend:
_saved_suspend is good for me even though it took me a while to get what it means. Maybe _persistent_suspend would be better? Hopefully this won't turn into some bikeshedding, but since we are not reusing the libvirt naming,I feelt it was worth making an alternate suggestion :)
+ * @dom: the domain to save and suspend + * @flags: extra flags, currently unused + * @err: Place-holder for possible errors + * + * Just like #gvir_domain_suspend but also saves the state of the domain on disk + * and therefore makes it possible to restore the domain to its previous state + * even after shutdown/reboot of host machine. + * + * Returns: TRUE if domain was saved and suspended successfully, FALSE otherwise. + */ +gboolean gvir_domain_saved_suspend (GVirDomain *dom, + unsigned int flags, + GError **err) +{ + gboolean ret = FALSE; + + g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE); + + if (virDomainManagedSave(dom->priv->handle, flags) < 0) { + gvir_set_error_literal(err, GVIR_DOMAIN_ERROR, + 0, + "Unable to saved and suspend domain");
"to save" (no -d)
+ goto cleanup; + } + + ret = TRUE; +cleanup: + return ret; +}
Imo, the code would flow more nicely with a return FALSE; in the error case, return TRUE; otherwise, and no cleanup: and gboolean ret. If you prefer it that way, that's fine with me.
+ +typedef struct { + guint flags; +} DomainSavedSuspendData; + +static void +gvir_domain_saved_suspend_helper(GSimpleAsyncResult *res, + GObject *object, + GCancellable *cancellable G_GNUC_UNUSED) +{ + GVirDomain *dom = GVIR_DOMAIN(object); + DomainSavedSuspendData *data; + GError *err = NULL; + + data = (DomainSavedSuspendData *) g_object_get_data(G_OBJECT(res), + "DomainSavedSuspendData"); + + if (!gvir_domain_saved_suspend(dom, data->flags, &err)) { + g_simple_async_result_set_from_error(res, err); + g_error_free(err);
There's a g_simple_async_result_take_error
+ } + + g_slice_free (DomainSavedSuspendData, data); +} + +/** + * gir_domain_saved_suspend_async: + * @dom: the domain to save and suspend + * @flags: extra flags, currently unused + * @cancellable: (allow-none)(transfer none): cancellation object + * @callback: (scope async): completion callback + * @user_data: (closure): opaque data for callback + * + * Asynchronous variant of #gvir_domain_saved_suspend. + */ +void gvir_domain_saved_suspend_async (GVirDomain *dom, + unsigned int flags, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data) +{ + GSimpleAsyncResult *res; + DomainSavedSuspendData *data; + + g_return_if_fail(GVIR_IS_DOMAIN(dom)); + + data = g_new0(DomainSavedSuspendData, 1);
Don't forget to change this to g_slice_new0 as Marc-André and Daniel have pointed out. Rest of the patch looks good to me, so ACK from me. Christophe
+ data->flags = flags; + + res = g_simple_async_result_new(G_OBJECT(dom), + callback, + user_data, + gvir_domain_saved_suspend); + g_object_set_data(G_OBJECT(res), "DomainSavedSuspendData", data); + g_simple_async_result_run_in_thread(res, + gvir_domain_saved_suspend_helper, + G_PRIORITY_DEFAULT, + cancellable); + g_object_unref(res); +} + +/** + * gir_domain_saved_suspend_finish: + * @dom: the domain to save and suspend + * @result: (transfer none): async method result + * @err: Place-holder for possible errors + * + * Finishes the operation started by #gvir_domain_saved_suspend_async. + * + * Returns: TRUE if domain was saved and suspended successfully, FALSE otherwise. + */ +gboolean gvir_domain_saved_suspend_finish (GVirDomain *dom, + GAsyncResult *result, + GError **err) +{ + g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE); + g_return_val_if_fail(G_IS_ASYNC_RESULT(result), FALSE); + + if (G_IS_SIMPLE_ASYNC_RESULT(result)) { + GSimpleAsyncResult *simple = G_SIMPLE_ASYNC_RESULT(result); + g_warn_if_fail (g_simple_async_result_get_source_tag(simple) == + gvir_domain_saved_suspend); + if (g_simple_async_result_propagate_error(simple, err)) + return FALSE; + } + + return TRUE; +} diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index a5923f4..96a1560 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -155,6 +155,17 @@ gboolean gvir_domain_open_graphics(GVirDomain *dom,
gboolean gvir_domain_suspend (GVirDomain *dom, GError **err); +gboolean gvir_domain_saved_suspend (GVirDomain *dom, + unsigned int flags, + GError **err); +void gvir_domain_saved_suspend_async (GVirDomain *dom, + unsigned int flags, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data); +gboolean gvir_domain_saved_suspend_finish (GVirDomain *dom, + GAsyncResult *result, + GError **err);
G_END_DECLS
diff --git a/libvirt-gobject/libvirt-gobject.sym b/libvirt-gobject/libvirt-gobject.sym index 526098d..c04d67a 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -52,6 +52,9 @@ LIBVIRT_GOBJECT_0.0.3 { gvir_domain_resume; gvir_domain_stop; gvir_domain_suspend; + gvir_domain_saved_suspend; + gvir_domain_saved_suspend_async; + gvir_domain_saved_suspend_finish; gvir_domain_delete; gvir_domain_open_console; gvir_domain_open_graphics; -- 1.7.7.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Jan 06, 2012 at 12:19:34PM +0400, Christophe Fergeau wrote:
Hey,
On Fri, Dec 23, 2011 at 09:58:23PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
Its just a set of synchronous and asynchronous wrappers around virDomainManagedSave. --- libvirt-gobject/libvirt-gobject-domain.c | 120 ++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 11 +++ libvirt-gobject/libvirt-gobject.sym | 3 + 3 files changed, 134 insertions(+), 0 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index e4963ed..8496257 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -708,3 +708,123 @@ gboolean gvir_domain_suspend (GVirDomain *dom, cleanup: return ret; } + +/** + * gvir_domain_saved_suspend:
_saved_suspend is good for me even though it took me a while to get what it means. Maybe _persistent_suspend would be better? Hopefully this won't turn into some bikeshedding, but since we are not reusing the libvirt naming,I feelt it was worth making an alternate suggestion :)
I'd suggest just calling it 'gvir_domain_save' really. 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 :|

Hi, Pushed this patch with all the corrections/suggestions acted-upon. Let me know if I missed something and we can still correct it of course. :) Thanks everyone for your details reviews on this. -- Regards, Zeeshan Ali (Khattak) FSF member#5124

ACK On Fri, Dec 23, 2011 at 09:58:22PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- libvirt-gobject/libvirt-gobject-domain.c | 30 ++++++++++++++++++++++++++++++ libvirt-gobject/libvirt-gobject-domain.h | 3 +++ libvirt-gobject/libvirt-gobject.sym | 1 + 3 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 4301b14..e4963ed 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -678,3 +678,33 @@ gboolean gvir_domain_open_graphics(GVirDomain *dom, cleanup: return ret; } + +/** + * gir_domain_suspend: + * @dom: the domain to suspend + * @err: Place-holder for possible errors + * + * Suspends an active domain, the process is frozen without further access to + * CPU resources and I/O but the memory used by the domain at the hypervisor + * level will stay allocated. Use gvir_domain_resume() to reactivate the domain. + * + * Returns: TRUE if domain was suspended successfully, FALSE otherwise. + */ +gboolean gvir_domain_suspend (GVirDomain *dom, + GError **err) +{ + gboolean ret = FALSE; + + g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE); + + if (virDomainSuspend(dom->priv->handle) < 0) { + gvir_set_error_literal(err, GVIR_DOMAIN_ERROR, + 0, + "Unable to suspend domain"); + goto cleanup; + } + + ret = TRUE; +cleanup: + return ret; +} diff --git a/libvirt-gobject/libvirt-gobject-domain.h b/libvirt-gobject/libvirt-gobject-domain.h index 6fcec8d..a5923f4 100644 --- a/libvirt-gobject/libvirt-gobject-domain.h +++ b/libvirt-gobject/libvirt-gobject-domain.h @@ -153,6 +153,9 @@ gboolean gvir_domain_open_graphics(GVirDomain *dom, unsigned int flags, GError **err);
+gboolean gvir_domain_suspend (GVirDomain *dom, + 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 a523adc..526098d 100644 --- a/libvirt-gobject/libvirt-gobject.sym +++ b/libvirt-gobject/libvirt-gobject.sym @@ -51,6 +51,7 @@ LIBVIRT_GOBJECT_0.0.3 { gvir_domain_start; gvir_domain_resume; gvir_domain_stop; + gvir_domain_suspend; gvir_domain_delete; gvir_domain_open_console; gvir_domain_open_graphics; -- 1.7.7.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (4)
-
Christophe Fergeau
-
Daniel P. Berrange
-
Marc-André Lureau
-
Zeeshan Ali (Khattak)