[libvirt] RFC: Add virDomainBlockPull API family to libvirt

To help speed the provisioning process for large domains, new QED disks are created with backing to a template image. These disks are configured with copy on read such that blocks that are read from the backing file are copied to the new disk. This reduces I/O over a potentially costly path to the backing image. In such a configuration, there is a desire to remove the dependency on the backing image as the domain runs. To accomplish this, qemu will provide an interface to perform sequential copy on read operations during normal VM operation. Once all data has been copied, the disk image's link to the backing file is removed. The virDomainBlockPull API family brings this functionality to libvirt. virDomainBlockPullAll() instructs the hypervisor to stream the entire device in the background. Progress of this operation can be checked with the function virDomainBlockPullInfo(). An ongoing stream can be cancelled with virDomainBlockPullAbort(). If a more controlled IO rate is desired, virDomainBlockPull() can be used to perform a single increment of IO. Subsequent calls to this function will automatically stream the appropriate next increment until the disk has been fully populated. An event (VIR_DOMAIN_EVENT_ID_BLOCK_PULL) will be emitted when a disk has been fully populated or if a BlockPullAll() operation was terminated due to an error. This event is useful to avoid polling on virDomainBlockPullInfo() for completion and could also be used by the security driver to revoke access to the backing file when it is no longer needed. Note: I am sending this series out now (even though image streaming is not quite committed to qemu upstream) because I want to start the review process. At this stage, I expect only minor changes to the qemu implementation. make check: PASS make syntax-check: PASS make -C tests valgrind: PASS I am testing this API with Python Unittest (see the last patch). [PATCH 0/7] virDomainBlockPull: Add public symbols to libvirt API [PATCH 2/7] Add virDomainBlockPull support to the remote driver [PATCH 3/7] Implement virDomainBlockPull for the qemu driver [PATCH 4/7] Enable the virDomainBlockPull API in virsh [PATCH 5/7] Enable virDomainBlockPull in the python API. [PATCH 6/7] Asynchronous event for BlockPull completion [PATCH 7/7] test: Python Unittests for DomainBlockPull API

