[PATCH 00/16] Clean up of virJSONValueFree usage and few related refactors

[PATCH 00/17] qemu: Add support...

[libvirt PATCH] libxl:...

Peter Krempa

Friday, 3 December 2021 Fri, 3 Dec '21

8:57 a.m.

Peter Krempa (16): qemu: hotplug: Use automatic freeing for virJSONValue qemuDomainHotplugAddIOThread: Automatically free virJSONValue qemuMonitorBlockdevCreate: Use double pointer instead of always consuming '@props' virLockDaemonClientPreExecRestart: Modernize JSON object construction virLockDaemonPostExecRestart: Automatically free temporary variables virLockDaemonPostExecRestart: Refactor cleanup virLogDaemonPostExecRestart: Use automatic freeing for variables virLogDaemonPostExecRestart: Refactor cleanup virCHProcessUpdateInfo: Automatically free virJSONValue tests/virnetdaemontest.c: testExecRestart: Automatically free virJSONValue-s qemuAgentGuestSync: Don't use goto for looping qemuMonitorJSONGetCPUModelExpansion: Don't use goto for looping qemuMonitorAddObject: Use g_clear_pointer for a free and reset operation qemuAgentIOProcessLine: refactor cleanup qemu: agent: Automatically free virJSONValue-s qemu: agent: Remove unneeded cleanup sections src/ch/ch_process.c | 4 +- src/locking/lock_daemon.c | 85 ++++------ src/logging/log_daemon.c | 42 ++--- src/qemu/qemu_agent.c | 304 ++++++++++++++++------------------- src/qemu/qemu_block.c | 3 +- src/qemu/qemu_driver.c | 3 +- src/qemu/qemu_hotplug.c | 13 +- src/qemu/qemu_monitor.c | 20 +-- src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c | 79 +++++---- src/qemu/qemu_monitor_json.h | 2 +- tests/virnetdaemontest.c | 6 +- 12 files changed, 254 insertions(+), 309 deletions(-) -- 2.31.1

Show replies by date

Peter Krempa

Friday, 3 December Fri, 3 Dec

8:57 a.m.

New subject: [PATCH 01/16] qemu: hotplug: Use automatic freeing for virJSONValue

