Re: [PATCH 3/3] block rebase: initial qemu implementation

[Laine Stump <laine@xxxxxxxxx>]

On 02/01/2012 12:06 AM, Eric Blake wrote:
> This is a trivial implementation, which works with the current
> released qemu 1.0 with backports of preliminary block pull but
> no partial rebase.  Future patches will update the monitor handling
> to support an optional parameter for partial rebase; but as qemu
> 1.1 is unreleased, it can be in later patches, designed to be
> backported on top of the supported API.
>
> * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Add parameter,
> and adjust callers.  Drop redundant check.
> (qemuDomainBlockPull): Move guts...
> (qemuDomainBlockRebase): ...to new function.
> ---
>   src/qemu/qemu_driver.c |   40 ++++++++++++++++++++++++++--------------
>   1 files changed, 26 insertions(+), 14 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 1b147a9..beb6e71 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -11308,7 +11308,7 @@ cleanup:
>   }
>
>   static int
> -qemuDomainBlockJobImpl(virDomainPtr dom, const char *path,
> +qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
>                          unsigned long bandwidth, virDomainBlockJobInfoPtr info,
>                          int mode)
>   {
> @@ -11328,16 +11328,18 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path,
>           goto cleanup;
>       }
>
> -    if (!virDomainObjIsActive(vm)) {
> -        qemuReportError(VIR_ERR_OPERATION_INVALID,
> -                        "%s", _("domain is not running"));
> -        goto cleanup;
> -    }
> -

For all the existing virDomainBlockxxx() APIs I think this is a change 
in behavior, right? THey used to fial for inactive domains, and now they 
may succeed. Could this create any problems?

If not, then ACK. This is all pretty mechanical.

>       device = qemuDiskPathToAlias(vm, path);
>       if (!device) {
>           goto cleanup;
>       }
> +    /* XXX - add a qemu capability check; if qemu 1.1 or newer, then
> +     *  validate and convert non-NULL base into something that can
> +     * be passed as optional base argument.  */
> +    if (base)  {
> +        qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> +                        _("partial block pull is not supported with this QEMU binary"));
> +        goto cleanup;
> +    }

So the API is in, and the actual functionality missing until qemu that 
supports it is available.


>       if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY)<  0)
>           goto cleanup;
> @@ -11371,7 +11373,7 @@ static int
>   qemuDomainBlockJobAbort(virDomainPtr dom, const char *path, unsigned int flags)
>   {
>       virCheckFlags(0, -1);
> -    return qemuDomainBlockJobImpl(dom, path, 0, NULL, BLOCK_JOB_ABORT);
> +    return qemuDomainBlockJobImpl(dom, path, NULL, 0, NULL, BLOCK_JOB_ABORT);
>   }
>
>   static int
> @@ -11379,7 +11381,7 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, const char *path,
>                              virDomainBlockJobInfoPtr info, unsigned int flags)
>   {
>       virCheckFlags(0, -1);
> -    return qemuDomainBlockJobImpl(dom, path, 0, info, BLOCK_JOB_INFO);
> +    return qemuDomainBlockJobImpl(dom, path, NULL, 0, info, BLOCK_JOB_INFO);
>   }
>
>   static int
> @@ -11387,24 +11389,33 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom, const char *path,
>                              unsigned long bandwidth, unsigned int flags)
>   {
>       virCheckFlags(0, -1);
> -    return qemuDomainBlockJobImpl(dom, path, bandwidth, NULL, BLOCK_JOB_SPEED);
> +    return qemuDomainBlockJobImpl(dom, path, NULL, bandwidth, NULL,
> +                                  BLOCK_JOB_SPEED);
>   }
>
>   static int
> -qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth,
> -                    unsigned int flags)
> +qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
> +                      unsigned long bandwidth, unsigned int flags)
>   {
>       int ret;
>
>       virCheckFlags(0, -1);
> -    ret = qemuDomainBlockJobImpl(dom, path, bandwidth, NULL, BLOCK_JOB_PULL);
> +    ret = qemuDomainBlockJobImpl(dom, path, base, bandwidth, NULL,
> +                                 BLOCK_JOB_PULL);
>       if (ret == 0&&  bandwidth != 0)
> -        ret = qemuDomainBlockJobImpl(dom, path, bandwidth, NULL,
> +        ret = qemuDomainBlockJobImpl(dom, path, NULL, bandwidth, NULL,
>                                        BLOCK_JOB_SPEED);
>       return ret;
>   }
>
>   static int
> +qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth,
> +                    unsigned int flags)
> +{
> +    return qemuDomainBlockRebase(dom, path, NULL, bandwidth, flags);
> +}
> +
> +static int
>   qemuDomainOpenGraphics(virDomainPtr dom,
>                          unsigned int idx,
>                          int fd,
> @@ -11885,6 +11896,7 @@ static virDriver qemuDriver = {
>       .domainGetBlockJobInfo = qemuDomainGetBlockJobInfo, /* 0.9.4 */
>       .domainBlockJobSetSpeed = qemuDomainBlockJobSetSpeed, /* 0.9.4 */
>       .domainBlockPull = qemuDomainBlockPull, /* 0.9.4 */
> +    .domainBlockRebase = qemuDomainBlockRebase, /* 0.9.10 */
>       .isAlive = qemuIsAlive, /* 0.9.8 */
>       .nodeSuspendForDuration = nodeSuspendForDuration, /* 0.9.8 */
>       .domainSetBlockIoTune = qemuDomainSetBlockIoTune, /* 0.9.8 */

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