* src/libvirt.c: implement the main entry points * src/libvirt_public.syms: add them to the exported symbols Signed-off-by: Adam Litke <agl@us.ibm.com> --- src/libvirt.c | 252 +++++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 7 ++ 2 files changed, 259 insertions(+), 0 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index abacf85..8f344de 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -13467,3 +13467,255 @@ error: virDispatchError(conn); return -1; } + +/** + * virDomainBlockPull: + * @dom: pointer to domain object + * @path: Fully-qualified filename of disk + * @info: A pointer to a virDomainBlockPullInfo structure, or NULL + * @flags: currently unused, for future extension + * + * Populate a disk image with data from its backing image. Once all data from + * its backing image has been pulled, the disk no longer depends on a backing + * image. This function works incrementally, performing a small amount of work + * each time it is called. When successful, @info is updated with the current + * progress. + * + * Returns -1 in case of failure, 0 when successful. + */ +int virDomainBlockPull(virDomainPtr dom, + const char *path, + virDomainBlockPullInfoPtr info, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "path=%p, info=%p, flags=%u", path, info, 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; + } + + if (!path) { + virLibDomainError(VIR_ERR_INVALID_ARG, + _("path is NULL")); + goto error; + } + + if (flags != 0) { + virLibDomainError(VIR_ERR_INVALID_ARG, + _("flags must be zero")); + goto error; + } + + if (conn->driver->domainBlockPull) { + int ret; + ret = conn->driver->domainBlockPull(dom, path, info, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return -1; +} + +/** + * virDomainBlockPullAll: + * @dom: pointer to domain object + * @path: Fully-qualified filename of disk + * @flags: currently unused, for future extension + * + * Populate a disk image with data from its backing image. Once all data from + * its backing image has been pulled, the disk no longer depends on a backing + * image. This function pulls data for the entire device in the background. + * Progress of the operation can be checked with virDomainGetBlockPullInfo() and + * the operation can be aborted with virDomainBlockPullAbort(). When finished, + * an asynchronous event is raised to indicate the final status. + * + * Returns 0 if the operation has started, -1 on failure. + */ +int virDomainBlockPullAll(virDomainPtr dom, + const char *path, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "path=%p, flags=%u", path, 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; + } + + if (!path) { + virLibDomainError(VIR_ERR_INVALID_ARG, + _("path is NULL")); + goto error; + } + + if (flags != 0) { + virLibDomainError(VIR_ERR_INVALID_ARG, + _("flags must be zero")); + goto error; + } + + if (conn->driver->domainBlockPullAll) { + int ret; + ret = conn->driver->domainBlockPullAll(dom, path, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return -1; +} + +/** + * virDomainBlockPullAbort: + * @dom: pointer to domain object + * @path: fully-qualified filename of disk + * @flags: currently unused, for future extension + * + * Cancel a pull operation previously started by virDomainBlockPullAll(). + * + * Returns -1 in case of failure, 0 when successful. + */ +int virDomainBlockPullAbort(virDomainPtr dom, + const char *path, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "path=%p, flags=%u", path, 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; + } + + if (!path) { + virLibDomainError(VIR_ERR_INVALID_ARG, + _("path is NULL")); + goto error; + } + + if (flags != 0) { + virLibDomainError(VIR_ERR_INVALID_ARG, + _("flags must be zero")); + goto error; + } + + if (conn->driver->domainBlockPullAbort) { + int ret; + ret = conn->driver->domainBlockPullAbort(dom, path, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return -1; +} + +/** + * virDomainGetBlockPullInfo: + * @dom: pointer to domain object + * @path: fully-qualified filename of disk + * @info: pointer to a virDomainBlockPullInfo structure + * @flags: currently unused, for future extension + * + * Request progress information on a block pull operation that has been started + * with virDomainBlockPullAll(). If an operation is active for the given + * parameters, @info will be updated with the current progress. + * + * Returns -1 in case of failure, 0 when successful. + */ +int virDomainGetBlockPullInfo(virDomainPtr dom, + const char *path, + virDomainBlockPullInfoPtr info, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "path=%p, info=%p, flags=%u", path, info, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN (dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + conn = dom->conn; + + if (!path) { + virLibDomainError(VIR_ERR_INVALID_ARG, + _("path is NULL")); + goto error; + } + + if (!info) { + virLibDomainError(VIR_ERR_INVALID_ARG, + _("info is NULL")); + goto error; + } + + if (flags != 0) { + virLibDomainError(VIR_ERR_INVALID_ARG, + _("flags must be zero")); + goto error; + } + + if (conn->driver->domainGetBlockPullInfo) { + int ret; + ret = conn->driver->domainGetBlockPullInfo(dom, path, info, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibDomainError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(dom->conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index b4aed41..78d8a21 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -437,3 +437,10 @@ LIBVIRT_0.9.0 { } LIBVIRT_0.8.8; # .... define new API here using predicted next version number .... +LIBVIRT_0.9.1 { + global: + virDomainBlockPull; + virDomainBlockPullAll; + virDomainBlockPullAbort; + virDomainGetBlockPullInfo; +} LIBVIRT_0.9.0; -- 1.7.3

* src/remote/remote_protocol.x: provide defines for the new entry points * src/remote/remote_driver.c daemon/remote.c: implement the client and server side * daemon/remote_generator.pl: Specify the manually-written functions Signed-off-by: Adam Litke <agl@us.ibm.com> --- daemon/remote.c | 71 +++++++++++++++++++++++++++++++++++++++++ daemon/remote_generator.pl | 8 +++- src/remote/remote_driver.c | 72 +++++++++++++++++++++++++++++++++++++++-- src/remote/remote_protocol.x | 44 +++++++++++++++++++++++++- 4 files changed, 188 insertions(+), 7 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 2220655..f6aa78e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1692,6 +1692,77 @@ no_memory: goto cleanup; } +static int +remoteDispatchDomainBlockPull(struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error * rerr, + remote_domain_block_pull_args *args, + remote_domain_block_pull_ret *ret) +{ + virDomainPtr dom = NULL; + virDomainBlockPullInfo tmp; + int rv = -1; + + if (!conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + + if (virDomainBlockPull(dom, args->path, &tmp, args->flags) < 0) + goto cleanup; + rv = 0; + ret->info.cur = tmp.cur; + ret->info.end = tmp.end; + +cleanup: + if (rv < 0) + remoteDispatchError(rerr); + if (dom) + virDomainFree(dom); + return rv; +} + +static int +remoteDispatchDomainGetBlockPullInfo(struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error * rerr, + remote_domain_get_block_pull_info_args *args, + remote_domain_get_block_pull_info_ret *ret) +{ + virDomainPtr dom = NULL; + virDomainBlockPullInfo tmp; + int rv = -1; + + if (!conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + + if (virDomainGetBlockPullInfo(dom, args->path, &tmp, args->flags) < 0) + goto cleanup; + rv = 0; + ret->info.cur = tmp.cur; + ret->info.end = tmp.end; + +cleanup: + if (rv < 0) + remoteDispatchError(rerr); + if (dom) + virDomainFree(dom); + return rv; +} + + /*-------------------------------------------------------------*/ static int diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl index 062ccc1..d7e0383 100755 --- a/daemon/remote_generator.pl +++ b/daemon/remote_generator.pl @@ -278,7 +278,9 @@ elsif ($opt_b) { "GetType", "NodeDeviceGetParent", "NodeGetSecurityModel", - "SecretGetValue"); + "SecretGetValue", + "DomainBlockPull", + "DomainGetBlockPullInfo"); } elsif ($structprefix eq "qemu") { @ungeneratable = ("MonitorCommand"); } @@ -779,7 +781,9 @@ elsif ($opt_k) { "GetType", "NodeDeviceGetParent", "NodeGetSecurityModel", - "SecretGetValue"); + "SecretGetValue", + "DomainBlockPull", + "DomainGetBlockPullInfo"); } elsif ($structprefix eq "qemu") { @ungeneratable = ("MonitorCommand"); } diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 07bc629..0a885a9 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2844,6 +2844,70 @@ done: return rv; } +static int remoteDomainBlockPull(virDomainPtr domain, + const char *path, + virDomainBlockPullInfoPtr info, + unsigned int flags) +{ + int rv = -1; + remote_domain_block_pull_args args; + remote_domain_block_pull_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.path = (char *)path; + args.flags = flags; + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_BLOCK_PULL, + (xdrproc_t)xdr_remote_domain_block_pull_args, (char *)&args, + (xdrproc_t)xdr_remote_domain_block_pull_ret, (char *)&ret) == -1) + goto done; + + if (info) { + info->cur = ret.info.cur; + info->end = ret.info.end; + } + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainGetBlockPullInfo(virDomainPtr domain, + const char *path, + virDomainBlockPullInfoPtr info, + unsigned int flags) +{ + int rv = -1; + remote_domain_get_block_pull_info_args args; + remote_domain_get_block_pull_info_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.path = (char *)path; + args.flags = flags; + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_BLOCK_PULL_INFO, + (xdrproc_t)xdr_remote_domain_get_block_pull_info_args, + (char *)&args, + (xdrproc_t)xdr_remote_domain_get_block_pull_info_ret, + (char *)&ret) == -1) + goto done; + + info->cur = ret.info.cur; + info->end = ret.info.end; + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +} + /*----------------------------------------------------------------------*/ static virDrvOpenStatus ATTRIBUTE_NONNULL (1) @@ -6493,10 +6557,10 @@ static virDriver remote_driver = { remoteDomainSnapshotDelete, /* domainSnapshotDelete */ remoteQemuDomainMonitorCommand, /* qemuDomainMonitorCommand */ remoteDomainOpenConsole, /* domainOpenConsole */ - NULL, /* domainBlockPull */ - NULL, /* domainBlockPullAll */ - NULL, /* domainBlockPullAbort */ - NULL, /* domainGetBlockPullInfo */ + remoteDomainBlockPull, /* domainBlockPull */ + remoteDomainBlockPullAll, /* domainBlockPullAll */ + remoteDomainBlockPullAbort, /* domainBlockPullAbort */ + remoteDomainGetBlockPullInfo, /* domainGetBlockPullInfo */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index c706c36..2f52ceb 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -917,6 +917,43 @@ struct remote_domain_set_autostart_args { int autostart; }; +struct remote_domain_block_pull_info { + unsigned hyper cur; + unsigned hyper end; +}; + +struct remote_domain_block_pull_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + unsigned int flags; +}; + +struct remote_domain_block_pull_ret { + remote_domain_block_pull_info info; +}; + +struct remote_domain_block_pull_all_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + unsigned int flags; +}; + +struct remote_domain_block_pull_abort_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + unsigned int flags; +}; + +struct remote_domain_get_block_pull_info_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + unsigned int flags; +}; + +struct remote_domain_get_block_pull_info_ret { + remote_domain_block_pull_info info; +}; + /* Network calls: */ struct remote_num_of_networks_ret { @@ -2176,7 +2213,12 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_BLKIO_PARAMETERS = 206, REMOTE_PROC_DOMAIN_MIGRATE_SET_MAX_SPEED = 207, REMOTE_PROC_STORAGE_VOL_UPLOAD = 208, - REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209 + REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209, + REMOTE_PROC_DOMAIN_BLOCK_PULL = 210, + + REMOTE_PROC_DOMAIN_BLOCK_PULL_ALL = 211, + REMOTE_PROC_DOMAIN_BLOCK_PULL_ABORT = 212, + REMOTE_PROC_DOMAIN_GET_BLOCK_PULL_INFO = 213 /* * Notice how the entries are grouped in sets of 10 ? -- 1.7.3

2011/6/1 Adam Litke <agl@us.ibm.com>: This commit has a pretty long summary line.
* src/remote/remote_protocol.x: provide defines for the new entry points * src/remote/remote_driver.c daemon/remote.c: implement the client and server side * daemon/remote_generator.pl: Specify the manually-written functions
Signed-off-by: Adam Litke <agl@us.ibm.com> --- daemon/remote.c | 71 +++++++++++++++++++++++++++++++++++++++++ daemon/remote_generator.pl | 8 +++- src/remote/remote_driver.c | 72 +++++++++++++++++++++++++++++++++++++++-- src/remote/remote_protocol.x | 44 +++++++++++++++++++++++++- 4 files changed, 188 insertions(+), 7 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 2220655..f6aa78e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1692,6 +1692,77 @@ no_memory: goto cleanup; }
+static int +remoteDispatchDomainBlockPull(struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error * rerr, + remote_domain_block_pull_args *args, + remote_domain_block_pull_ret *ret) +{ + virDomainPtr dom = NULL; + virDomainBlockPullInfo tmp; + int rv = -1; + + if (!conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + + if (virDomainBlockPull(dom, args->path, &tmp, args->flags) < 0) + goto cleanup; + rv = 0; + ret->info.cur = tmp.cur; + ret->info.end = tmp.end; + +cleanup: + if (rv < 0) + remoteDispatchError(rerr); + if (dom) + virDomainFree(dom); + return rv; +} + +static int +remoteDispatchDomainGetBlockPullInfo(struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error * rerr, + remote_domain_get_block_pull_info_args *args, + remote_domain_get_block_pull_info_ret *ret) +{ + virDomainPtr dom = NULL; + virDomainBlockPullInfo tmp; + int rv = -1; + + if (!conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + + if (virDomainGetBlockPullInfo(dom, args->path, &tmp, args->flags) < 0) + goto cleanup; + rv = 0; + ret->info.cur = tmp.cur; + ret->info.end = tmp.end; + +cleanup: + if (rv < 0) + remoteDispatchError(rerr); + if (dom) + virDomainFree(dom); + return rv; +}
The generator should be able to deal with this. I might have to tweak it to handle multi-return-value procedures more general.
static int diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl index 062ccc1..d7e0383 100755 --- a/daemon/remote_generator.pl +++ b/daemon/remote_generator.pl @@ -278,7 +278,9 @@ elsif ($opt_b) { "GetType", "NodeDeviceGetParent", "NodeGetSecurityModel", - "SecretGetValue"); + "SecretGetValue", + "DomainBlockPull", + "DomainGetBlockPullInfo"); } elsif ($structprefix eq "qemu") { @ungeneratable = ("MonitorCommand"); } @@ -779,7 +781,9 @@ elsif ($opt_k) { "GetType", "NodeDeviceGetParent", "NodeGetSecurityModel", - "SecretGetValue"); + "SecretGetValue", + "DomainBlockPull", + "DomainGetBlockPullInfo"); } elsif ($structprefix eq "qemu") { @ungeneratable = ("MonitorCommand"); }
You need to rebase your series to git head as the generator has changed much recently.
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 07bc629..0a885a9 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2844,6 +2844,70 @@ done: return rv; }
+static int remoteDomainBlockPull(virDomainPtr domain, + const char *path, + virDomainBlockPullInfoPtr info, + unsigned int flags) +{ + int rv = -1; + remote_domain_block_pull_args args; + remote_domain_block_pull_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.path = (char *)path; + args.flags = flags; + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_BLOCK_PULL, + (xdrproc_t)xdr_remote_domain_block_pull_args, (char *)&args, + (xdrproc_t)xdr_remote_domain_block_pull_ret, (char *)&ret) == -1) + goto done; + + if (info) { + info->cur = ret.info.cur; + info->end = ret.info.end; + } + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainGetBlockPullInfo(virDomainPtr domain, + const char *path, + virDomainBlockPullInfoPtr info, + unsigned int flags) +{ + int rv = -1; + remote_domain_get_block_pull_info_args args; + remote_domain_get_block_pull_info_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.path = (char *)path; + args.flags = flags; + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_BLOCK_PULL_INFO, + (xdrproc_t)xdr_remote_domain_get_block_pull_info_args, + (char *)&args, + (xdrproc_t)xdr_remote_domain_get_block_pull_info_ret, + (char *)&ret) == -1) + goto done; + + info->cur = ret.info.cur; + info->end = ret.info.end; + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +}
This should be generatable as well.
static virDrvOpenStatus ATTRIBUTE_NONNULL (1) @@ -6493,10 +6557,10 @@ static virDriver remote_driver = { remoteDomainSnapshotDelete, /* domainSnapshotDelete */ remoteQemuDomainMonitorCommand, /* qemuDomainMonitorCommand */ remoteDomainOpenConsole, /* domainOpenConsole */ - NULL, /* domainBlockPull */ - NULL, /* domainBlockPullAll */ - NULL, /* domainBlockPullAbort */ - NULL, /* domainGetBlockPullInfo */ + remoteDomainBlockPull, /* domainBlockPull */ + remoteDomainBlockPullAll, /* domainBlockPullAll */ + remoteDomainBlockPullAbort, /* domainBlockPullAbort */ + remoteDomainGetBlockPullInfo, /* domainGetBlockPullInfo */ };
Again, you need to rebase this, this was changed to named C99 initializers recently. Yes, libvirt is a fast moving target :)
static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index c706c36..2f52ceb 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -917,6 +917,43 @@ struct remote_domain_set_autostart_args { int autostart; };
+struct remote_domain_block_pull_info { + unsigned hyper cur; + unsigned hyper end; +}; + +struct remote_domain_block_pull_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + unsigned int flags; +}; + +struct remote_domain_block_pull_ret { + remote_domain_block_pull_info info; +}; + +struct remote_domain_block_pull_all_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + unsigned int flags; +}; + +struct remote_domain_block_pull_abort_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + unsigned int flags; +}; + +struct remote_domain_get_block_pull_info_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + unsigned int flags; +}; + +struct remote_domain_get_block_pull_info_ret { + remote_domain_block_pull_info info; +}; + /* Network calls: */
struct remote_num_of_networks_ret { @@ -2176,7 +2213,12 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_BLKIO_PARAMETERS = 206, REMOTE_PROC_DOMAIN_MIGRATE_SET_MAX_SPEED = 207, REMOTE_PROC_STORAGE_VOL_UPLOAD = 208, - REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209 + REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209, + REMOTE_PROC_DOMAIN_BLOCK_PULL = 210, + + REMOTE_PROC_DOMAIN_BLOCK_PULL_ALL = 211, + REMOTE_PROC_DOMAIN_BLOCK_PULL_ABORT = 212, + REMOTE_PROC_DOMAIN_GET_BLOCK_PULL_INFO = 213
Annotations for the generator go here. I also miss corresponding updates to src/remote_protocol-structs. Matthias

On 06/01/2011 12:13 PM, Matthias Bolte wrote:
2011/6/1 Adam Litke <agl@us.ibm.com>:
This commit has a pretty long summary line.
Fixed.
* src/remote/remote_protocol.x: provide defines for the new entry points * src/remote/remote_driver.c daemon/remote.c: implement the client and server side * daemon/remote_generator.pl: Specify the manually-written functions
Signed-off-by: Adam Litke <agl@us.ibm.com> --- daemon/remote.c | 71 +++++++++++++++++++++++++++++++++++++++++ daemon/remote_generator.pl | 8 +++- src/remote/remote_driver.c | 72 +++++++++++++++++++++++++++++++++++++++-- src/remote/remote_protocol.x | 44 +++++++++++++++++++++++++- 4 files changed, 188 insertions(+), 7 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 2220655..f6aa78e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1692,6 +1692,77 @@ no_memory: goto cleanup; }
+static int +remoteDispatchDomainBlockPull(struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error * rerr, + remote_domain_block_pull_args *args, + remote_domain_block_pull_ret *ret) +{ + virDomainPtr dom = NULL; + virDomainBlockPullInfo tmp; + int rv = -1; + + if (!conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + + if (virDomainBlockPull(dom, args->path, &tmp, args->flags) < 0) + goto cleanup; + rv = 0; + ret->info.cur = tmp.cur; + ret->info.end = tmp.end; + +cleanup: + if (rv < 0) + remoteDispatchError(rerr); + if (dom) + virDomainFree(dom); + return rv; +} + +static int +remoteDispatchDomainGetBlockPullInfo(struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error * rerr, + remote_domain_get_block_pull_info_args *args, + remote_domain_get_block_pull_info_ret *ret) +{ + virDomainPtr dom = NULL; + virDomainBlockPullInfo tmp; + int rv = -1; + + if (!conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + + if (virDomainGetBlockPullInfo(dom, args->path, &tmp, args->flags) < 0) + goto cleanup; + rv = 0; + ret->info.cur = tmp.cur; + ret->info.end = tmp.end; + +cleanup: + if (rv < 0) + remoteDispatchError(rerr); + if (dom) + virDomainFree(dom); + return rv; +}
The generator should be able to deal with this. I might have to tweak it to handle multi-return-value procedures more general.
That would be excellent. I am not doing anything particularly special.
static int diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl index 062ccc1..d7e0383 100755 --- a/daemon/remote_generator.pl +++ b/daemon/remote_generator.pl @@ -278,7 +278,9 @@ elsif ($opt_b) { "GetType", "NodeDeviceGetParent", "NodeGetSecurityModel", - "SecretGetValue"); + "SecretGetValue", + "DomainBlockPull", + "DomainGetBlockPullInfo"); } elsif ($structprefix eq "qemu") { @ungeneratable = ("MonitorCommand"); } @@ -779,7 +781,9 @@ elsif ($opt_k) { "GetType", "NodeDeviceGetParent", "NodeGetSecurityModel", - "SecretGetValue"); + "SecretGetValue", + "DomainBlockPull", + "DomainGetBlockPullInfo"); } elsif ($structprefix eq "qemu") { @ungeneratable = ("MonitorCommand"); }
You need to rebase your series to git head as the generator has changed much recently.
Ok. I will do that this afternoon.
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 07bc629..0a885a9 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2844,6 +2844,70 @@ done: return rv; }
+static int remoteDomainBlockPull(virDomainPtr domain, + const char *path, + virDomainBlockPullInfoPtr info, + unsigned int flags) +{ + int rv = -1; + remote_domain_block_pull_args args; + remote_domain_block_pull_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.path = (char *)path; + args.flags = flags; + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_BLOCK_PULL, + (xdrproc_t)xdr_remote_domain_block_pull_args, (char *)&args, + (xdrproc_t)xdr_remote_domain_block_pull_ret, (char *)&ret) == -1) + goto done; + + if (info) { + info->cur = ret.info.cur; + info->end = ret.info.end; + } + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +} + +static int remoteDomainGetBlockPullInfo(virDomainPtr domain, + const char *path, + virDomainBlockPullInfoPtr info, + unsigned int flags) +{ + int rv = -1; + remote_domain_get_block_pull_info_args args; + remote_domain_get_block_pull_info_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.path = (char *)path; + args.flags = flags; + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_BLOCK_PULL_INFO, + (xdrproc_t)xdr_remote_domain_get_block_pull_info_args, + (char *)&args, + (xdrproc_t)xdr_remote_domain_get_block_pull_info_ret, + (char *)&ret) == -1) + goto done; + + info->cur = ret.info.cur; + info->end = ret.info.end; + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +}
This should be generatable as well.
static virDrvOpenStatus ATTRIBUTE_NONNULL (1) @@ -6493,10 +6557,10 @@ static virDriver remote_driver = { remoteDomainSnapshotDelete, /* domainSnapshotDelete */ remoteQemuDomainMonitorCommand, /* qemuDomainMonitorCommand */ remoteDomainOpenConsole, /* domainOpenConsole */ - NULL, /* domainBlockPull */ - NULL, /* domainBlockPullAll */ - NULL, /* domainBlockPullAbort */ - NULL, /* domainGetBlockPullInfo */ + remoteDomainBlockPull, /* domainBlockPull */ + remoteDomainBlockPullAll, /* domainBlockPullAll */ + remoteDomainBlockPullAbort, /* domainBlockPullAbort */ + remoteDomainGetBlockPullInfo, /* domainGetBlockPullInfo */ };
Again, you need to rebase this, this was changed to named C99 initializers recently. Yes, libvirt is a fast moving target :)
static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index c706c36..2f52ceb 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -917,6 +917,43 @@ struct remote_domain_set_autostart_args { int autostart; };
+struct remote_domain_block_pull_info { + unsigned hyper cur; + unsigned hyper end; +}; + +struct remote_domain_block_pull_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + unsigned int flags; +}; + +struct remote_domain_block_pull_ret { + remote_domain_block_pull_info info; +}; + +struct remote_domain_block_pull_all_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + unsigned int flags; +}; + +struct remote_domain_block_pull_abort_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + unsigned int flags; +}; + +struct remote_domain_get_block_pull_info_args { + remote_nonnull_domain dom; + remote_nonnull_string path; + unsigned int flags; +}; + +struct remote_domain_get_block_pull_info_ret { + remote_domain_block_pull_info info; +}; + /* Network calls: */
struct remote_num_of_networks_ret { @@ -2176,7 +2213,12 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_BLKIO_PARAMETERS = 206, REMOTE_PROC_DOMAIN_MIGRATE_SET_MAX_SPEED = 207, REMOTE_PROC_STORAGE_VOL_UPLOAD = 208, - REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209 + REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209, + REMOTE_PROC_DOMAIN_BLOCK_PULL = 210, + + REMOTE_PROC_DOMAIN_BLOCK_PULL_ALL = 211, + REMOTE_PROC_DOMAIN_BLOCK_PULL_ABORT = 212, + REMOTE_PROC_DOMAIN_GET_BLOCK_PULL_INFO = 213
Annotations for the generator go here.
I am not sure what you mean by annotations for the generator. Could you explain further?
I also miss corresponding updates to src/remote_protocol-structs.
Isn't this file generated from remote-protocol.x? It seems to have exactly the same information.
Matthias
-- Adam Litke IBM Linux Technology Center

2011/6/1 Adam Litke <agl@us.ibm.com>:
On 06/01/2011 12:13 PM, Matthias Bolte wrote:
2011/6/1 Adam Litke <agl@us.ibm.com>:
This commit has a pretty long summary line.
Fixed.
* src/remote/remote_protocol.x: provide defines for the new entry points * src/remote/remote_driver.c daemon/remote.c: implement the client and server side * daemon/remote_generator.pl: Specify the manually-written functions
Signed-off-by: Adam Litke <agl@us.ibm.com> --- daemon/remote.c | 71 +++++++++++++++++++++++++++++++++++++++++ daemon/remote_generator.pl | 8 +++- src/remote/remote_driver.c | 72 +++++++++++++++++++++++++++++++++++++++-- src/remote/remote_protocol.x | 44 +++++++++++++++++++++++++- 4 files changed, 188 insertions(+), 7 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 2220655..f6aa78e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1692,6 +1692,77 @@ no_memory: goto cleanup; }
+static int +remoteDispatchDomainBlockPull(struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error * rerr, + remote_domain_block_pull_args *args, + remote_domain_block_pull_ret *ret) +{ + virDomainPtr dom = NULL; + virDomainBlockPullInfo tmp; + int rv = -1; + + if (!conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + + if (virDomainBlockPull(dom, args->path, &tmp, args->flags) < 0) + goto cleanup; + rv = 0; + ret->info.cur = tmp.cur; + ret->info.end = tmp.end; + +cleanup: + if (rv < 0) + remoteDispatchError(rerr); + if (dom) + virDomainFree(dom); + return rv; +} + +static int +remoteDispatchDomainGetBlockPullInfo(struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client ATTRIBUTE_UNUSED, + virConnectPtr conn, + remote_message_header *hdr ATTRIBUTE_UNUSED, + remote_error * rerr, + remote_domain_get_block_pull_info_args *args, + remote_domain_get_block_pull_info_ret *ret) +{ + virDomainPtr dom = NULL; + virDomainBlockPullInfo tmp; + int rv = -1; + + if (!conn) { + virNetError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + + if (virDomainGetBlockPullInfo(dom, args->path, &tmp, args->flags) < 0) + goto cleanup; + rv = 0; + ret->info.cur = tmp.cur; + ret->info.end = tmp.end; + +cleanup: + if (rv < 0) + remoteDispatchError(rerr); + if (dom) + virDomainFree(dom); + return rv; +}
The generator should be able to deal with this. I might have to tweak it to handle multi-return-value procedures more general.
That would be excellent. I am not doing anything particularly special.
Yes works almost fine. I've attached a patch for the generator to deal with the placement of the struct in the signature.
struct remote_num_of_networks_ret { @@ -2176,7 +2213,12 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_BLKIO_PARAMETERS = 206, REMOTE_PROC_DOMAIN_MIGRATE_SET_MAX_SPEED = 207, REMOTE_PROC_STORAGE_VOL_UPLOAD = 208, - REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209 + REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209, + REMOTE_PROC_DOMAIN_BLOCK_PULL = 210, + + REMOTE_PROC_DOMAIN_BLOCK_PULL_ALL = 211, + REMOTE_PROC_DOMAIN_BLOCK_PULL_ABORT = 212, + REMOTE_PROC_DOMAIN_GET_BLOCK_PULL_INFO = 213
Annotations for the generator go here.
I am not sure what you mean by annotations for the generator. Could you explain further?
Instead of the blacklist in the generator each procedure has a comment that tells the generator how to handle it. It's explained in the .x file.
I also miss corresponding updates to src/remote_protocol-structs.
Isn't this file generated from remote-protocol.x? It seems to have exactly the same information.
This file is not generated, it is used to verify that the existing XDR protocol is not changed, but only extended. When you have pdwtags installed the make check will verify this. Matthias

2011/6/1 Adam Litke <agl@us.ibm.com>:
* src/remote/remote_protocol.x: provide defines for the new entry points * src/remote/remote_driver.c daemon/remote.c: implement the client and server side * daemon/remote_generator.pl: Specify the manually-written functions
Signed-off-by: Adam Litke <agl@us.ibm.com> ---
From the generator point-of-view I would want to avoid having the info
+static int remoteDomainBlockPull(virDomainPtr domain, + const char *path, + virDomainBlockPullInfoPtr info, + unsigned int flags) +{ + int rv = -1; + remote_domain_block_pull_args args; + remote_domain_block_pull_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.path = (char *)path; + args.flags = flags; + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_BLOCK_PULL, + (xdrproc_t)xdr_remote_domain_block_pull_args, (char *)&args, + (xdrproc_t)xdr_remote_domain_block_pull_ret, (char *)&ret) == -1) + goto done; + + if (info) { + info->cur = ret.info.cur; + info->end = ret.info.end; + } + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +} parameter being NULLable, as this differs from the common pattern and results in special case code in the generator.
+struct remote_domain_block_pull_info { + unsigned hyper cur; + unsigned hyper end; +};
+struct remote_domain_block_pull_ret { + remote_domain_block_pull_info info; +};
+struct remote_domain_get_block_pull_info_ret { + remote_domain_block_pull_info info; +};
From the generator point-of-view I would avoid this approach of putting a struct in a struct because that differs from the common approach and results in special case code in the generator. It should look like this
struct remote_domain_block_pull_ret { unsigned hyper cur; unsigned hyper end; }; struct remote_domain_get_block_pull_info_ret { unsigned hyper cur; unsigned hyper end; }; Matthias

On 06/01/2011 12:27 PM, Matthias Bolte wrote:
2011/6/1 Adam Litke <agl@us.ibm.com>:
* src/remote/remote_protocol.x: provide defines for the new entry points * src/remote/remote_driver.c daemon/remote.c: implement the client and server side * daemon/remote_generator.pl: Specify the manually-written functions
Signed-off-by: Adam Litke <agl@us.ibm.com> ---
+static int remoteDomainBlockPull(virDomainPtr domain, + const char *path, + virDomainBlockPullInfoPtr info, + unsigned int flags) +{ + int rv = -1; + remote_domain_block_pull_args args; + remote_domain_block_pull_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.path = (char *)path; + args.flags = flags; + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_BLOCK_PULL, + (xdrproc_t)xdr_remote_domain_block_pull_args, (char *)&args, + (xdrproc_t)xdr_remote_domain_block_pull_ret, (char *)&ret) == -1) + goto done; + + if (info) { + info->cur = ret.info.cur; + info->end = ret.info.end; + } + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +}
From the generator point-of-view I would want to avoid having the info parameter being NULLable, as this differs from the common pattern and results in special case code in the generator.
From an API user's point of view, I think it's nice to allow NULL for the info struct. The user may not care about the current progress information. I could fix this at the global libvirt level by allocating a dummy struct to pass down to the driver implementations if the user passed NULL. Would that be acceptable?
+struct remote_domain_block_pull_info { + unsigned hyper cur; + unsigned hyper end; +};
+struct remote_domain_block_pull_ret { + remote_domain_block_pull_info info; +};
+struct remote_domain_get_block_pull_info_ret { + remote_domain_block_pull_info info; +};
From the generator point-of-view I would avoid this approach of putting a struct in a struct because that differs from the common approach and results in special case code in the generator. It should look like this
struct remote_domain_block_pull_ret { unsigned hyper cur; unsigned hyper end; };
struct remote_domain_get_block_pull_info_ret { unsigned hyper cur; unsigned hyper end; };
Ok, I will make this change. -- Adam Litke IBM Linux Technology Center

