[libvirt] [PATCH] blockstats: support lookup by path in blockstats
Daniel Veillard
veillard at redhat.com
Wed Nov 23 06:05:20 UTC 2011
On Tue, Nov 22, 2011 at 04:42:33PM -0700, Eric Blake wrote:
> Commit 89b6284f made it possible to pass either a source name or
> the target device to most API demanding a disk designation, but
> forgot to update the documentation. It also failed to update
> virDomainBlockStats to take both forms. This patch fixes both the
> documentation and the remaining function.
>
> Xen continues to use just device shorthand (that is, I did not
> implement path lookup there, since xen does not track a domain_conf
> to quickly tie a path back to the device shorthand).
>
> * src/libvirt.c (virDomainBlockStats, virDomainBlockStatsFlags)
> (virDomainGetBlockInfo, virDomainBlockPeek)
> (virDomainBlockJobAbort, virDomainGetBlockJobInfo)
> (virDomainBlockJobSetSpeed, virDomainBlockPull): Document
> acceptable disk naming conventions.
> * src/qemu/qemu_driver.c (qemuDomainBlockStats)
> (qemuDomainBlockStatsFlags): Allow lookup by source name.
> * src/test/test_driver.c (testDomainBlockStats): Likewise.
> ---
>
> I would like to apply this prior to patch 1/8 in Lei's series:
> https://www.redhat.com/archives/libvir-list/2011-November/msg01145.html
> since that patch should be using the same copy-and-paste documentation.
>
> src/libvirt.c | 82 ++++++++++++++++++++++++++++++++++++-----------
> src/qemu/qemu_driver.c | 20 ++---------
> src/test/test_driver.c | 11 +-----
> 3 files changed, 69 insertions(+), 44 deletions(-)
>
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 17e073e..811dde6 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -6659,16 +6659,19 @@ error:
> /**
> * virDomainBlockStats:
> * @dom: pointer to the domain object
> - * @path: path to the block device
> + * @path: path to the block device, or device shorthand
> * @stats: block device stats (returned)
> * @size: size of stats structure
> *
> * This function returns block device (disk) stats for block
> * devices attached to the domain.
> *
> - * The path parameter is the name of the block device. Get this
> - * by calling virDomainGetXMLDesc and finding the <target dev='...'>
> - * attribute within //domain/devices/disk. (For example, "xvda").
> + * The @path parameter is either the device target shorthand (the
> + * <target dev='...'/> sub-element, such as "xvda"), or (since 0.9.8)
> + * an unambiguous source name of the block device (the <source
> + * file='...'/> sub-element, such as "/path/to/image"). Valid names
> + * can be found by calling virDomainGetXMLDesc() and inspecting
> + * elements within //domain/devices/disk.
> *
> * Domains may have more than one block device. To get stats for
> * each you should make multiple calls to this function.
> @@ -6719,7 +6722,7 @@ error:
> /**
> * virDomainBlockStatsFlags:
> * @dom: pointer to domain object
> - * @path: path to the block device
> + * @path: path to the block device, or device shorthand
> * @params: pointer to block stats parameter object
> * (return value)
> * @nparams: pointer to number of block stats; input and output
> @@ -6728,9 +6731,12 @@ error:
> * This function is to get block stats parameters for block
> * devices attached to the domain.
> *
> - * The @path is the name of the block device. Get this
> - * by calling virDomainGetXMLDesc and finding the <target dev='...'>
> - * attribute within //domain/devices/disk. (For example, "xvda").
> + * The @path parameter is either the device target shorthand (the
> + * <target dev='...'/> sub-element, such as "xvda"), or (since 0.9.8)
> + * an unambiguous source name of the block device (the <source
> + * file='...'/> sub-element, such as "/path/to/image"). Valid names
> + * can be found by calling virDomainGetXMLDesc() and inspecting
> + * elements within //domain/devices/disk.
> *
> * Domains may have more than one block device. To get stats for
> * each you should make multiple calls to this function.
> @@ -6927,7 +6933,7 @@ error:
> /**
> * virDomainBlockPeek:
> * @dom: pointer to the domain object
> - * @path: path to the block device
> + * @path: path to the block device, or device shorthand
> * @offset: offset within block device
> * @size: size to read
> * @buffer: return buffer (must be at least size bytes)
> @@ -6946,10 +6952,12 @@ error:
> * remote case, nor if you don't have sufficient permission.
> * Hence the need for this call).
> *
> - * 'path' must be a device or file corresponding to the domain.
> - * In other words it must be the precise string returned in
> - * a <disk><source dev='...'/></disk> from
> - * virDomainGetXMLDesc.
> + * The @path parameter is either an unambiguous source name of the
> + * block device (the <source file='...'/> sub-element, such as
> + * "/path/to/image"), or (since 0.9.5) the device target shorthand
> + * (the <target dev='...'/> sub-element, such as "xvda"). Valid names
> + * can be found by calling virDomainGetXMLDesc() and inspecting
> + * elements within //domain/devices/disk.
> *
> * 'offset' and 'size' represent an area which must lie entirely
> * within the device or file. 'size' may be 0 to test if the
> @@ -7133,16 +7141,24 @@ error:
> /**
> * virDomainGetBlockInfo:
> * @domain: a domain object
> - * @path: path to the block device or file
> + * @path: path to the block device, or device shorthand
> * @info: pointer to a virDomainBlockInfo structure allocated by the user
> * @flags: currently unused, pass zero
> *
> * Extract information about a domain's block device.
> *
> + * The @path parameter is either an unambiguous source name of the
> + * block device (the <source file='...'/> sub-element, such as
> + * "/path/to/image"), or (since 0.9.5) the device target shorthand
> + * (the <target dev='...'/> sub-element, such as "xvda"). Valid names
> + * can be found by calling virDomainGetXMLDesc() and inspecting
> + * elements within //domain/devices/disk.
> + *
> * Returns 0 in case of success and -1 in case of failure.
> */
> int
> -virDomainGetBlockInfo(virDomainPtr domain, const char *path, virDomainBlockInfoPtr info, unsigned int flags)
> +virDomainGetBlockInfo(virDomainPtr domain, const char *path,
> + virDomainBlockInfoPtr info, unsigned int flags)
> {
> virConnectPtr conn;
>
> @@ -16837,11 +16853,18 @@ error:
> /**
> * virDomainBlockJobAbort:
> * @dom: pointer to domain object
> - * @path: fully-qualified filename of disk
> + * @path: path to the block device, or device shorthand
> * @flags: currently unused, for future extension
> *
> * Cancel the active block job on the given disk.
> *
> + * The @path parameter is either an unambiguous source name of the
> + * block device (the <source file='...'/> sub-element, such as
> + * "/path/to/image"), or (since 0.9.5) the device target shorthand
> + * (the <target dev='...'/> sub-element, such as "xvda"). Valid names
> + * can be found by calling virDomainGetXMLDesc() and inspecting
> + * elements within //domain/devices/disk.
> + *
> * Returns -1 in case of failure, 0 when successful.
> */
> int virDomainBlockJobAbort(virDomainPtr dom, const char *path,
> @@ -16889,13 +16912,20 @@ error:
> /**
> * virDomainGetBlockJobInfo:
> * @dom: pointer to domain object
> - * @path: fully-qualified filename of disk
> + * @path: path to the block device, or device shorthand
> * @info: pointer to a virDomainBlockJobInfo structure
> * @flags: currently unused, for future extension
> *
> * Request block job information for the given disk. If an operation is active
> * @info will be updated with the current progress.
> *
> + * The @path parameter is either an unambiguous source name of the
> + * block device (the <source file='...'/> sub-element, such as
> + * "/path/to/image"), or (since 0.9.5) the device target shorthand
> + * (the <target dev='...'/> sub-element, such as "xvda"). Valid names
> + * can be found by calling virDomainGetXMLDesc() and inspecting
> + * elements within //domain/devices/disk.
> + *
> * Returns -1 in case of failure, 0 when nothing found, 1 when info was found.
> */
> int virDomainGetBlockJobInfo(virDomainPtr dom, const char *path,
> @@ -16944,13 +16974,20 @@ error:
> /**
> * virDomainBlockJobSetSpeed:
> * @dom: pointer to domain object
> - * @path: fully-qualified filename of disk
> + * @path: path to the block device, or device shorthand
> * @bandwidth: specify bandwidth limit in Mbps
> * @flags: currently unused, for future extension
> *
> * Set the maximimum allowable bandwidth that a block job may consume. If
> * bandwidth is 0, the limit will revert to the hypervisor default.
> *
> + * The @path parameter is either an unambiguous source name of the
> + * block device (the <source file='...'/> sub-element, such as
> + * "/path/to/image"), or (since 0.9.5) the device target shorthand
> + * (the <target dev='...'/> sub-element, such as "xvda"). Valid names
> + * can be found by calling virDomainGetXMLDesc() and inspecting
> + * elements within //domain/devices/disk.
> + *
> * Returns -1 in case of failure, 0 when successful.
> */
> int virDomainBlockJobSetSpeed(virDomainPtr dom, const char *path,
> @@ -16999,7 +17036,7 @@ error:
> /**
> * virDomainBlockPull:
> * @dom: pointer to domain object
> - * @path: Fully-qualified filename of disk
> + * @path: path to the block device, or device shorthand
> * @bandwidth: (optional) specify copy bandwidth limit in Mbps
> * @flags: currently unused, for future extension
> *
> @@ -17010,6 +17047,13 @@ error:
> * the operation can be aborted with virDomainBlockJobAbort(). When finished,
> * an asynchronous event is raised to indicate the final status.
> *
> + * The @path parameter is either an unambiguous source name of the
> + * block device (the <source file='...'/> sub-element, such as
> + * "/path/to/image"), or (since 0.9.5) the device target shorthand
> + * (the <target dev='...'/> sub-element, such as "xvda"). Valid names
> + * can be found by calling virDomainGetXMLDesc() and inspecting
> + * elements within //domain/devices/disk.
> + *
> * The maximum bandwidth (in Mbps) that will be used to do the copy can be
> * specified with the bandwidth parameter. If set to 0, libvirt will choose a
> * suitable default. Some hypervisors do not support this feature and will
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index fe2ab85..98ce695 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7082,18 +7082,12 @@ qemuDomainBlockStats(virDomainPtr dom,
> goto cleanup;
> }
>
> - for (i = 0 ; i < vm->def->ndisks ; i++) {
> - if (STREQ(path, vm->def->disks[i]->dst)) {
> - disk = vm->def->disks[i];
> - break;
> - }
> - }
> -
> - if (!disk) {
> + if ((i = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
> qemuReportError(VIR_ERR_INVALID_ARG,
> _("invalid path: %s"), path);
> goto cleanup;
> }
> + disk = vm->def->disks[i];
>
> if (!disk->info.alias) {
> qemuReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -7174,18 +7168,12 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
> }
>
> if (*nparams != 0) {
> - for (i = 0 ; i < vm->def->ndisks ; i++) {
> - if (STREQ(path, vm->def->disks[i]->dst)) {
> - disk = vm->def->disks[i];
> - break;
> - }
> - }
> -
> - if (!disk) {
> + if ((i = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
> qemuReportError(VIR_ERR_INVALID_ARG,
> _("invalid path: %s"), path);
> goto cleanup;
> }
> + disk = vm->def->disks[i];
>
> if (!disk->info.alias) {
> qemuReportError(VIR_ERR_INTERNAL_ERROR,
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index e6ff696..f365bf4 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -2803,7 +2803,7 @@ static int testDomainBlockStats(virDomainPtr domain,
> virDomainObjPtr privdom;
> struct timeval tv;
> unsigned long long statbase;
> - int i, found = 0, ret = -1;
> + int ret = -1;
>
> testDriverLock(privconn);
> privdom = virDomainFindByName(&privconn->domains,
> @@ -2815,14 +2815,7 @@ static int testDomainBlockStats(virDomainPtr domain,
> goto error;
> }
>
> - for (i = 0 ; i < privdom->def->ndisks ; i++) {
> - if (STREQ(path, privdom->def->disks[i]->dst)) {
> - found = 1;
> - break;
> - }
> - }
> -
> - if (!found) {
> + if (virDomainDiskIndexByName(privdom->def, path, false) < 0) {
> testError(VIR_ERR_INVALID_ARG,
> _("invalid path: %s"), path);
> goto error;
ACK, looks good, and nice cleanup too
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel at veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
More information about the libvir-list
mailing list