[libvirt] [PATCH 0/4] qemu: Don't use API flags in monitor code (blockdev-add saga)

Peter Krempa (4): qemu: migration: Don't pass around flags for different API qemu: monitor: Don't pass full flags to qemuMonitorJSONBlockdevMirror qemu: monitor: Don't pass full flags to qemuMonitorJSONDriveMirror qemu: monitor: Use VIR_AUTOPTR in qemuMonitorJSON(Drive/Blockdev)Mirror src/qemu/qemu_driver.c | 4 +- src/qemu/qemu_migration.c | 21 ++++------ src/qemu/qemu_monitor.c | 17 ++++---- src/qemu/qemu_monitor.h | 5 ++- src/qemu/qemu_monitor_json.c | 79 ++++++++++++++---------------------- src/qemu/qemu_monitor_json.h | 5 ++- tests/qemumonitorjsontest.c | 6 +-- 7 files changed, 60 insertions(+), 77 deletions(-) -- 2.21.0

The NBD migration code uses drive/blockdev-mirror internally. In those APIs we pass around flags for the monitor commands which are based on the flags for the virDomainBlockRebase API. Since there's only one flag which changes, pass it around explicitly rather than obscuring it in a bitfield. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e3ad4e52a7..7b1e392f73 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -785,7 +785,7 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver, const char *host, int port, unsigned long long mirror_speed, - unsigned int mirror_flags, + unsigned int mirror_shallow, const char *tlsAlias) { VIR_AUTOPTR(qemuBlockStorageSourceAttachData) data = NULL; @@ -793,6 +793,10 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver, int mon_ret = 0; int ret = -1; VIR_AUTOUNREF(virStorageSourcePtr) copysrc = NULL; + unsigned int mirror_flags = 0; + + if (mirror_shallow) + mirror_flags |= VIR_DOMAIN_BLOCK_REBASE_SHALLOW; VIR_DEBUG("starting blockdev mirror for disk=%s to host=%s", diskAlias, host); @@ -861,11 +865,15 @@ qemuMigrationSrcNBDStorageCopyDriveMirror(virQEMUDriverPtr driver, const char *host, int port, unsigned long long mirror_speed, - unsigned int mirror_flags) + bool mirror_shallow) { char *nbd_dest = NULL; int mon_ret; int ret = -1; + unsigned int mirror_flags = VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT; + + if (mirror_shallow) + mirror_flags |= VIR_DOMAIN_BLOCK_REBASE_SHALLOW; if (strchr(host, ':')) { if (virAsprintf(&nbd_dest, "nbd:[%s]:%d:exportname=%s", @@ -903,7 +911,7 @@ qemuMigrationSrcNBDStorageCopyOne(virQEMUDriverPtr driver, const char *host, int port, unsigned long long mirror_speed, - unsigned int mirror_flags, + bool mirror_shallow, const char *tlsAlias, unsigned int flags) { @@ -926,13 +934,13 @@ qemuMigrationSrcNBDStorageCopyOne(virQEMUDriverPtr driver, disk, diskAlias, host, port, mirror_speed, - mirror_flags, + mirror_shallow, tlsAlias); } else { rc = qemuMigrationSrcNBDStorageCopyDriveMirror(driver, vm, diskAlias, host, port, mirror_speed, - mirror_flags); + mirror_shallow); } if (rc < 0) @@ -987,7 +995,7 @@ qemuMigrationSrcNBDStorageCopy(virQEMUDriverPtr driver, int port; size_t i; unsigned long long mirror_speed = speed; - unsigned int mirror_flags = VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT; + bool mirror_shallow = *migrate_flags & QEMU_MONITOR_MIGRATE_NON_SHARED_INC; int rv; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); @@ -1005,9 +1013,6 @@ qemuMigrationSrcNBDStorageCopy(virQEMUDriverPtr driver, port = mig->nbd->port; mig->nbd->port = 0; - if (*migrate_flags & QEMU_MONITOR_MIGRATE_NON_SHARED_INC) - mirror_flags |= VIR_DOMAIN_BLOCK_REBASE_SHALLOW; - for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; @@ -1016,7 +1021,7 @@ qemuMigrationSrcNBDStorageCopy(virQEMUDriverPtr driver, continue; if (qemuMigrationSrcNBDStorageCopyOne(driver, vm, disk, host, port, - mirror_speed, mirror_flags, + mirror_speed, mirror_shallow, tlsAlias, flags) < 0) goto cleanup; -- 2.21.0