2011/6/1 Adam Litke <agl@us.ibm.com>:
On 06/01/2011 12:27 PM, Matthias Bolte wrote:
2011/6/1 Adam Litke <agl@us.ibm.com>:
* src/remote/remote_protocol.x: provide defines for the new entry points * src/remote/remote_driver.c daemon/remote.c: implement the client and server side * daemon/remote_generator.pl: Specify the manually-written functions
Signed-off-by: Adam Litke <agl@us.ibm.com> ---
+static int remoteDomainBlockPull(virDomainPtr domain, + const char *path, + virDomainBlockPullInfoPtr info, + unsigned int flags) +{ + int rv = -1; + remote_domain_block_pull_args args; + remote_domain_block_pull_ret ret; + struct private_data *priv = domain->conn->privateData; + + remoteDriverLock(priv); + + make_nonnull_domain(&args.dom, domain); + args.path = (char *)path; + args.flags = flags; + + if (call(domain->conn, priv, 0, REMOTE_PROC_DOMAIN_BLOCK_PULL, + (xdrproc_t)xdr_remote_domain_block_pull_args, (char *)&args, + (xdrproc_t)xdr_remote_domain_block_pull_ret, (char *)&ret) == -1) + goto done; + + if (info) { + info->cur = ret.info.cur; + info->end = ret.info.end; + } + rv = 0; + +done: + remoteDriverUnlock(priv); + return rv; +}
From the generator point-of-view I would want to avoid having the info parameter being NULLable, as this differs from the common pattern and results in special case code in the generator.
From an API user's point of view, I think it's nice to allow NULL for the info struct. The user may not care about the current progress information. I could fix this at the global libvirt level by allocating a dummy struct to pass down to the driver implementations if the user passed NULL. Would that be acceptable?
No, don't make runtime compromises to simplify the generator, we can special case this later.
+struct remote_domain_block_pull_info { + unsigned hyper cur; + unsigned hyper end; +};
+struct remote_domain_block_pull_ret { + remote_domain_block_pull_info info; +};
+struct remote_domain_get_block_pull_info_ret { + remote_domain_block_pull_info info; +};
From the generator point-of-view I would avoid this approach of putting a struct in a struct because that differs from the common approach and results in special case code in the generator. It should look like this
struct remote_domain_block_pull_ret { unsigned hyper cur; unsigned hyper end; };
struct remote_domain_get_block_pull_info_ret { unsigned hyper cur; unsigned hyper end; };
Ok, I will make this change.
The patch from my other response require this change and makes the generator deal with the virDomainGetBlockPullInfo. virDomainBlockPull requires some additional special case for the optional struct so you should just keep that one manually written and we'll deal with that later. Matthias

