Allow updating capacity for the block devices returned by qemuDomainBlocksStatsGather and replace the open-coded call to qemuMonitorGetAllBlockStatsInfo by the helper. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 41 ++++++++++++----------------------------- 1 file changed, 12 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 847dab2edc..4f2855a26b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11063,6 +11063,7 @@ qemuDomainBlockStatsGatherTotals(void *payload, * @driver: driver object * @vm: domain object * @path: to gather the statistics for + * @capacity: refresh capacity of the backing image * @retstats: returns pointer to structure holding the stats * * Gathers the block statistics for use in qemuDomainBlockStats* APIs. @@ -11073,6 +11074,7 @@ static int qemuDomainBlocksStatsGather(virQEMUDriverPtr driver, virDomainObjPtr vm, const char *path, + bool capacity, qemuBlockStatsPtr *retstats) { qemuDomainObjPrivatePtr priv = vm->privateData; @@ -11101,6 +11103,11 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); nstats = qemuMonitorGetAllBlockStatsInfo(priv->mon, &blockstats, false); + + if (capacity && nstats >= 0 && + qemuMonitorBlockStatsUpdateCapacity(priv->mon, blockstats, false) < 0) + nstats = -1; + if (qemuDomainObjExitMonitor(driver, vm) < 0 || nstats < 0) goto cleanup; @@ -11154,7 +11161,7 @@ qemuDomainBlockStats(virDomainPtr dom, if (virDomainObjCheckActive(vm) < 0) goto endjob; - if (qemuDomainBlocksStatsGather(driver, vm, path, &blockstats) < 0) + if (qemuDomainBlocksStatsGather(driver, vm, path, false, &blockstats) < 0) goto endjob; stats->rd_req = blockstats->rd_req; @@ -11208,7 +11215,7 @@ qemuDomainBlockStatsFlags(virDomainPtr dom, if (virDomainObjCheckActive(vm) < 0) goto endjob; - if ((nstats = qemuDomainBlocksStatsGather(driver, vm, path, + if ((nstats = qemuDomainBlocksStatsGather(driver, vm, path, false, &blockstats)) < 0) goto endjob; @@ -12021,10 +12028,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom, int ret = -1; virDomainDiskDefPtr disk; virQEMUDriverConfigPtr cfg = NULL; - int rc; - virHashTablePtr stats = NULL; - qemuBlockStats *entry; - char *alias = NULL; + qemuBlockStatsPtr entry = NULL; virCheckFlags(0, -1); @@ -12065,28 +12069,8 @@ qemuDomainGetBlockInfo(virDomainPtr dom, goto endjob; } - if (!disk->info.alias || - !(alias = qemuDomainStorageAlias(disk->info.alias, 0))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("missing disk device alias name for %s"), disk->dst); + if (qemuDomainBlocksStatsGather(driver, vm, path, true, &entry) < 0) goto endjob; - } - - qemuDomainObjEnterMonitor(driver, vm); - rc = qemuMonitorGetAllBlockStatsInfo(qemuDomainGetMonitor(vm), - &stats, false); - if (rc >= 0) - rc = qemuMonitorBlockStatsUpdateCapacity(qemuDomainGetMonitor(vm), - stats, false); - - if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) - goto endjob; - - if (!(entry = virHashLookup(stats, alias))) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to gather stats for disk '%s'"), disk->dst); - goto endjob; - } if (!entry->wr_highest_offset_valid) { info->allocation = entry->physical; @@ -12131,8 +12115,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom, endjob: qemuDomainObjEndJob(driver, vm); cleanup: - VIR_FREE(alias); - virHashFree(stats); + VIR_FREE(entry); virDomainObjEndAPI(&vm); virObjectUnref(cfg); return ret; -- 2.16.2

In some cases backing chain needs to be cleared prior to re-detection. Move this step out of qemuDomainDetermineDiskChain as only certain places need it and the function itself is able to skip to the end of the chain to perform detection. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_blockjob.c | 4 ++-- src/qemu/qemu_domain.c | 4 ---- src/qemu/qemu_domain.h | 1 - src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 7 +++++-- 6 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index b08e047490..0f52996ade 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -175,8 +175,8 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver, disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; disk->src->id = 0; - ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk, - true, true)); + virStorageSourceBackingStoreClear(disk->src); + ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk, true)); ignore_value(qemuBlockNodeNamesDetect(driver, vm, asyncJob)); diskPriv->blockjob = false; break; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ed76495309..c92ac8c926 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8352,7 +8352,6 @@ int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, - bool force_probe, bool report_broken) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); @@ -8368,9 +8367,6 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, goto cleanup; } - if (force_probe) - virStorageSourceBackingStoreClear(src); - /* There is no need to check the backing chain for disks without backing * support */ if (virStorageSourceIsLocalStorage(src) && diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index e748d78adb..1692fa9838 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -718,7 +718,6 @@ int qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, - bool force_probe, bool report_broken); bool qemuDomainDiskSourceDiffers(virDomainDiskDefPtr disk, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4f2855a26b..64e2c274c2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7850,7 +7850,7 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm, if (virDomainDiskTranslateSourcePool(disk) < 0) goto cleanup; - if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0) + if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0) goto cleanup; if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def, @@ -16931,7 +16931,7 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver, oldsrc = disk->src; disk->src = disk->mirror; - if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0) + if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0) goto cleanup; if (disk->mirror->format && diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2b6633a998..98f7a28826 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -669,7 +669,7 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver, if (qemuSetUnprivSGIO(dev) < 0) goto cleanup; - if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0) + if (qemuDomainDetermineDiskChain(driver, vm, disk, true) < 0) goto cleanup; switch ((virDomainDiskDevice) disk->device) { diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c903a8e5c8..138e128807 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6015,7 +6015,9 @@ qemuProcessPrepareHostStorage(virQEMUDriverPtr driver, if (virStorageSourceIsEmpty(disk->src)) continue; - if (qemuDomainDetermineDiskChain(driver, vm, disk, true, true) >= 0) + virStorageSourceBackingStoreClear(disk->src); + + if (qemuDomainDetermineDiskChain(driver, vm, disk, true) >= 0) continue; if (qemuDomainCheckDiskStartupPolicy(driver, vm, idx, cold_boot) >= 0) @@ -7691,7 +7693,8 @@ qemuProcessReconnect(void *opaque) /* This should be the only place that calls * qemuDomainDetermineDiskChain with @report_broken == false * to guarantee best-effort domain reconnect */ - if (qemuDomainDetermineDiskChain(driver, obj, disk, true, false) < 0) + virStorageSourceBackingStoreClear(disk->src); + if (qemuDomainDetermineDiskChain(driver, obj, disk, false) < 0) goto error; } else { VIR_DEBUG("skipping backing chain detection for '%s'", disk->dst); -- 2.16.2

The disk backend alias was historically the alias of the -drive backing the storage. For setups with -blockdev this will become more complex as it will depend on other configs and generally will differ. --- src/qemu/qemu_domain.c | 28 ++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 6 ++++++ 2 files changed, 34 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c92ac8c926..f8a621a5c9 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8449,6 +8449,34 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, } +/** + * qemuDomainDiskGetBackendAlias: + * @disk: disk definition + * @qemuCaps: emulator capabilities + * @backendAlias: filled with the alias of the disk storage backend + * + * Returns the correct alias for the disk backend. This may be the alias of + * -drive for legacy setup or the correct node name for -blockdev setups. + * + * @backendAlias may be NULL on success if the backend does not exist + * (disk is empty). Caller is responsible for freeing @backendAlias. + * + * Returns 0 on success, -1 on error with libvirt error reported. + */ +int +qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk, + virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED, + char **backendAlias) +{ + *backendAlias = NULL; + + if (!(*backendAlias = qemuAliasDiskDriveFromDisk(disk))) + return -1; + + return 0; +} + + /** * qemuDomainDiskChainElementRevoke: * diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 1692fa9838..22c3a51354 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -732,6 +732,12 @@ int qemuDomainStorageFileInit(virQEMUDriverPtr driver, virStorageSourcePtr parent); char *qemuDomainStorageAlias(const char *device, int depth); +int qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk, + virQEMUCapsPtr qemuCaps, + char **backendAlias) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) + ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; + void qemuDomainDiskChainElementRevoke(virQEMUDriverPtr driver, virDomainObjPtr vm, virStorageSourcePtr elem); -- 2.16.2

Use the proper backend for the block device both when using -drive and when using -blockdev for disk drives and floppy disks. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 44ae8dcef7..6e550cf951 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1818,7 +1818,7 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, virBuffer opt = VIR_BUFFER_INITIALIZER; const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus); const char *contAlias; - char *drivealias; + char *backendAlias = NULL; int controllerModel; if (qemuCheckDiskConfig(disk, qemuCaps) < 0) @@ -2077,10 +2077,14 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, virQEMUCapsGet(qemuCaps, QEMU_CAPS_DISK_SHARE_RW)) virBufferAddLit(&opt, ",share-rw=on"); - if (!(drivealias = qemuAliasDiskDriveFromDisk(disk))) + if (qemuDomainDiskGetBackendAlias(disk, qemuCaps, &backendAlias) < 0) goto error; - virBufferAsprintf(&opt, ",drive=%s,id=%s", drivealias, disk->info.alias); - VIR_FREE(drivealias); + + if (backendAlias) + virBufferAsprintf(&opt, ",drive=%s", backendAlias); + VIR_FREE(backendAlias); + + virBufferAsprintf(&opt, ",id=%s", disk->info.alias); if (bootindex && virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX)) virBufferAsprintf(&opt, ",bootindex=%u", bootindex); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKIO)) { @@ -2139,6 +2143,7 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, return virBufferContentAndReset(&opt); error: + VIR_FREE(backendAlias); virBufferFreeAndReset(&opt); return NULL; } @@ -2148,8 +2153,8 @@ static int qemuBuildFloppyCommandLineOptions(virCommandPtr cmd, const virDomainDef *def, virDomainDiskDefPtr disk, + virQEMUCapsPtr qemuCaps, unsigned int bootindex) - { virBuffer fdc_opts = VIR_BUFFER_INITIALIZER; char driveLetter; @@ -2163,10 +2168,11 @@ qemuBuildFloppyCommandLineOptions(virCommandPtr cmd, else driveLetter = 'A'; - if (!(backendAlias = qemuAliasDiskDriveFromDisk(disk))) - return -1; + if (qemuDomainDiskGetBackendAlias(disk, qemuCaps, &backendAlias) < 0) + goto cleanup; - if (virAsprintf(&backendStr, "drive%c=%s", driveLetter, backendAlias) < 0) + if (backendAlias && + virAsprintf(&backendStr, "drive%c=%s", driveLetter, backendAlias) < 0) goto cleanup; if (bootindex && @@ -2281,7 +2287,7 @@ qemuBuildDiskCommandLine(virCommandPtr cmd, if (!qemuDiskBusNeedsDriveArg(disk->bus)) { if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) { - if (qemuBuildFloppyCommandLineOptions(cmd, def, disk, + if (qemuBuildFloppyCommandLineOptions(cmd, def, disk, qemuCaps, bootindex) < 0) return -1; } else { -- 2.16.2

The 'query-blockstats' command does not return statistics for the explicitly named nodes unless the new argument is specified. Add infrastrucuture that will allow us to use the new approach if desired. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_block.c | 2 +- src/qemu/qemu_monitor.c | 8 ++++++-- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 9 ++++++--- src/qemu/qemu_monitor_json.h | 3 ++- 5 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 7ad79c7e7d..a633bfa0b0 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -356,7 +356,7 @@ qemuBlockNodeNamesDetect(virQEMUDriverPtr driver, return -1; data = qemuMonitorQueryNamedBlockNodes(qemuDomainGetMonitor(vm)); - blockstats = qemuMonitorQueryBlockstats(qemuDomainGetMonitor(vm)); + blockstats = qemuMonitorQueryBlockstats(qemuDomainGetMonitor(vm), false); if (qemuDomainObjExitMonitor(driver, vm) < 0 || !data || !blockstats) goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index bc116e4e2d..8618a44f32 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2243,15 +2243,19 @@ qemuMonitorGetBlockInfo(qemuMonitorPtr mon) /** * qemuMonitorQueryBlockstats: * @mon: monitor object + * @nodenames: include backing chain nodes with explicitly specified name * * Returns data from a call to 'query-blockstats'. */ virJSONValuePtr -qemuMonitorQueryBlockstats(qemuMonitorPtr mon) +qemuMonitorQueryBlockstats(qemuMonitorPtr mon, + bool nodenames) { QEMU_CHECK_MONITOR_NULL(mon); - return qemuMonitorJSONQueryBlockstats(mon); + VIR_DEBUG("nodenames: %d", nodenames); + + return qemuMonitorJSONQueryBlockstats(mon, nodenames); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 81474a04f6..b4b9cf91eb 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -563,7 +563,8 @@ int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon, int qemuMonitorBlockIOStatusToError(const char *status); virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr mon); -virJSONValuePtr qemuMonitorQueryBlockstats(qemuMonitorPtr mon); +virJSONValuePtr qemuMonitorQueryBlockstats(qemuMonitorPtr mon, + bool nodenames); typedef struct _qemuBlockStats qemuBlockStats; typedef qemuBlockStats *qemuBlockStatsPtr; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index cc3c8f2dd6..e41c1d47aa 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2326,13 +2326,16 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, virJSONValuePtr -qemuMonitorJSONQueryBlockstats(qemuMonitorPtr mon) +qemuMonitorJSONQueryBlockstats(qemuMonitorPtr mon, + bool nodenames) { virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr ret = NULL; - if (!(cmd = qemuMonitorJSONMakeCommand("query-blockstats", NULL))) + if (!(cmd = qemuMonitorJSONMakeCommand("query-blockstats", + "B:query-nodes", nodenames, + NULL))) return NULL; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) @@ -2361,7 +2364,7 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, size_t i; virJSONValuePtr devices; - if (!(devices = qemuMonitorJSONQueryBlockstats(mon))) + if (!(devices = qemuMonitorJSONQueryBlockstats(mon, false))) return -1; for (i = 0; i < virJSONValueArraySize(devices); i++) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 66536ceb97..a6b6579849 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -82,7 +82,8 @@ int qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon, int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, virHashTablePtr table); -virJSONValuePtr qemuMonitorJSONQueryBlockstats(qemuMonitorPtr mon); +virJSONValuePtr qemuMonitorJSONQueryBlockstats(qemuMonitorPtr mon, + bool nodenames); int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, virHashTablePtr hash, bool backingChain); -- 2.16.2