There are a few uses which still explicitly free JSON objects, fix them. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_hotplug.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 71c0686190..2e1d18c633 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -411,7 +411,7 @@ qemuHotplugAttachManagedPR(virQEMUDriver *driver, qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivate *priv = vm->privateData; - virJSONValue *props = NULL; + g_autoptr(virJSONValue) props = NULL; bool daemonStarted = false; int ret = -1; int rc; @@ -442,7 +442,6 @@ qemuHotplugAttachManagedPR(virQEMUDriver *driver, cleanup: if (ret < 0 && daemonStarted) qemuProcessKillManagedPRDaemon(vm); - virJSONValueFree(props); return ret; } @@ -2286,7 +2285,7 @@ qemuDomainAttachRNGDevice(virQEMUDriver *driver, bool teardowncgroup = false; bool teardowndevice = false; bool chardevAdded = false; - virJSONValue *props = NULL; + g_autoptr(virJSONValue) props = NULL; int ret = -1; qemuAssignDeviceRNGAlias(vm->def, rng); @@ -2349,7 +2348,6 @@ qemuDomainAttachRNGDevice(virQEMUDriver *driver, audit: virDomainAuditRNG(vm, NULL, rng, "attach", ret == 0); cleanup: - virJSONValueFree(props); if (ret < 0) { if (releaseaddr) qemuDomainReleaseDeviceAddress(vm, &rng->info); @@ -2403,7 +2401,7 @@ qemuDomainAttachMemory(virQEMUDriver *driver, bool teardownlabel = false; bool teardowncgroup = false; bool teardowndevice = false; - virJSONValue *props = NULL; + g_autoptr(virJSONValue) props = NULL; virObjectEvent *event; int id; int ret = -1; @@ -2492,7 +2490,6 @@ qemuDomainAttachMemory(virQEMUDriver *driver, qemuDomainReleaseMemoryDeviceSlot(vm, mem); } - virJSONValueFree(props); virDomainMemoryDefFree(mem); return ret; @@ -2970,7 +2967,7 @@ qemuDomainAttachShmemDevice(virQEMUDriver *driver, bool release_backing = false; bool release_address = true; virErrorPtr orig_err = NULL; - virJSONValue *props = NULL; + g_autoptr(virJSONValue) props = NULL; qemuDomainObjPrivate *priv = vm->privateData; virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_SHMEM, { .shmem = shmem } }; @@ -3046,8 +3043,6 @@ qemuDomainAttachShmemDevice(virQEMUDriver *driver, if (release_address) qemuDomainReleaseDeviceAddress(vm, &shmem->info); - virJSONValueFree(props); - return ret; exit_monitor: -- 2.31.1

Peter Krempa

8:57 a.m.

New subject: [PATCH 02/16] qemuDomainHotplugAddIOThread: Automatically free virJSONValue

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6333d0af36..13e33a2289 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5233,7 +5233,7 @@ qemuDomainHotplugAddIOThread(virQEMUDriver *driver, int new_niothreads = 0; qemuMonitorIOThreadInfo **new_iothreads = NULL; virDomainIOThreadIDDef *iothrid; - virJSONValue *props = NULL; + g_autoptr(virJSONValue) props = NULL; bool threadAdded = false; bool objectAdded = false; @@ -5316,7 +5316,6 @@ qemuDomainHotplugAddIOThread(virQEMUDriver *driver, } virDomainAuditIOThread(vm, orig_niothreads, new_niothreads, "update", ret == 0); - virJSONValueFree(props); return ret; exit_monitor: -- 2.31.1

Peter Krempa

8:57 a.m.

New subject: [PATCH 03/16] qemuMonitorBlockdevCreate: Use double pointer instead of always consuming '@props'

We use this approach for other APIs which take a virJSONValue as argument and the logic is also simpler. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_block.c | 3 +-- src/qemu/qemu_monitor.c | 13 +++---------- src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c | 5 ++--- src/qemu/qemu_monitor_json.h | 2 +- 5 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 8a10d2aa0c..f6a7f2b750 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -2710,8 +2710,7 @@ qemuBlockStorageSourceCreateGeneric(virDomainObj *vm, if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0) goto cleanup; - rc = qemuMonitorBlockdevCreate(priv->mon, job->name, props); - props = NULL; + rc = qemuMonitorBlockdevCreate(priv->mon, job->name, &props); qemuDomainObjExitMonitor(priv->driver, vm); if (rc < 0) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 26b59801b8..d752b299ab 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4242,30 +4242,23 @@ qemuMonitorSetWatchdogAction(qemuMonitor *mon, * qemuMonitorBlockdevCreate: * @mon: monitor object * @jobname: name of the job - * @props: JSON object describing the blockdev to add + * @props: JSON object describing the blockdev to add (consumed on success) * * Instructs qemu to create/format a new storage or format layer. Note that * the job does not add the created/formatted image into qemu and * qemuMonitorBlockdevAdd needs to be called separately with corresponding * arguments. Note that the arguments for creating and adding are different. - * - * Note that @props is always consumed by this function and should not be - * accessed after calling this function. */ int qemuMonitorBlockdevCreate(qemuMonitor *mon, const char *jobname, - virJSONValue *props) + virJSONValue **props) { VIR_DEBUG("jobname=%s props=%p", jobname, props); - QEMU_CHECK_MONITOR_GOTO(mon, error); + QEMU_CHECK_MONITOR(mon); return qemuMonitorJSONBlockdevCreate(mon, jobname, props); - - error: - virJSONValueFree(props); - return -1; } /** diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 99ecebc648..8b0c8a99ab 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1416,7 +1416,7 @@ int qemuMonitorSetWatchdogAction(qemuMonitor *mon, int qemuMonitorBlockdevCreate(qemuMonitor *mon, const char *jobname, - virJSONValue *props); + virJSONValue **props); int qemuMonitorBlockdevAdd(qemuMonitor *mon, virJSONValue **props); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c10ea583fd..c05a2f3cff 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -7964,16 +7964,15 @@ qemuMonitorJSONSetWatchdogAction(qemuMonitor *mon, int qemuMonitorJSONBlockdevCreate(qemuMonitor *mon, const char *jobname, - virJSONValue *props) + virJSONValue **props) { g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; cmd = qemuMonitorJSONMakeCommand("blockdev-create", "s:job-id", jobname, - "a:options", &props, + "a:options", props, NULL); - virJSONValueFree(props); if (!cmd) return -1; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index f7fb13f56c..3b88ae7363 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -586,7 +586,7 @@ int qemuMonitorJSONSetWatchdogAction(qemuMonitor *mon, int qemuMonitorJSONBlockdevCreate(qemuMonitor *mon, const char *jobname, - virJSONValue *props) + virJSONValue **props) ATTRIBUTE_NONNULL(1); int qemuMonitorJSONBlockdevAdd(qemuMonitor *mon, -- 2.31.1

Peter Krempa

8:57 a.m.

New subject: [PATCH 04/16] virLockDaemonClientPreExecRestart: Modernize JSON object construction

Use virJSONValueObjectAdd instead of step-by-step construction of the object. This also removes a handful impossible to reach errors with translatable messages. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/locking/lock_daemon.c | 40 ++++++++++----------------------------- 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index cf33d9732b..fc714052be 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -549,41 +549,21 @@ virLockDaemonClientPreExecRestart(virNetServerClient *client G_GNUC_UNUSED, void *opaque) { virLockDaemonClient *priv = opaque; - virJSONValue *object = virJSONValueNewObject(); + virJSONValue *object = NULL; char uuidstr[VIR_UUID_STRING_BUFLEN]; - if (virJSONValueObjectAppendBoolean(object, "restricted", priv->restricted) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot set restricted data in JSON document")); - goto error; - } - if (virJSONValueObjectAppendNumberUint(object, "ownerPid", priv->ownerPid) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot set ownerPid data in JSON document")); - goto error; - } - if (virJSONValueObjectAppendNumberUint(object, "ownerId", priv->ownerId) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot set ownerId data in JSON document")); - goto error; - } - if (virJSONValueObjectAppendString(object, "ownerName", priv->ownerName) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot set ownerName data in JSON document")); - goto error; - } virUUIDFormat(priv->ownerUUID, uuidstr); - if (virJSONValueObjectAppendString(object, "ownerUUID", uuidstr) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot set ownerUUID data in JSON document")); - goto error; - } - return object; + if (virJSONValueObjectAdd(&object, + "b:restricted", priv->restricted, + "u:ownerPid", priv->ownerPid, + "u:ownerId", priv->ownerId, + "s:ownerName", priv->ownerName, + "s:ownerUUID", uuidstr, + NULL) < 0) + return NULL; - error: - virJSONValueFree(object); - return NULL; + return object; } -- 2.31.1

Peter Krempa

8:57 a.m.

New subject: [PATCH 05/16] virLockDaemonPostExecRestart: Automatically free temporary variables

Convert two temp strings and one virJSONValue to g_auto(free|ptr). Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/locking/lock_daemon.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index fc714052be..1edd912c3e 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -607,10 +607,10 @@ virLockDaemonPostExecRestart(const char *state_file, bool privileged) { const char *gotmagic; - char *wantmagic = NULL; + g_autofree char *wantmagic = NULL; int ret = -1; - char *state = NULL; - virJSONValue *object = NULL; + g_autofree char *state = NULL; + g_autoptr(virJSONValue) object = NULL; VIR_DEBUG("Running post-restart exec"); @@ -660,9 +660,6 @@ virLockDaemonPostExecRestart(const char *state_file, cleanup: unlink(state_file); - VIR_FREE(wantmagic); - VIR_FREE(state); - virJSONValueFree(object); return ret; } -- 2.31.1

Peter Krempa

8:57 a.m.

New subject: [PATCH 06/16] virLockDaemonPostExecRestart: Refactor cleanup

Move the unlinking of the state file earlier and get rid of the cleanup label. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/locking/lock_daemon.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 1edd912c3e..107fb22bc2 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -608,59 +608,57 @@ virLockDaemonPostExecRestart(const char *state_file, { const char *gotmagic; g_autofree char *wantmagic = NULL; - int ret = -1; g_autofree char *state = NULL; g_autoptr(virJSONValue) object = NULL; + int rc; VIR_DEBUG("Running post-restart exec"); if (!virFileExists(state_file)) { VIR_DEBUG("No restart state file %s present", state_file); - ret = 0; - goto cleanup; + return 0; } - if (virFileReadAll(state_file, - 1024 * 1024 * 10, /* 10 MB */ - &state) < 0) - goto cleanup; + rc = virFileReadAll(state_file, + 1024 * 1024 * 10, /* 10 MB */ + &state); + + unlink(state_file); + + if (rc < 0) + return -1; VIR_DEBUG("Loading state %s", state); if (!(object = virJSONValueFromString(state))) - goto cleanup; + return -1; gotmagic = virJSONValueObjectGetString(object, "magic"); if (!gotmagic) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing magic data in JSON document")); - goto cleanup; + return -1; } if (!(wantmagic = virLockDaemonGetExecRestartMagic())) - goto cleanup; + return -1; if (STRNEQ(gotmagic, wantmagic)) { VIR_WARN("Found restart exec file with old magic %s vs wanted %s", gotmagic, wantmagic); - ret = 0; - goto cleanup; + return 0; } /* Re-claim PID file now as we will not be daemonizing */ if (pid_file && (*pid_file_fd = virPidFileAcquirePath(pid_file, false, getpid())) < 0) - goto cleanup; + return -1; if (!(lockDaemon = virLockDaemonNewPostExecRestart(object, privileged))) - goto cleanup; - - ret = 1; + return -1; - cleanup: - unlink(state_file); - return ret; + return 1; } -- 2.31.1

Peter Krempa

8:57 a.m.

New subject: [PATCH 07/16] virLogDaemonPostExecRestart: Use automatic freeing for variables

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/logging/log_daemon.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index cc7889399b..a2a5b5f547 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -435,10 +435,10 @@ virLogDaemonPostExecRestart(const char *state_file, virLogDaemonConfig *config) { const char *gotmagic; - char *wantmagic = NULL; + g_autofree char *wantmagic = NULL; int ret = -1; - char *state = NULL; - virJSONValue *object = NULL; + g_autofree char *state = NULL; + g_autoptr(virJSONValue) object = NULL; VIR_DEBUG("Running post-restart exec"); @@ -490,9 +490,6 @@ virLogDaemonPostExecRestart(const char *state_file, cleanup: unlink(state_file); - VIR_FREE(wantmagic); - VIR_FREE(state); - virJSONValueFree(object); return ret; } -- 2.31.1

Peter Krempa

8:57 a.m.

New subject: [PATCH 08/16] virLogDaemonPostExecRestart: Refactor cleanup

Move the unlinking of the state file right after reading it so that we can get rid of the cleanup section. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/logging/log_daemon.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index a2a5b5f547..de6bf082e8 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -436,61 +436,56 @@ virLogDaemonPostExecRestart(const char *state_file, { const char *gotmagic; g_autofree char *wantmagic = NULL; - int ret = -1; g_autofree char *state = NULL; g_autoptr(virJSONValue) object = NULL; + int rc; VIR_DEBUG("Running post-restart exec"); if (!virFileExists(state_file)) { VIR_DEBUG("No restart state file %s present", state_file); - ret = 0; - goto cleanup; + return 0; } - if (virFileReadAll(state_file, - 1024 * 1024 * 10, /* 10 MB */ - &state) < 0) - goto cleanup; + rc = virFileReadAll(state_file, 1024 * 1024 * 10, /* 10 MB */ &state); + unlink(state_file); + + if (rc < 0) + return -1; VIR_DEBUG("Loading state %s", state); if (!(object = virJSONValueFromString(state))) - goto cleanup; + return -1; gotmagic = virJSONValueObjectGetString(object, "magic"); if (!gotmagic) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing magic data in JSON document")); - goto cleanup; + return -1; } if (!(wantmagic = virLogDaemonGetExecRestartMagic())) - goto cleanup; + return -1; if (STRNEQ(gotmagic, wantmagic)) { VIR_WARN("Found restart exec file with old magic %s vs wanted %s", gotmagic, wantmagic); - ret = 0; - goto cleanup; + return 0; } /* Re-claim PID file now as we will not be daemonizing */ if (pid_file && (*pid_file_fd = virPidFileAcquirePath(pid_file, false, getpid())) < 0) - goto cleanup; + return -1; if (!(logDaemon = virLogDaemonNewPostExecRestart(object, privileged, config))) - goto cleanup; - - ret = 1; + return -1; - cleanup: - unlink(state_file); - return ret; + return 1; } -- 2.31.1

Peter Krempa

8:57 a.m.

New subject: [PATCH 09/16] virCHProcessUpdateInfo: Automatically free virJSONValue

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/ch/ch_process.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c index 1a01ca9384..9f82e04485 100644 --- a/src/ch/ch_process.c +++ b/src/ch/ch_process.c @@ -121,15 +121,13 @@ virCHProcessUpdateConsole(virDomainObj *vm, static int virCHProcessUpdateInfo(virDomainObj *vm) { - virJSONValue *info; + g_autoptr(virJSONValue) info = NULL; virCHDomainObjPrivate *priv = vm->privateData; if (virCHMonitorGetInfo(priv->monitor, &info) < 0) return -1; virCHProcessUpdateConsole(vm, info); - virJSONValueFree(info); - return 0; } -- 2.31.1

Peter Krempa

8:57 a.m.

New subject: [PATCH 10/16] tests/virnetdaemontest.c: testExecRestart: Automatically free virJSONValue-s

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- tests/virnetdaemontest.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index 70af1480df..24f4761bb7 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -273,8 +273,8 @@ static int testExecRestart(const void *opaque) g_autofree char *outfile = NULL; g_autofree char *injsonstr = NULL; g_autofree char *outjsonstr = NULL; - virJSONValue *injson = NULL; - virJSONValue *outjson = NULL; + g_autoptr(virJSONValue) injson = NULL; + g_autoptr(virJSONValue) outjson = NULL; int fdclient[2] = { -1, -1 }, fdserver[2] = { -1, -1 }; if (socketpair(PF_UNIX, SOCK_STREAM, 0, fdclient) < 0) { @@ -351,8 +351,6 @@ static int testExecRestart(const void *opaque) VIR_TEST_DEBUG("Test should have failed"); ret = -1; } - virJSONValueFree(injson); - virJSONValueFree(outjson); virObjectUnref(dmn); VIR_FORCE_CLOSE(fdserver[0]); VIR_FORCE_CLOSE(fdserver[1]); -- 2.31.1

Peter Krempa

8:57 a.m.

New subject: [PATCH 11/16] qemuAgentGuestSync: Don't use goto for looping

Don't use 'goto' for looping. Extract the sync sending code into a new function and restructure the logic to avoid jumping back in the code. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_agent.c | 119 +++++++++++++++++++++++++----------------- 1 file changed, 72 insertions(+), 47 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 1fe680f121..fc402e936f 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -846,6 +846,60 @@ static int qemuAgentSend(qemuAgent *agent, } +/** + * qemuAgentGuestSyncSend: + * @agent: agent object + * @timeout: timeout for the command + * @first: true when this is the first invocation to drain possible leftovers + * from the pipe + * + * Sends a sync request to the guest agent. + * Returns: -1 on error + * 0 on successful send, but when no reply was received + * 1 when a reply was received + */ +static int +qemuAgentGuestSyncSend(qemuAgent *agent, + int timeout, + bool first) +{ + g_autofree char *txMsg = NULL; + g_autoptr(virJSONValue) rxObj = NULL; + unsigned long long id; + qemuAgentMessage sync_msg; + int rc; + + memset(&sync_msg, 0, sizeof(sync_msg)); + + if (virTimeMillisNow(&id) < 0) + return -1; + + txMsg = g_strdup_printf("{\"execute\":\"guest-sync\", " + "\"arguments\":{\"id\":%llu}}\n", id); + + sync_msg.txBuffer = txMsg; + sync_msg.txLength = strlen(txMsg); + sync_msg.sync = true; + sync_msg.id = id; + sync_msg.first = first; + + VIR_DEBUG("Sending guest-sync command with ID: %llu", id); + + rc = qemuAgentSend(agent, &sync_msg, timeout); + rxObj = g_steal_pointer(&sync_msg.rxObject); + + VIR_DEBUG("qemuAgentSend returned: %d", rc); + + if (rc < 0) + return -1; + + if (rxObj) + return 1; + + return 0; +} + + /** * qemuAgentGuestSync: * @agent: agent object @@ -860,11 +914,8 @@ static int qemuAgentSend(qemuAgent *agent, static int qemuAgentGuestSync(qemuAgent *agent) { - int ret = -1; - int send_ret; - unsigned long long id; - qemuAgentMessage sync_msg; int timeout = VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT; + int rc; if (agent->singleSync && agent->inSync) return 0; @@ -874,55 +925,29 @@ qemuAgentGuestSync(qemuAgent *agent) if ((agent->timeout >= 0) && (agent->timeout < QEMU_AGENT_WAIT_TIME)) timeout = agent->timeout; - memset(&sync_msg, 0, sizeof(sync_msg)); - /* set only on first sync */ - sync_msg.first = true; - - retry: - if (virTimeMillisNow(&id) < 0) + if ((rc = qemuAgentGuestSyncSend(agent, timeout, true)) < 0) return -1; - sync_msg.txBuffer = g_strdup_printf("{\"execute\":\"guest-sync\", " - "\"arguments\":{\"id\":%llu}}\n", id); - - sync_msg.txLength = strlen(sync_msg.txBuffer); - sync_msg.sync = true; - sync_msg.id = id; - - VIR_DEBUG("Sending guest-sync command with ID: %llu", id); - - send_ret = qemuAgentSend(agent, &sync_msg, timeout); - - VIR_DEBUG("qemuAgentSend returned: %d", send_ret); - - if (send_ret < 0) - goto cleanup; + /* successfully sync'd */ + if (rc == 1) + return 0; - if (!sync_msg.rxObject) { - if (sync_msg.first) { - VIR_FREE(sync_msg.txBuffer); - memset(&sync_msg, 0, sizeof(sync_msg)); - goto retry; - } else { - if (agent->running) - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing agent reply object")); - else - virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", - _("Guest agent disappeared while executing command")); - goto cleanup; - } - } + /* send another sync */ + if ((rc = qemuAgentGuestSyncSend(agent, timeout, false)) < 0) + return -1; - if (agent->singleSync) - agent->inSync = true; + /* successfully sync'd */ + if (rc == 1) + return 0; - ret = 0; + if (agent->running) + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing agent reply object")); + else + virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s", + _("Guest agent disappeared while executing command")); - cleanup: - virJSONValueFree(sync_msg.rxObject); - VIR_FREE(sync_msg.txBuffer); - return ret; + return -1; } static const char * -- 2.31.1

Peter Krempa

8:57 a.m.

New subject: [PATCH 12/16] qemuMonitorJSONGetCPUModelExpansion: Don't use goto for looping

Don't use 'goto' for looping. Extract the monitor interaction code into a new function and restructure the logic to avoid jumping back in the code. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_monitor_json.c | 74 +++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c05a2f3cff..ec13f270eb 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5469,29 +5469,16 @@ qemuMonitorJSONParseCPUModel(const char *cpu_name, } -int -qemuMonitorJSONGetCPUModelExpansion(qemuMonitor *mon, - qemuMonitorCPUModelExpansionType type, - virCPUDef *cpu, - bool migratable, - bool fail_no_props, - qemuMonitorCPUModelInfo **model_info) +static int +qemuMonitorJSONQueryCPUModelExpansionOne(qemuMonitor *mon, + qemuMonitorCPUModelExpansionType type, + virJSONValue **model, + virJSONValue **data) { - g_autoptr(virJSONValue) model = NULL; g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; - virJSONValue *data; - virJSONValue *cpu_model; - virJSONValue *cpu_props = NULL; - const char *cpu_name = ""; const char *typeStr = ""; - *model_info = NULL; - - if (!(model = qemuMonitorJSONMakeCPUModel(cpu, migratable))) - return -1; - - retry: switch (type) { case QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC: case QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL: @@ -5502,10 +5489,9 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitor *mon, typeStr = "full"; break; } - if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-expansion", "s:type", typeStr, - "a:model", &model, + "a:model", model, NULL))) return -1; @@ -5522,7 +5508,35 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitor *mon, if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0) return -1; - data = virJSONValueObjectGetObject(reply, "return"); + *data = virJSONValueObjectStealObject(reply, "return"); + + return 1; +} + + +int +qemuMonitorJSONGetCPUModelExpansion(qemuMonitor *mon, + qemuMonitorCPUModelExpansionType type, + virCPUDef *cpu, + bool migratable, + bool fail_no_props, + qemuMonitorCPUModelInfo **model_info) +{ + g_autoptr(virJSONValue) model = NULL; + g_autoptr(virJSONValue) data = NULL; + g_autoptr(virJSONValue) fullData = NULL; + virJSONValue *cpu_model; + virJSONValue *cpu_props = NULL; + const char *cpu_name = ""; + int rc; + + *model_info = NULL; + + if (!(model = qemuMonitorJSONMakeCPUModel(cpu, migratable))) + return -1; + + if ((rc = qemuMonitorJSONQueryCPUModelExpansionOne(mon, type, &model, &data)) <= 0) + return rc; if (qemuMonitorJSONParseCPUModelData(data, "query-cpu-model-expansion", fail_no_props, &cpu_model, &cpu_props, @@ -5530,16 +5544,22 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitor *mon, return -1; /* QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL requests "full" expansion - * on the result of the initial "static" expansion. - */ + * on the result of the initial "static" expansion. */ if (type == QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL) { - if (!(model = virJSONValueCopy(cpu_model))) + g_autoptr(virJSONValue) fullModel = virJSONValueCopy(cpu_model); + + if (!fullModel) return -1; - virJSONValueFree(cmd); - virJSONValueFree(reply); type = QEMU_MONITOR_CPU_MODEL_EXPANSION_FULL; - goto retry; + + if ((rc = qemuMonitorJSONQueryCPUModelExpansionOne(mon, type, &fullModel, &fullData)) <= 0) + return rc; + + if (qemuMonitorJSONParseCPUModelData(fullData, "query-cpu-model-expansion", + fail_no_props, &cpu_model, &cpu_props, + &cpu_name) < 0) + return -1; } return qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, model_info); -- 2.31.1

Peter Krempa

8:57 a.m.

New subject: [PATCH 13/16] qemuMonitorAddObject: Use g_clear_pointer for a free and reset operation

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_monitor.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index d752b299ab..6beb23e9f7 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2956,10 +2956,9 @@ qemuMonitorAddObject(qemuMonitor *mon, ignore_value(virJSONValueObjectRemoveKey(*props, "qom-type", &typeobj)); ignore_value(virJSONValueObjectRemoveKey(*props, "id", &idobj)); - if (!virJSONValueObjectGetKey(*props, 0)) { - virJSONValueFree(*props); - *props = NULL; - } + /* avoid empty 'props' member */ + if (!virJSONValueObjectGetKey(*props, 0)) + g_clear_pointer(props, virJSONValueFree); if (virJSONValueObjectAdd(&pr, "s:qom-type", type, -- 2.31.1

Peter Krempa

8:57 a.m.

New subject: [PATCH 14/16] qemuAgentIOProcessLine: refactor cleanup

Refactor the control flow so we can remove the cleanup label. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_agent.c | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index fc402e936f..c573e0fdf4 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -234,8 +234,7 @@ qemuAgentIOProcessLine(qemuAgent *agent, const char *line, qemuAgentMessage *msg) { - virJSONValue *obj = NULL; - int ret = -1; + g_autoptr(virJSONValue) obj = NULL; VIR_DEBUG("Line [%s]", line); @@ -247,19 +246,19 @@ qemuAgentIOProcessLine(qemuAgent *agent, return 0; } - goto cleanup; + return -1; } if (virJSONValueGetType(obj) != VIR_JSON_TYPE_OBJECT) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Parsed JSON reply '%s' isn't an object"), line); - goto cleanup; + return -1; } if (virJSONValueObjectHasKey(obj, "QMP") == 1) { - ret = 0; + return 0; } else if (virJSONValueObjectHasKey(obj, "event") == 1) { - ret = qemuAgentIOProcessEvent(agent, obj); + return qemuAgentIOProcessEvent(agent, obj); } else if (virJSONValueObjectHasKey(obj, "error") == 1 || virJSONValueObjectHasKey(obj, "return") == 1) { if (msg) { @@ -268,8 +267,7 @@ qemuAgentIOProcessLine(qemuAgent *agent, if (virJSONValueObjectGetNumberUlong(obj, "return", &id) < 0) { VIR_DEBUG("Ignoring delayed reply on sync"); - ret = 0; - goto cleanup; + return 0; } VIR_DEBUG("Guest returned ID: %llu", id); @@ -277,8 +275,7 @@ qemuAgentIOProcessLine(qemuAgent *agent, if (msg->id != id) { VIR_DEBUG("Guest agent returned ID: %llu instead of %llu", id, msg->id); - ret = 0; - goto cleanup; + return 0; } } msg->rxObject = g_steal_pointer(&obj); @@ -287,15 +284,13 @@ qemuAgentIOProcessLine(qemuAgent *agent, /* we are out of sync */ VIR_DEBUG("Ignoring delayed reply"); } - ret = 0; - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unknown JSON reply '%s'"), line); + + return 0; } - cleanup: - virJSONValueFree(obj); - return ret; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown JSON reply '%s'"), line); + return -1; } static int qemuAgentIOProcessData(qemuAgent *agent, -- 2.31.1

Peter Krempa

8:57 a.m.

New subject: [PATCH 15/16] qemu: agent: Automatically free virJSONValue-s

Convert the code to use g_autoptr for the few cases sill using explicit cleanup. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_agent.c | 60 ++++++++++++++----------------------------- 1 file changed, 19 insertions(+), 41 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index c573e0fdf4..1eb31ffca2 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1206,8 +1206,8 @@ int qemuAgentShutdown(qemuAgent *agent, qemuAgentShutdownMode mode) { int ret = -1; - virJSONValue *cmd; - virJSONValue *reply = NULL; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; cmd = qemuAgentMakeCommand("guest-shutdown", "s:mode", qemuAgentShutdownModeTypeToString(mode), @@ -1222,8 +1222,6 @@ int qemuAgentShutdown(qemuAgent *agent, ret = qemuAgentCommand(agent, cmd, &reply, VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN); - virJSONValueFree(cmd); - virJSONValueFree(reply); return ret; } @@ -1245,12 +1243,11 @@ int qemuAgentFSFreeze(qemuAgent *agent, const char **mountpoints, unsigned int nmountpoints) { int ret = -1; - virJSONValue *cmd; - virJSONValue *arg = NULL; - virJSONValue *reply = NULL; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; if (mountpoints && nmountpoints) { - arg = qemuAgentMakeStringsArray(mountpoints, nmountpoints); + g_autoptr(virJSONValue) arg = qemuAgentMakeStringsArray(mountpoints, nmountpoints); if (!arg) return -1; @@ -1272,9 +1269,6 @@ int qemuAgentFSFreeze(qemuAgent *agent, const char **mountpoints, } cleanup: - virJSONValueFree(arg); - virJSONValueFree(cmd); - virJSONValueFree(reply); return ret; } @@ -1292,10 +1286,8 @@ int qemuAgentFSFreeze(qemuAgent *agent, const char **mountpoints, int qemuAgentFSThaw(qemuAgent *agent) { int ret = -1; - virJSONValue *cmd; - virJSONValue *reply = NULL; - - cmd = qemuAgentMakeCommand("guest-fsfreeze-thaw", NULL); + g_autoptr(virJSONValue) cmd = qemuAgentMakeCommand("guest-fsfreeze-thaw", NULL); + g_autoptr(virJSONValue) reply = NULL; if (!cmd) return -1; @@ -1309,8 +1301,6 @@ int qemuAgentFSThaw(qemuAgent *agent) } cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); return ret; } @@ -1328,8 +1318,8 @@ qemuAgentSuspend(qemuAgent *agent, unsigned int target) { int ret = -1; - virJSONValue *cmd; - virJSONValue *reply = NULL; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; cmd = qemuAgentMakeCommand(qemuAgentSuspendModeTypeToString(target), NULL); @@ -1339,8 +1329,6 @@ qemuAgentSuspend(qemuAgent *agent, agent->await_event = QEMU_AGENT_EVENT_SUSPEND; ret = qemuAgentCommand(agent, cmd, &reply, agent->timeout); - virJSONValueFree(cmd); - virJSONValueFree(reply); return ret; } @@ -1351,8 +1339,8 @@ qemuAgentArbitraryCommand(qemuAgent *agent, int timeout) { int ret = -1; - virJSONValue *cmd = NULL; - virJSONValue *reply = NULL; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; *result = NULL; if (timeout < VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN) { @@ -1374,8 +1362,6 @@ qemuAgentArbitraryCommand(qemuAgent *agent, cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); return ret; } @@ -1384,8 +1370,8 @@ qemuAgentFSTrim(qemuAgent *agent, unsigned long long minimum) { int ret = -1; - virJSONValue *cmd; - virJSONValue *reply = NULL; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; cmd = qemuAgentMakeCommand("guest-fstrim", "U:minimum", minimum, @@ -1395,8 +1381,6 @@ qemuAgentFSTrim(qemuAgent *agent, ret = qemuAgentCommand(agent, cmd, &reply, agent->timeout); - virJSONValueFree(cmd); - virJSONValueFree(reply); return ret; } @@ -1406,8 +1390,8 @@ qemuAgentGetVCPUs(qemuAgent *agent, { int ret = -1; size_t i; - virJSONValue *cmd; - virJSONValue *reply = NULL; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; virJSONValue *data = NULL; size_t ndata; @@ -1461,8 +1445,6 @@ qemuAgentGetVCPUs(qemuAgent *agent, ret = ndata; cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); return ret; } @@ -1696,8 +1678,8 @@ qemuAgentGetTime(qemuAgent *agent, { int ret = -1; unsigned long long json_time; - virJSONValue *cmd; - virJSONValue *reply = NULL; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; cmd = qemuAgentMakeCommand("guest-get-time", NULL); @@ -1720,8 +1702,6 @@ qemuAgentGetTime(qemuAgent *agent, ret = 0; cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); return ret; } @@ -1738,8 +1718,8 @@ qemuAgentSetTime(qemuAgent *agent, bool rtcSync) { int ret = -1; - virJSONValue *cmd; - virJSONValue *reply = NULL; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; if (rtcSync) { cmd = qemuAgentMakeCommand("guest-set-time", NULL); @@ -1774,8 +1754,6 @@ qemuAgentSetTime(qemuAgent *agent, ret = 0; cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); return ret; } -- 2.31.1

Peter Krempa

8:57 a.m.

New subject: [PATCH 16/16] qemu: agent: Remove unneeded cleanup sections

Remove the cleanup sections where not needed after we've converted to automatic freeing of virJSONValue. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- src/qemu/qemu_agent.c | 96 +++++++++++++++++-------------------------- 1 file changed, 38 insertions(+), 58 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 1eb31ffca2..e2107e5cbf 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1205,7 +1205,6 @@ VIR_ENUM_IMPL(qemuAgentShutdownMode, int qemuAgentShutdown(qemuAgent *agent, qemuAgentShutdownMode mode) { - int ret = -1; g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; @@ -1219,10 +1218,9 @@ int qemuAgentShutdown(qemuAgent *agent, agent->await_event = QEMU_AGENT_EVENT_RESET; else agent->await_event = QEMU_AGENT_EVENT_SHUTDOWN; - ret = qemuAgentCommand(agent, cmd, &reply, - VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN); - return ret; + return qemuAgentCommand(agent, cmd, &reply, + VIR_DOMAIN_QEMU_AGENT_COMMAND_SHUTDOWN); } /* @@ -1242,7 +1240,7 @@ int qemuAgentShutdown(qemuAgent *agent, int qemuAgentFSFreeze(qemuAgent *agent, const char **mountpoints, unsigned int nmountpoints) { - int ret = -1; + int nfrozen = 0; g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; @@ -1258,18 +1256,18 @@ int qemuAgentFSFreeze(qemuAgent *agent, const char **mountpoints, } if (!cmd) - goto cleanup; + return -1; if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) - goto cleanup; + return -1; - if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) { + if (virJSONValueObjectGetNumberInt(reply, "return", &nfrozen) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed return value")); + return -1; } - cleanup: - return ret; + return nfrozen; } /* @@ -1285,7 +1283,7 @@ int qemuAgentFSFreeze(qemuAgent *agent, const char **mountpoints, */ int qemuAgentFSThaw(qemuAgent *agent) { - int ret = -1; + int nthawed = 0; g_autoptr(virJSONValue) cmd = qemuAgentMakeCommand("guest-fsfreeze-thaw", NULL); g_autoptr(virJSONValue) reply = NULL; @@ -1293,15 +1291,15 @@ int qemuAgentFSThaw(qemuAgent *agent) return -1; if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) - goto cleanup; + return -1; - if (virJSONValueObjectGetNumberInt(reply, "return", &ret) < 0) { + if (virJSONValueObjectGetNumberInt(reply, "return", &nthawed) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed return value")); + return -1; } - cleanup: - return ret; + return nthawed; } VIR_ENUM_DECL(qemuAgentSuspendMode); @@ -1317,7 +1315,6 @@ int qemuAgentSuspend(qemuAgent *agent, unsigned int target) { - int ret = -1; g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; @@ -1327,9 +1324,8 @@ qemuAgentSuspend(qemuAgent *agent, return -1; agent->await_event = QEMU_AGENT_EVENT_SUSPEND; - ret = qemuAgentCommand(agent, cmd, &reply, agent->timeout); - return ret; + return qemuAgentCommand(agent, cmd, &reply, agent->timeout); } int @@ -1338,7 +1334,7 @@ qemuAgentArbitraryCommand(qemuAgent *agent, char **result, int timeout) { - int ret = -1; + int rc; g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; @@ -1348,28 +1344,25 @@ qemuAgentArbitraryCommand(qemuAgent *agent, _("guest agent timeout '%d' is " "less than the minimum '%d'"), timeout, VIR_DOMAIN_QEMU_AGENT_COMMAND_MIN); - goto cleanup; + return -1; } if (!(cmd = virJSONValueFromString(cmd_str))) - goto cleanup; + return -1; - if ((ret = qemuAgentCommand(agent, cmd, &reply, timeout)) < 0) - goto cleanup; + if ((rc = qemuAgentCommand(agent, cmd, &reply, timeout)) < 0) + return rc; if (!(*result = virJSONValueToString(reply, false))) - ret = -1; - + return -1; - cleanup: - return ret; + return rc; } int qemuAgentFSTrim(qemuAgent *agent, unsigned long long minimum) { - int ret = -1; g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; @@ -1377,18 +1370,15 @@ qemuAgentFSTrim(qemuAgent *agent, "U:minimum", minimum, NULL); if (!cmd) - return ret; - - ret = qemuAgentCommand(agent, cmd, &reply, agent->timeout); + return -1; - return ret; + return qemuAgentCommand(agent, cmd, &reply, agent->timeout); } int qemuAgentGetVCPUs(qemuAgent *agent, qemuAgentCPUInfo **info) { - int ret = -1; size_t i; g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; @@ -1399,12 +1389,12 @@ qemuAgentGetVCPUs(qemuAgent *agent, return -1; if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) - goto cleanup; + return -1; if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("guest-get-vcpus reply was missing return data")); - goto cleanup; + return -1; } ndata = virJSONValueArraySize(data); @@ -1419,33 +1409,30 @@ qemuAgentGetVCPUs(qemuAgent *agent, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("array element missing in guest-get-vcpus return " "value")); - goto cleanup; + return -1; } if (virJSONValueObjectGetNumberUint(entry, "logical-id", &in->id) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("'logical-id' missing in reply of guest-get-vcpus")); - goto cleanup; + return -1; } if (virJSONValueObjectGetBoolean(entry, "online", &in->online) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("'online' missing in reply of guest-get-vcpus")); - goto cleanup; + return -1; } if (virJSONValueObjectGetBoolean(entry, "can-offline", &in->offlinable) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("'can-offline' missing in reply of guest-get-vcpus")); - goto cleanup; + return -1; } } - ret = ndata; - - cleanup: - return ret; + return ndata; } @@ -1676,7 +1663,6 @@ qemuAgentGetTime(qemuAgent *agent, long long *seconds, unsigned int *nseconds) { - int ret = -1; unsigned long long json_time; g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; @@ -1684,25 +1670,22 @@ qemuAgentGetTime(qemuAgent *agent, cmd = qemuAgentMakeCommand("guest-get-time", NULL); if (!cmd) - return ret; + return -1; if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) - goto cleanup; + return -1; if (virJSONValueObjectGetNumberUlong(reply, "return", &json_time) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed return value")); - goto cleanup; + return -1; } /* guest agent returns time in nanoseconds, * we need it in seconds here */ *seconds = json_time / 1000000000LL; *nseconds = json_time % 1000000000LL; - ret = 0; - - cleanup: - return ret; + return 0; } @@ -1717,7 +1700,6 @@ qemuAgentSetTime(qemuAgent *agent, unsigned int nseconds, bool rtcSync) { - int ret = -1; g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; @@ -1736,7 +1718,7 @@ qemuAgentSetTime(qemuAgent *agent, virReportError(VIR_ERR_INVALID_ARG, _("Time '%lld' is too big for guest agent"), seconds); - return ret; + return -1; } json_time = seconds * 1000000000LL; @@ -1747,14 +1729,12 @@ qemuAgentSetTime(qemuAgent *agent, } if (!cmd) - return ret; + return -1; if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - return ret; + return 0; } void -- 2.31.1

Ján Tomko

9:47 a.m.

On a Friday in 2021, Peter Krempa wrote:

...

Reviewed-by: Ján Tomko <jtomko(a)redhat.com> Jano

1085

days inactive

1085

days old

devel@lists.libvirt.org

Manage subscription

17 comments

2 participants

tags (0)

participants (2)

Ján Tomko
Peter Krempa

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

[PATCH 00/16] Clean up of virJSONValueFree usage and few related refactors