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

Re: [libvirt] [PATCH 2/3] qemu: pass block pull backing file to monitor



On 02/18/2012 12:44 PM, Eric Blake wrote:
> This actually wires up the new optional parameter to block_stream:
> http://wiki.qemu.org/Features/LiveBlockMigration/ImageStreamingAPI
>
> The error checking is still sparse, since libvirt must not use
> qemu-img or header probing on a qcow2 file in use by qemu to
> check if the backing file name is valid; so for now, libvirt is
> relying on qemu to diagnose an incorrect backing name.  Fixing this
> will require libvirt to track the entire backing file chain at the
> time qemu is started and keeps it updated with snapshot and pull
> operations.

Looks like mostly pushing the new parameter down the callstack, moving
the location (and conditions)
of the check for base being specified at an inappropriate time, and
changing the json command sent to actually use base in certain cases.

ACK.

>
> * src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockJob): Add
> parameter, and update callers.
> * src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockJob): Update
> signature.
> * src/qemu/qemu_monitor.h (qemuMonitorBlockJob): Likewise.
> * src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Update caller.
> * src/qemu/qemu_monitor.c (qemuMonitorBlockJob): Likewise.
> ---
>  src/qemu/qemu_driver.c       |   15 ++++++---------
>  src/qemu/qemu_monitor.c      |    7 ++++---
>  src/qemu/qemu_monitor.h      |    6 ++++--
>  src/qemu/qemu_monitor_json.c |   26 +++++++++++++++++++-------
>  src/qemu/qemu_monitor_json.h |    3 ++-
>  5 files changed, 35 insertions(+), 22 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 717bdf1..8308b3b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -11367,14 +11367,6 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
>      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;
> -    }
>
>      if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
>          goto cleanup;
> @@ -11387,7 +11379,12 @@ qemuDomainBlockJobImpl(virDomainPtr dom, const char *path, const char *base,
>
>      qemuDomainObjEnterMonitorWithDriver(driver, vm);
>      priv = vm->privateData;
> -    ret = qemuMonitorBlockJob(priv->mon, device, bandwidth, info, mode);
> +    /* XXX - add a qemu capability check, since only qemu 1.1 or newer
> +     * supports the base argument.
> +     * XXX - libvirt should really be tracking the backing file chain
> +     * itself, and validating that base is on the chain, rather than
> +     * relying on qemu to do this.  */
> +    ret = qemuMonitorBlockJob(priv->mon, device, base, bandwidth, info, mode);
>      qemuDomainObjExitMonitorWithDriver(driver, vm);
>
>  endjob:
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 903463d..70ef1d6 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -2653,17 +2653,18 @@ int qemuMonitorScreendump(qemuMonitorPtr mon,
>
>  int qemuMonitorBlockJob(qemuMonitorPtr mon,
>                          const char *device,
> +                        const char *base,
>                          unsigned long bandwidth,
>                          virDomainBlockJobInfoPtr info,
>                          int mode)
>  {
>      int ret = -1;
>
> -    VIR_DEBUG("mon=%p, device=%s, bandwidth=%lu, info=%p, mode=%o",
> -              mon, device, bandwidth, info, mode);
> +    VIR_DEBUG("mon=%p, device=%s, base=%s, bandwidth=%lu, info=%p, mode=%o",
> +              mon, device, NULLSTR(base), bandwidth, info, mode);
>
>      if (mon->json)
> -        ret = qemuMonitorJSONBlockJob(mon, device, bandwidth, info, mode);
> +        ret = qemuMonitorJSONBlockJob(mon, device, base, bandwidth, info, mode);
>      else
>          qemuReportError(VIR_ERR_INVALID_ARG, "%s",
>                          _("block jobs require JSON monitor"));
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 7c6c52b..1d2caea 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -1,7 +1,7 @@
>  /*
>   * qemu_monitor.h: interaction with QEMU monitor console
>   *
> - * Copyright (C) 2006-2011 Red Hat, Inc.
> + * Copyright (C) 2006-2012 Red Hat, Inc.
>   * Copyright (C) 2006 Daniel P. Berrange
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -518,9 +518,11 @@ typedef enum {
>
>  int qemuMonitorBlockJob(qemuMonitorPtr mon,
>                          const char *device,
> +                        const char *back,
>                          unsigned long bandwidth,
>                          virDomainBlockJobInfoPtr info,
> -                        int mode);
> +                        int mode)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5);
>
>  int qemuMonitorOpenGraphics(qemuMonitorPtr mon,
>                              const char *protocol,
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 513788a..2677423 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -3229,17 +3229,25 @@ static int qemuMonitorJSONGetBlockJobInfo(virJSONValuePtr reply,
>  }
>
>
> -int qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
> -                             const char *device,
> -                             unsigned long bandwidth,
> -                             virDomainBlockJobInfoPtr info,
> -                             int mode)
> +int
> +qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
> +                        const char *device,
> +                        const char *base,
> +                        unsigned long bandwidth,
> +                        virDomainBlockJobInfoPtr info,
> +                        int mode)
>  {
>      int ret = -1;
>      virJSONValuePtr cmd = NULL;
>      virJSONValuePtr reply = NULL;
>      const char *cmd_name = NULL;
>
> +    if (base && mode != BLOCK_JOB_PULL) {
> +        qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                        _("only block pull supports base: %s"), base);
> +        return -1;
> +    }
> +
>      if (mode == BLOCK_JOB_ABORT) {
>          cmd_name = "block_job_cancel";
>          cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device", device, NULL);
> @@ -3254,8 +3262,12 @@ int qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
>                                           NULL);
>      } else if (mode == BLOCK_JOB_PULL) {
>          cmd_name = "block_stream";
> -        cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device",
> -                                         device, NULL);
> +        if (base)
> +            cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device",
> +                                             device, "s:base", base, NULL);
> +        else
> +            cmd = qemuMonitorJSONMakeCommand(cmd_name, "s:device",
> +                                             device, NULL);
>      }
>
>      if (!cmd)
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index d0b3000..e307649 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -1,7 +1,7 @@
>  /*
>   * qemu_monitor_json.h: interaction with QEMU monitor console
>   *
> - * Copyright (C) 2006-2009, 2011 Red Hat, Inc.
> + * Copyright (C) 2006-2009, 2011-2012 Red Hat, Inc.
>   * Copyright (C) 2006 Daniel P. Berrange
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -246,6 +246,7 @@ int qemuMonitorJSONScreendump(qemuMonitorPtr mon,
>
>  int qemuMonitorJSONBlockJob(qemuMonitorPtr mon,
>                              const char *device,
> +                            const char *base,
>                              unsigned long bandwidth,
>                              virDomainBlockJobInfoPtr info,
>                              int mode);


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