On Mon, May 20, 2019 at 04:16:30PM +0200, Peter Krempa wrote:
The NBD migration code uses drive/blockdev-mirror internally. In those APIs we pass around flags for the monitor commands which are based on the flags for the virDomainBlockRebase API. Since there's only one flag which changes, pass it around explicitly rather than obscuring it in a bitfield.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Split out the 'shallow' flag as a boolean argument rather than passing in flags and constructing them in irrelevant APIs. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 6 +----- src/qemu/qemu_monitor.c | 8 ++++---- src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c | 3 +-- src/qemu/qemu_monitor_json.h | 2 +- tests/qemumonitorjsontest.c | 3 +-- 6 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7b1e392f73..832f98346d 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -793,10 +793,6 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver, int mon_ret = 0; int ret = -1; VIR_AUTOUNREF(virStorageSourcePtr) copysrc = NULL; - unsigned int mirror_flags = 0; - - if (mirror_shallow) - mirror_flags |= VIR_DOMAIN_BLOCK_REBASE_SHALLOW; VIR_DEBUG("starting blockdev mirror for disk=%s to host=%s", diskAlias, host); @@ -841,7 +837,7 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver, if (mon_ret == 0) mon_ret = qemuMonitorBlockdevMirror(qemuDomainGetMonitor(vm), NULL, diskAlias, copysrc->nodeformat, - mirror_speed, 0, 0, mirror_flags); + mirror_speed, 0, 0, mirror_shallow); if (mon_ret != 0) qemuBlockStorageSourceAttachRollback(qemuDomainGetMonitor(vm), data); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index e1fcbac13f..9a796db3cb 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3266,17 +3266,17 @@ qemuMonitorBlockdevMirror(qemuMonitorPtr mon, unsigned long long bandwidth, unsigned int granularity, unsigned long long buf_size, - unsigned int flags) + bool shallow) { VIR_DEBUG("jobname=%s, device=%s, target=%s, bandwidth=%lld, " - "granularity=%#x, buf_size=%lld, flags=0x%x", + "granularity=%#x, buf_size=%lld, shallow=%d", NULLSTR(jobname), device, target, bandwidth, granularity, - buf_size, flags); + buf_size, shallow); QEMU_CHECK_MONITOR(mon); return qemuMonitorJSONBlockdevMirror(mon, jobname, device, target, bandwidth, - granularity, buf_size, flags); + granularity, buf_size, shallow); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 9242d37407..438c3c38ed 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -893,7 +893,7 @@ int qemuMonitorBlockdevMirror(qemuMonitorPtr mon, unsigned long long bandwidth, unsigned int granularity, unsigned long long buf_size, - unsigned int flags) + bool shallow) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); int qemuMonitorDrivePivot(qemuMonitorPtr mon, const char *jobname) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5e3df5e759..a75e660d07 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4320,12 +4320,11 @@ qemuMonitorJSONBlockdevMirror(qemuMonitorPtr mon, unsigned long long speed, unsigned int granularity, unsigned long long buf_size, - unsigned int flags) + bool shallow) { int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; - bool shallow = (flags & VIR_DOMAIN_BLOCK_REBASE_SHALLOW) != 0; cmd = qemuMonitorJSONMakeCommand("blockdev-mirror", "S:job-id", jobname, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 746b7072ca..174db7a2e9 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -259,7 +259,7 @@ int qemuMonitorJSONBlockdevMirror(qemuMonitorPtr mon, unsigned long long speed, unsigned int granularity, unsigned long long buf_size, - unsigned int flags) + bool shallow) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); int qemuMonitorJSONDrivePivot(qemuMonitorPtr mon, const char *jobname) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 923f6d7d9c..c2bb3a6c4f 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1343,8 +1343,7 @@ GEN_TEST_FUNC(qemuMonitorJSONDelDevice, "ide0") GEN_TEST_FUNC(qemuMonitorJSONAddDevice, "some_dummy_devicestr") GEN_TEST_FUNC(qemuMonitorJSONDriveMirror, "vdb", "/foo/bar", "formatstr", 1024, 1234, 31234, VIR_DOMAIN_BLOCK_REBASE_SHALLOW | VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT) -GEN_TEST_FUNC(qemuMonitorJSONBlockdevMirror, "jobname", "vdb", "targetnode", 1024, 1234, 31234, - VIR_DOMAIN_BLOCK_REBASE_SHALLOW | VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT) +GEN_TEST_FUNC(qemuMonitorJSONBlockdevMirror, "jobname", "vdb", "targetnode", 1024, 1234, 31234, true) GEN_TEST_FUNC(qemuMonitorJSONBlockStream, "vdb", "/foo/bar1", "backingfilename", 1024) GEN_TEST_FUNC(qemuMonitorJSONBlockCommit, "vdb", "/foo/bar1", "/foo/bar2", "backingfilename", 1024) GEN_TEST_FUNC(qemuMonitorJSONDrivePivot, "vdb") -- 2.21.0

