Re: [libvirt] [PATCH v2 1/8] blockcopy: virDomainBlockCopy with XML destination, typed params

This commit (finally) adds the virDomainBlockCopy API, with the intent that it will provide more power to the existing 'virsh blockcopy' command. 'virsh blockcopy' was first added in Apr 2012 (v0.9.12), which corresponds to the upstream qemu 1.2 timeframe. It was done as a hack on top of the existing virDomainBlockRebase() API call, for two reasons: 1) it was targetting a feature that landed first in downstream RHEL qemu, but had not stabilized in upstream qemu at the time (and indeed, 'drive-mirror' only landed upstream in qemu 1.3 with slight differences to the first RHEL attempt, and later gained further parameters like granularity and buf-size that are also worth exposing), and 2) extending an existing API allowed it to be backported without worrying about bumping .so versions. A virDomainBlockCopy() API was proposed at that time [1], but we decided not to accept it into libvirt until after upstream qemu stabilized, and it ended up getting scrapped. Whether or not RHEL should have attempted adding a new feature without getting it upstream first is a debate that can be held another day; but enough time has now elapsed that we are ready to do the interface cleanly. [1] https://www.redhat.com/archives/libvir-list/2012-April/msg00768.html Delaying the creation of a clean API until now has also had a benefit: we've only recently learned of a shortcoming in the original design, namely, that it is unable to target a network destination (such as a gluster volume) because it hard-coded the assumption that the destination is a local file name. Because of all the refactoring we've done to add virStorageSourcePtr, we are in a better position to declare an API that parses XML describing a host storage source as the copy destination, which was not possible had we implemented virDomainBlockCopy as it had been originally envisioned. At least I had the foresight to create 'virsh blockcopy' as a separate command at the UI level (commit 1f06c00) rather than leaking the underlying API overload of virDomainBlockRebase onto shell users. A note on the bandwidth option: virTypedParameters intentionally lacks unsigned long (since variable-width interaction between mixed 32- vs. 64-bit client/server setups is nasty), but we have to deal with the fact that we are interacting with existing older code that mistakenly chose unsigned long bandwidth at a point before we decided to prohibit it in all new API. The typed parameter is therefore unsigned long long, and the implementation (in a later patch) will have to do overflow detection on 32-bit platforms. (Actually, qemu treats bandwidth as bytes/second, while we document it as megabytes/second, so we already do overflow detection even on 64-bit hosts to prohibit anything larger than LLONG_MAX >> 20). * include/libvirt/libvirt.h.in (virDomainBlockCopy): New API. (virDomainBlockJobType, virConnectDomainEventBlockJobStatus): Update related documentation. * src/libvirt.c (virDomainBlockCopy): Implement it. * src/libvirt_public.syms (LIBVIRT_1.2.8): Export it. * src/driver.h (_virDriver): New driver callback. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- include/libvirt/libvirt.h.in | 61 ++++++++++++++++++++-- src/driver.h | 11 +++- src/libvirt.c | 120 ++++++++++++++++++++++++++++++++++++++++++- src/libvirt_public.syms | 5 ++ 4 files changed, 190 insertions(+), 7 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 47ea695..ebed373 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2518,16 +2518,16 @@ typedef enum { * flags), job ends on completion */ VIR_DOMAIN_BLOCK_JOB_TYPE_COPY = 2, - /* Block Copy (virDomainBlockRebase with flags), job exists as - * long as mirroring is active */ + /* Block Copy (virDomainBlockCopy, or virDomainBlockRebase with + * flags), job exists as long as mirroring is active */ VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT = 3, /* Block Commit (virDomainBlockCommit without flags), job ends on * completion */ VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT = 4, - /* Active Block Commit (virDomainBlockCommit with flags), job - * exists as long as sync is active */ + /* Active Block Commit (virDomainBlockCommit with flags), job exists + * as long as sync is active */ #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_BLOCK_JOB_TYPE_LAST @@ -2597,6 +2597,57 @@ int virDomainBlockRebase(virDomainPtr dom, const char *disk, 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; + +/** + * VIR_DOMAIN_BLOCK_COPY_BANDWIDTH: + * Macro for the virDomainBlockCopy bandwidth tunable: it represents + * the maximum bandwidth (in MiB/s) used while getting the copy

+ * operation into the mirrored phase, with a type of ullong (although + * the hypervisor may restrict the set of valid values to a smaller + * range). Specifying 0 is the same as omitting this parameter, to + * request the hypervisor default. Some hypervisors may lack support + * for this parameter, while still allowing a subsequent change of + * bandwidth via virDomainBlockJobSetSpeed(). The actual speed can + * be determined with virDomainGetBlockJobInfo(). + */ +#define VIR_DOMAIN_BLOCK_COPY_BANDWIDTH "bandwidth"