Re: [PATCHv2 14/20] snapshot: qemu: Add flag VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/01/2012 10:22 AM, Peter Krempa wrote:
> The current snapshot reverting api supported changing the state of the
> machine after the snapshot was reverted to either started or paused.
> 
> This patch adds the ability to revert the state but to stopped state.
> ---
>  include/libvirt/libvirt.h.in |  1 +
>  src/libvirt.c                |  9 +++++----
>  tools/virsh-snapshot.c       |  3 +++
>  tools/virsh.pod              | 15 +++++++++------
>  4 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index fe58c08..ea57d00 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -3872,6 +3872,7 @@ typedef enum {
>      VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING = 1 << 0, /* Run after revert */
>      VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED  = 1 << 1, /* Pause after revert */
>      VIR_DOMAIN_SNAPSHOT_REVERT_FORCE   = 1 << 2, /* Allow risky reverts */
> +    VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED = 1 << 3, /* Revert into stopped state */

Hmm, this might even be the argument I was looking for earlier about
whether it makes sense to mix QUIESCE and memory state (still, using
QUIESCE only makes sense for non-LIVE checkpoints).  If we are going to
revert into a stopped state, that means that we are going to be using
the disk state without any memory and so no in-flight I/O; if that is to
be allowed, we want a way to quiesce then pause the domain then save
state, so we can make up our mind whether to restore just the disk state
or everything; but it would also mean that the saved ram state needs to
flag that it was done while the guest was quiesced, so that the first
thing done on a non-stopped revert is to thaw the guest file system.

On the other hand, I don't know how many people will revert to just disk
state and not also load up the associated RAM state.  This flag might
not get much use.

> +++ b/src/libvirt.c
> @@ -18698,11 +18698,12 @@ virDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
>          goto error;
>      }
> 
> -    if ((flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) &&
> -        (flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)) {
> +    if ((!!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING) +
> +         !!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED)  +
> +         !!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_STOPPED)) > 1) {
>          virReportInvalidArg(flags,
> -                            _("running and paused flags in %s are mutually exclusive"),
> -                            __FUNCTION__);
> +                            _("running, paused and stopped flags in %s are "
> +                              "mutually exclusive"), __FUNCTION__);

Also needs documentation several lines earlier.

> +++ b/tools/virsh.pod
> @@ -2803,7 +2803,7 @@ Output the name of the parent snapshot, if any, for the given
>  I<snapshot>, or for the current snapshot with I<--current>.
> 
>  =item B<snapshot-revert> I<domain> {I<snapshot> | I<--current>}
> -[{I<--running> | I<--paused>}] [I<--force>]
> +[{I<--running> | I<--paused>} | I<--stopped>] [I<--force>]

Since it is mutually exclusive, this should be:

[{I<--running> | I<--paused> | I<--stopped>}] [I<--force>]

ACK with the 2 doc changes; we can save decisions about use of quiesce
for later.

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]