[libvirt] [PATCH v2 3/5] qemu: Track domain quiesced status

[Daniel P. Berrange]

On Wed, Mar 05, 2014 at 02:53:10PM -0500, Tomoki Sekiyama wrote:
> Adds an quiesced flag into qemuDomainObjPrivate that tracks whether guest
> filesystems of the domain is quiesced of not.
> It also modify error code from qemuDomainSnapshotFSFreeze and
> qemuDomainSnapshotFSThaw, so that a caller can know whether the command is
> actually sent to the guest agent. If the error is caused before sending a
> freeze command, a counterpart thaw command shouldn't be sent to avoid
> confusing the tracking of quiesced status.
> ---
>  src/qemu/qemu_domain.h |    2 ++
>  src/qemu/qemu_driver.c |   41 +++++++++++++++++++++++++++++++----------
>  2 files changed, 33 insertions(+), 10 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 0bed50b..8a79a00 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -176,6 +176,8 @@ struct _qemuDomainObjPrivate {
>      char **qemuDevices; /* NULL-terminated list of devices aliases known to QEMU */
>  
>      bool hookRun;  /* true if there was a hook run over this domain */
> +
> +    bool quiesced; /* true if the domain filesystems are quiesced */
>  };
>  
>  typedef enum {

You will also want to update qemuDomainObjPrivateXMLFormat and
qemuDomainObjPrivateXMLParse so that this new 'quiesced' attribute
is preserved in the live XML state across libvirtd restarts.

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 9aad2dc..8109e46 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -12040,6 +12040,8 @@ cleanup:
>  }
>  
>  
> +/* Return -1 if request is not sent to agent due to misconfig, -2 on agent
> + * failure, and number of frozen filesystems on success. */
>  static int
>  qemuDomainSnapshotFSFreeze(virDomainObjPtr vm) {
>      qemuDomainObjPrivatePtr priv = vm->privateData;
> @@ -12056,14 +12058,24 @@ qemuDomainSnapshotFSFreeze(virDomainObjPtr vm) {
>                         _("QEMU guest agent is not configured"));
>          return -1;
>      }
> +    if (priv->quiesced) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("domain is already quiesced"));
> +        return -1;
> +    }
>  
>      qemuDomainObjEnterAgent(vm);
>      freezed = qemuAgentFSFreeze(priv->agent);
>      qemuDomainObjExitAgent(vm);
>  
> -    return freezed;
> +    if (freezed >= 0)
> +        priv->quiesced = true;
> +
> +    return freezed < 0 ? -2 : freezed;
>  }
>  
> +/* Return -1 if request is not sent to agent due to misconfig, -2 on agent
> + * failure, and number of thawed filesystems on success. */
>  static int
>  qemuDomainSnapshotFSThaw(virDomainObjPtr vm, bool report)
>  {
> @@ -12084,6 +12096,11 @@ qemuDomainSnapshotFSThaw(virDomainObjPtr vm, bool report)
>                             _("QEMU guest agent is not configured"));
>          return -1;
>      }
> +    if (!priv->quiesced && report) {
> +        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                       _("domain is not quiesced"));
> +        return -1;
> +    }
>  
>      qemuDomainObjEnterAgent(vm);
>      if (!report)
> @@ -12093,8 +12110,11 @@ qemuDomainSnapshotFSThaw(virDomainObjPtr vm, bool report)
>          virSetError(err);
>      qemuDomainObjExitAgent(vm);
>  
> +    if (thawed >= 0)
> +        priv->quiesced = false;
> +
>      virFreeError(err);
> -    return thawed;
> +    return thawed < 0 ? -2 : thawed;
>  }

And in both these methods you want to call virDomainSaveStatus after changing
the priv->quiesced flag, to save the status XML to disk


>  
>  /* The domain is expected to be locked and inactive. */
> @@ -13069,17 +13089,18 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn,
>          goto cleanup;
>  
>      /* If quiesce was requested, then issue a freeze command, and a
> -     * counterpart thaw command, no matter what.  The command will
> -     * fail if the guest is paused or the guest agent is not
> -     * running.  */
> +     * counterpart thaw command when the it is actually sent to agent.
> +     * The command will fail if the guest is paused or the guest agent is not
> +     * running, or is already quiesced.  */
>      if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) {
> -        if (qemuDomainSnapshotFSFreeze(vm) < 0) {
> -            /* helper reported the error */
> -            thaw = -1;
> +        int freeze = qemuDomainSnapshotFSFreeze(vm);
> +        if (freeze < 0) {
> +            /* the helper reported the error */
> +            if (freeze != -1)
> +                thaw = -1; /* the command is sent but agent failed */
>              goto endjob;
> -        } else {
> -            thaw = 1;
>          }
> +        thaw = 1;
>      }

I'm not sure I understand why you're changing this ? Are you trying to
deal with the case where the guest is already frozen, but someone has
still supplied the VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag ?

I'd probably argue that such usage is a bug we should report to the
caller as an error.


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 :|