On Mon, May 20, 2019 at 04:16:31PM +0200, Peter Krempa wrote:
Split out the 'shallow' flag as a boolean argument rather than passing in flags and constructing them in irrelevant APIs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_migration.c | 6 +----- src/qemu/qemu_monitor.c | 8 ++++---- src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c | 3 +-- src/qemu/qemu_monitor_json.h | 2 +- tests/qemumonitorjsontest.c | 3 +-- 6 files changed, 9 insertions(+), 15 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Split out the 'shallow' and 'reuse' flags as booleans rather than passing in flags and constructing them in irrelevant APIs. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 4 +++- src/qemu/qemu_migration.c | 6 +----- src/qemu/qemu_monitor.c | 9 +++++---- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 5 ++--- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 3 +-- 7 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9c0796b0a4..95c84b46ae 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17764,7 +17764,9 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, /* qemuMonitorDriveMirror needs to honor the REUSE_EXT flag as specified * by the user regardless of how @reuse was modified */ ret = qemuMonitorDriveMirror(priv->mon, device, mirror->path, format, - bandwidth, granularity, buf_size, flags); + bandwidth, granularity, buf_size, + flags & VIR_DOMAIN_BLOCK_COPY_SHALLOW, + flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT); virDomainAuditDisk(vm, NULL, mirror, "mirror", ret >= 0); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 832f98346d..32b3040473 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -866,10 +866,6 @@ qemuMigrationSrcNBDStorageCopyDriveMirror(virQEMUDriverPtr driver, char *nbd_dest = NULL; int mon_ret; int ret = -1; - unsigned int mirror_flags = VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT; - - if (mirror_shallow) - mirror_flags |= VIR_DOMAIN_BLOCK_REBASE_SHALLOW; if (strchr(host, ':')) { if (virAsprintf(&nbd_dest, "nbd:[%s]:%d:exportname=%s", @@ -887,7 +883,7 @@ qemuMigrationSrcNBDStorageCopyDriveMirror(virQEMUDriverPtr driver, mon_ret = qemuMonitorDriveMirror(qemuDomainGetMonitor(vm), diskAlias, nbd_dest, "raw", - mirror_speed, 0, 0, mirror_flags); + mirror_speed, 0, 0, mirror_shallow, true); if (qemuDomainObjExitMonitor(driver, vm) < 0 || mon_ret < 0) goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 9a796db3cb..6b731cd91a 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3244,17 +3244,18 @@ qemuMonitorDriveMirror(qemuMonitorPtr mon, const char *device, const char *file, const char *format, unsigned long long bandwidth, unsigned int granularity, unsigned long long buf_size, - unsigned int flags) + bool shallow, + bool reuse) { VIR_DEBUG("device=%s, file=%s, format=%s, bandwidth=%lld, " - "granularity=%#x, buf_size=%lld, flags=0x%x", + "granularity=%#x, buf_size=%lld, shallow=%d, reuse=%d", device, file, NULLSTR(format), bandwidth, granularity, - buf_size, flags); + buf_size, shallow, reuse); QEMU_CHECK_MONITOR(mon); return qemuMonitorJSONDriveMirror(mon, device, file, format, bandwidth, - granularity, buf_size, flags); + granularity, buf_size, shallow, reuse); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 438c3c38ed..dee594fa66 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -884,7 +884,8 @@ int qemuMonitorDriveMirror(qemuMonitorPtr mon, unsigned long long bandwidth, unsigned int granularity, unsigned long long buf_size, - unsigned int flags) + bool shallow, + bool reuse) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int qemuMonitorBlockdevMirror(qemuMonitorPtr mon, const char *jobname, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index a75e660d07..4152d44331 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4277,13 +4277,12 @@ qemuMonitorJSONDriveMirror(qemuMonitorPtr mon, const char *format, unsigned long long speed, unsigned int granularity, unsigned long long buf_size, - unsigned int flags) + bool shallow, + bool reuse) { int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; - bool shallow = (flags & VIR_DOMAIN_BLOCK_REBASE_SHALLOW) != 0; - bool reuse = (flags & VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT) != 0; cmd = qemuMonitorJSONMakeCommand("drive-mirror", "s:device", device, diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 174db7a2e9..acef1a0a79 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -250,7 +250,8 @@ int qemuMonitorJSONDriveMirror(qemuMonitorPtr mon, unsigned long long speed, unsigned int granularity, unsigned long long buf_size, - unsigned int flags) + bool shallow, + bool reuse) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); int qemuMonitorJSONBlockdevMirror(qemuMonitorPtr mon, const char *jobname, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index c2bb3a6c4f..0894e748ae 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1341,8 +1341,7 @@ GEN_TEST_FUNC(qemuMonitorJSONAddNetdev, "id=net0,type=test") GEN_TEST_FUNC(qemuMonitorJSONRemoveNetdev, "net0") GEN_TEST_FUNC(qemuMonitorJSONDelDevice, "ide0") GEN_TEST_FUNC(qemuMonitorJSONAddDevice, "some_dummy_devicestr") -GEN_TEST_FUNC(qemuMonitorJSONDriveMirror, "vdb", "/foo/bar", "formatstr", 1024, 1234, 31234, - VIR_DOMAIN_BLOCK_REBASE_SHALLOW | VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT) +GEN_TEST_FUNC(qemuMonitorJSONDriveMirror, "vdb", "/foo/bar", "formatstr", 1024, 1234, 31234, true, true) GEN_TEST_FUNC(qemuMonitorJSONBlockdevMirror, "jobname", "vdb", "targetnode", 1024, 1234, 31234, true) GEN_TEST_FUNC(qemuMonitorJSONBlockStream, "vdb", "/foo/bar1", "backingfilename", 1024) GEN_TEST_FUNC(qemuMonitorJSONBlockCommit, "vdb", "/foo/bar1", "/foo/bar2", "backingfilename", 1024) -- 2.21.0

On Mon, May 20, 2019 at 04:16:32PM +0200, Peter Krempa wrote:
Split out the 'shallow' and 'reuse' flags as booleans rather than passing in flags and constructing them in irrelevant APIs.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 4 +++- src/qemu/qemu_migration.c | 6 +----- src/qemu/qemu_monitor.c | 9 +++++---- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 5 ++--- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 3 +-- 7 files changed, 16 insertions(+), 17 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 71 ++++++++++++++---------------------- 1 file changed, 28 insertions(+), 43 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 4152d44331..4f9a0cb960 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4280,34 +4280,25 @@ qemuMonitorJSONDriveMirror(qemuMonitorPtr mon, bool shallow, bool reuse) { - int ret = -1; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; + VIR_AUTOPTR(virJSONValue) cmd = NULL; + VIR_AUTOPTR(virJSONValue) reply = NULL; - cmd = qemuMonitorJSONMakeCommand("drive-mirror", - "s:device", device, - "s:target", file, - "Y:speed", speed, - "z:granularity", granularity, - "P:buf-size", buf_size, - "s:sync", shallow ? "top" : "full", - "s:mode", reuse ? "existing" : "absolute-paths", - "S:format", format, - NULL); - if (!cmd) + if (!(cmd = qemuMonitorJSONMakeCommand("drive-mirror", + "s:device", device, + "s:target", file, + "Y:speed", speed, + "z:granularity", granularity, + "P:buf-size", buf_size, + "s:sync", shallow ? "top" : "full", + "s:mode", reuse ? "existing" : "absolute-paths", + "S:format", format, + NULL))) return -1; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) - goto cleanup; - - if (qemuMonitorJSONCheckError(cmd, reply) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + return qemuMonitorJSONCheckError(cmd, reply); } @@ -4321,30 +4312,24 @@ qemuMonitorJSONBlockdevMirror(qemuMonitorPtr mon, unsigned long long buf_size, bool shallow) { - int ret = -1; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; + VIR_AUTOPTR(virJSONValue) cmd = NULL; + VIR_AUTOPTR(virJSONValue) reply = NULL; - cmd = qemuMonitorJSONMakeCommand("blockdev-mirror", - "S:job-id", jobname, - "s:device", device, - "s:target", target, - "Y:speed", speed, - "z:granularity", granularity, - "P:buf-size", buf_size, - "s:sync", shallow ? "top" : "full", - NULL); - if (!cmd) + if (!(cmd = qemuMonitorJSONMakeCommand("blockdev-mirror", + "S:job-id", jobname, + "s:device", device, + "s:target", target, + "Y:speed", speed, + "z:granularity", granularity, + "P:buf-size", buf_size, + "s:sync", shallow ? "top" : "full", + NULL))) return -1; - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) - goto cleanup; - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; - cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + return qemuMonitorJSONCheckError(cmd, reply); } -- 2.21.0

On Mon, May 20, 2019 at 04:16:33PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 71 ++++++++++++++---------------------- 1 file changed, 28 insertions(+), 43 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 4152d44331..4f9a0cb960 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4280,34 +4280,25 @@ qemuMonitorJSONDriveMirror(qemuMonitorPtr mon, bool shallow, bool reuse) { - int ret = -1; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; + VIR_AUTOPTR(virJSONValue) cmd = NULL; + VIR_AUTOPTR(virJSONValue) reply = NULL;
- cmd = qemuMonitorJSONMakeCommand("drive-mirror", - "s:device", device, - "s:target", file, - "Y:speed", speed, - "z:granularity", granularity, - "P:buf-size", buf_size, - "s:sync", shallow ? "top" : "full", - "s:mode", reuse ? "existing" : "absolute-paths", - "S:format", format, - NULL); - if (!cmd) + if (!(cmd = qemuMonitorJSONMakeCommand("drive-mirror", + "s:device", device, + "s:target", file, + "Y:speed", speed, + "z:granularity", granularity, + "P:buf-size", buf_size, + "s:sync", shallow ? "top" : "full", + "s:mode", reuse ? "existing" : "absolute-paths", + "S:format", format, + NULL))) return -1;
Merging the assignment and the condition is not necessary for using AUTOPTR.
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) - goto cleanup; - - if (qemuMonitorJSONCheckError(cmd, reply) < 0) - goto cleanup; + return -1;
- ret = 0; - cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + return qemuMonitorJSONCheckError(cmd, reply); }
@@ -4321,30 +4312,24 @@ qemuMonitorJSONBlockdevMirror(qemuMonitorPtr mon, unsigned long long buf_size, bool shallow) { - int ret = -1; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; + VIR_AUTOPTR(virJSONValue) cmd = NULL; + VIR_AUTOPTR(virJSONValue) reply = NULL;
- cmd = qemuMonitorJSONMakeCommand("blockdev-mirror", - "S:job-id", jobname, - "s:device", device, - "s:target", target, - "Y:speed", speed, - "z:granularity", granularity, - "P:buf-size", buf_size, - "s:sync", shallow ? "top" : "full", - NULL); - if (!cmd) + if (!(cmd = qemuMonitorJSONMakeCommand("blockdev-mirror", + "S:job-id", jobname, + "s:device", device, + "s:target", target, + "Y:speed", speed, + "z:granularity", granularity, + "P:buf-size", buf_size, + "s:sync", shallow ? "top" : "full", + NULL))) return -1;
Same here.
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko
-
Peter Krempa