[libvirt] [PATCH v3 5/5] blockcommit: turn on active commit

[Peter Krempa]

On 06/11/14 18:27, Eric Blake wrote:
> With this in place, I can (finally!) now do:
> 
> virsh blockcommit $dom vda --shallow --wait --verbose --pivot
> 
> and watch qemu shorten the backing chain by one, followed by
> libvirt automatically updating the dumpxml output, effectively
> undoing the work of virsh snapshot-commit --no-metadata --disk-only.
> Commit is SOOOO much faster than blockpull, when I'm still fairly
> close in time to when the temporary qcow2 wrapper file was created
> via a snapshot operation!
> 
> * src/qemu/qemu_driver.c (qemuDomainBlockCommit): Implement live
> commit to regular files.
> 
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>  src/qemu/qemu_driver.c | 43 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 38 insertions(+), 5 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 728aa85..60dc9c6 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -15529,9 +15529,11 @@ qemuDomainBlockCommit(virDomainPtr dom,
>      char *topPath = NULL;
>      char *basePath = NULL;
>      char *backingPath = NULL;
> +    virStorageSourcePtr mirror = NULL;
> 
> -    /* XXX Add support for COMMIT_ACTIVE, COMMIT_DELETE */
> +    /* XXX Add support for COMMIT_DELETE */
>      virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
> +                  VIR_DOMAIN_BLOCK_COMMIT_ACTIVE |

Rebase needed if this should go before my series.

>                    VIR_DOMAIN_BLOCK_COMMIT_RELATIVE, -1);
> 
>      if (!(vm = qemuDomObjFromDomain(dom)))
> @@ -15578,13 +15580,14 @@ qemuDomainBlockCommit(virDomainPtr dom,
>                                                       &top_parent)))
>          goto endjob;
> 
> -    /* FIXME: qemu 2.0 supports active commit, but as a two-stage
> -     * process; qemu 2.1 is further improving active commit. We need
> -     * to start supporting it in libvirt. */
>      if (topSource == disk->src) {
>          /* We assume that no one will backport qemu 2.0 active commit
>           * to an earlier qemu without also backporting the block job
> -         * ready event; but this makes sure of that fact */
> +         * ready event; but this makes sure of that fact.
> +         *
> +         * XXX Also need to check other capability bit(s): qemu 1.7
> +         * supports async blockjob but not active commit; and qemu 2.0
> +         * active commit misbehaves on 0-byte file.  */

How about tying the support to qemu 2.1 if it makes things easier? I
don't see any problem with supporting stuff from later-than-introduced
versions. The only question that remains is whether there's anything
relevant to trigger the capability of.


>          if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)) {
>              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                             _("active commit not supported with this QEMU binary"));

> @@ -15667,6 +15678,21 @@ qemuDomainBlockCommit(virDomainPtr dom,
>          }
>      }
> 
> +    /* For an active commit, clone enough of the base to act as the mirror */
> +    if (mirror) {
> +        /* XXX Support network commits */
> +        if (baseSource->type != VIR_STORAGE_TYPE_FILE &&
> +            baseSource->type != VIR_STORAGE_TYPE_BLOCK) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("active commit to non-file not supported yet"));
> +            goto endjob;
> +        }
> +        mirror->type = baseSource->type;
> +        if (VIR_STRDUP(mirror->path, baseSource->path) < 0)
> +            goto endjob;
> +        mirror->format = baseSource->format;
> +    }
> +
>      /* Start the commit operation.  Pass the user's original spelling,
>       * if any, through to qemu, since qemu may behave differently
>       * depending on whether the input was specified as relative or

Hmmm, this commit is invalid here as we are not passing user's spelling
to qemu. It also doesn't make sense to do that. I should revert that
patch explicitly apparently as a part of my series.


Still needs the capability bit figured out. Otherwise looks good.

Peter

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 901 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140612/e9919d9b/attachment-0001.sig>