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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list