The virDomainBlockPull* family of commands are enabled by the 'block_stream' and 'info block_stream' qemu monitor commands. * src/qemu/qemu_driver.c src/qemu/qemu_monitor_text.[ch]: implement disk streaming by using the stream and info stream text monitor commands * src/qemu/qemu_monitor_json.[ch]: implement commands using the qmp monitor Signed-off-by: Adam Litke <agl@us.ibm.com> --- src/qemu/qemu_driver.c | 112 +++++++++++++++++++++++++++++++- src/qemu/qemu_monitor.c | 16 +++++ src/qemu/qemu_monitor.h | 13 ++++ src/qemu/qemu_monitor_json.c | 118 ++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 5 ++ src/qemu/qemu_monitor_text.c | 145 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 5 ++ 7 files changed, 410 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cfbe199..481b168 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7080,6 +7080,110 @@ cleanup: return ret; } +static const char * +qemuDiskPathToAlias(virDomainObjPtr vm, const char *path) { + int i; + char *ret = NULL; + + for (i = 0 ; i < vm->def->ndisks ; i++) { + virDomainDiskDefPtr disk = vm->def->disks[i]; + + if (disk->src != NULL && STREQ(disk->src, path)) { + if (virAsprintf(&ret, "drive-%s", disk->info.alias) < 0) { + virReportOOMError(); + return NULL; + } + break; + } + } + + if (!ret) { + qemuReportError(VIR_ERR_INVALID_ARG, + "%s", _("No device found for specified path")); + } + return ret; +} + +static int +qemuDomainBlockPullImpl(virDomainPtr dom, const char *path, + virDomainBlockPullInfoPtr info, + int mode) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + qemuDomainObjPrivatePtr priv; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + const char *device = NULL; + int ret = -1; + + qemuDriverLock(driver); + virUUIDFormat(dom->uuid, uuidstr); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + if (!vm) { + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + device = qemuDiskPathToAlias(vm, path); + if (!device) { + goto cleanup; + } + + if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0) + goto cleanup; + qemuDomainObjEnterMonitorWithDriver(driver, vm); + priv = vm->privateData; + ret = qemuMonitorBlockPull(priv->mon, device, info, mode); + qemuDomainObjExitMonitorWithDriver(driver, vm); + if (qemuDomainObjEndJob(vm) == 0) { + vm = NULL; + goto cleanup; + } + +cleanup: + VIR_FREE(device); + if (vm) + virDomainObjUnlock(vm); + qemuDriverUnlock(driver); + return ret; +} + +static int +qemuDomainBlockPull(virDomainPtr dom, const char *path, + virDomainBlockPullInfoPtr info, unsigned int flags) +{ + virCheckFlags(0, -1); + return qemuDomainBlockPullImpl(dom, path, info, BLOCK_PULL_MODE_ONE); +} + +static int +qemuDomainBlockPullAll(virDomainPtr dom, const char *path, unsigned int flags) +{ + virCheckFlags(0, -1); + return qemuDomainBlockPullImpl(dom, path, NULL, BLOCK_PULL_MODE_ALL); +} + +static int +qemuDomainBlockPullAbort(virDomainPtr dom, const char *path, unsigned int flags) +{ + virCheckFlags(0, -1); + return qemuDomainBlockPullImpl(dom, path, NULL, BLOCK_PULL_MODE_ABORT); +} + +static int +qemuDomainGetBlockPullInfo(virDomainPtr dom, const char *path, + virDomainBlockPullInfoPtr info, unsigned int flags) +{ + virCheckFlags(0, -1); + return qemuDomainBlockPullImpl(dom, path, info, BLOCK_PULL_MODE_INFO); +} static virDriver qemuDriver = { VIR_DRV_QEMU, @@ -7192,10 +7296,10 @@ static virDriver qemuDriver = { qemuDomainSnapshotDelete, /* domainSnapshotDelete */ qemuDomainMonitorCommand, /* qemuDomainMonitorCommand */ qemuDomainOpenConsole, /* domainOpenConsole */ - NULL, /* domainBlockPull */ - NULL, /* domainBlockPullAll */ - NULL, /* domainBlockPullAbort */ - NULL, /* domainGetBlockPullInfo */ + qemuDomainBlockPull, /* domainBlockPull */ + qemuDomainBlockPullAll, /* domainBlockPullAll */ + qemuDomainBlockPullAbort, /* domainBlockPullAbort */ + qemuDomainGetBlockPullInfo, /* domainGetBlockPullInfo */ }; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index f89038e..60e4ee2 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2228,3 +2228,19 @@ int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, ret = qemuMonitorTextArbitraryCommand(mon, cmd, reply); return ret; } + +int qemuMonitorBlockPull(qemuMonitorPtr mon, + const char *path, + virDomainBlockPullInfoPtr info, + int mode) +{ + int ret; + + VIR_DEBUG("mon=%p, path=%p, info=%p, mode=%i", mon, path, info, mode); + + if (mon->json) + ret = qemuMonitorJSONBlockPull(mon, path, info, mode); + else + ret = qemuMonitorTextBlockPull(mon, path, info, mode); + return ret; +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index c90219b..62875a3 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -423,6 +423,19 @@ int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, char **reply, bool hmp); +typedef enum { + BLOCK_PULL_MODE_ONE = 0, + BLOCK_PULL_MODE_ALL = 1, + BLOCK_PULL_MODE_ABORT = 2, + BLOCK_PULL_MODE_INFO = 3, +} BLOCK_PULL_MODE; + + +int qemuMonitorBlockPull(qemuMonitorPtr mon, + const char *path, + virDomainBlockPullInfoPtr info, + int mode); + /** * When running two dd process and using <> redirection, we need a * shell that will not truncate files. These two strings serve that diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 20a78e1..bc491e0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2513,3 +2513,121 @@ cleanup: return ret; } + +static int qemuMonitorJSONGetBlockPullInfoOne(virJSONValuePtr entry, + const char *device, + virDomainBlockPullInfoPtr info) +{ + const char *this_dev; + + if ((this_dev = virJSONValueObjectGetString(entry, "device")) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("entry was missing 'device'")); + return -1; + } + if (!STREQ(this_dev, device)) + return -1; + + if (virJSONValueObjectGetNumberUlong(entry, "offset", &info->cur) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("entry was missing 'offset'")); + return -1; + } + + if (virJSONValueObjectGetNumberUlong(entry, "len", &info->end) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("entry was missing 'len'")); + return -1; + } + return 0; +} + +/** qemuMonitorJSONGetBlockPullInfo: + * Parse Block Pull information. + * The reply can be a JSON array of objects or just an object. + */ +static int qemuMonitorJSONGetBlockPullInfo(virJSONValuePtr reply, + const char *device, + virDomainBlockPullInfoPtr info) +{ + virJSONValuePtr data; + int nr_results, i = 0; + + if (!info) + return -1; + + if ((data = virJSONValueObjectGet(reply, "return")) == NULL) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("reply was missing block_pull progress information")); + return -1; + } + + if (data->type == VIR_JSON_TYPE_OBJECT) { + if (qemuMonitorJSONGetBlockPullInfoOne(data, device, info) != 0) + goto not_found; + else + return 0; + } else if (data->type != VIR_JSON_TYPE_ARRAY) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("urecognized format of block pull information")); + return -1; + } + + if ((nr_results = virJSONValueArraySize(data)) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("unable to determine array size")); + return -1; + } + + for (i = 0; i < nr_results; i++) { + virJSONValuePtr entry = virJSONValueArrayGet(data, i); + if (qemuMonitorJSONGetBlockPullInfoOne(entry, device, info) == 0) + return 0; + } + +not_found: + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("No associated information for the specified disk")); + return -1; +} + + +int qemuMonitorJSONBlockPull(qemuMonitorPtr mon, + const char *device, + virDomainBlockPullInfoPtr info, + int mode) +{ + int ret = -1; + virJSONValuePtr cmd = NULL; + virJSONValuePtr reply = NULL; + int parse_info = 0; + + if (mode == BLOCK_PULL_MODE_ONE) { + cmd = qemuMonitorJSONMakeCommand("block_stream", "s:device", device, NULL); + parse_info = 1; + } else if (mode == BLOCK_PULL_MODE_ALL) { + cmd = qemuMonitorJSONMakeCommand("block_stream", "s:device", device, + "b:all", 1, NULL); + } else if (mode == BLOCK_PULL_MODE_ABORT) { + cmd = qemuMonitorJSONMakeCommand("block_stream", "s:device", device, + "b:stop", 1, NULL); + } else if (mode == BLOCK_PULL_MODE_INFO) { + cmd = qemuMonitorJSONMakeCommand("query-block-stream", NULL); + parse_info = 1; + } + + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + if (ret == 0 && parse_info) + ret = qemuMonitorJSONGetBlockPullInfo(reply, device, info); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 086f0e1..129fbbe 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -204,4 +204,9 @@ int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon, char **reply_str, bool hmp); +int qemuMonitorJSONBlockPull(qemuMonitorPtr mon, + const char *device, + virDomainBlockPullInfoPtr info, + int mode); + #endif /* QEMU_MONITOR_JSON_H */ diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 53781c8..c4c469e 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2628,3 +2628,148 @@ int qemuMonitorTextArbitraryCommand(qemuMonitorPtr mon, const char *cmd, return ret; } + +static int qemuMonitorTextParseBlockPullOne(const char *text, + const char *device, + virDomainBlockPullInfoPtr info, + const char **next) +{ + virDomainBlockPullInfo tmp; + char *p; + int mismatch = 0; + + if (next == NULL) + return -1; + *next = NULL; + + /* + * Each active stream will appear on its own line in the following format: + * Streaming device <device>: Completed <cur> of <end> bytes + */ + if ((text = STRSKIP(text, "Streaming device ")) == NULL) + return -EINVAL; + + if (!STREQLEN(text, device, strlen(device))) + mismatch = 1; + + if ((text = strstr(text, ": Completed ")) == NULL) + return -EINVAL; + text += 11; + + if (virStrToLong_ull (text, &p, 10, &tmp.cur)) + return -EINVAL; + text = p; + + if (!STRPREFIX(text, " of ")) + return -EINVAL; + text += 4; + + if (virStrToLong_ull (text, &p, 10, &tmp.end)) + return -EINVAL; + text = p; + + if (!STRPREFIX(text, " bytes")) + return -EINVAL; + + if (mismatch) { + *next = STRSKIP(text, "\n"); + return -EAGAIN; + } + + if (info) { + info->cur = tmp.cur; + info->end = tmp.end; + } + return 0; +} + +static int qemuMonitorTextParseBlockPull(const char *text, + const char *device, + virDomainBlockPullInfoPtr info) +{ + const char *next = NULL; + int ret = 0; + + /* Check error: Device not found */ + if (strstr(text, "Device '") && strstr(text, "' not found")) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Device not found")); + return -1; + } + + /* Check if we have exceeded the number of simultaneous streams */ + if (strstr(text, "Device '") && strstr(text, "' is in use")) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("Another streaming operation is in progress")); + return -1; + } + + /* Check error: Non-existent stream */ + if (strstr(text, "No such process") || + strstr(text, "No active stream")) { + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("No active stream")); + return -1; + } + + /* No output indicates success for BlockPullAll and BlockPullAbort */ + if (STREQ(text, "")) + return 0; + + /* Now try to parse lines of block_stream output */ + do { + ret = qemuMonitorTextParseBlockPullOne(text, device, info, &next); + text = next; + } while (ret == -EAGAIN); + + if (ret != 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("No associated information for the specified disk")); + ret = -1; + } + return ret; +} + +int qemuMonitorTextBlockPull(qemuMonitorPtr mon, + const char *device, + virDomainBlockPullInfoPtr info, + int mode) +{ + char *cmd = NULL; + char *reply = NULL; + int ret, parse_info = 0; + + if (mode == BLOCK_PULL_MODE_ONE) { + ret = virAsprintf(&cmd, "block_stream %s", device); + parse_info = 1; + } else if (mode == BLOCK_PULL_MODE_ALL) { + ret = virAsprintf(&cmd, "block_stream -a %s", device); + } else if (mode == BLOCK_PULL_MODE_ABORT) { + ret = virAsprintf(&cmd, "block_stream -s %s", device); + } else if (mode == BLOCK_PULL_MODE_INFO) { + ret = virAsprintf(&cmd, "info block_stream"); + parse_info = 1; + } else { + return -1; + } + + if (ret < 0) { + virReportOOMError(); + return -1; + } + + ret = 0; + if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot run monitor command")); + ret = -1; + goto cleanup; + } + + if (parse_info && (qemuMonitorTextParseBlockPull(reply, device, info) != 0)) + ret = -1; + +cleanup: + VIR_FREE(cmd); + VIR_FREE(reply); + return ret; +} diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index 0838a2b..e72f654 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -198,4 +198,9 @@ int qemuMonitorTextDeleteSnapshot(qemuMonitorPtr mon, const char *name); int qemuMonitorTextArbitraryCommand(qemuMonitorPtr mon, const char *cmd, char **reply); +int qemuMonitorTextBlockPull(qemuMonitorPtr mon, + const char *device, + virDomainBlockPullInfoPtr info, + int mode); + #endif /* QEMU_MONITOR_TEXT_H */ -- 1.7.3