The code is useful also when gathering statistics per node name, so extract it to a separate functions. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_monitor_json.c | 51 ++++++++++++++++++++++++++++++-------------- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e41c1d47aa..37a4e59189 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2253,26 +2253,15 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon, } -static int -qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, - const char *dev_name, - int depth, - virHashTablePtr hash, - bool backingChain) +static qemuBlockStatsPtr +qemuMonitorJSONBlockStatsCollectData(virJSONValuePtr dev, + int *nstats) { qemuBlockStatsPtr bstats = NULL; - virJSONValuePtr stats; + qemuBlockStatsPtr ret = NULL; virJSONValuePtr parent; virJSONValuePtr parentstats; - int ret = -1; - int nstats = 0; - char *entry_name = qemuDomainStorageAlias(dev_name, depth); - virJSONValuePtr backing; - - if (!entry_name) - goto cleanup; - if (VIR_ALLOC(bstats) < 0) - goto cleanup; + virJSONValuePtr stats; if ((stats = virJSONValueObjectGetObject(dev, "stats")) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -2281,6 +2270,9 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, goto cleanup; } + if (VIR_ALLOC(bstats) < 0) + goto cleanup; + #define QEMU_MONITOR_BLOCK_STAT_GET(NAME, VAR, MANDATORY) \ if (MANDATORY || virJSONValueObjectHasKey(stats, NAME)) { \ nstats++; \ @@ -2307,6 +2299,33 @@ qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, bstats->wr_highest_offset_valid = true; } + VIR_STEAL_PTR(ret, bstats); + + cleanup: + VIR_FREE(bstats); + return ret; +} + + +static int +qemuMonitorJSONGetOneBlockStatsInfo(virJSONValuePtr dev, + const char *dev_name, + int depth, + virHashTablePtr hash, + bool backingChain) +{ + qemuBlockStatsPtr bstats = NULL; + int ret = -1; + int nstats = 0; + char *entry_name = qemuDomainStorageAlias(dev_name, depth); + virJSONValuePtr backing; + + if (!entry_name) + goto cleanup; + + if (!(bstats = qemuMonitorJSONBlockStatsCollectData(dev, &nstats))) + goto cleanup; + if (virHashAddEntry(hash, entry_name, bstats) < 0) goto cleanup; bstats = NULL; -- 2.16.2

Extract the code for future reuse. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_monitor_json.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 37a4e59189..81043e2af5 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2423,39 +2423,50 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon, static int -qemuMonitorJSONBlockStatsUpdateCapacityOne(virJSONValuePtr image, - const char *dev_name, - int depth, - virHashTablePtr stats, - bool backingChain) +qemuMonitorJSONBlockStatsUpdateCapacityData(virJSONValuePtr image, + const char *name, + virHashTablePtr stats) { qemuBlockStatsPtr bstats; - int ret = -1; - char *entry_name = qemuDomainStorageAlias(dev_name, depth); - virJSONValuePtr backing; - if (!(bstats = virHashLookup(stats, entry_name))) { + if (!(bstats = virHashLookup(stats, name))) { if (VIR_ALLOC(bstats) < 0) - goto cleanup; + return -1; - if (virHashAddEntry(stats, entry_name, bstats) < 0) { + if (virHashAddEntry(stats, name, bstats) < 0) { VIR_FREE(bstats); - goto cleanup; + return -1; } } - /* After this point, we ignore failures; the stats were - * zero-initialized when created which is a sane fallback. */ - ret = 0; + /* failures can be ignored after this point */ if (virJSONValueObjectGetNumberUlong(image, "virtual-size", &bstats->capacity) < 0) - goto cleanup; + return 0; /* if actual-size is missing, image is not thin provisioned */ if (virJSONValueObjectGetNumberUlong(image, "actual-size", &bstats->physical) < 0) bstats->physical = bstats->capacity; + return 0; +} + + +static int +qemuMonitorJSONBlockStatsUpdateCapacityOne(virJSONValuePtr image, + const char *dev_name, + int depth, + virHashTablePtr stats, + bool backingChain) +{ + int ret = -1; + char *entry_name = qemuDomainStorageAlias(dev_name, depth); + virJSONValuePtr backing; + + if (qemuMonitorJSONBlockStatsUpdateCapacityData(image, entry_name, stats) < 0) + goto cleanup; + if (backingChain && (backing = virJSONValueObjectGetObject(image, "backing-image"))) { ret = qemuMonitorJSONBlockStatsUpdateCapacityOne(backing, -- 2.16.2

To allow checking whether a storage source points to the same location add a helper which checks the relevant fields. This will allow replacing a similar check done by formatting the command line arguments for qemu-like syntax. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virstoragefile.c | 43 +++++++++++++++++++++++++++++++++++++++++++ src/util/virstoragefile.h | 3 +++ 3 files changed, 47 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1caecb96b6..fb7c8c724d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2855,6 +2855,7 @@ virStorageSourceIsBlockLocal; virStorageSourceIsEmpty; virStorageSourceIsLocalStorage; virStorageSourceIsRelative; +virStorageSourceIsSameLocation; virStorageSourceNetworkAssignDefaultPorts; virStorageSourceNewFromBacking; virStorageSourceNewFromBackingAbsolute; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 58f67278da..96ed4b1489 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2259,6 +2259,49 @@ virStorageSourceCopy(const virStorageSource *src, } +/** + * virStorageSourceIsSameLocation: + * + * Returns true if the sources @a and @b point to the same storage location. + * This does not compare any other configuration option + */ +bool +virStorageSourceIsSameLocation(virStorageSourcePtr a, + virStorageSourcePtr b) +{ + size_t i; + + /* there are multiple possibilities to define an empty source */ + if (virStorageSourceIsEmpty(a) && + virStorageSourceIsEmpty(b)) + return true; + + if (virStorageSourceGetActualType(a) != virStorageSourceGetActualType(b)) + return false; + + if (STRNEQ_NULLABLE(a->path, b->path) || + STRNEQ_NULLABLE(a->volume, b->volume) || + STRNEQ_NULLABLE(a->snapshot, b->snapshot)) + return false; + + if (a->type == VIR_STORAGE_TYPE_NETWORK) { + if (a->protocol != b->protocol || + a->nhosts != b->nhosts) + return false; + + for (i = 0; i < a->nhosts; i++) { + if (a->hosts[i].transport != b->hosts[i].transport || + a->hosts[i].port != b->hosts[i].port || + STRNEQ_NULLABLE(a->hosts[i].name, b->hosts[i].name) || + STRNEQ_NULLABLE(a->hosts[i].socket, b->hosts[i].socket)) + return false; + } + } + + return true; +} + + /** * virStorageSourceInitChainElement: * @newelem: New backing chain element disk source diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 991098e6c6..c2c40edf68 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -436,6 +436,9 @@ virStorageSourcePtr virStorageSourceNewFromBacking(virStorageSourcePtr parent); virStorageSourcePtr virStorageSourceCopy(const virStorageSource *src, bool backingChain) ATTRIBUTE_NONNULL(1); +bool virStorageSourceIsSameLocation(virStorageSourcePtr a, + virStorageSourcePtr b) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virStorageSourceParseRBDColonString(const char *rbdstr, virStorageSourcePtr src) -- 2.16.2

Now that we have a saner replacement for checking if the disk source is the same use it instead of formatting qemu command-line chunks. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 34 ---------------------------------- src/qemu/qemu_domain.h | 3 --- src/qemu/qemu_driver.c | 2 +- 3 files changed, 1 insertion(+), 38 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f8a621a5c9..acf5cc8dd2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8555,40 +8555,6 @@ qemuDomainDiskChainElementPrepare(virQEMUDriverPtr driver, } -bool -qemuDomainDiskSourceDiffers(virDomainDiskDefPtr disk, - virDomainDiskDefPtr origDisk) -{ - char *diskSrc = NULL, *origDiskSrc = NULL; - bool diskEmpty, origDiskEmpty; - bool ret = true; - - diskEmpty = virStorageSourceIsEmpty(disk->src); - origDiskEmpty = virStorageSourceIsEmpty(origDisk->src); - - if (diskEmpty && origDiskEmpty) - return false; - - if (diskEmpty ^ origDiskEmpty) - return true; - - /* This won't be a network storage, so no need to get the diskPriv - * in order to fetch the secret, thus NULL for param2 */ - if (qemuGetDriveSourceString(disk->src, NULL, &diskSrc) < 0 || - qemuGetDriveSourceString(origDisk->src, NULL, &origDiskSrc) < 0) - goto cleanup; - - /* So far in qemu disk sources are considered different - * if either path to disk or its format changes. */ - ret = virDomainDiskGetFormat(disk) != virDomainDiskGetFormat(origDisk) || - STRNEQ_NULLABLE(diskSrc, origDiskSrc); - cleanup: - VIR_FREE(diskSrc); - VIR_FREE(origDiskSrc); - return ret; -} - - /* * Makes sure the @disk differs from @orig_disk only by the source * path and nothing else. Fields that are being checked and the diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 22c3a51354..bff293fc0a 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -720,9 +720,6 @@ int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, virDomainDiskDefPtr disk, bool report_broken); -bool qemuDomainDiskSourceDiffers(virDomainDiskDefPtr disk, - virDomainDiskDefPtr origDisk); - bool qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk, virDomainDiskDefPtr orig_disk); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 64e2c274c2..dea548f982 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7871,7 +7871,7 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm, if (!qemuDomainDiskChangeSupported(disk, orig_disk)) goto cleanup; - if (qemuDomainDiskSourceDiffers(disk, orig_disk)) { + if (!virStorageSourceIsSameLocation(disk->src, orig_disk->src)) { /* Disk source can be changed only for removable devices */ if (disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM && disk->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY) { -- 2.16.2