[libvirt] [PATCHv4 16/18] blockjob: add virDomainBlockCopy

[Jiri Denemark]

On Mon, Apr 09, 2012 at 21:52:25 -0600, Eric Blake wrote:
> This new API provides additional flexibility over what can be
> crammed on top of virDomainBlockRebase (namely, the ability to
> specify an arbitrary destination format, for things like copying
> qcow2 into qed without having to pre-create the destination), at
> the expense that it cannot be backported without bumping the .so
> version.
> 
> * include/libvirt/libvirt.h.in (virDomainBlockCopy): New API.
> * src/libvirt.c (virDomainBlockCopy): Implement it.
> * src/libvirt_public.syms (LIBVIRT_0.9.12): Export it.
> * src/driver.h (virDrvDomainBlockCopy): New driver callback.
> * docs/apibuild.py (CParser.parseSignature): Add exception.
> ---
>  docs/apibuild.py             |    1 +
>  include/libvirt/libvirt.h.in |   18 ++++++-
>  src/driver.h                 |    5 ++
>  src/libvirt.c                |  119 +++++++++++++++++++++++++++++++++++++++++-
>  src/libvirt_public.syms      |    5 ++
>  5 files changed, 145 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/apibuild.py b/docs/apibuild.py
> index 1ac0281..bf06f3b 100755
> --- a/docs/apibuild.py
> +++ b/docs/apibuild.py
> @@ -1650,6 +1650,7 @@ class CParser:
>          "virDomainBlockJobSetSpeed"      : (False, ("bandwidth")),
>          "virDomainBlockPull"             : (False, ("bandwidth")),
>          "virDomainBlockRebase"           : (False, ("bandwidth")),
> +        "virDomainBlockCopy"             : (False, ("bandwidth")),
>          "virDomainMigrateGetMaxSpeed"    : (False, ("bandwidth")) }
> 
>      def checkLongLegacyFunction(self, name, return_type, signature):
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index ac5df95..7cd5bbe 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1937,7 +1937,7 @@ int virDomainUpdateDeviceFlags(virDomainPtr domain,
>   * VIR_DOMAIN_BLOCK_JOB_TYPE_PULL: Block Pull (virDomainBlockPull, or
>   * virDomainBlockRebase without flags), job ends on completion
>   * VIR_DOMAIN_BLOCK_JOB_TYPE_COPY: Block Copy (virDomainBlockRebase with
> - * flags), job exists as long as mirroring is active
> + * flags, or virDomainBlockCopy), job exists as long as mirroring is active
>   */
>  typedef enum {
>      VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN = 0,
> @@ -2007,6 +2007,22 @@ int           virDomainBlockRebase(virDomainPtr dom, const char *disk,
>                                     const char *base, unsigned long bandwidth,
>                                     unsigned int flags);
> 
> +/**
> + * virDomainBlockCopyFlags:
> + *
> + * Flags available for virDomainBlockCopy().
> + */
> +typedef enum {
> +    VIR_DOMAIN_BLOCK_COPY_SHALLOW   = 1 << 0, /* Limit copy to top of source
> +                                                 backing chain */
> +    VIR_DOMAIN_BLOCK_COPY_REUSE_EXT = 1 << 1, /* Reuse existing external
> +                                                 file for a copy */
> +} virDomainBlockCopyFlags;

Hmm, we have several flags enums that end up being passed to a single internal
API and one has to be extra careful when adding new flags. Should we make a
note about this to the affected enums?

> +
> +int virDomainBlockCopy(virDomainPtr dom, const char *disk,  const char *dest,
> +                       const char *format, unsigned long bandwidth,
> +                       unsigned int flags);
> +
> 
>  /* Block I/O throttling support */
> 
> diff --git a/src/driver.h b/src/driver.h
> index 03d249b..e10ba71 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -788,6 +788,10 @@ typedef int
>      (*virDrvDomainBlockRebase)(virDomainPtr dom, const char *path,
>                                 const char *base, unsigned long bandwidth,
>                                 unsigned int flags);
> +typedef int
> +    (*virDrvDomainBlockCopy)(virDomainPtr dom, const char *path,
> +                             const char *dest, const char *format,
> +                             unsigned long bandwidth, unsigned int flags);
> 
>  typedef int
>      (*virDrvSetKeepAlive)(virConnectPtr conn,
> @@ -1005,6 +1009,7 @@ struct _virDriver {
>      virDrvDomainBlockJobSetSpeed domainBlockJobSetSpeed;
>      virDrvDomainBlockPull domainBlockPull;
>      virDrvDomainBlockRebase domainBlockRebase;
> +    virDrvDomainBlockCopy domainBlockCopy;
>      virDrvSetKeepAlive setKeepAlive;
>      virDrvConnectIsAlive isAlive;
>      virDrvNodeSuspendForDuration nodeSuspendForDuration;
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 753a2e0..bdde654 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -18263,7 +18263,11 @@ error:
>   * return an error if bandwidth is not 0.
>   *
>   * When @base is NULL and @flags is 0, this is identical to
> - * virDomainBlockPull().
> + * virDomainBlockPull().  Conversely, when @flags includes
> + * VIR_DOMAIN_BLOCK_REBASE_COPY, this is shorthand for
> + * virDomainBlockCopy(dom, disk, base,
> + * flags & VIR_DOMAIN_BLOCK_COPY_RAW ? "raw" : NULL, bandwidth,
> + * flags & (VIR_DOMAIN_BLOCK_REBASE_SHALLOW|VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)).
>   *
>   * Returns 0 if the operation has started, -1 on failure.
>   */
> @@ -18273,7 +18277,7 @@ int virDomainBlockRebase(virDomainPtr dom, const char *disk,
>  {
>      virConnectPtr conn;
> 
> -    VIR_DOMAIN_DEBUG(dom, "disk=%s, base=%s bandwidth=%lu, flags=%x",
> +    VIR_DOMAIN_DEBUG(dom, "disk=%s, base=%s, bandwidth=%lu, flags=%x",
>                       disk, NULLSTR(base), bandwidth, flags);
> 
>      virResetLastError();
> @@ -18328,6 +18332,117 @@ error:
> 
> 
>  /**
> + * virDomainBlockCopy:
> + * @dom: pointer to domain object
> + * @disk: path to the block device, or device shorthand
> + * @dest: path to the copy destination
> + * @format: format of the destination
> + * @bandwidth: (optional) specify copy bandwidth limit in Mbps
> + * @flags: bitwise-OR of virDomainBlockCopyFlags

OK, so this new API may be used to avoid format guessing involved in
virDomainBlockRebase. Shouldn't we introduce an enhanced version of
virDomainBlockRebase with format parameter instead of introducing an API with
a different name that does almost the same as virDomainBlockRebase?

Jirka