Define three new virsh commands: * blockpull: Perform incremental block pull * blockpull: Start/stop full device block pull * blockpullinfo: Retrieve progress info for full device block pull Share print_job_progress() with the migration code. * tools/virsh.c: implement the new commands Signed-off-by: Adam Litke <agl@us.ibm.com> --- tools/virsh.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 130 insertions(+), 4 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 2b16714..e04de60 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -3818,7 +3818,8 @@ out_sig: } static void -print_job_progress(unsigned long long remaining, unsigned long long total) +print_job_progress(const char *label, unsigned long long remaining, + unsigned long long total) { int progress; @@ -3838,7 +3839,7 @@ print_job_progress(unsigned long long remaining, unsigned long long total) } } - fprintf(stderr, "\rMigration: [%3d %%]", progress); + fprintf(stderr, "\r%s: [%3d %%]", label, progress); } static bool @@ -3925,7 +3926,7 @@ repoll: functionReturn = true; if (verbose) { /* print [100 %] */ - print_job_progress(0, 1); + print_job_progress("Migration", 0, 1); } } else functionReturn = false; @@ -3964,7 +3965,8 @@ repoll: pthread_sigmask(SIG_SETMASK, &oldsigmask, NULL); #endif if (ret == 0) - print_job_progress(jobinfo.dataRemaining, jobinfo.dataTotal); + print_job_progress("Migration", jobinfo.dataRemaining, + jobinfo.dataTotal); } } @@ -4023,6 +4025,127 @@ done: return ret; } +typedef enum { + VSH_CMD_BLOCK_PULL_ONE = 0, + VSH_CMD_BLOCK_PULL_ALL = 1, + VSH_CMD_BLOCK_PULL_ABORT = 2, + VSH_CMD_BLOCK_PULL_INFO = 3 +} VSH_CMD_BLOCK_PULL_MODE; + +static int +blockPullImpl(vshControl *ctl, const vshCmd *cmd, + virDomainBlockPullInfoPtr info, int mode) +{ + virDomainPtr dom; + const char *name, *path; + int ret = -1; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) + return false; + + if (vshCommandOptString(cmd, "path", &path) < 0) + return false; + + if (mode == VSH_CMD_BLOCK_PULL_ONE) + ret = virDomainBlockPull(dom, path, info, 0); + else if (mode == VSH_CMD_BLOCK_PULL_ALL) + ret = virDomainBlockPullAll(dom, path, 0); + else if (mode == VSH_CMD_BLOCK_PULL_ABORT) + ret = virDomainBlockPullAbort(dom, path, 0); + else if (mode == VSH_CMD_BLOCK_PULL_INFO) + ret = virDomainGetBlockPullInfo(dom, path, info, 0); + + virDomainFree(dom); + return ret; +} + +/* + * "blockpull" command + */ +static const vshCmdInfo info_block_pull[] = { + {"help", N_("Iteratively populate a disk from its backing image.")}, + {"desc", N_("Iteratively populate a disk from its backing image.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_block_pull[] = { + {"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")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdBlockPull(vshControl *ctl, const vshCmd *cmd) +{ + virDomainBlockPullInfo info; + + if (blockPullImpl(ctl, cmd, &info, VSH_CMD_BLOCK_PULL_ONE) != 0) + return false; + print_job_progress("Block pull", info.end - info.cur, info.end); + return true; +} + +/* + * "blockpullall" command + */ +static const vshCmdInfo info_block_pull_all[] = { + {"help", N_("Start or stop populating a disk from its backing image.")}, + {"desc", N_("Start or stop populating a disk from its backing image.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_block_pull_all[] = { + {"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")}, + {"abort", VSH_OT_BOOL, VSH_OFLAG_NONE, N_("Abort the current operation")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdBlockPullAll(vshControl *ctl, const vshCmd *cmd) +{ + int mode; + + if (vshCommandOptBool (cmd, "abort")) + mode = VSH_CMD_BLOCK_PULL_ABORT; + else + mode = VSH_CMD_BLOCK_PULL_ALL; + + if (blockPullImpl(ctl, cmd, NULL, mode) != 0) + return false; + return true; +} + +/* + * "blockpullinfo" command + */ +static const vshCmdInfo info_block_pull_info[] = { + {"help", N_("Check progress of an active block pull operation.")}, + {"desc", N_("Check progress of an active block pull operation.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_block_pull_info[] = { + {"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")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdBlockPullInfo(vshControl *ctl, const vshCmd *cmd) +{ + virDomainBlockPullInfo info; + + if (blockPullImpl(ctl, cmd, &info, VSH_CMD_BLOCK_PULL_INFO) != 0) + return false; + print_job_progress("Block pull", info.end - info.cur, info.end); + return true; +} + + /* * "net-autostart" command */ @@ -10685,6 +10808,9 @@ static const vshCmdDef domManagementCmds[] = { {"attach-interface", cmdAttachInterface, opts_attach_interface, info_attach_interface}, {"autostart", cmdAutostart, opts_autostart, info_autostart}, {"blkiotune", cmdBlkiotune, opts_blkiotune, info_blkiotune}, + {"blockpull", cmdBlockPull, opts_block_pull, info_block_pull}, + {"blockpullall", cmdBlockPullAll, opts_block_pull_all, info_block_pull_all}, + {"blockpullinfo", cmdBlockPullInfo, opts_block_pull_info, info_block_pull_info}, #ifndef WIN32 {"console", cmdConsole, opts_console, info_console}, #endif -- 1.7.3

