[libvirt] [PATCH 0/3] block commit support

I'm still working on the qemu implementation of this API, but want to get the API reviewed and included in 0.10.2 before adding piecewise support for more and more of the API. My biggest issues for current implementations are: online qemu: Jeff's block-commit API is still not part of qemu.git, and might be subject to change, in particular based on review from Paolo's drive-mirror changes (since both patch series will be sharing related code on how to handle error reporting). offline qemu: qemu-img is a blocking command, but this API promises to be asynchronous. I'll have to run qemu-img in a worker thread, and I still need to figure out whether qemu-img can display progress for virDomainBlockJobInfo (can you send it a SIGUSR1, like for dd?), otherwise, the job will look like it is making no progress. But as neither of those issues invalidate the API, I'm comfortable with committing the API now (after all, that's what we did back in 0.9.12 with the virDomainBlockRebase API - we committed code in preparation for qemu to implement drive-mirror, and we are STILL waiting on qemu to follow through with that promise...) Obviously, there will be more patches to come as I get more things working at the qemu layer (I'm not sure whether online or offline qemu will be done first, as I have branches working on both approaches while I am testing against Jeff's qemu patch proposals). Eric Blake (3): blockjob: add virDomainBlockCommit blockjob: add virsh blockcommit blockjob: add blockcommit support to rpc docs/apibuild.py | 1 + include/libvirt/libvirt.h.in | 20 ++++++ src/driver.h | 5 ++ src/libvirt.c | 105 +++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 +++- src/remote_protocol-structs | 9 +++ src/rpc/gendispatch.pl | 1 + tools/virsh-domain.c | 153 ++++++++++++++++++++++++++++++++++++++++++- tools/virsh.pod | 30 +++++++++ 11 files changed, 336 insertions(+), 2 deletions(-) -- 1.7.11.4

