[libvirt] [PATCHv4 13/18] blockjob: allow for existing files

[Jiri Denemark]

On Mon, Apr 09, 2012 at 21:52:22 -0600, Eric Blake wrote:
> This copies some of the checks from snapshots regarding testing
> when a file already exists.  In the process, I noticed a missing
> sanity check in snapshots: REUSE_EXT should require the destination
> to already be present.
> 
> * src/qemu/qemu_driver.c (qemuDomainBlockRebase): Allow REUSE_EXT
> flag.
> (qemuDomainBlockCopy): Wire up flag, and add some sanity checks.
> (qemuDomainSnapshotDiskPrepare): Require destination on REUSE_EXT.
> ---
>  src/qemu/qemu_driver.c |   48 +++++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 43 insertions(+), 5 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 6d5a5da..661ccb4 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -9812,6 +9812,11 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def,
>                                           _("unable to stat for disk %s: %s"),
>                                           disk->name, disk->file);
>                      goto cleanup;
> +                } else if (allow_reuse) {
> +                    virReportSystemError(errno,
> +                                         _("missing existing file for disk %s: %s"),
> +                                         disk->name, disk->file);
> +                    goto cleanup;
>                  }
>              } else if (!(S_ISBLK(st.st_mode) || !st.st_size || allow_reuse)) {
>                  qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -11889,9 +11894,11 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path,
>      virDomainDiskDefPtr disk;
>      int ret = -1;
>      int idx;
> +    struct stat st;
> 
>      /* Preliminaries: find the disk we are editing, sanity checks */
> -    virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW, -1);
> +    virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
> +                  VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT, -1);
> 
>      qemuDriverLock(driver);
>      virUUIDFormat(dom->uuid, uuidstr);
> @@ -11936,12 +11943,42 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path,
>          goto endjob;
>      }
> 
> +    /* XXX this is pessimistic; we could use 'query-block' or even
> +     * keep track of the backing chain ourselves, rather than assuming
> +     * that all non-raw source files have a current backing image */
> +    if ((flags & VIR_DOMAIN_BLOCK_REBASE_SHALLOW) &&
> +        STREQ_NULLABLE(format, "raw") &&
> +        STRNEQ_NULLABLE(disk->driverType, "raw")) {
> +        qemuReportError(VIR_ERR_INVALID_ARG,
> +                        _("raw shallow copy of non-raw disk '%s' not possible"),
> +                        disk->dst);
> +        goto endjob;
> +    }
> +
>      /* Prepare the destination file.  */
> +    if (stat(dest, &st) < 0) {
> +        if (errno != ENOENT) {
> +            virReportSystemError(errno, _("unable to stat for disk %s: %s"),
> +                                 disk->dst, dest);
> +            goto endjob;
> +        } else if (flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT) {
> +            virReportSystemError(errno,
> +                                 _("missing destination file for disk %s: %s"),
> +                                 disk->dst, dest);
> +            goto endjob;
> +        }
> +    } else if (!(S_ISBLK(st.st_mode) || !st.st_size ||
> +                 (flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT))) {

Eh, this is quite hard to parse, I'd move the negation inside;

              if (!S_ISBLK(st.st_mode) && st.st_size &&
                  !(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT))

> +        qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                        _("external destination file for disk %s already "
> +                          "exists and is not a block device: %s"),
> +                        disk->dst, dest);
> +        goto endjob;
> +    }
> +
>      /* XXX We also need to add security labeling, lock manager lease,
> -     * and auditing of those events, as well as to support reuse of
> -     * existing images, including probing the existing format of an
> -     * existing image.  */
> -    if (!format)
> +     * and auditing of those events.  */
> +    if (!format && !(flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT))
>          format = disk->driverType;
>      if ((format && !(disk->mirrorFormat = strdup(format))) ||
>          !(disk->mirror = strdup(dest))) {
> @@ -11980,6 +12017,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
>                        unsigned long bandwidth, unsigned int flags)
>  {
>      virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
> +                  VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
>                    VIR_DOMAIN_BLOCK_REBASE_COPY |
>                    VIR_DOMAIN_BLOCK_REBASE_COPY_RAW, -1);
> 

OK

Jirka