virDomainBlockPullAll and virDomainBlockPullAbort are handled automatically. virDomainBlockPull and virDomainBlockPullInfo require manual overrides since they return a custom type. * python/generator.py: reenable bindings for this entry point * python/libvirt-override-api.xml python/libvirt-override.c: manual overrides Signed-off-by: Adam Litke <agl@us.ibm.com> --- python/generator.py | 5 +-- python/libvirt-override-api.xml | 14 ++++++++++ python/libvirt-override.c | 53 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/python/generator.py b/python/generator.py index 52be85d..3684722 100755 --- a/python/generator.py +++ b/python/generator.py @@ -166,8 +166,6 @@ def enum(type, name, value): functions_failed = [] functions_skipped = [ "virConnectListDomains", - 'virDomainBlockPull', - 'virDomainGetBlockPullInfo', ] skipped_modules = { @@ -182,7 +180,6 @@ skipped_types = { 'virConnectDomainEventIOErrorCallback': "No function types in python", 'virConnectDomainEventGraphicsCallback': "No function types in python", 'virEventAddHandleFunc': "No function types in python", - 'virDomainBlockPullInfoPtr': "Not implemented yet", } ####################################################################### @@ -344,6 +341,8 @@ skip_impl = ( 'virNodeDeviceListCaps', 'virConnectBaselineCPU', 'virDomainRevertToSnapshot', + 'virDomainBlockPull', + 'virDomainGetBlockPullInfo', ) diff --git a/python/libvirt-override-api.xml b/python/libvirt-override-api.xml index 54deeb5..af560e8 100644 --- a/python/libvirt-override-api.xml +++ b/python/libvirt-override-api.xml @@ -308,5 +308,19 @@ <arg name='flags' type='unsigned int' info='flags, curently unused'/> <return type='int' info="0 on success, -1 on error"/> </function> + <function name='virDomainBlockPull' file='python'> + <info>Initiate an incremental BlockPull for the given disk</info> + <arg name='dom' type='virDomainPtr' info='pointer to the domain'/> + <arg name='path' type='const char *' info='Fully-qualified filename of disk'/> + <arg name='flags' type='unsigned int' info='fine-tuning flags, currently unused, pass 0.'/> + <return type='virDomainBlockPullInfo' info='A dictionary containing progress information.' /> + </function> + <function name='virDomainGetBlockPullInfo' file='python'> + <info>Get progress information for a background BlockPull operation</info> + <arg name='dom' type='virDomainPtr' info='pointer to the domain'/> + <arg name='path' type='const char *' info='Fully-qualified filename of disk'/> + <arg name='flags' type='unsigned int' info='fine-tuning flags, currently unused, pass 0.'/> + <return type='virDomainBlockPullInfo' info='A dictionary containing progress information.' /> + </function> </symbols> </api> diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 4a9b432..ca90ef0 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -2350,6 +2350,57 @@ libvirt_virDomainGetJobInfo(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) { return(py_retval); } +static PyObject * +libvirt_virDomainBlockPullImpl(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args, int infoOnly) { + virDomainPtr domain; + PyObject *pyobj_domain; + const char *path; + unsigned int flags; + virDomainBlockPullInfo info; + int c_ret; + PyObject *ret; + + if (!PyArg_ParseTuple(args, (char *)"Ozi:virDomainStreamDiskInfo", + &pyobj_domain, &path, &flags)) + return(NULL); + domain = (virDomainPtr) PyvirDomain_Get(pyobj_domain); + +LIBVIRT_BEGIN_ALLOW_THREADS; + if (infoOnly) + c_ret = virDomainGetBlockPullInfo(domain, path, &info, flags); + else + c_ret = virDomainBlockPull(domain, path, &info, flags); +LIBVIRT_END_ALLOW_THREADS; + + if (c_ret == -1) + return VIR_PY_NONE; + + if ((ret = PyDict_New()) == NULL) + return VIR_PY_NONE; + + PyDict_SetItem(ret, libvirt_constcharPtrWrap("cur"), + libvirt_ulonglongWrap(info.cur)); + PyDict_SetItem(ret, libvirt_constcharPtrWrap("end"), + libvirt_ulonglongWrap(info.end)); + + return ret; +} + +static PyObject * +libvirt_virDomainBlockPull(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + return libvirt_virDomainBlockPullImpl(self, args, 0); +} + +static PyObject * +libvirt_virDomainGetBlockPullInfo(PyObject *self ATTRIBUTE_UNUSED, + PyObject *args) +{ + return libvirt_virDomainBlockPullImpl(self, args, 1); +} + /******************************************* * Helper functions to avoid importing modules @@ -3585,6 +3636,8 @@ static PyMethodDef libvirtMethods[] = { {(char *) "virDomainGetJobInfo", libvirt_virDomainGetJobInfo, METH_VARARGS, NULL}, {(char *) "virDomainSnapshotListNames", libvirt_virDomainSnapshotListNames, METH_VARARGS, NULL}, {(char *) "virDomainRevertToSnapshot", libvirt_virDomainRevertToSnapshot, METH_VARARGS, NULL}, + {(char *) "virDomainBlockPull", libvirt_virDomainBlockPull, METH_VARARGS, NULL}, + {(char *) "virDomainGetBlockPullInfo", libvirt_virDomainGetBlockPullInfo, METH_VARARGS, NULL}, {NULL, NULL, 0, NULL} }; -- 1.7.3

When an operation started by virDomainBlockPullAll completes (either with success or with failure), raise an event to indicate the final status. This allows an API user to avoid polling on virDomainBlockPullInfo if they would prefer to use the event mechanism. * daemon/remote.c: Dispatch events to client * include/libvirt/libvirt.h.in: Define event ID and callback signature * src/conf/domain_event.c, src/conf/domain_event.h, src/libvirt_private.syms: Extend API to handle the new event * src/qemu/qemu_driver.c: Connect to the QEMU monitor event for block_stream completion and emit a libvirt block pull event * src/remote/remote_driver.c: Receive and dispatch events to application * src/remote/remote_protocol.x: Wire protocol definition for the event * src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h, src/qemu/qemu_monitor_json.c: Watch for BLOCK_STREAM_COMPLETED event from QEMU monitor Signed-off-by: Adam Litke <agl@us.ibm.com> --- daemon/remote.c | 32 ++++++++++++++++++++ include/libvirt/libvirt.h.in | 27 +++++++++++++++++ python/libvirt-override-virConnect.py | 12 ++++++++ python/libvirt-override.c | 51 +++++++++++++++++++++++++++++++++ src/conf/domain_event.c | 50 ++++++++++++++++++++++++++++++++ src/conf/domain_event.h | 7 ++++- src/libvirt_private.syms | 2 + src/qemu/qemu_monitor.c | 12 ++++++++ src/qemu/qemu_monitor.h | 8 +++++ src/qemu/qemu_monitor_json.c | 30 +++++++++++++++++++ src/qemu/qemu_process.c | 28 ++++++++++++++++++ src/remote/remote_driver.c | 30 +++++++++++++++++++ src/remote/remote_protocol.x | 9 +++++- 13 files changed, 296 insertions(+), 2 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index f6aa78e..f45052c 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -378,6 +378,37 @@ static int remoteRelayDomainEventGraphics(virConnectPtr conn ATTRIBUTE_UNUSED, return 0; } +static int remoteRelayDomainEventBlockPull(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + const char *path, + int status, + void *opaque) +{ + struct qemud_client *client = opaque; + remote_domain_event_block_pull_msg data; + + if (!client) + return -1; + + VIR_DEBUG("Relaying domain block pull event %s %d %s %i", dom->name, dom->id, path, status); + + virMutexLock(&client->lock); + + /* build return data */ + memset(&data, 0, sizeof data); + make_nonnull_domain(&data.dom, dom); + data.path = (char*)path; + data.status = status; + + remoteDispatchDomainEventSend(client, + REMOTE_PROC_DOMAIN_EVENT_BLOCK_PULL, + (xdrproc_t)xdr_remote_domain_event_block_pull_msg, &data); + + virMutexUnlock(&client->lock); + + return 0; +} + static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventLifecycle), @@ -387,6 +418,7 @@ static virConnectDomainEventGenericCallback domainEventCallbacks[] = { VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventIOError), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventGraphics), VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventIOErrorReason), + VIR_DOMAIN_EVENT_CALLBACK(remoteRelayDomainEventBlockPull), }; verify(ARRAY_CARDINALITY(domainEventCallbacks) == VIR_DOMAIN_EVENT_ID_LAST); diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 9af1b76..c85e204 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -2505,6 +2505,32 @@ typedef void (*virConnectDomainEventGraphicsCallback)(virConnectPtr conn, void *opaque); /** + * virConnectDomainEventBlockPullStatus: + * + * The final status of a virDomainBlockPullAll() operation + */ +typedef enum { + VIR_DOMAIN_BLOCK_PULL_COMPLETED = 0, + VIR_DOMAIN_BLOCK_PULL_FAILED = 1, +} virConnectDomainEventBlockPullStatus; + +/** + * virConnectDomainEventBlockPullCallback: + * @conn: connection object + * @dom: domain on which the event occurred + * @path: fully-qualified filename of the affected disk + * @status: final status of the operation (virConnectDomainEventBlockPullStatus) + * + * The callback signature to use when registering for an event of type + * VIR_DOMAIN_EVENT_ID_BLOCK_PULL with virConnectDomainEventRegisterAny() + */ +typedef void (*virConnectDomainEventBlockPullCallback)(virConnectPtr conn, + virDomainPtr dom, + const char *path, + int status, + void *opaque); + +/** * VIR_DOMAIN_EVENT_CALLBACK: * * Used to cast the event specific callback into the generic one @@ -2521,6 +2547,7 @@ typedef enum { VIR_DOMAIN_EVENT_ID_IO_ERROR = 4, /* virConnectDomainEventIOErrorCallback */ VIR_DOMAIN_EVENT_ID_GRAPHICS = 5, /* virConnectDomainEventGraphicsCallback */ VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON = 6, /* virConnectDomainEventIOErrorReasonCallback */ + VIR_DOMAIN_EVENT_ID_BLOCK_PULL = 7, /* virConnectDomainEventBlockPullCallback */ /* * NB: this enum value will increase over time as new events are diff --git a/python/libvirt-override-virConnect.py b/python/libvirt-override-virConnect.py index e344303..362be75 100644 --- a/python/libvirt-override-virConnect.py +++ b/python/libvirt-override-virConnect.py @@ -124,6 +124,18 @@ except AttributeError: pass + def dispatchDomainEventBlockPullCallback(self, dom, path, status, cbData): + """Dispatches events to python user domain blockPull event callbacks + """ + try: + cb = cbData["cb"] + opaque = cbData["opaque"] + + cb(self, virDomain(self, _obj=dom), path, status, opaque) + return 0 + except AttributeError: + pass + def domainEventDeregisterAny(self, callbackID): """Removes a Domain Event Callback. De-registering for a domain callback will disable delivery of this event type """ diff --git a/python/libvirt-override.c b/python/libvirt-override.c index ca90ef0..9950ea8 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -3452,6 +3452,54 @@ libvirt_virConnectDomainEventGraphicsCallback(virConnectPtr conn ATTRIBUTE_UNUSE return ret; } +static int +libvirt_virConnectDomainEventBlockPullCallback(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainPtr dom, + const char *path, + int status, + void *opaque) +{ + PyObject *pyobj_cbData = (PyObject*)opaque; + PyObject *pyobj_dom; + PyObject *pyobj_ret; + PyObject *pyobj_conn; + PyObject *dictKey; + int ret = -1; + + LIBVIRT_ENSURE_THREAD_STATE; + + /* Create a python instance of this virDomainPtr */ + virDomainRef(dom); + pyobj_dom = libvirt_virDomainPtrWrap(dom); + Py_INCREF(pyobj_cbData); + + dictKey = libvirt_constcharPtrWrap("conn"); + pyobj_conn = PyDict_GetItem(pyobj_cbData, dictKey); + Py_DECREF(dictKey); + + /* Call the Callback Dispatcher */ + pyobj_ret = PyObject_CallMethod(pyobj_conn, + (char*)"dispatchDomainEventBlockPullCallback", + (char*)"OsiO", + pyobj_dom, path, status, pyobj_cbData); + + Py_DECREF(pyobj_cbData); + Py_DECREF(pyobj_dom); + + if(!pyobj_ret) { +#if DEBUG_ERROR + printf("%s - ret:%p\n", __FUNCTION__, pyobj_ret); +#endif + PyErr_Print(); + } else { + Py_DECREF(pyobj_ret); + ret = 0; + } + + LIBVIRT_RELEASE_THREAD_STATE; + return ret; +} + static PyObject * libvirt_virConnectDomainEventRegisterAny(ATTRIBUTE_UNUSED PyObject * self, PyObject * args) @@ -3507,6 +3555,9 @@ libvirt_virConnectDomainEventRegisterAny(ATTRIBUTE_UNUSED PyObject * self, case VIR_DOMAIN_EVENT_ID_GRAPHICS: cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventGraphicsCallback); break; + case VIR_DOMAIN_EVENT_ID_BLOCK_PULL: + cb = VIR_DOMAIN_EVENT_CALLBACK(libvirt_virConnectDomainEventBlockPullCallback); + break; } if (!cb) { diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 688bf6c..94dfb8e 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -83,6 +83,10 @@ struct _virDomainEvent { char *authScheme; virDomainEventGraphicsSubjectPtr subject; } graphics; + struct { + char *path; + int status; + } blockPull; } data; }; @@ -499,6 +503,11 @@ void virDomainEventFree(virDomainEventPtr event) } VIR_FREE(event->data.graphics.subject); } + break; + + case VIR_DOMAIN_EVENT_ID_BLOCK_PULL: + VIR_FREE(event->data.blockPull.path); + break; } VIR_FREE(event->dom.name); @@ -782,6 +791,40 @@ virDomainEventPtr virDomainEventGraphicsNewFromObj(virDomainObjPtr obj, return ev; } +static virDomainEventPtr +virDomainEventBlockPullNew(int id, const char *name, unsigned char *uuid, + const char *path, int status) +{ + virDomainEventPtr ev = + virDomainEventNewInternal(VIR_DOMAIN_EVENT_ID_BLOCK_PULL, + id, name, uuid); + + if (ev) { + if (!(ev->data.blockPull.path = strdup(path))) { + virDomainEventFree(ev); + return NULL; + } + ev->data.blockPull.status = status; + } + + return ev; +} + +virDomainEventPtr virDomainEventBlockPullNewFromObj(virDomainObjPtr obj, + const char *path, + int status) +{ + return virDomainEventBlockPullNew(obj->def->id, obj->def->name, + obj->def->uuid, path, status); +} + +virDomainEventPtr virDomainEventBlockPullNewFromDom(virDomainPtr dom, + const char *path, + int status) +{ + return virDomainEventBlockPullNew(dom->id, dom->name, dom->uuid, + path, status); +} /** * virDomainEventQueueFree: @@ -932,6 +975,13 @@ void virDomainEventDispatchDefaultFunc(virConnectPtr conn, cbopaque); break; + case VIR_DOMAIN_EVENT_ID_BLOCK_PULL: + ((virConnectDomainEventBlockPullCallback)cb)(conn, dom, + event->data.blockPull.path, + event->data.blockPull.status, + cbopaque); + break; + default: VIR_WARN("Unexpected event ID %d", event->eventID); break; diff --git a/src/conf/domain_event.h b/src/conf/domain_event.h index c03a159..383b5e6 100644 --- a/src/conf/domain_event.h +++ b/src/conf/domain_event.h @@ -154,7 +154,12 @@ virDomainEventPtr virDomainEventGraphicsNewFromObj(virDomainObjPtr obj, const char *authScheme, virDomainEventGraphicsSubjectPtr subject); - +virDomainEventPtr virDomainEventBlockPullNewFromObj(virDomainObjPtr obj, + const char *path, + int status); +virDomainEventPtr virDomainEventBlockPullNewFromDom(virDomainPtr dom, + const char *path, + int status); int virDomainEventQueuePush(virDomainEventQueuePtr evtQueue, virDomainEventPtr event); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e2e706d..e36f23f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -352,6 +352,8 @@ virDomainWatchdogModelTypeToString; # domain_event.h +virDomainEventBlockPullNewFromObj; +virDomainEventBlockPullNewFromDom; virDomainEventCallbackListAdd; virDomainEventCallbackListAddID; virDomainEventCallbackListCount; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 60e4ee2..3f9afcf 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -893,6 +893,18 @@ int qemuMonitorEmitGraphics(qemuMonitorPtr mon, return ret; } +int qemuMonitorEmitBlockPull(qemuMonitorPtr mon, + const char *diskAlias, + int status) +{ + int ret = -1; + VIR_DEBUG("mon=%p", mon); + + QEMU_MONITOR_CALLBACK(mon, ret, domainBlockPull, mon->vm, + diskAlias, status); + return ret; +} + int qemuMonitorSetCapabilities(qemuMonitorPtr mon) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 62875a3..1a2cd56 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -112,6 +112,10 @@ struct _qemuMonitorCallbacks { const char *authScheme, const char *x509dname, const char *saslUsername); + int (*domainBlockPull)(qemuMonitorPtr mon, + virDomainObjPtr vm, + const char *diskAlias, + int status); }; @@ -173,6 +177,10 @@ int qemuMonitorEmitGraphics(qemuMonitorPtr mon, const char *authScheme, const char *x509dname, const char *saslUsername); +int qemuMonitorEmitBlockPull(qemuMonitorPtr mon, + const char *diskAlias, + int status); + int qemuMonitorStartCPUs(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index bc491e0..dea491f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -56,6 +56,7 @@ static void qemuMonitorJSONHandleIOError(qemuMonitorPtr mon, virJSONValuePtr dat static void qemuMonitorJSONHandleVNCConnect(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleVNCInitialize(qemuMonitorPtr mon, virJSONValuePtr data); static void qemuMonitorJSONHandleVNCDisconnect(qemuMonitorPtr mon, virJSONValuePtr data); +static void qemuMonitorJSONHandleBlockPull(qemuMonitorPtr mon, virJSONValuePtr data); struct { const char *type; @@ -71,6 +72,7 @@ struct { { "VNC_CONNECTED", qemuMonitorJSONHandleVNCConnect, }, { "VNC_INITIALIZED", qemuMonitorJSONHandleVNCInitialize, }, { "VNC_DISCONNECTED", qemuMonitorJSONHandleVNCDisconnect, }, + { "BLOCK_STREAM_COMPLETED", qemuMonitorJSONHandleBlockPull, }, }; @@ -675,6 +677,34 @@ static void qemuMonitorJSONHandleVNCDisconnect(qemuMonitorPtr mon, virJSONValueP qemuMonitorJSONHandleVNC(mon, data, VIR_DOMAIN_EVENT_GRAPHICS_DISCONNECT); } +static void qemuMonitorJSONHandleBlockPull(qemuMonitorPtr mon, virJSONValuePtr data) +{ + const char *device; + unsigned long long offset, len; + int status = VIR_DOMAIN_BLOCK_PULL_FAILED; + + if ((device = virJSONValueObjectGetString(data, "device")) == NULL) { + VIR_WARN0("missing device in disk io error event"); + goto out; + } + + if (virJSONValueObjectGetNumberUlong(data, "offset", &offset) < 0) { + VIR_WARN0("missing offset in block pull event"); + goto out; + } + + if (virJSONValueObjectGetNumberUlong(data, "len", &len) < 0) { + VIR_WARN0("missing len in block pull event"); + goto out; + } + + if (offset != 0 && offset == len) + status = VIR_DOMAIN_BLOCK_PULL_COMPLETED; + +out: + qemuMonitorEmitBlockPull(mon, device, status); +} + int qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index bd7c932..ab48981 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -515,6 +515,33 @@ qemuProcessHandleIOError(qemuMonitorPtr mon ATTRIBUTE_UNUSED, return 0; } +static int +qemuProcessHandleBlockPull(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + const char *diskAlias, + int status) +{ + struct qemud_driver *driver = qemu_driver; + virDomainEventPtr blockPullEvent = NULL; + const char *path; + virDomainDiskDefPtr disk; + + virDomainObjLock(vm); + disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias); + + if (disk) + path = disk->src; + else + path = ""; + blockPullEvent = virDomainEventBlockPullNewFromObj(vm, path, status); + + virDomainObjUnlock(vm); + qemuDriverLock(driver); + qemuDomainEventQueue(driver, blockPullEvent); + qemuDriverUnlock(driver); + + return 0; +} static int qemuProcessHandleGraphics(qemuMonitorPtr mon ATTRIBUTE_UNUSED, @@ -631,6 +658,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .domainWatchdog = qemuProcessHandleWatchdog, .domainIOError = qemuProcessHandleIOError, .domainGraphics = qemuProcessHandleGraphics, + .domainBlockPull = qemuProcessHandleBlockPull, }; static int diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 0a885a9..a07ce2d 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4094,6 +4094,32 @@ remoteDomainReadEventIOErrorReason(virConnectPtr conn, XDR *xdr) return event; } +static virDomainEventPtr +remoteDomainReadEventBlockPull(virConnectPtr conn, XDR *xdr) +{ + remote_domain_event_block_pull_msg msg; + virDomainPtr dom; + virDomainEventPtr event = NULL; + memset (&msg, 0, sizeof msg); + + /* unmarshall parameters, and process it*/ + if (! xdr_remote_domain_event_block_pull_msg(xdr, &msg) ) { + remoteError(VIR_ERR_RPC, "%s", + _("unable to demarshall block_pull event")); + return NULL; + } + + dom = get_nonnull_domain(conn,msg.dom); + if (!dom) + return NULL; + + event = virDomainEventBlockPullNewFromDom(dom, msg.path, msg.status); + xdr_free ((xdrproc_t) &xdr_remote_domain_event_block_pull_msg, (char *) &msg); + + virDomainFree(dom); + return event; +} + static virDomainEventPtr remoteDomainReadEventGraphics(virConnectPtr conn, XDR *xdr) @@ -5603,6 +5629,10 @@ processCallDispatchMessage(virConnectPtr conn, struct private_data *priv, event = remoteDomainReadEventGraphics(conn, xdr); break; + case REMOTE_PROC_DOMAIN_EVENT_BLOCK_PULL: + event = remoteDomainReadEventBlockPull(conn, xdr); + break; + default: VIR_DEBUG("Unexpected event proc %d", hdr->proc); break; diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 2f52ceb..8f4e5e4 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1862,6 +1862,12 @@ struct remote_domain_event_graphics_msg { remote_domain_event_graphics_identity subject<REMOTE_DOMAIN_EVENT_GRAPHICS_IDENTITY_MAX>; }; +struct remote_domain_event_block_pull_msg { + remote_nonnull_domain dom; + remote_nonnull_string path; + int status; +}; + struct remote_domain_managed_save_args { remote_nonnull_domain dom; unsigned flags; @@ -2218,7 +2224,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_BLOCK_PULL_ALL = 211, REMOTE_PROC_DOMAIN_BLOCK_PULL_ABORT = 212, - REMOTE_PROC_DOMAIN_GET_BLOCK_PULL_INFO = 213 + REMOTE_PROC_DOMAIN_GET_BLOCK_PULL_INFO = 213, + REMOTE_PROC_DOMAIN_EVENT_BLOCK_PULL = 214 /* * Notice how the entries are grouped in sets of 10 ? -- 1.7.3

*** Please do not consider for merging, example tests only *** Here are the testcases I am currently running to verify the correctness of this API. These are continuing to evolve. Signed-off-by: Adam Litke <agl@us.ibm.com> --- blockPull-test.py | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 215 insertions(+), 0 deletions(-) create mode 100755 blockPull-test.py diff --git a/blockPull-test.py b/blockPull-test.py new file mode 100755 index 0000000..8433b70 --- /dev/null +++ b/blockPull-test.py @@ -0,0 +1,215 @@ +#!/usr/bin/env python + +import sys +import subprocess +import time +import unittest +import re +import threading +import libvirt + +qemu_img_bin = "/home/aglitke/src/qemu/qemu-img" + +dom_xml = """ +<domain type='kvm'> + <name>blockPull-test</name> + <memory>131072</memory> + <currentMemory>131072</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='x86_64' machine='pc-0.13'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/home/aglitke/src/qemu/x86_64-softmmu/qemu-system-x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qed'/> + <source file='/tmp/disk1.qed' /> + <target dev='vda' bus='virtio'/> + </disk> + <disk type='file' device='disk'> + <driver name='qemu' type='qed'/> + <source file='/tmp/disk2.qed' /> + <target dev='vdb' bus='virtio'/> + </disk> + <graphics type='vnc' port='-1' autoport='yes'/> + </devices> +</domain> +""" + +def qemu_img(*args): + global qemu_img_bin + + devnull = open('/dev/null', 'r+') + return subprocess.call([qemu_img_bin] + list(args), stdin=devnull, stdout=devnull) + +def make_baseimage(name, size_mb): + devnull = open('/dev/null', 'r+') + return subprocess.call(['dd', 'if=/dev/zero', "of=%s" % name, 'bs=1M', + 'count=%i' % size_mb], stdin=devnull, stdout=devnull, stderr=devnull) + +def has_backing_file(path): + global qemu_img_bin + p1 = subprocess.Popen([qemu_img_bin, "info", path], + stdout=subprocess.PIPE).communicate()[0] + matches = re.findall("^backing file:", p1, re.M) + if len(matches) > 0: + return True + return False + +class BlockPullTestCase(unittest.TestCase): + def _error_handler(self, ctx, error, dummy=None): + pass + + def create_disks(self, sparse): + self.disks = [ '/tmp/disk1.qed', '/tmp/disk2.qed' ] + if sparse: + qemu_img('create', '-f', 'raw', '/tmp/backing1.img', '100M') + qemu_img('create', '-f', 'raw', '/tmp/backing2.img', '100M') + else: + make_baseimage('/tmp/backing1.img', 100) + make_baseimage('/tmp/backing2.img', 100) + qemu_img('create', '-f', 'qed', '-o', 'backing_file=/tmp/backing1.img,copy_on_read=on', self.disks[0]) + qemu_img('create', '-f', 'qed', '-o', 'backing_file=/tmp/backing2.img,copy_on_read=on', self.disks[1]) + + def begin(self, sparse=True): + global dom_xml + + libvirt.registerErrorHandler(self._error_handler, None) + self.create_disks(sparse) + self.conn = libvirt.open('qemu:///system') + self.dom = self.conn.createXML(dom_xml, 0) + + def end(self): + self.dom.destroy() + self.conn.close() + +class TestBasicErrors(BlockPullTestCase): + def setUp(self): + self.begin() + + def tearDown(self): + self.end() + + def test_bad_path(self): + try: + self.dom.blockPull('/dev/null', 0) + except libvirt.libvirtError, e: + self.assertEqual(libvirt.VIR_ERR_INVALID_ARG, e.get_error_code()) + else: + e = self.conn.virConnGetLastError() + self.assertEqual(libvirt.VIR_ERR_INVALID_ARG, e[0]) + + def test_abort_no_stream(self): + try: + self.dom.blockPullAbort(self.disks[0], 0) + except libvirt.libvirtError, e: + self.assertEqual(libvirt.VIR_ERR_INTERNAL_ERROR, e.get_error_code()) + else: + e = self.conn.virConnGetLastError() + self.assertEqual(libvirt.VIR_ERR_INTERNAL_ERROR, e[0]) + +class TestBasicCommands(BlockPullTestCase): + def setUp(self): + pass + + def tearDown(self): + self.end() + + def test_start_stop(self): + self.begin(sparse=False) + self.dom.blockPullAll(self.disks[0], 0) + time.sleep(1) + self.assertIsNot(None, self.dom.blockPullInfo(self.disks[0], 0)) + self.dom.blockPullAbort(self.disks[0], 0) + time.sleep(1) + self.assertIs(None, self.dom.blockPullInfo(self.disks[0], 0)) + + self.dom.blockPull(self.disks[0], 0) + info = self.dom.blockPullInfo(self.disks[0], 0) + self.assertNotEqual(info['cur'], 0) + + def test_whole_disk(self): + self.begin() + self.assertTrue(has_backing_file(self.disks[0])) + self.dom.blockPullAll(self.disks[0], 0) + while self.dom.blockPullInfo(self.disks[0], 0) is not None: + time.sleep(1) + self.assertFalse(has_backing_file(self.disks[0])) + + def test_incremental(self): + self.begin() + last_cur = -1 + while True: + info = self.dom.blockPull(self.disks[0], 0) + if info is None or info['cur'] == info['end']: + break + self.assertGreater(info['cur'], last_cur) + last_cur = info['cur'] + self.assertFalse(has_backing_file(self.disks[0])) + +class TestEvents(BlockPullTestCase): + def eventLoopRun(self): + while self.do_events: + libvirt.virEventRunDefaultImpl() + + def eventLoopStart(self): + libvirt.virEventRegisterDefaultImpl() + self.eventLoopThread = threading.Thread(target=self.eventLoopRun, name="libvirtEventLoop") + self.eventLoopThread.setDaemon(True) + self.do_events = True + self.eventLoopThread.start() + + def eventLoopStop(self): + self.do_events = False + + def setUp(self): + self.eventLoopStart() + + def tearDown(self): + self.end() + + @staticmethod + def recordBlockPullEvent(conn, dom, path, status, inst): + inst.event = (dom, path, status) + + def test_event_complete(self): + self.begin() + self.event = None + self.conn.domainEventRegisterAny(self.dom, libvirt.VIR_DOMAIN_EVENT_ID_BLOCK_PULL, + TestEvents.recordBlockPullEvent, self) + self.dom.blockPullAll(self.disks[0], 0) + for i in xrange(1, 5): + if self.event is not None: + break + time.sleep(1) + self.eventLoopStop() + self.assertIsNot(None, self.event) + self.assertFalse(has_backing_file(self.disks[0])) + self.assertEqual(self.event[1], self.disks[0]) + self.assertEqual(self.event[2], libvirt.VIR_DOMAIN_BLOCK_PULL_COMPLETED) + +if __name__ == '__main__': + unittest.main() + + + + + + + + + + + + + -- 1.7.3

Set up the types for the block pull functions and insert them into the virDriver structure definition. Because of static initializers, update every driver and set the new fields to NULL. * include/libvirt/libvirt.h.in: new API * src/driver.h src/*/*_driver.c src/vbox/vbox_tmpl.c: add the new entry to the driver structure * python/generator.py: fix compiler errors, the actual python bindings are implemented later Signed-off-by: Adam Litke <agl@us.ibm.com> --- include/libvirt/libvirt.h.in | 92 ++++++++++++++++++++++++++++++++++++++++++ python/generator.py | 3 + src/driver.h | 21 ++++++++++ src/esx/esx_driver.c | 4 ++ src/lxc/lxc_driver.c | 4 ++ src/openvz/openvz_driver.c | 4 ++ src/phyp/phyp_driver.c | 4 ++ src/qemu/qemu_driver.c | 4 ++ src/remote/remote_driver.c | 4 ++ src/test/test_driver.c | 4 ++ src/uml/uml_driver.c | 4 ++ src/vbox/vbox_tmpl.c | 4 ++ src/xen/xen_driver.c | 4 ++ 13 files changed, 156 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5783303..9af1b76 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1145,6 +1145,98 @@ int virDomainUpdateDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags); /* + * BlockPull API + */ + +/* An iterator for initiating and monitoring block pull operations */ +typedef unsigned long long virDomainBlockPullCursor; + +typedef struct _virDomainBlockPullInfo virDomainBlockPullInfo; +struct _virDomainBlockPullInfo { + /* + * The following fields provide an indication of block pull progress. @cur + * indicates the current position and will be between 0 and @end. @end is + * the final cursor position for this operation and represents completion. + * To approximate progress, divide @cur by @end. + */ + virDomainBlockPullCursor cur; + virDomainBlockPullCursor end; +}; +typedef virDomainBlockPullInfo *virDomainBlockPullInfoPtr; + +/** + * virDomainBlockPull: + * @dom: pointer to domain object + * @path: Fully-qualified filename of disk + * @info: A pointer to a virDomainBlockPullInfo structure, or NULL + * @flags: currently unused, for future extension + * + * Populate a disk image with data from its backing image. Once all data from + * its backing image has been pulled, the disk no longer depends on a backing + * image. This function works incrementally, performing a small amount of work + * each time it is called. When successful, @info is updated with the current + * progress. + * + * Returns -1 in case of failure, 0 when successful. + */ +int virDomainBlockPull(virDomainPtr dom, + const char *path, + virDomainBlockPullInfoPtr info, + unsigned int flags); + +/** + * virDomainBlockPullAll: + * @dom: pointer to domain object + * @path: Fully-qualified filename of disk + * @flags: currently unused, for future extension + * + * Populate a disk image with data from its backing image. Once all data from + * its backing image has been pulled, the disk no longer depends on a backing + * image. This function pulls data for the entire device in the background. + * Progress of the operation can be checked with virDomainGetBlockPullInfo() and + * the operation can be aborted with virDomainBlockPullAbort(). When finished, + * an asynchronous event is raised to indicate the final status. + * + * Returns 0 if the operation has started, -1 on failure. + */ +int virDomainBlockPullAll(virDomainPtr dom, + const char *path, + unsigned int flags); + +/** + * virDomainBlockPullAbort: + * @dom: pointer to domain object + * @path: fully-qualified filename of disk + * @flags: currently unused, for future extension + * + * Cancel a pull operation previously started by virDomainBlockPullAll(). + * + * Returns -1 in case of failure, 0 when successful. + */ +int virDomainBlockPullAbort(virDomainPtr dom, + const char *path, + unsigned int flags); + +/** + * virDomainGetBlockPullInfo: + * @dom: pointer to domain object + * @path: fully-qualified filename of disk + * @info: pointer to a virDomainBlockPullInfo structure + * @flags: currently unused, for future extension + * + * Request progress information on a block pull operation that has been started + * with virDomainBlockPullAll(). If an operation is active for the given + * parameters, @info will be updated with the current progress. + * + * Returns -1 in case of failure, 0 when successful. + */ +int virDomainGetBlockPullInfo(virDomainPtr dom, + const char *path, + virDomainBlockPullInfoPtr info, + unsigned int flags); + + +/* * NUMA support */ diff --git a/python/generator.py b/python/generator.py index 4fa4f65..52be85d 100755 --- a/python/generator.py +++ b/python/generator.py @@ -166,6 +166,8 @@ def enum(type, name, value): functions_failed = [] functions_skipped = [ "virConnectListDomains", + 'virDomainBlockPull', + 'virDomainGetBlockPullInfo', ] skipped_modules = { @@ -180,6 +182,7 @@ skipped_types = { 'virConnectDomainEventIOErrorCallback': "No function types in python", 'virConnectDomainEventGraphicsCallback': "No function types in python", 'virEventAddHandleFunc': "No function types in python", + 'virDomainBlockPullInfoPtr': "Not implemented yet", } ####################################################################### diff --git a/src/driver.h b/src/driver.h index a8b79e6..3a0391d 100644 --- a/src/driver.h +++ b/src/driver.h @@ -515,6 +515,23 @@ typedef int virStreamPtr st, unsigned int flags); +typedef int + (*virDrvDomainBlockPull)(virDomainPtr dom, const char *path, + virDomainBlockPullInfoPtr info, + unsigned int flags); + +typedef int + (*virDrvDomainBlockPullAll)(virDomainPtr dom, const char *path, + unsigned int flags); + +typedef int + (*virDrvDomainBlockPullAbort)(virDomainPtr dom, const char *path, + unsigned int flags); + +typedef int + (*virDrvDomainGetBlockPullInfo)(virDomainPtr dom, const char *path, + virDomainBlockPullInfoPtr info, + unsigned int flags); /** * _virDriver: @@ -639,6 +656,10 @@ struct _virDriver { virDrvDomainSnapshotDelete domainSnapshotDelete; virDrvQemuDomainMonitorCommand qemuDomainMonitorCommand; virDrvDomainOpenConsole domainOpenConsole; + virDrvDomainBlockPull domainBlockPull; + virDrvDomainBlockPullAll domainBlockPullAll; + virDrvDomainBlockPullAbort domainBlockPullAbort; + virDrvDomainGetBlockPullInfo domainGetBlockPullInfo; }; typedef int diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 7933f11..adf75b9 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -4699,6 +4699,10 @@ static virDriver esxDriver = { esxDomainSnapshotDelete, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ NULL, /* domainOpenConsole */ + NULL, /* domainBlockPull */ + NULL, /* domainBlockPullAll */ + NULL, /* domainBlockPullAbort */ + NULL, /* domainGetBlockPullInfo */ }; diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index b94941d..7d3eb7a 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2905,6 +2905,10 @@ static virDriver lxcDriver = { NULL, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ lxcDomainOpenConsole, /* domainOpenConsole */ + NULL, /* domainBlockPull */ + NULL, /* domainBlockPullAll */ + NULL, /* domainBlockPullAbort */ + NULL, /* domainGetBlockPullInfo */ }; static virStateDriver lxcStateDriver = { diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 0bd007a..fc3f7f0 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1667,6 +1667,10 @@ static virDriver openvzDriver = { NULL, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ NULL, /* domainOpenConsole */ + NULL, /* domainBlockPull */ + NULL, /* domainBlockPullAll */ + NULL, /* domainBlockPullAbort */ + NULL, /* domainGetBlockPullInfo */ }; int openvzRegister(void) { diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 30d4adf..817d570 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3828,6 +3828,10 @@ static virDriver phypDriver = { NULL, /* domainSnapshotDelete */ NULL, /* qemuMonitorCommand */ NULL, /* domainOpenConsole */ + NULL, /* domainBlockPull */ + NULL, /* domainBlockPullAll */ + NULL, /* domainBlockPullAbort */ + NULL, /* domainGetBlockPullInfo */ }; static virStorageDriver phypStorageDriver = { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0fd0f10..cfbe199 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7192,6 +7192,10 @@ static virDriver qemuDriver = { qemuDomainSnapshotDelete, /* domainSnapshotDelete */ qemuDomainMonitorCommand, /* qemuDomainMonitorCommand */ qemuDomainOpenConsole, /* domainOpenConsole */ + NULL, /* domainBlockPull */ + NULL, /* domainBlockPullAll */ + NULL, /* domainBlockPullAbort */ + NULL, /* domainGetBlockPullInfo */ }; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index d076a90..07bc629 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6493,6 +6493,10 @@ static virDriver remote_driver = { remoteDomainSnapshotDelete, /* domainSnapshotDelete */ remoteQemuDomainMonitorCommand, /* qemuDomainMonitorCommand */ remoteDomainOpenConsole, /* domainOpenConsole */ + NULL, /* domainBlockPull */ + NULL, /* domainBlockPullAll */ + NULL, /* domainBlockPullAbort */ + NULL, /* domainGetBlockPullInfo */ }; static virNetworkDriver network_driver = { diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 0978214..b35183d 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5447,6 +5447,10 @@ static virDriver testDriver = { NULL, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ NULL, /* domainOpenConsole */ + NULL, /* domainBlockPull */ + NULL, /* domainBlockPullAll */ + NULL, /* domainBlockPullAbort */ + NULL, /* domainGetBlockPullInfo */ }; static virNetworkDriver testNetworkDriver = { diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 33849a0..86356d5 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -2253,6 +2253,10 @@ static virDriver umlDriver = { NULL, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ umlDomainOpenConsole, /* domainOpenConsole */ + NULL, /* domainBlockPull */ + NULL, /* domainBlockPullAll */ + NULL, /* domainBlockPullAbort */ + NULL, /* domainGetBlockPullInfo */ }; static int diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 8241d34..5c0da44 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -8652,6 +8652,10 @@ virDriver NAME(Driver) = { vboxDomainSnapshotDelete, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ NULL, /* domainOpenConsole */ + NULL, /* domainBlockPull */ + NULL, /* domainBlockPullAll */ + NULL, /* domainBlockPullAbort */ + NULL, /* domainGetBlockPullInfo */ }; virNetworkDriver NAME(NetworkDriver) = { diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index dd94fbc..f218222 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -2208,6 +2208,10 @@ static virDriver xenUnifiedDriver = { NULL, /* domainSnapshotDelete */ NULL, /* qemuDomainMonitorCommand */ xenUnifiedDomainOpenConsole, /* domainOpenConsole */ + NULL, /* domainBlockPull */ + NULL, /* domainBlockPullAll */ + NULL, /* domainBlockPullAbort */ + NULL, /* domainGetBlockPullInfo */ }; /** -- 1.7.3

Sorry, this one was left off from the initial post. It comes first in the series. -- Adam Litke IBM Linux Technology Center

2011/6/1 Adam Litke <agl@us.ibm.com>:
Set up the types for the block pull functions and insert them into the virDriver structure definition. Because of static initializers, update every driver and set the new fields to NULL.
* include/libvirt/libvirt.h.in: new API * src/driver.h src/*/*_driver.c src/vbox/vbox_tmpl.c: add the new entry to the driver structure * python/generator.py: fix compiler errors, the actual python bindings are implemented later
Signed-off-by: Adam Litke <agl@us.ibm.com> --- include/libvirt/libvirt.h.in | 92 ++++++++++++++++++++++++++++++++++++++++++ python/generator.py | 3 + src/driver.h | 21 ++++++++++ src/esx/esx_driver.c | 4 ++ src/lxc/lxc_driver.c | 4 ++ src/openvz/openvz_driver.c | 4 ++ src/phyp/phyp_driver.c | 4 ++ src/qemu/qemu_driver.c | 4 ++ src/remote/remote_driver.c | 4 ++ src/test/test_driver.c | 4 ++ src/uml/uml_driver.c | 4 ++ src/vbox/vbox_tmpl.c | 4 ++ src/xen/xen_driver.c | 4 ++
Changing all the other driver files will go away when you rebase to git head because of named initializers.
13 files changed, 156 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5783303..9af1b76 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1145,6 +1145,98 @@ int virDomainUpdateDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags);
/* + * BlockPull API + */ + +/* An iterator for initiating and monitoring block pull operations */ +typedef unsigned long long virDomainBlockPullCursor; + +typedef struct _virDomainBlockPullInfo virDomainBlockPullInfo; +struct _virDomainBlockPullInfo { + /* + * The following fields provide an indication of block pull progress. @cur + * indicates the current position and will be between 0 and @end. @end is + * the final cursor position for this operation and represents completion. + * To approximate progress, divide @cur by @end. + */ + virDomainBlockPullCursor cur; + virDomainBlockPullCursor end; +}; +typedef virDomainBlockPullInfo *virDomainBlockPullInfoPtr; + +/** + * virDomainBlockPull: + * @dom: pointer to domain object + * @path: Fully-qualified filename of disk + * @info: A pointer to a virDomainBlockPullInfo structure, or NULL + * @flags: currently unused, for future extension + * + * Populate a disk image with data from its backing image. Once all data from + * its backing image has been pulled, the disk no longer depends on a backing + * image. This function works incrementally, performing a small amount of work + * each time it is called. When successful, @info is updated with the current + * progress. + * + * Returns -1 in case of failure, 0 when successful. + */ +int virDomainBlockPull(virDomainPtr dom, + const char *path, + virDomainBlockPullInfoPtr info, + unsigned int flags); +
You're documenting the functions twice, in libvirt.h and in libvirt.c. Typically the functions are just documented in libvirt.c. Matthias
participants (2)
-
Adam Litke
-
Matthias Bolte