This API matches a proposed qemu command 'block-commit': https://lists.gnu.org/archive/html/qemu-devel/2012-09/msg02226.html Jeff's command is still in the works for qemu 1.3, and may gain further enhancements, such as the ability to control on-error handling (it will be comparable to the error handling Paolo is adding to 'drive-mirror', so a similar solution will be needed when I finally propose virDomainBlockCopy). However, even without qemu support, this API will be useful for _offline_ block commits, by wrapping qemu-img calls and turning them into a block job, so this API is worth committing now. For some examples of how this will be implemented, all starting with the chain: base <- snap1 <- snap2 <- active + These are equivalent: virDomainBlockCommit(dom, disk, NULL, NULL, 0, 0) virDomainBlockCommit(dom, disk, NULL, "active", 0, 0) virDomainBlockCommit(dom, disk, "base", NULL, 0, 0) virDomainBlockCommit(dom, disk, "base", "active", 0, 0) but cannot be implemented for online qemu with round 1 of Jeff's patches; and for offline images, it would require three back-to-back qemu-img invocations unless qemu-img is patched to allow more efficient multi-layer commits; the end result would be 'base' as the active disk with contents from all three other files, where 'snap1' and 'snap2' are invalid right away, and 'active' is invalid once any further changes to 'base' are made, for a final chain of base. + These are equivalent: virDomainBlockCommit(dom, disk, "snap2", NULL, 0, 0) virDomainBlockCommit(dom, disk, NULL, NULL, 0, _SHALLOW) they cannot be implemented for online qemu, but for offline, it is a matter of 'qemu-img commit active', so that 'snap2' is now the active disk with contents formerly in 'active', for a final chain of base <- snap1 <- snap2. + Similarly: virDomainBlockCommit(dom, disk, "snap2", NULL, 0, _DELETE) for an offline domain will merge 'active' into 'snap2', then delete 'active' to avoid leaving a potentially invalid file around. + This version: virDomainBlockCommit(dom, disk, NULL, "snap2", 0, _SHALLOW) can be implemented online with 'block-commit' passing a base of snap1 and a top of snap2; and can be implemented offline by 'qemu-img commit snap2' followed by 'qemu-img rebase -u -b snap1 active', for a final chain of base <- snap1 <- active. * include/libvirt/libvirt.h.in (virDomainBlockCommit): New API. * src/libvirt.c (virDomainBlockCommit): Implement it. * src/libvirt_public.syms (LIBVIRT_0.10.2): Export it. * src/driver.h (virDrvDomainBlockCommit): New driver callback. * docs/apibuild.py (CParser.parseSignature): Add exception. --- docs/apibuild.py | 1 + include/libvirt/libvirt.h.in | 20 +++++++++ src/driver.h | 5 +++ src/libvirt.c | 105 +++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 5 files changed, 132 insertions(+) diff --git a/docs/apibuild.py b/docs/apibuild.py index 3f0ecd9..ae84be9 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -1759,6 +1759,7 @@ class CParser: "virDomainSetMaxMemory" : (False, ("memory")), "virDomainSetMemory" : (False, ("memory")), "virDomainSetMemoryFlags" : (False, ("memory")), + "virDomainBlockCommit" : (False, ("bandwidth")), "virDomainBlockJobSetSpeed" : (False, ("bandwidth")), "virDomainBlockPull" : (False, ("bandwidth")), "virDomainBlockRebase" : (False, ("bandwidth")), diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 8f72c03..2fd8ae1 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2062,11 +2062,14 @@ int virDomainUpdateDeviceFlags(virDomainPtr domain, * 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 + * VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT: Block Commit (virDomainBlockCommit), + * job ends on completion */ typedef enum { VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN = 0, VIR_DOMAIN_BLOCK_JOB_TYPE_PULL = 1, VIR_DOMAIN_BLOCK_JOB_TYPE_COPY = 2, + VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT = 3, #ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_BLOCK_JOB_TYPE_LAST @@ -2131,6 +2134,23 @@ int virDomainBlockRebase(virDomainPtr dom, const char *disk, const char *base, unsigned long bandwidth, unsigned int flags); +/** + * virDomainBlockCommitFlags: + * + * Flags available for virDomainBlockCommit(). + */ +typedef enum { + VIR_DOMAIN_BLOCK_COMMIT_SHALLOW = 1 << 0, /* NULL base means next backing + file, not whole chain */ + VIR_DOMAIN_BLOCK_COMMIT_DELETE = 1 << 1, /* Delete any files that are now + invalid after their contents + have been committed */ +} virDomainBlockCommitFlags; + +int virDomainBlockCommit(virDomainPtr dom, const char *disk, const char *base, + const char *top, unsigned long bandwidth, + unsigned int flags); + /* Block I/O throttling support */ diff --git a/src/driver.h b/src/driver.h index bb470fe..063bbbf 100644 --- a/src/driver.h +++ b/src/driver.h @@ -832,6 +832,10 @@ typedef int (*virDrvDomainBlockRebase)(virDomainPtr dom, const char *path, const char *base, unsigned long bandwidth, unsigned int flags); +typedef int + (*virDrvDomainBlockCommit)(virDomainPtr dom, const char *disk, + const char *base, const char *top, + unsigned long bandwidth, unsigned int flags); typedef int (*virDrvSetKeepAlive)(virConnectPtr conn, @@ -1071,6 +1075,7 @@ struct _virDriver { virDrvDomainBlockJobSetSpeed domainBlockJobSetSpeed; virDrvDomainBlockPull domainBlockPull; virDrvDomainBlockRebase domainBlockRebase; + virDrvDomainBlockCommit domainBlockCommit; virDrvSetKeepAlive setKeepAlive; virDrvConnectIsAlive isAlive; virDrvNodeSuspendForDuration nodeSuspendForDuration; diff --git a/src/libvirt.c b/src/libvirt.c index 9848993..41ecda1 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -19219,6 +19219,111 @@ error: /** + * virDomainBlockCommit: + * @dom: pointer to domain object + * @disk: path to the block device, or device shorthand + * @base: path to backing file to merge into, or NULL for default + * @top: path to file within backing chain that contains data to be merged, + * or NULL to merge all possible data + * @bandwidth: (optional) specify commit bandwidth limit in MiB/s + * @flags: bitwise-OR of virDomainBlockCommitFlags + * + * Commit changes that were made to temporary top-level files within a disk + * image backing file chain into a lower-level base file. In other words, + * take all the difference between @base and @top, and update @base to contain + * that difference; after the commit, any portion of the chain that previously + * depended on @top will now depend on @base, and all files after @base up + * to and including @top will now be invalidated. A typical use of this + * command is to reduce the length of a backing file chain after taking an + * external disk snapshot. + * + * This command starts a long-running commit block job, whose status may + * be tracked by virDomainBlockJobInfo() with a job type of + * VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT, and the operation can be aborted with + * virDomainBlockJobAbort(). When finished, an asynchronous event is + * raised to indicate the final status, and the job no longer exists. If + * the job is aborted, it is up to the hypervisor whether starting a new + * job will resume from the same point, or start over. + * + * Be aware that this command may invalidate files even if it is aborted; + * the user is cautioned against relying on the contents of invalidated + * files such as @top without manually rebasing those files to use a + * backing file of a read-only copy of @base prior to the point where the + * commit operation was restarted (although such a rebase cannot be safely + * done until the commit has successfully completed). As a convenience, + * if @flags contains VIR_DOMAIN_BLOCK_COMMIT_DELETE, this command will + * unlink all files that were invalidated, after the commit successfully + * completes. + * + * By default, if @base is NULL, the bottom of the backing chain will be + * the commit target; if @flags contains VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, + * then the immediate backing file of @top will be used instead. If @top + * is NULL, the active image at the top of the chain will be used. Some + * hypervisors place restrictions on how much can be committed, and might + * fail if @base is not the immediate backing file of @top, or if @top is + * in use by a running domain, or if @top is not the top-most file. + * + * The @disk parameter is either an unambiguous source name of the + * block device (the <source file='...'/> sub-element, such as + * "/path/to/image"), or the device target shorthand (the + * <target dev='...'/> sub-element, such as "xvda"). Valid names + * can be found by calling virDomainGetXMLDesc() and inspecting + * elements within //domain/devices/disk. + * + * The maximum bandwidth (in MiB/s) that will be used to do the commit can be + * specified with the bandwidth parameter. If set to 0, libvirt will choose a + * suitable default. Some hypervisors do not support this feature and will + * return an error if bandwidth is not 0; in this case, it might still be + * possible for a later call to virDomainBlockJobSetSpeed() to succeed. + * The actual speed can be determined with virDomainGetBlockJobInfo(). + * + * Returns 0 if the operation has started, -1 on failure. + */ +int virDomainBlockCommit(virDomainPtr dom, const char *disk, + const char *base, const char *top, + unsigned long bandwidth, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "disk=%s, base=%s, top=%s, bandwidth=%lu, flags=%x", + disk, NULLSTR(base), NULLSTR(top), bandwidth, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + conn = dom->conn; + + if (dom->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + virCheckNonNullArgGoto(disk, error); + if (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW) + virCheckNullArgGoto(base, error); + + if (conn->driver->domainBlockCommit) { + int ret; + ret = conn->driver->domainBlockCommit(dom, disk, base, top, bandwidth, + flags); + if (ret < 0) + goto error; + return ret; + } + + virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return -1; +} + + +/** * virDomainOpenGraphics: * @dom: pointer to domain object * @idx: index of graphics config to open diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 28b92ad..d965c7f 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -562,6 +562,7 @@ LIBVIRT_0.10.2 { virConnectListAllNWFilters; virConnectListAllSecrets; virConnectListAllStoragePools; + virDomainBlockCommit; virNodeGetMemoryParameters; virNodeSetMemoryParameters; virStoragePoolListAllVolumes; -- 1.7.11.4

