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

Re: [libvirt] [PATCH] blockcopy: allow block device destination



Ping

On 08/13/2014 02:00 PM, Eric Blake wrote:
> To date, anyone performing a block copy and pivot ends up with
> the destination being treated as <disk type='file'>.  While this
> works for data access for a block device, it has at least one
> noticeable shortcoming: virDomainGetBlockInfo() reports allocation
> differently for block devices visited as files (the size of the
> device) than for block devices visited as <disk type='block'>
> (the maximum sector used, as reported by qemu); and this difference
> is significant when trying to manage qcow2 format on block devices
> that can be grown as needed.
> 
> I still plan to add a virDomainBlockCopy() API that takes the
> destination disk as XML, allowing full expressive capability
> to copy to a network disk.  But a new API can't be backported,
> while a new flag to an existing API can.  So this patch enhances
> blockcopy to let the user flag when the resulting XML after the
> copy must list the device as type='block'.
> 
> * include/libvirt/libvirt.h.in (VIR_DOMAIN_BLOCK_REBASE_COPY_DEV):
> New flag.
> * src/libvirt.c (virDomainBlockRebase): Document it.
> * tools/virsh-domain.c (opts_block_copy, blockJobImpl): Add
> --blockdev option.
> * tools/virsh.pod (blockcopy): Document it.
> * src/qemu/qemu_driver.c (qemuDomainBlockRebase): Allow new flag.
> (qemuDomainBlockCopy): Remember the flag.
> * tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Test it.
> 
> Signed-off-by: Eric Blake <eblake redhat com>
> ---
>  include/libvirt/libvirt.h.in                       |    2 ++
>  src/libvirt.c                                      |    8 ++++++--
>  src/qemu/qemu_driver.c                             |   12 ++++++++----
>  .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml  |    4 ++--
>  tools/virsh-domain.c                               |    6 ++++++
>  tools/virsh.pod                                    |    7 +++++--
>  6 files changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 47ea695..f2a02ea 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -2590,6 +2590,8 @@ typedef enum {
>      VIR_DOMAIN_BLOCK_REBASE_RELATIVE  = 1 << 4, /* Keep backing chain
>                                                     referenced using relative
>                                                     names */
> +    VIR_DOMAIN_BLOCK_REBASE_COPY_DEV  = 1 << 5, /* Treat destination as block
> +                                                   device instead of file */
>  } virDomainBlockRebaseFlags;
> 
>  int           virDomainBlockRebase(virDomainPtr dom, const char *disk,
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 992e4f2..c4643e8 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -19881,7 +19881,10 @@ virDomainBlockPull(virDomainPtr dom, const char *disk,
>   * pre-create files with relative backing file names, rather than the default
>   * of absolute backing file names; as a security precaution, you should
>   * generally only use reuse_ext with the shallow flag and a non-raw
> - * destination file.
> + * destination file.  By default, the copy destination will be treated as
> + * type='file', but using VIR_DOMAIN_BLOCK_REBASE_COPY_DEV treats the
> + * destination as type='block' (affecting how virDomainGetBlockInfo() will
> + * report allocation after pivoting).
>   *
>   * A copy job has two parts; in the first phase, the @bandwidth parameter
>   * affects how fast the source is pulled into the destination, and the job
> @@ -19950,7 +19953,8 @@ virDomainBlockRebase(virDomainPtr dom, const char *disk,
>          virCheckNonNullArgGoto(base, error);
>      } else if (flags & (VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
>                          VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
> -                        VIR_DOMAIN_BLOCK_REBASE_COPY_RAW)) {
> +                        VIR_DOMAIN_BLOCK_REBASE_COPY_RAW |
> +                        VIR_DOMAIN_BLOCK_REBASE_COPY_DEV)) {
>          virReportInvalidArg(flags,
>                              _("use of flags in %s requires a copy job"),
>                              __FUNCTION__);
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index b6219ba..e74227e 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -15295,7 +15295,8 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
> 
>      /* Preliminaries: find the disk we are editing, sanity checks */
>      virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
> -                  VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT, -1);
> +                  VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
> +                  VIR_DOMAIN_BLOCK_REBASE_COPY_DEV, -1);
> 
>      priv = vm->privateData;
>      cfg = virQEMUDriverGetConfig(driver);
> @@ -15374,7 +15375,8 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
>      if (VIR_ALLOC(mirror) < 0)
>          goto endjob;
>      /* XXX Allow non-file mirror destinations */
> -    mirror->type = VIR_STORAGE_TYPE_FILE;
> +    mirror->type = flags & VIR_DOMAIN_BLOCK_REBASE_COPY_DEV ?
> +        VIR_STORAGE_TYPE_BLOCK : VIR_STORAGE_TYPE_FILE;
> 
>      if (format) {
>          if ((mirror->format = virStorageFileFormatTypeFromString(format)) <= 0) {
> @@ -15466,7 +15468,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
>                    VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
>                    VIR_DOMAIN_BLOCK_REBASE_COPY |
>                    VIR_DOMAIN_BLOCK_REBASE_COPY_RAW |
> -                  VIR_DOMAIN_BLOCK_REBASE_RELATIVE, -1);
> +                  VIR_DOMAIN_BLOCK_REBASE_RELATIVE |
> +                  VIR_DOMAIN_BLOCK_REBASE_COPY_DEV, -1);
> 
>      if (!(vm = qemuDomObjFromDomain(dom)))
>          return -1;
> @@ -15482,7 +15485,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
>              format = "raw";
>          flags &= ~(VIR_DOMAIN_BLOCK_REBASE_COPY |
>                     VIR_DOMAIN_BLOCK_REBASE_COPY_RAW);
> -        return qemuDomainBlockCopy(vm, dom->conn, path, base, format, bandwidth, flags);
> +        return qemuDomainBlockCopy(vm, dom->conn, path, base, format,
> +                                   bandwidth, flags);
>      }
> 
>      return qemuDomainBlockJobImpl(vm, dom->conn, path, base, bandwidth, NULL,
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
> index 46f2a3e..7495a45 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
> @@ -17,8 +17,8 @@
>      <disk type='block' device='disk'>
>        <source dev='/dev/HostVG/QEMUGuest1'/>
>        <backingStore/>
> -      <mirror type='file' file='/dev/HostVG/QEMUGuest1Copy' job='copy' ready='yes'>
> -        <source file='/dev/HostVG/QEMUGuest1Copy'/>
> +      <mirror type='block' job='copy' ready='yes'>
> +        <source dev='/dev/HostVG/QEMUGuest1Copy'/>
>        </mirror>
>        <target dev='hda' bus='ide'/>
>        <address type='drive' controller='0' bus='0' target='0' unit='0'/>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index f7193cb..1fbd36e 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -1521,6 +1521,8 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
>              flags |= VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT;
>          if (vshCommandOptBool(cmd, "raw"))
>              flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_RAW;
> +        if (vshCommandOptBool(cmd, "blockdev"))
> +            flags |= VIR_DOMAIN_BLOCK_REBASE_COPY_DEV;
>          if (vshCommandOptStringReq(ctl, cmd, "dest", &base) < 0)
>              goto cleanup;
>          ret = virDomainBlockRebase(dom, path, base, bandwidth, flags);
> @@ -1828,6 +1830,10 @@ static const vshCmdOptDef opts_block_copy[] = {
>       .type = VSH_OT_BOOL,
>       .help = N_("use raw destination file")
>      },
> +    {.name = "blockdev",
> +     .type = VSH_OT_BOOL,
> +     .help = N_("copy destination is block device instead of regular file")
> +    },
>      {.name = "wait",
>       .type = VSH_OT_BOOL,
>       .help = N_("wait for job to reach mirroring phase")
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index f07deec..8178868 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -873,7 +873,8 @@ unlimited. The hypervisor can choose whether to reject the value or
>  convert it to the maximum value allowed.
> 
>  =item B<blockcopy> I<domain> I<path> I<dest> [I<bandwidth>] [I<--shallow>]
> -[I<--reuse-external>] [I<--raw>] [I<--wait> [I<--async>] [I<--verbose>]]
> +[I<--reuse-external>] [I<--raw>] [I<--blockdev>]
> +[I<--wait> [I<--async>] [I<--verbose>]]
>  [{I<--pivot> | I<--finish>}] [I<--timeout> B<seconds>]
> 
>  Copy a disk backing image chain to I<dest>. By default, this command
> @@ -891,7 +892,9 @@ The format of the destination is determined by the first match in the
>  following list: if I<--raw> is specified, it will be raw; if
>  I<--reuse-external> is specified, the existing destination is probed
>  for a format; and in all other cases, the destination format will
> -match the source format.
> +match the source format.  The destination is treated as a regular
> +file unless I<--blockdev> is used to signal that it is a block
> +device.
> 
>  By default, the copy job runs in the background, and consists of two
>  phases.  Initially, the job must copy all data from the source, and
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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