From 8de6544e98992cf8203b9e246409451e1583dbbe Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 16 Dec 2014 23:23:24 -0700 Subject: [PATCH] qemu: refactor blockinfo data gathering Create a helper function that can be reused for gathering block info from virDomainListGetStats. * src/qemu/qemu_driver.c (qemuDomainGetBlockInfo): Split guts... (qemuStorageLimitsRefresh): ...into new helper function. Signed-off-by: Eric Blake --- src/qemu/qemu_driver.c | 170 +++++++++++++++++++++++------------------ 1 file changed, 94 insertions(+), 76 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f7373a5a68..c23136fc1c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11003,67 +11003,24 @@ qemuDomainMemoryPeek(virDomainPtr dom, } +/* Refresh the capacity and allocation limits of a given storage + * source. Assumes that the caller has already obtained a domain + * job. */ static int -qemuDomainGetBlockInfo(virDomainPtr dom, - const char *path, - virDomainBlockInfoPtr info, - unsigned int flags) +qemuStorageLimitsRefresh(virQEMUDriverPtr driver, + virQEMUDriverConfigPtr cfg, + virDomainObjPtr vm, + virStorageSourcePtr src) { - virQEMUDriverPtr driver = dom->conn->privateData; - virDomainObjPtr vm; int ret = -1; int fd = -1; off_t end; virStorageSourcePtr meta = NULL; - virDomainDiskDefPtr disk = NULL; - virStorageSourcePtr src; struct stat sb; - int idx; int format; - bool activeFail = false; - virQEMUDriverConfigPtr cfg = NULL; char *buf = NULL; ssize_t len; - virCheckFlags(0, -1); - - if (!(vm = qemuDomObjFromDomain(dom))) - return -1; - - cfg = virQEMUDriverGetConfig(driver); - - if (virDomainGetBlockInfoEnsureACL(dom->conn, vm->def) < 0) - goto cleanup; - - if (!path || path[0] == '\0') { - virReportError(VIR_ERR_INVALID_ARG, "%s", _("NULL or empty path")); - goto cleanup; - } - - /* Technically, we only need a job if we are going to query the - * monitor, which is only for active domains that are using - * non-raw block devices. But it is easier to share code if we - * always grab a job; furthermore, grabbing the job ensures that - * hot-plug won't change disk behind our backs. */ - if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) - goto cleanup; - - /* Check the path belongs to this domain. */ - if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) { - virReportError(VIR_ERR_INVALID_ARG, - _("invalid path %s not assigned to domain"), path); - goto endjob; - } - - disk = vm->def->disks[idx]; - src = disk->src; - if (virStorageSourceIsEmpty(src)) { - virReportError(VIR_ERR_INVALID_ARG, - _("disk '%s' does not currently have a source assigned"), - path); - goto endjob; - } - /* FIXME: For an offline domain, we always want to check current * on-disk statistics (as users have been known to change offline * images behind our backs). For a running domain, however, it @@ -11087,31 +11044,31 @@ qemuDomainGetBlockInfo(virDomainPtr dom, if (virStorageSourceIsLocalStorage(src)) { if ((fd = qemuOpenFile(driver, vm, src->path, O_RDONLY, NULL, NULL)) == -1) - goto endjob; + goto cleanup; if (fstat(fd, &sb) < 0) { virReportSystemError(errno, _("cannot stat file '%s'"), src->path); - goto endjob; + goto cleanup; } if ((len = virFileReadHeaderFD(fd, VIR_STORAGE_MAX_HEADER, &buf)) < 0) { virReportSystemError(errno, _("cannot read header '%s'"), src->path); - goto endjob; + goto cleanup; } } else { if (virStorageFileInitAs(src, cfg->user, cfg->group) < 0) - goto endjob; + goto cleanup; if ((len = virStorageFileReadHeader(src, VIR_STORAGE_MAX_HEADER, &buf)) < 0) - goto endjob; + goto cleanup; if (virStorageFileStat(src, &sb) < 0) { virReportSystemError(errno, _("failed to stat remote file '%s'"), NULLSTR(src->path)); - goto endjob; + goto cleanup; } } @@ -11137,8 +11094,8 @@ qemuDomainGetBlockInfo(virDomainPtr dom, end = lseek(fd, 0, SEEK_END); if (end == (off_t)-1) { virReportSystemError(errno, - _("failed to seek to end of %s"), path); - goto endjob; + _("failed to seek to end of %s"), src->path); + goto cleanup; } src->physical = end; src->allocation = end; @@ -11151,31 +11108,102 @@ qemuDomainGetBlockInfo(virDomainPtr dom, if (!cfg->allowDiskFormatProbing) { virReportError(VIR_ERR_INTERNAL_ERROR, _("no disk format for %s and probing is disabled"), - path); - goto endjob; + src->path); + goto cleanup; } if ((format = virStorageFileProbeFormatFromBuf(src->path, buf, len)) < 0) - goto endjob; + goto cleanup; } if (!(meta = virStorageFileGetMetadataFromBuf(src->path, buf, len, format, NULL))) - goto endjob; + goto cleanup; if (format == VIR_STORAGE_FILE_RAW) src->capacity = src->physical; else if ((meta = virStorageFileGetMetadataFromBuf(src->path, buf, len, format, NULL))) src->capacity = meta->capacity ? meta->capacity : src->physical; else - goto endjob; + goto cleanup; - /* If guest is not using raw disk format and on a block device, - * then query highest allocated extent from QEMU + /* If guest is not using raw disk format and is on a host block + * device, then leave the value unspecified, so caller knows to + * query the highest allocated extent from QEMU */ if (virStorageSourceGetActualType(src) == VIR_STORAGE_TYPE_BLOCK && format != VIR_STORAGE_FILE_RAW && - S_ISBLK(sb.st_mode)) { + S_ISBLK(sb.st_mode)) + src->allocation = 0; + + ret = 0; + cleanup: + VIR_FREE(buf); + virStorageSourceFree(meta); + VIR_FORCE_CLOSE(fd); + virStorageFileDeinit(src); + return ret; +} + + +static int +qemuDomainGetBlockInfo(virDomainPtr dom, + const char *path, + virDomainBlockInfoPtr info, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + virDomainDiskDefPtr disk = NULL; + virStorageSourcePtr src; + int idx; + bool activeFail = false; + virQEMUDriverConfigPtr cfg = NULL; + + virCheckFlags(0, -1); + + if (!(vm = qemuDomObjFromDomain(dom))) + return -1; + + cfg = virQEMUDriverGetConfig(driver); + + if (virDomainGetBlockInfoEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (!path || path[0] == '\0') { + virReportError(VIR_ERR_INVALID_ARG, "%s", _("NULL or empty path")); + goto cleanup; + } + + /* Technically, we only need a job if we are going to query the + * monitor, which is only for active domains that are using + * non-raw block devices. But it is easier to share code if we + * always grab a job; furthermore, grabbing the job ensures that + * hot-plug won't change disk behind our backs. */ + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + + /* Check the path belongs to this domain. */ + if ((idx = virDomainDiskIndexByName(vm->def, path, false)) < 0) { + virReportError(VIR_ERR_INVALID_ARG, + _("invalid path %s not assigned to domain"), path); + goto endjob; + } + + disk = vm->def->disks[idx]; + src = disk->src; + if (virStorageSourceIsEmpty(src)) { + virReportError(VIR_ERR_INVALID_ARG, + _("disk '%s' does not currently have a source assigned"), + path); + goto endjob; + } + + if ((ret = qemuStorageLimitsRefresh(driver, cfg, vm, src)) < 0) + goto endjob; + + if (!src->allocation) { qemuDomainObjPrivatePtr priv = vm->privateData; /* If the guest is not running, then success/failure return @@ -11183,7 +11211,6 @@ qemuDomainGetBlockInfo(virDomainPtr dom, */ if (!virDomainObjIsActive(vm)) { activeFail = true; - ret = 0; goto endjob; } @@ -11192,9 +11219,6 @@ qemuDomainGetBlockInfo(virDomainPtr dom, disk->info.alias, &src->allocation); qemuDomainObjExitMonitor(driver, vm); - - } else { - ret = 0; } if (ret == 0) { @@ -11207,12 +11231,6 @@ qemuDomainGetBlockInfo(virDomainPtr dom, if (!qemuDomainObjEndJob(driver, vm)) vm = NULL; cleanup: - VIR_FREE(buf); - virStorageSourceFree(meta); - VIR_FORCE_CLOSE(fd); - if (disk) - virStorageFileDeinit(disk->src); - /* If we failed to get data from a domain because it's inactive and * it's not a persistent domain, then force failure. */ -- 2.39.5