On 09/18/12 00:08, Eric Blake wrote:
This API matches a proposed qemu command 'block-commit': https://lists.gnu.org/archive/html/qemu-devel/2012-09/msg02226.html
Jeff's command is still in the works for qemu 1.3, and may gain further enhancements, such as the ability to control on-error handling (it will be comparable to the error handling Paolo is adding to 'drive-mirror', so a similar solution will be needed when I finally propose virDomainBlockCopy). However, even without qemu support, this API will be useful for _offline_ block commits, by wrapping qemu-img calls and turning them into a block job, so this API is worth committing now.
For some examples of how this will be implemented, all starting with the chain: base <- snap1 <- snap2 <- active
+ These are equivalent: virDomainBlockCommit(dom, disk, NULL, NULL, 0, 0) virDomainBlockCommit(dom, disk, NULL, "active", 0, 0) virDomainBlockCommit(dom, disk, "base", NULL, 0, 0) virDomainBlockCommit(dom, disk, "base", "active", 0, 0) but cannot be implemented for online qemu with round 1 of Jeff's patches; and for offline images, it would require three back-to-back qemu-img invocations unless qemu-img is patched to allow more efficient multi-layer commits; the end result would be 'base' as the active disk with contents from all three other files, where 'snap1' and 'snap2' are invalid right away, and 'active' is invalid once any further changes to 'base' are made, for a final chain of base.
+ These are equivalent: virDomainBlockCommit(dom, disk, "snap2", NULL, 0, 0) virDomainBlockCommit(dom, disk, NULL, NULL, 0, _SHALLOW) they cannot be implemented for online qemu, but for offline, it is a matter of 'qemu-img commit active', so that 'snap2' is now the active disk with contents formerly in 'active', for a final chain of base <- snap1 <- snap2.
+ Similarly: virDomainBlockCommit(dom, disk, "snap2", NULL, 0, _DELETE) for an offline domain will merge 'active' into 'snap2', then delete 'active' to avoid leaving a potentially invalid file around.
+ This version: virDomainBlockCommit(dom, disk, NULL, "snap2", 0, _SHALLOW) can be implemented online with 'block-commit' passing a base of snap1 and a top of snap2; and can be implemented offline by 'qemu-img commit snap2' followed by 'qemu-img rebase -u -b snap1 active', for a final chain of base <- snap1 <- active.
* include/libvirt/libvirt.h.in (virDomainBlockCommit): New API. * src/libvirt.c (virDomainBlockCommit): Implement it. * src/libvirt_public.syms (LIBVIRT_0.10.2): Export it. * src/driver.h (virDrvDomainBlockCommit): New driver callback. * docs/apibuild.py (CParser.parseSignature): Add exception. --- docs/apibuild.py | 1 + include/libvirt/libvirt.h.in | 20 +++++++++ src/driver.h | 5 +++ src/libvirt.c | 105 +++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 5 files changed, 132 insertions(+)
diff --git a/docs/apibuild.py b/docs/apibuild.py index 3f0ecd9..ae84be9 100755 --- a/docs/apibuild.py +++ b/docs/apibuild.py @@ -1759,6 +1759,7 @@ class CParser: "virDomainSetMaxMemory" : (False, ("memory")), "virDomainSetMemory" : (False, ("memory")), "virDomainSetMemoryFlags" : (False, ("memory")), + "virDomainBlockCommit" : (False, ("bandwidth")), "virDomainBlockJobSetSpeed" : (False, ("bandwidth")), "virDomainBlockPull" : (False, ("bandwidth")), "virDomainBlockRebase" : (False, ("bandwidth")), diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 8f72c03..2fd8ae1 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2062,11 +2062,14 @@ int virDomainUpdateDeviceFlags(virDomainPtr domain, * 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 + * VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT: Block Commit (virDomainBlockCommit), + * job ends on completion */ typedef enum { VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN = 0, VIR_DOMAIN_BLOCK_JOB_TYPE_PULL = 1, VIR_DOMAIN_BLOCK_JOB_TYPE_COPY = 2, + VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT = 3,
#ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_BLOCK_JOB_TYPE_LAST @@ -2131,6 +2134,23 @@ int virDomainBlockRebase(virDomainPtr dom, const char *disk, const char *base, unsigned long bandwidth, unsigned int flags);
+/** + * virDomainBlockCommitFlags: + * + * Flags available for virDomainBlockCommit(). + */ +typedef enum { + VIR_DOMAIN_BLOCK_COMMIT_SHALLOW = 1 << 0, /* NULL base means next backing + file, not whole chain */ + VIR_DOMAIN_BLOCK_COMMIT_DELETE = 1 << 1, /* Delete any files that are now + invalid after their contents + have been committed */ +} virDomainBlockCommitFlags; + +int virDomainBlockCommit(virDomainPtr dom, const char *disk, const char *base, + const char *top, unsigned long bandwidth, + unsigned int flags);
Hm, I'm always wondering how things would be if we'd use libvirt "objects" instead of strings for doing such things: We could use a virStorageVolPtrs for the arguments but we would need to have a way to use storage volumes not present in libvirt's pools. (We would need a way to wrap them somehow into our storage objects). But anyways, with the current state a the objects are still looked up by matching name or UUID so this solution is probably more versatile and requires less layers of code to traverse, so I'm fine with it.
+
/* Block I/O throttling support */
diff --git a/src/driver.h b/src/driver.h index bb470fe..063bbbf 100644 --- a/src/driver.h +++ b/src/driver.h @@ -832,6 +832,10 @@ typedef int (*virDrvDomainBlockRebase)(virDomainPtr dom, const char *path, const char *base, unsigned long bandwidth, unsigned int flags); +typedef int + (*virDrvDomainBlockCommit)(virDomainPtr dom, const char *disk, + const char *base, const char *top, + unsigned long bandwidth, unsigned int flags);
typedef int (*virDrvSetKeepAlive)(virConnectPtr conn, @@ -1071,6 +1075,7 @@ struct _virDriver { virDrvDomainBlockJobSetSpeed domainBlockJobSetSpeed; virDrvDomainBlockPull domainBlockPull; virDrvDomainBlockRebase domainBlockRebase; + virDrvDomainBlockCommit domainBlockCommit; virDrvSetKeepAlive setKeepAlive; virDrvConnectIsAlive isAlive; virDrvNodeSuspendForDuration nodeSuspendForDuration; diff --git a/src/libvirt.c b/src/libvirt.c index 9848993..41ecda1 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -19219,6 +19219,111 @@ error:
/** + * virDomainBlockCommit: + * @dom: pointer to domain object + * @disk: path to the block device, or device shorthand + * @base: path to backing file to merge into, or NULL for default + * @top: path to file within backing chain that contains data to be merged, + * or NULL to merge all possible data + * @bandwidth: (optional) specify commit bandwidth limit in MiB/s + * @flags: bitwise-OR of virDomainBlockCommitFlags + * + * Commit changes that were made to temporary top-level files within a disk + * image backing file chain into a lower-level base file. In other words, + * take all the difference between @base and @top, and update @base to contain + * that difference; after the commit, any portion of the chain that previously + * depended on @top will now depend on @base, and all files after @base up + * to and including @top will now be invalidated. A typical use of this + * command is to reduce the length of a backing file chain after taking an + * external disk snapshot. + * + * This command starts a long-running commit block job, whose status may + * be tracked by virDomainBlockJobInfo() with a job type of + * VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT, and the operation can be aborted with + * virDomainBlockJobAbort(). When finished, an asynchronous event is + * raised to indicate the final status, and the job no longer exists. If + * the job is aborted, it is up to the hypervisor whether starting a new + * job will resume from the same point, or start over. + * + * Be aware that this command may invalidate files even if it is aborted; + * the user is cautioned against relying on the contents of invalidated + * files such as @top without manually rebasing those files to use a + * backing file of a read-only copy of @base prior to the point where the + * commit operation was restarted (although such a rebase cannot be safely + * done until the commit has successfully completed). As a convenience, + * if @flags contains VIR_DOMAIN_BLOCK_COMMIT_DELETE, this command will + * unlink all files that were invalidated, after the commit successfully + * completes. + * + * By default, if @base is NULL, the bottom of the backing chain will be + * the commit target; if @flags contains VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, + * then the immediate backing file of @top will be used instead. If @top + * is NULL, the active image at the top of the chain will be used. Some + * hypervisors place restrictions on how much can be committed, and might + * fail if @base is not the immediate backing file of @top, or if @top is + * in use by a running domain, or if @top is not the top-most file. + * + * The @disk parameter is either an unambiguous source name of the + * block device (the <source file='...'/> sub-element, such as + * "/path/to/image"), or the device target shorthand (the + * <target dev='...'/> sub-element, such as "xvda"). Valid names + * can be found by calling virDomainGetXMLDesc() and inspecting + * elements within //domain/devices/disk. + * + * The maximum bandwidth (in MiB/s) that will be used to do the commit can be + * specified with the bandwidth parameter. If set to 0, libvirt will choose a + * suitable default. Some hypervisors do not support this feature and will + * return an error if bandwidth is not 0; in this case, it might still be + * possible for a later call to virDomainBlockJobSetSpeed() to succeed. + * The actual speed can be determined with virDomainGetBlockJobInfo(). + * + * Returns 0 if the operation has started, -1 on failure.
Wow, exhaustive docs!
+ */ +int virDomainBlockCommit(virDomainPtr dom, const char *disk, + const char *base, const char *top, + unsigned long bandwidth, unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "disk=%s, base=%s, top=%s, bandwidth=%lu, flags=%x", + disk, NULLSTR(base), NULLSTR(top), bandwidth, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + conn = dom->conn; + + if (dom->conn->flags & VIR_CONNECT_RO) { + virLibDomainError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + virCheckNonNullArgGoto(disk, error); + if (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW) + virCheckNullArgGoto(base, error); + + if (conn->driver->domainBlockCommit) { + int ret; + ret = conn->driver->domainBlockCommit(dom, disk, base, top, bandwidth, + flags); + if (ret < 0) + goto error; + return ret; + } + + virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return -1; +} + + +/** * virDomainOpenGraphics: * @dom: pointer to domain object * @idx: index of graphics config to open diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 28b92ad..d965c7f 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -562,6 +562,7 @@ LIBVIRT_0.10.2 { virConnectListAllNWFilters; virConnectListAllSecrets; virConnectListAllStoragePools; + virDomainBlockCommit; virNodeGetMemoryParameters; virNodeSetMemoryParameters; virStoragePoolListAllVolumes;
This patch as well as the API looks fine from my point of view and contains a ton of documentation, so ACK if nobody else objects. Peter

On 09/17/2012 05:08 PM, Peter Krempa wrote:
+ +int virDomainBlockCommit(virDomainPtr dom, const char *disk, const char *base, + const char *top, unsigned long bandwidth, + unsigned int flags);
Hm, I'm always wondering how things would be if we'd use libvirt "objects" instead of strings for doing such things:
We could use a virStorageVolPtrs for the arguments but we would need to have a way to use storage volumes not present in libvirt's pools. (We would need a way to wrap them somehow into our storage objects).
Yeah, having a way to get a list of virStorageVolPtrs for each domain, and then for each volume, get another list of virStorageVolPtrs representing the backing chain, would be cool. But it affects more than just this API (virDomainBlockRebase, virDomainBlockPull, and probably several other chain manipulation commands could benefit from an alternative version that operates on objects instead of strings).
+++ b/src/libvirt_public.syms @@ -562,6 +562,7 @@ LIBVIRT_0.10.2 { virConnectListAllNWFilters; virConnectListAllSecrets; virConnectListAllStoragePools; + virDomainBlockCommit; virNodeGetMemoryParameters; virNodeSetMemoryParameters; virStoragePoolListAllVolumes;
This patch as well as the API looks fine from my point of view and contains a ton of documentation, so ACK if nobody else objects.
I've gone ahead and pushed it, then, after adding a cross-reference between virDomainBlockPull and virDomainBlockCommit as being in the opposite direction. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

The wait loop logic borrows heavily from blockcommit. * tools/virsh-domain.c (cmdBlockCommit): New function. * tools/virsh.pod (blockcommit): Document it. --- tools/virsh-domain.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++- tools/virsh.pod | 30 ++++++++++ 2 files changed, 182 insertions(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index b6de9a8..aa23011 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1154,6 +1154,7 @@ typedef enum { VSH_CMD_BLOCK_JOB_SPEED = 2, VSH_CMD_BLOCK_JOB_PULL = 3, VSH_CMD_BLOCK_JOB_COPY = 4, + VSH_CMD_BLOCK_JOB_COMMIT = 5, } vshCmdBlockJobMode; static int @@ -1166,6 +1167,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, unsigned long bandwidth = 0; int ret = -1; const char *base = NULL; + const char *top = NULL; unsigned int flags = 0; if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) @@ -1180,7 +1182,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, } switch ((vshCmdBlockJobMode) mode) { - case VSH_CMD_BLOCK_JOB_ABORT: + case VSH_CMD_BLOCK_JOB_ABORT: if (vshCommandOptBool(cmd, "async")) flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC; if (vshCommandOptBool(cmd, "pivot")) @@ -1201,6 +1203,16 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, else ret = virDomainBlockPull(dom, path, bandwidth, 0); break; + case VSH_CMD_BLOCK_JOB_COMMIT: + if (vshCommandOptString(cmd, "base", &base) < 0 || + vshCommandOptString(cmd, "top", &top) < 0) + goto cleanup; + if (vshCommandOptBool(cmd, "shallow")) + flags |= VIR_DOMAIN_BLOCK_COMMIT_SHALLOW; + if (vshCommandOptBool(cmd, "delete")) + flags |= VIR_DOMAIN_BLOCK_COMMIT_DELETE; + ret = virDomainBlockCommit(dom, path, base, top, bandwidth, flags); + break; case VSH_CMD_BLOCK_JOB_COPY: flags |= VIR_DOMAIN_BLOCK_REBASE_COPY; if (vshCommandOptBool(cmd, "shallow")) @@ -1260,6 +1272,144 @@ static void vshCatchInt(int sig ATTRIBUTE_UNUSED, } /* + * "blockcommit" command + */ +static const vshCmdInfo info_block_commit[] = { + {"help", N_("Start a block commit operation.")}, + {"desc", N_("Commit changes from a snapshot down to its backing image.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_block_commit[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"path", VSH_OT_DATA, VSH_OFLAG_REQ, N_("fully-qualified path of disk")}, + {"bandwidth", VSH_OT_DATA, VSH_OFLAG_NONE, N_("bandwidth limit in MiB/s")}, + {"base", VSH_OT_DATA, VSH_OFLAG_NONE, + N_("path of base file to commit into (default bottom of chain)")}, + {"shallow", VSH_OT_BOOL, 0, N_("use backing file of top as base")}, + {"top", VSH_OT_DATA, VSH_OFLAG_NONE, + N_("path of top file to commit from (default top of chain)")}, + {"delete", VSH_OT_BOOL, 0, + N_("delete files that were successfully committed")}, + {"wait", VSH_OT_BOOL, 0, N_("wait for job to complete")}, + {"verbose", VSH_OT_BOOL, 0, N_("with --wait, display the progress")}, + {"timeout", VSH_OT_INT, VSH_OFLAG_NONE, + N_("with --wait, abort if copy exceeds timeout (in seconds)")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + bool ret = false; + bool blocking = vshCommandOptBool(cmd, "wait"); + bool verbose = vshCommandOptBool(cmd, "verbose"); + int timeout = 0; + struct sigaction sig_action; + struct sigaction old_sig_action; + sigset_t sigmask; + struct timeval start; + struct timeval curr; + const char *path = NULL; + bool quit = false; + int abort_flags = 0; + + if (blocking) { + if (vshCommandOptInt(cmd, "timeout", &timeout) > 0) { + if (timeout < 1) { + vshError(ctl, "%s", _("invalid timeout")); + return false; + } + + /* Ensure that we can multiply by 1000 without overflowing. */ + if (timeout > INT_MAX / 1000) { + vshError(ctl, "%s", _("timeout is too big")); + return false; + } + } + if (vshCommandOptString(cmd, "path", &path) < 0) + return false; + if (vshCommandOptBool(cmd, "async")) + abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC; + + sigemptyset(&sigmask); + sigaddset(&sigmask, SIGINT); + + intCaught = 0; + sig_action.sa_sigaction = vshCatchInt; + sig_action.sa_flags = SA_SIGINFO; + sigemptyset(&sig_action.sa_mask); + sigaction(SIGINT, &sig_action, &old_sig_action); + + GETTIMEOFDAY(&start); + } else if (verbose || vshCommandOptBool(cmd, "timeout") || + vshCommandOptBool(cmd, "async")) { + vshError(ctl, "%s", _("blocking control options require --wait")); + return false; + } + + if (blockJobImpl(ctl, cmd, NULL, VSH_CMD_BLOCK_JOB_COMMIT, &dom) < 0) + goto cleanup; + + if (!blocking) { + vshPrint(ctl, "%s", _("Block Commit started")); + ret = true; + goto cleanup; + } + + while (blocking) { + virDomainBlockJobInfo info; + int result = virDomainGetBlockJobInfo(dom, path, &info, 0); + + if (result < 0) { + vshError(ctl, _("failed to query job for disk %s"), path); + goto cleanup; + } + if (result == 0) + break; + + if (verbose) + print_job_progress(_("Block Commit"), + info.end - info.cur, info.end); + + GETTIMEOFDAY(&curr); + if (intCaught || (timeout && + (((int)(curr.tv_sec - start.tv_sec) * 1000 + + (int)(curr.tv_usec - start.tv_usec) / 1000) > + timeout * 1000))) { + vshDebug(ctl, VSH_ERR_DEBUG, + intCaught ? "interrupted" : "timeout"); + intCaught = 0; + timeout = 0; + quit = true; + if (virDomainBlockJobAbort(dom, path, abort_flags) < 0) { + vshError(ctl, _("failed to abort job for disk %s"), path); + goto cleanup; + } + if (abort_flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC) + break; + } else { + usleep(500 * 1000); + } + } + + if (verbose && !quit) { + /* printf [100 %] */ + print_job_progress(_("Block Commit"), 0, 1); + } + vshPrint(ctl, "\n%s", quit ? _("Commit aborted") : _("Commit complete")); + + ret = true; +cleanup: + if (dom) + virDomainFree(dom); + if (blocking) + sigaction(SIGINT, &old_sig_action, NULL); + return ret; +} + +/* * "blockcopy" command */ static const vshCmdInfo info_block_copy[] = { @@ -8112,6 +8262,7 @@ const vshCmdDef domManagementCmds[] = { {"autostart", cmdAutostart, opts_autostart, info_autostart, 0}, {"blkdeviotune", cmdBlkdeviotune, opts_blkdeviotune, info_blkdeviotune, 0}, {"blkiotune", cmdBlkiotune, opts_blkiotune, info_blkiotune, 0}, + {"blockcommit", cmdBlockCommit, opts_block_commit, info_block_commit, 0}, {"blockcopy", cmdBlockCopy, opts_block_copy, info_block_copy, 0}, {"blockjob", cmdBlockJob, opts_block_job, info_block_job, 0}, {"blockpull", cmdBlockPull, opts_block_pull, info_block_pull, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index bb135da..4a79e12 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -694,6 +694,36 @@ currently in use by a running domain. Other contexts that require a MAC address of virtual interface (such as I<detach-interface> or I<domif-setlink>) will accept the MAC address printed by this command. +=item B<blockcommit> I<domain> I<path> [I<bandwidth>] +{[I<base>] | [I<--shallow>]} [I<top>] [I<--delete>] +[I<--wait> [I<--verbose>] [I<--timeout> B<seconds>]] + +Reduce the length of a backing image chain, by committing changes at the +top of the chain (snapshot or delta files) into backing images. By +default, this command attempts to flatten the entire chain. If I<base> +and/or I<top> are specified as files within the backing chain, then the +operation is constrained to committing just that portion of the chain; +I<--shallow> can be used instead of I<base> to specify the immediate +backing file of the resulting top image to be committed. The files +being committed are rendered invalid, possibly as soon as the operation +starts; using the I<--delete> flag will remove these files at the successful +completion of the commit operation. + +By default, this command returns as soon as possible, and data for +the entire disk is committed in the background; the progress of the +operation can be checked with B<blockjob>. However, if I<--wait> is +specified, then this command will block until the operation completes, +or cancel the operation if the optional I<timeout> in seconds elapses +or SIGINT is sent (usually with C<Ctrl-C>). Using I<--verbose> along +with I<--wait> will produce periodic status updates. + +I<path> specifies fully-qualified path of the disk; it corresponds +to a unique target name (<target dev='name'/>) or source file (<source +file='name'/>) for one of the disk devices attached to I<domain> (see +also B<domblklist> for listing these names). +I<bandwidth> specifies copying bandwidth limit in MiB/s, although for +qemu, it may be non-zero only for an online domain. + =item B<blockcopy> I<domain> I<path> I<dest> [I<bandwidth>] [I<--shallow>] [I<--reuse-external>] [I<--raw>] [I<--wait> [I<--verbose] [{I<--pivot> | I<--finish>}] [I<--timeout> B<seconds>] [I<--async>]] -- 1.7.11.4

On 09/18/12 00:08, Eric Blake wrote:
The wait loop logic borrows heavily from blockcommit.
Maybe a little too sparse on the commit message, but the man page addition makes it up.
* tools/virsh-domain.c (cmdBlockCommit): New function. * tools/virsh.pod (blockcommit): Document it. --- tools/virsh-domain.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++- tools/virsh.pod | 30 ++++++++++ 2 files changed, 182 insertions(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index b6de9a8..aa23011 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1260,6 +1272,144 @@ static void vshCatchInt(int sig ATTRIBUTE_UNUSED, }
/* + * "blockcommit" command + */ +static const vshCmdInfo info_block_commit[] = { + {"help", N_("Start a block commit operation.")}, + {"desc", N_("Commit changes from a snapshot down to its backing image.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_block_commit[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"path", VSH_OT_DATA, VSH_OFLAG_REQ, N_("fully-qualified path of disk")},
The API says you can use "vda" and similar, is that considered fully-qualified? / this question was answered below /
+ {"bandwidth", VSH_OT_DATA, VSH_OFLAG_NONE, N_("bandwidth limit in MiB/s")},
Megabits look like a reasonable granularity to limit this. Well maybe apart from testing purposes where somebody would like to do it really slow to be able to poke around.
+ {"base", VSH_OT_DATA, VSH_OFLAG_NONE, + N_("path of base file to commit into (default bottom of chain)")}, + {"shallow", VSH_OT_BOOL, 0, N_("use backing file of top as base")}, + {"top", VSH_OT_DATA, VSH_OFLAG_NONE, + N_("path of top file to commit from (default top of chain)")}, + {"delete", VSH_OT_BOOL, 0, + N_("delete files that were successfully committed")}, + {"wait", VSH_OT_BOOL, 0, N_("wait for job to complete")}, + {"verbose", VSH_OT_BOOL, 0, N_("with --wait, display the progress")}, + {"timeout", VSH_OT_INT, VSH_OFLAG_NONE, + N_("with --wait, abort if copy exceeds timeout (in seconds)")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdBlockCommit(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + bool ret = false; + bool blocking = vshCommandOptBool(cmd, "wait"); + bool verbose = vshCommandOptBool(cmd, "verbose"); + int timeout = 0; + struct sigaction sig_action; + struct sigaction old_sig_action; + sigset_t sigmask; + struct timeval start; + struct timeval curr; + const char *path = NULL; + bool quit = false; + int abort_flags = 0; + + if (blocking) { + if (vshCommandOptInt(cmd, "timeout", &timeout) > 0) { + if (timeout < 1) { + vshError(ctl, "%s", _("invalid timeout")); + return false; + } + + /* Ensure that we can multiply by 1000 without overflowing. */ + if (timeout > INT_MAX / 1000) { + vshError(ctl, "%s", _("timeout is too big")); + return false; + } + } + if (vshCommandOptString(cmd, "path", &path) < 0) + return false; + if (vshCommandOptBool(cmd, "async")) + abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC; + + sigemptyset(&sigmask); + sigaddset(&sigmask, SIGINT); + + intCaught = 0; + sig_action.sa_sigaction = vshCatchInt; + sig_action.sa_flags = SA_SIGINFO; + sigemptyset(&sig_action.sa_mask); + sigaction(SIGINT, &sig_action, &old_sig_action); + + GETTIMEOFDAY(&start); + } else if (verbose || vshCommandOptBool(cmd, "timeout") || + vshCommandOptBool(cmd, "async")) { + vshError(ctl, "%s", _("blocking control options require --wait"));
This error message isn't 100% clear on the first read. What about: "--verbose and --timeout require --wait" ?
+ return false; + } + + if (blockJobImpl(ctl, cmd, NULL, VSH_CMD_BLOCK_JOB_COMMIT, &dom) < 0) + goto cleanup; + + if (!blocking) { + vshPrint(ctl, "%s", _("Block Commit started")); + ret = true; + goto cleanup; + } + + while (blocking) { + virDomainBlockJobInfo info; + int result = virDomainGetBlockJobInfo(dom, path, &info, 0); + + if (result < 0) { + vshError(ctl, _("failed to query job for disk %s"), path); + goto cleanup; + } + if (result == 0) + break; + + if (verbose) + print_job_progress(_("Block Commit"), + info.end - info.cur, info.end); + + GETTIMEOFDAY(&curr); + if (intCaught || (timeout && + (((int)(curr.tv_sec - start.tv_sec) * 1000 + + (int)(curr.tv_usec - start.tv_usec) / 1000) > + timeout * 1000))) {
Wouldn't it be better to multiply timeout by 1000 at the beginning and not bother re-doing it on every iteration?
+ vshDebug(ctl, VSH_ERR_DEBUG, + intCaught ? "interrupted" : "timeout"); + intCaught = 0; + timeout = 0; + quit = true; + if (virDomainBlockJobAbort(dom, path, abort_flags) < 0) { + vshError(ctl, _("failed to abort job for disk %s"), path); + goto cleanup; + } + if (abort_flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC) + break; + } else { + usleep(500 * 1000); + } + } + + if (verbose && !quit) { + /* printf [100 %] */ + print_job_progress(_("Block Commit"), 0, 1); + } + vshPrint(ctl, "\n%s", quit ? _("Commit aborted") : _("Commit complete"));
Meh a ternary ... but saves lines.
+ + ret = true; +cleanup: + if (dom) + virDomainFree(dom); + if (blocking) + sigaction(SIGINT, &old_sig_action, NULL); + return ret; +} + +/* * "blockcopy" command */ static const vshCmdInfo info_block_copy[] = { @@ -8112,6 +8262,7 @@ const vshCmdDef domManagementCmds[] = { {"autostart", cmdAutostart, opts_autostart, info_autostart, 0}, {"blkdeviotune", cmdBlkdeviotune, opts_blkdeviotune, info_blkdeviotune, 0}, {"blkiotune", cmdBlkiotune, opts_blkiotune, info_blkiotune, 0}, + {"blockcommit", cmdBlockCommit, opts_block_commit, info_block_commit, 0}, {"blockcopy", cmdBlockCopy, opts_block_copy, info_block_copy, 0}, {"blockjob", cmdBlockJob, opts_block_job, info_block_job, 0}, {"blockpull", cmdBlockPull, opts_block_pull, info_block_pull, 0}, diff --git a/tools/virsh.pod b/tools/virsh.pod index bb135da..4a79e12 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -694,6 +694,36 @@ currently in use by a running domain. Other contexts that require a MAC address of virtual interface (such as I<detach-interface> or I<domif-setlink>) will accept the MAC address printed by this command.
+=item B<blockcommit> I<domain> I<path> [I<bandwidth>] +{[I<base>] | [I<--shallow>]} [I<top>] [I<--delete>] +[I<--wait> [I<--verbose>] [I<--timeout> B<seconds>]] + +Reduce the length of a backing image chain, by committing changes at the +top of the chain (snapshot or delta files) into backing images. By +default, this command attempts to flatten the entire chain. If I<base> +and/or I<top> are specified as files within the backing chain, then the +operation is constrained to committing just that portion of the chain; +I<--shallow> can be used instead of I<base> to specify the immediate +backing file of the resulting top image to be committed. The files +being committed are rendered invalid, possibly as soon as the operation +starts; using the I<--delete> flag will remove these files at the successful +completion of the commit operation. + +By default, this command returns as soon as possible, and data for +the entire disk is committed in the background; the progress of the +operation can be checked with B<blockjob>. However, if I<--wait> is +specified, then this command will block until the operation completes, +or cancel the operation if the optional I<timeout> in seconds elapses +or SIGINT is sent (usually with C<Ctrl-C>). Using I<--verbose> along +with I<--wait> will produce periodic status updates. + +I<path> specifies fully-qualified path of the disk; it corresponds +to a unique target name (<target dev='name'/>) or source file (<source +file='name'/>) for one of the disk devices attached to I<domain> (see
Okay, this answers my question from the beginning.
+also B<domblklist> for listing these names). +I<bandwidth> specifies copying bandwidth limit in MiB/s, although for +qemu, it may be non-zero only for an online domain. + =item B<blockcopy> I<domain> I<path> I<dest> [I<bandwidth>] [I<--shallow>] [I<--reuse-external>] [I<--raw>] [I<--wait> [I<--verbose] [{I<--pivot> | I<--finish>}] [I<--timeout> B<seconds>] [I<--async>]]
Your docs are always great. ACK Peter

On 09/17/2012 05:29 PM, Peter Krempa wrote:
On 09/18/12 00:08, Eric Blake wrote:
The wait loop logic borrows heavily from blockcommit.
I _meant_ to say 'blockpull'. And because of my cryptic reference, instead of a proper attribution,
Maybe a little too sparse on the commit message, but the man page addition makes it up.
...I understand why you had a harder time reviewing it.
+ {"bandwidth", VSH_OT_DATA, VSH_OFLAG_NONE, N_("bandwidth limit in MiB/s")},
Megabits look like a reasonable granularity to limit this. Well maybe apart from testing purposes where somebody would like to do it really slow to be able to poke around.
That, and the existing 'virsh blockjob --bandwidth' command sets MiB/s, so we really don't have a choice in our units since this is just another case of a block job.
+ } else if (verbose || vshCommandOptBool(cmd, "timeout") || + vshCommandOptBool(cmd, "async")) { + vshError(ctl, "%s", _("blocking control options require --wait"));
This error message isn't 100% clear on the first read. What about: "--verbose and --timeout require --wait" ?
Sure. Again, this was copied from 'virsh blockpull', so I'll make the same change there.
+ GETTIMEOFDAY(&curr); + if (intCaught || (timeout && + (((int)(curr.tv_sec - start.tv_sec) * 1000 + + (int)(curr.tv_usec - start.tv_usec) / 1000) > + timeout * 1000))) {
Wouldn't it be better to multiply timeout by 1000 at the beginning and not bother re-doing it on every iteration?
Copy and paste strikes again :)
+ vshPrint(ctl, "\n%s", quit ? _("Commit aborted") : _("Commit complete"));
Meh a ternary ... but saves lines.
and again.
Your docs are always great. ACK
I'll fix the nits, and push this shortly, then. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Relatively straightforward. Our decision to make block job speed a long keeps haunting us on new API. * src/remote/remote_protocol.x (remote_domain_block_commit_args): New struct. * src/remote/remote_driver.c (remote_driver): Enable it. * src/remote_protocol-structs: Regenerate. * src/rpc/gendispatch.pl (long_legacy): Exempt another bandwidth. --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 +++++++++++- src/remote_protocol-structs | 9 +++++++++ src/rpc/gendispatch.pl | 1 + 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 8f3895d..b6edf38 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6045,6 +6045,7 @@ static virDriver remote_driver = { .domainBlockJobSetSpeed = remoteDomainBlockJobSetSpeed, /* 0.9.4 */ .domainBlockPull = remoteDomainBlockPull, /* 0.9.4 */ .domainBlockRebase = remoteDomainBlockRebase, /* 0.9.10 */ + .domainBlockCommit = remoteDomainBlockCommit, /* 0.10.2 */ .setKeepAlive = remoteSetKeepAlive, /* 0.9.8 */ .isAlive = remoteIsAlive, /* 0.9.8 */ .nodeSuspendForDuration = remoteNodeSuspendForDuration, /* 0.9.8 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 6201ff7..9481f15 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1224,6 +1224,14 @@ struct remote_domain_block_rebase_args { unsigned hyper bandwidth; unsigned int flags; }; +struct remote_domain_block_commit_args { + remote_nonnull_domain dom; + remote_nonnull_string disk; + remote_string base; + remote_string top; + unsigned hyper bandwidth; + unsigned int flags; +}; struct remote_domain_set_block_io_tune_args { remote_nonnull_domain dom; @@ -2988,7 +2996,9 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_NWFILTERS = 286, /* skipgen skipgen priority:high */ REMOTE_PROC_CONNECT_LIST_ALL_SECRETS = 287, /* skipgen skipgen priority:high */ REMOTE_PROC_NODE_SET_MEMORY_PARAMETERS = 288, /* autogen autogen */ - REMOTE_PROC_NODE_GET_MEMORY_PARAMETERS = 289 /* skipgen skipgen */ + REMOTE_PROC_NODE_GET_MEMORY_PARAMETERS = 289, /* skipgen skipgen */ + REMOTE_PROC_DOMAIN_BLOCK_COMMIT = 290 /* autogen autogen */ + /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 52ccf80..8b0ae1f 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -875,6 +875,14 @@ struct remote_domain_block_rebase_args { uint64_t bandwidth; u_int flags; }; +struct remote_domain_block_commit_args { + remote_nonnull_domain dom; + remote_nonnull_string disk; + remote_string base; + remote_string top; + uint64_t bandwidth; + u_int flags; +}; struct remote_domain_set_block_io_tune_args { remote_nonnull_domain dom; remote_nonnull_string disk; @@ -2397,4 +2405,5 @@ enum remote_procedure { REMOTE_PROC_CONNECT_LIST_ALL_SECRETS = 287, REMOTE_PROC_NODE_SET_MEMORY_PARAMETERS = 288, REMOTE_PROC_NODE_GET_MEMORY_PARAMETERS = 289, + REMOTE_PROC_DOMAIN_BLOCK_COMMIT = 290, }; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 774977d..ae7ecba 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -276,6 +276,7 @@ my $long_legacy = { GetLibVersion => { ret => { lib_ver => 1 } }, GetVersion => { ret => { hv_ver => 1 } }, NodeGetInfo => { ret => { memory => 1 } }, + DomainBlockCommit => { arg => { bandwidth => 1 } }, DomainBlockPull => { arg => { bandwidth => 1 } }, DomainBlockRebase => { arg => { bandwidth => 1 } }, DomainBlockJobSetSpeed => { arg => { bandwidth => 1 } }, -- 1.7.11.4

On 09/18/12 00:08, Eric Blake wrote:
Relatively straightforward. Our decision to make block job speed a long keeps haunting us on new API.
* src/remote/remote_protocol.x (remote_domain_block_commit_args): New struct. * src/remote/remote_driver.c (remote_driver): Enable it. * src/remote_protocol-structs: Regenerate. * src/rpc/gendispatch.pl (long_legacy): Exempt another bandwidth. --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 +++++++++++- src/remote_protocol-structs | 9 +++++++++ src/rpc/gendispatch.pl | 1 + 4 files changed, 22 insertions(+), 1 deletion(-)
ACK. Automatically generated API rpc rocks! Peter

On 09/17/2012 05:33 PM, Peter Krempa wrote:
On 09/18/12 00:08, Eric Blake wrote:
Relatively straightforward. Our decision to make block job speed a long keeps haunting us on new API.
* src/remote/remote_protocol.x (remote_domain_block_commit_args): New struct. * src/remote/remote_driver.c (remote_driver): Enable it. * src/remote_protocol-structs: Regenerate. * src/rpc/gendispatch.pl (long_legacy): Exempt another bandwidth. --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 12 +++++++++++- src/remote_protocol-structs | 9 +++++++++ src/rpc/gendispatch.pl | 1 + 4 files changed, 22 insertions(+), 1 deletion(-)
ACK. Automatically generated API rpc rocks!
Yes it does. Pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Peter Krempa