[libvirt] [PATCH 0/4] qemu_monitor_json: Refactor even more

All these patches will be squashed into one, but I've split them into multiple because of easier review. Michal Privoznik (4): qemu_monitor_json: Follow refactor qemu_monitor_json: Follow refactor qemu_monitor_json: Follow refactor qemu_monitor_json: Follow refactor src/qemu/qemu_monitor_json.c | 673 ++++++++++++++++++++++--------------------- 1 file changed, 349 insertions(+), 324 deletions(-) -- 2.8.1

In 7884d089d2f I've started to refactor qemu_monitor_json.c. Thing is, it's current structure is nothing like the rest of our code. The @ret variable is rewritten all the time, if()-s are nested instead of using goto and so on. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_monitor_json.c | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 51fa790..dfb31a2 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2319,7 +2319,7 @@ int qemuMonitorJSONEjectMedia(qemuMonitorPtr mon, const char *dev_name, bool force) { - int ret; + int ret = -1; virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("eject", "s:device", dev_name, "b:force", force ? 1 : 0, @@ -2328,11 +2328,14 @@ int qemuMonitorJSONEjectMedia(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -2344,7 +2347,7 @@ int qemuMonitorJSONChangeMedia(qemuMonitorPtr mon, const char *newmedia, const char *format) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; @@ -2357,11 +2360,14 @@ int qemuMonitorJSONChangeMedia(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -2374,7 +2380,7 @@ static int qemuMonitorJSONSaveMemory(qemuMonitorPtr mon, size_t length, const char *path) { - int ret; + int ret = -1; virJSONValuePtr cmd = qemuMonitorJSONMakeCommand(cmdtype, "U:val", offset, "u:size", length, @@ -2384,11 +2390,14 @@ static int qemuMonitorJSONSaveMemory(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; -- 2.8.1

This is going to be squashed into previous commit. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_monitor_json.c | 162 +++++++++++++++++++++++++------------------ 1 file changed, 94 insertions(+), 68 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index dfb31a2..e9e1c51 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2424,7 +2424,7 @@ int qemuMonitorJSONSavePhysicalMemory(qemuMonitorPtr mon, int qemuMonitorJSONSetMigrationSpeed(qemuMonitorPtr mon, unsigned long bandwidth) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; cmd = qemuMonitorJSONMakeCommand("migrate_set_speed", @@ -2433,11 +2433,14 @@ int qemuMonitorJSONSetMigrationSpeed(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -2447,7 +2450,7 @@ int qemuMonitorJSONSetMigrationSpeed(qemuMonitorPtr mon, int qemuMonitorJSONSetMigrationDowntime(qemuMonitorPtr mon, unsigned long long downtime) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; @@ -2457,11 +2460,14 @@ int qemuMonitorJSONSetMigrationDowntime(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -2472,7 +2478,7 @@ int qemuMonitorJSONGetMigrationCacheSize(qemuMonitorPtr mon, unsigned long long *cacheSize) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; @@ -2482,16 +2488,13 @@ qemuMonitorJSONGetMigrationCacheSize(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret < 0) + if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; - ret = virJSONValueObjectGetNumberUlong(reply, "return", cacheSize); - if (ret < 0) { + if (virJSONValueObjectGetNumberUlong(reply, "return", cacheSize) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-migrate-cache-size reply was missing " "'return' data")); @@ -2510,7 +2513,7 @@ int qemuMonitorJSONSetMigrationCacheSize(qemuMonitorPtr mon, unsigned long long cacheSize) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; @@ -2520,11 +2523,14 @@ qemuMonitorJSONSetMigrationCacheSize(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -2543,10 +2549,10 @@ qemuMonitorJSONGetMigrationCompression(qemuMonitorPtr mon, if (!(cmd = qemuMonitorJSONMakeCommand("query-migrate-parameters", NULL))) return -1; - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; - if ((ret = qemuMonitorJSONCheckError(cmd, reply)) < 0) + if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; result = virJSONValueObjectGet(reply, "return"); @@ -2623,7 +2629,7 @@ qemuMonitorJSONSetMigrationCompression(qemuMonitorPtr mon, goto cleanup; args = NULL; - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; ret = qemuMonitorJSONCheckError(cmd, reply); @@ -2836,7 +2842,7 @@ qemuMonitorJSONGetMigrationStatsReply(virJSONValuePtr reply, int qemuMonitorJSONGetMigrationStats(qemuMonitorPtr mon, qemuMonitorMigrationStatsPtr stats) { - int ret; + int ret = -1; virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-migrate", NULL); virJSONValuePtr reply = NULL; @@ -2846,15 +2852,17 @@ int qemuMonitorJSONGetMigrationStats(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; - if (ret == 0 && - qemuMonitorJSONGetMigrationStatsReply(reply, stats) < 0) - ret = -1; + if (qemuMonitorJSONGetMigrationStatsReply(reply, stats) < 0) + goto cleanup; + ret = 0; + cleanup: if (ret < 0) memset(stats, 0, sizeof(*stats)); virJSONValueFree(cmd); @@ -2867,7 +2875,7 @@ int qemuMonitorJSONMigrate(qemuMonitorPtr mon, unsigned int flags, const char *uri) { - int ret; + int ret = -1; virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("migrate", "b:detach", flags & QEMU_MONITOR_MIGRATE_BACKGROUND ? 1 : 0, @@ -2880,11 +2888,14 @@ int qemuMonitorJSONMigrate(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -2892,17 +2903,20 @@ int qemuMonitorJSONMigrate(qemuMonitorPtr mon, int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon) { - int ret; + int ret = -1; virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("migrate_cancel", NULL); virJSONValuePtr reply = NULL; if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -2912,7 +2926,7 @@ int qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon, const char *capability) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr caps; @@ -2923,19 +2937,17 @@ qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon, NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) { - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) - goto cleanup; - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + ret = 0; + goto cleanup; } - if (ret < 0) + if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; - ret = -1; - if (!(caps = virJSONValueObjectGetObject(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing dump guest memory capabilities")); @@ -2961,10 +2973,9 @@ qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitorPtr mon, ret = 1; goto cleanup; } - - ret = 0; } + ret = 0; cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); @@ -2976,7 +2987,7 @@ qemuMonitorJSONDump(qemuMonitorPtr mon, const char *protocol, const char *dumpformat) { - int ret; + int ret = -1; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; @@ -2996,11 +3007,14 @@ qemuMonitorJSONDump(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -3026,11 +3040,14 @@ int qemuMonitorJSONGraphicsRelocate(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -3041,7 +3058,7 @@ int qemuMonitorJSONSendFileHandle(qemuMonitorPtr mon, const char *fdname, int fd) { - int ret; + int ret = -1; virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("getfd", "s:fdname", fdname, NULL); @@ -3049,11 +3066,14 @@ int qemuMonitorJSONSendFileHandle(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommandWithFd(mon, cmd, fd, &reply); + if (qemuMonitorJSONCommandWithFd(mon, cmd, fd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -3063,7 +3083,7 @@ int qemuMonitorJSONSendFileHandle(qemuMonitorPtr mon, int qemuMonitorJSONCloseFileHandle(qemuMonitorPtr mon, const char *fdname) { - int ret; + int ret = -1; virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("closefd", "s:fdname", fdname, NULL); @@ -3071,11 +3091,14 @@ int qemuMonitorJSONCloseFileHandle(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -3139,7 +3162,7 @@ qemuMonitorJSONAddFd(qemuMonitorPtr mon, int fdset, int fd, const char *name) int qemuMonitorJSONRemoveFd(qemuMonitorPtr mon, int fdset, int fd) { - int ret; + int ret = -1; virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("remove-fd", "i:fdset-id", fdset, fd < 0 ? NULL : "i:fd", @@ -3148,11 +3171,14 @@ qemuMonitorJSONRemoveFd(qemuMonitorPtr mon, int fdset, int fd) if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; -- 2.8.1

This is going to be squashed into previous commit. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_monitor_json.c | 266 ++++++++++++++++++++++--------------------- 1 file changed, 139 insertions(+), 127 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index e9e1c51..c99d300 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3205,11 +3205,13 @@ int qemuMonitorJSONAddNetdev(qemuMonitorPtr mon, goto cleanup; args = NULL; /* obj owns reference to args now */ - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; cleanup: virJSONValueFree(args); virJSONValueFree(cmd); @@ -3221,7 +3223,7 @@ int qemuMonitorJSONAddNetdev(qemuMonitorPtr mon, int qemuMonitorJSONRemoveNetdev(qemuMonitorPtr mon, const char *alias) { - int ret; + int ret = -1; virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("netdev_del", "s:id", alias, NULL); @@ -3229,11 +3231,14 @@ int qemuMonitorJSONRemoveNetdev(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -3539,7 +3544,7 @@ qemuMonitorJSONGetChardevInfo(qemuMonitorPtr mon, virHashTablePtr info) { - int ret; + int ret = -1; virJSONValuePtr cmd = qemuMonitorJSONMakeCommand("query-chardev", NULL); virJSONValuePtr reply = NULL; @@ -3547,14 +3552,14 @@ qemuMonitorJSONGetChardevInfo(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - if (ret == 0) - ret = qemuMonitorJSONExtractChardevInfo(reply, info); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = qemuMonitorJSONExtractChardevInfo(reply, info); + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -3573,7 +3578,7 @@ int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon ATTRIBUTE_UNUSED, int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, const char *devalias) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; @@ -3583,11 +3588,14 @@ int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -3614,11 +3622,13 @@ int qemuMonitorJSONAddDevice(qemuMonitorPtr mon, goto cleanup; args = NULL; /* obj owns reference to args now */ - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; cleanup: virJSONValueFree(args); virJSONValueFree(cmd); @@ -3644,14 +3654,16 @@ int qemuMonitorJSONAddObject(qemuMonitorPtr mon, if (!cmd) goto cleanup; - /* @props is part of @cmd now. Avoid double free */ + /* @props is part of @cmd now. Avoid double free */ props = NULL; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); @@ -3663,7 +3675,7 @@ int qemuMonitorJSONAddObject(qemuMonitorPtr mon, int qemuMonitorJSONDelObject(qemuMonitorPtr mon, const char *objalias) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; @@ -3673,11 +3685,14 @@ int qemuMonitorJSONDelObject(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -3688,7 +3703,7 @@ int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, const char *alias, const char *passphrase) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; char *drive; @@ -3704,11 +3719,14 @@ int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -3800,10 +3818,13 @@ qemuMonitorJSONDriveMirror(qemuMonitorPtr mon, if (!cmd) return -1; - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + ret = 0; cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); @@ -3826,11 +3847,13 @@ qemuMonitorJSONTransaction(qemuMonitorPtr mon, virJSONValuePtr actions) if (!cmd) goto cleanup; - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); @@ -3866,7 +3889,7 @@ qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device, if (!cmd) return -1; - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; if (!top && !base) { /* Normally we always specify top and base; but omitting them @@ -3896,7 +3919,7 @@ int qemuMonitorJSONDrivePivot(qemuMonitorPtr mon, const char *device) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; @@ -3906,10 +3929,13 @@ qemuMonitorJSONDrivePivot(qemuMonitorPtr mon, if (!cmd) return -1; - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + ret = 0; cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); @@ -4043,13 +4069,12 @@ int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon, cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); - return ret; } int qemuMonitorJSONInjectNMI(qemuMonitorPtr mon) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; @@ -4057,7 +4082,7 @@ int qemuMonitorJSONInjectNMI(qemuMonitorPtr mon) if (!cmd) return -1; - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { @@ -4125,7 +4150,7 @@ int qemuMonitorJSONSendKey(qemuMonitorPtr mon, /* @keys is part of @cmd now. Avoid double free */ keys = NULL; - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { @@ -4146,7 +4171,7 @@ int qemuMonitorJSONSendKey(qemuMonitorPtr mon, int qemuMonitorJSONScreendump(qemuMonitorPtr mon, const char *file) { - int ret; + int ret = -1; virJSONValuePtr cmd, reply = NULL; cmd = qemuMonitorJSONMakeCommand("screendump", @@ -4156,11 +4181,14 @@ int qemuMonitorJSONScreendump(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -4422,7 +4450,7 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon, const char *fdname, bool skipauth) { - int ret; + int ret = -1; virJSONValuePtr cmd, reply = NULL; cmd = qemuMonitorJSONMakeCommand("add_client", @@ -4434,11 +4462,14 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -4583,9 +4614,10 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &result); + if (qemuMonitorJSONCommand(mon, cmd, &result) < 0) + goto cleanup; - if (ret == 0 && virJSONValueObjectHasKey(result, "error")) { + if (virJSONValueObjectHasKey(result, "error")) { if (qemuMonitorJSONHasError(result, "DeviceNotActive")) virReportError(VIR_ERR_OPERATION_INVALID, _("No active operation on device: %s"), device); @@ -4595,9 +4627,11 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, else virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unexpected error")); - ret = -1; + goto cleanup; } + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(result); return ret; @@ -4616,9 +4650,10 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &result); + if (qemuMonitorJSONCommand(mon, cmd, &result) < 0) + goto cleanup; - if (ret == 0 && virJSONValueObjectHasKey(result, "error")) { + if (virJSONValueObjectHasKey(result, "error")) { if (qemuMonitorJSONHasError(result, "DeviceNotActive")) virReportError(VIR_ERR_OPERATION_INVALID, _("No active operation on device: %s"), device); @@ -4628,12 +4663,11 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr mon, else virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unexpected error")); - ret = -1; + goto cleanup; } - if (ret == 0) - ret = qemuMonitorJSONBlockIoThrottleInfo(result, device, reply, supportMaxOptions); - + ret = qemuMonitorJSONBlockIoThrottleInfo(result, device, reply, supportMaxOptions); + cleanup: virJSONValueFree(cmd); virJSONValueFree(result); return ret; @@ -4649,11 +4683,14 @@ int qemuMonitorJSONSystemWakeup(qemuMonitorPtr mon) if (!cmd) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -4665,7 +4702,7 @@ int qemuMonitorJSONGetVersion(qemuMonitorPtr mon, int *micro, char **package) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr data; @@ -4678,15 +4715,11 @@ int qemuMonitorJSONGetVersion(qemuMonitorPtr mon, if (!(cmd = qemuMonitorJSONMakeCommand("query-version", NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - if (ret < 0) + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; - ret = -1; + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; if (!(data = virJSONValueObjectGetObject(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -4739,7 +4772,7 @@ int qemuMonitorJSONGetVersion(qemuMonitorPtr mon, int qemuMonitorJSONGetMachines(qemuMonitorPtr mon, qemuMonitorMachineInfoPtr **machines) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr data; @@ -4752,15 +4785,11 @@ int qemuMonitorJSONGetMachines(qemuMonitorPtr mon, if (!(cmd = qemuMonitorJSONMakeCommand("query-machines", NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - if (ret < 0) + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; - ret = -1; + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -4839,7 +4868,7 @@ int qemuMonitorJSONGetMachines(qemuMonitorPtr mon, int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon, char ***cpus) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr data; @@ -4852,28 +4881,21 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon, if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-definitions", NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) { - /* Urgh, some QEMU architectures have the query-cpu-definitions - * command, but return 'GenericError' with string "Not supported", - * instead of simply omitting the command entirely :-( - */ - if (qemuMonitorJSONHasError(reply, "GenericError")) { - ret = 0; - goto cleanup; - } - ret = qemuMonitorJSONCheckError(cmd, reply); + /* Urgh, some QEMU architectures have the query-cpu-definitions + * command, but return 'GenericError' with string "Not supported", + * instead of simply omitting the command entirely :-( + */ + if (qemuMonitorJSONHasError(reply, "GenericError")) { + ret = 0; + goto cleanup; } - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - if (ret < 0) + if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; - ret = -1; - if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-cpu-definitions reply was missing return data")); @@ -4919,7 +4941,7 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon, int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, char ***commands) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr data; @@ -4932,15 +4954,11 @@ int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, if (!(cmd = qemuMonitorJSONMakeCommand("query-commands", NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - if (ret < 0) + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; - ret = -1; + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -4987,7 +5005,7 @@ int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, int qemuMonitorJSONGetEvents(qemuMonitorPtr mon, char ***events) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr data; @@ -5000,21 +5018,17 @@ int qemuMonitorJSONGetEvents(qemuMonitorPtr mon, if (!(cmd = qemuMonitorJSONMakeCommand("query-events", NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) { - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - ret = 0; - goto cleanup; - } - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + ret = 0; + goto cleanup; } - if (ret < 0) + if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; - ret = -1; - if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-events reply was missing return data")); @@ -5063,7 +5077,7 @@ qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon, char ***params, bool *found) { - int ret; + int ret = -1; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; virJSONValuePtr data = NULL; @@ -5084,15 +5098,15 @@ qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon, NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) { - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) - goto cleanup; - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + ret = 0; + goto cleanup; } - if (ret < 0) + if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; if (virJSONValueObjectRemoveKey(reply, "return", &array) <= 0) { @@ -5104,8 +5118,6 @@ qemuMonitorJSONGetCommandLineOptionParameters(qemuMonitorPtr mon, qemuMonitorSetOptions(mon, array); } - ret = -1; - if ((n = virJSONValueArraySize(array)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-command-line-options reply data was not " -- 2.8.1

This is going to be squashed into previous commit. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_monitor_json.c | 212 +++++++++++++++++++------------------------ 1 file changed, 95 insertions(+), 117 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c99d300..74d6346 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5197,7 +5197,7 @@ int qemuMonitorJSONGetKVMState(qemuMonitorPtr mon, bool *enabled, bool *present) { - int ret; + int ret = -1; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; virJSONValuePtr data = NULL; @@ -5208,20 +5208,17 @@ int qemuMonitorJSONGetKVMState(qemuMonitorPtr mon, if (!(cmd = qemuMonitorJSONMakeCommand("query-kvm", NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) { - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) - goto cleanup; - - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + ret = 0; + goto cleanup; } - if (ret < 0) + if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; - ret = -1; - if (!(data = virJSONValueObjectGetObject(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-kvm reply was missing return data")); @@ -5247,7 +5244,7 @@ int qemuMonitorJSONGetKVMState(qemuMonitorPtr mon, int qemuMonitorJSONGetObjectTypes(qemuMonitorPtr mon, char ***types) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr data; @@ -5260,15 +5257,11 @@ int qemuMonitorJSONGetObjectTypes(qemuMonitorPtr mon, if (!(cmd = qemuMonitorJSONMakeCommand("qom-list-types", NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - if (ret < 0) + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; - ret = -1; + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -5316,7 +5309,7 @@ int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon, const char *path, qemuMonitorJSONListPathPtr **paths) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr data; @@ -5331,15 +5324,11 @@ int qemuMonitorJSONGetObjectListPaths(qemuMonitorPtr mon, NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - if (ret < 0) + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; - ret = -1; + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -5416,7 +5405,7 @@ int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon, const char *property, qemuMonitorJSONObjectPropertyPtr prop) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr data; @@ -5428,15 +5417,11 @@ int qemuMonitorJSONGetObjectProperty(qemuMonitorPtr mon, NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - if (ret < 0) + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; - ret = -1; + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; data = virJSONValueObjectGet(reply, "return"); @@ -5508,9 +5493,9 @@ int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon, virJSONValuePtr reply = NULL; switch ((qemuMonitorJSONObjectPropertyType) prop->type) { - /* Simple cases of boolean, int, long, uint, ulong, double, and string - * will receive return value as part of {"return": xxx} statement - */ + /* Simple cases of boolean, int, long, uint, ulong, double, and string + * will receive return value as part of {"return": xxx} statement + */ case QEMU_MONITOR_OBJECT_PROPERTY_BOOLEAN: MAKE_SET_CMD("b:value", prop->val.b); break; @@ -5542,9 +5527,13 @@ int qemuMonitorJSONSetObjectProperty(qemuMonitorPtr mon, if (!cmd) return -1; - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + + ret = 0; cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); @@ -5558,7 +5547,7 @@ int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon, const char *type, char ***props) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr data; @@ -5573,21 +5562,17 @@ int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon, NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0 && - qemuMonitorJSONHasError(reply, "DeviceNotFound")) { + if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) { + ret = 0; goto cleanup; } - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - if (ret < 0) + if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; - ret = -1; - if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("device-list-properties reply was missing return data")); @@ -5634,7 +5619,6 @@ char * qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon) { char *ret = NULL; - int rv; const char *arch; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; @@ -5643,12 +5627,10 @@ qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon) if (!(cmd = qemuMonitorJSONMakeCommand("query-target", NULL))) return NULL; - rv = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (rv == 0) - rv = qemuMonitorJSONCheckError(cmd, reply); - - if (rv < 0) + if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; if (!(data = virJSONValueObjectGetObject(reply, "return"))) { @@ -5676,7 +5658,7 @@ int qemuMonitorJSONGetMigrationCapabilities(qemuMonitorPtr mon, char ***capabilities) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr caps; @@ -5690,19 +5672,17 @@ qemuMonitorJSONGetMigrationCapabilities(qemuMonitorPtr mon, NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) { - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) - goto cleanup; - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + ret = 0; + goto cleanup; } - if (ret < 0) + if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; - ret = -1; - if (!(caps = virJSONValueObjectGetArray(reply, "return")) || (n = virJSONValueArraySize(caps)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -5802,7 +5782,7 @@ qemuMonitorJSONSetMigrationCapability(qemuMonitorPtr mon, caps = NULL; - if ((ret = qemuMonitorJSONCommand(mon, cmd, &reply)) < 0) + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; ret = qemuMonitorJSONCheckError(cmd, reply); @@ -5835,7 +5815,7 @@ int qemuMonitorJSONGetGICCapabilities(qemuMonitorPtr mon, virGICCapability **capabilities) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr caps; @@ -5849,22 +5829,20 @@ qemuMonitorJSONGetGICCapabilities(qemuMonitorPtr mon, NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) { - /* If the 'query-gic-capabilities' QMP command was not available - * we simply successfully return zero capabilities. - * This is the case for QEMU <2.6 and all non-ARM architectures */ - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) - goto cleanup; - ret = qemuMonitorJSONCheckError(cmd, reply); + /* If the 'query-gic-capabilities' QMP command was not available + * we simply successfully return zero capabilities. + * This is the case for QEMU <2.6 and all non-ARM architectures */ + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + ret = 0; + goto cleanup; } - if (ret < 0) + if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; - ret = -1; - if (!(caps = virJSONValueObjectGetArray(reply, "return")) || (n = virJSONValueArraySize(caps)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -6033,11 +6011,14 @@ qemuMonitorJSONNBDServerAdd(qemuMonitorPtr mon, NULL))) return ret; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -6054,11 +6035,14 @@ qemuMonitorJSONNBDServerStop(qemuMonitorPtr mon) NULL))) return ret; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -6069,7 +6053,7 @@ static int qemuMonitorJSONGetStringArray(qemuMonitorPtr mon, const char *qmpCmd, char ***array) { - int ret; + int ret = -1; virJSONValuePtr cmd; virJSONValuePtr reply = NULL; virJSONValuePtr data; @@ -6082,20 +6066,17 @@ qemuMonitorJSONGetStringArray(qemuMonitorPtr mon, const char *qmpCmd, if (!(cmd = qemuMonitorJSONMakeCommand(qmpCmd, NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) { - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) - goto cleanup; - - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + ret = 0; + goto cleanup; } - if (ret < 0) + if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; - ret = -1; - if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("%s reply was missing return data"), @@ -6331,11 +6312,14 @@ qemuMonitorJSONDetachCharDev(qemuMonitorPtr mon, NULL))) return ret; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -6575,11 +6559,14 @@ qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon) NULL))) return ret; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; + ret = 0; + cleanup: virJSONValueFree(cmd); virJSONValueFree(reply); return ret; @@ -6608,15 +6595,11 @@ qemuMonitorJSONGetIOThreads(qemuMonitorPtr mon, if (!(cmd = qemuMonitorJSONMakeCommand("query-iothreads", NULL))) return ret; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); - - if (ret == 0) - ret = qemuMonitorJSONCheckError(cmd, reply); - - if (ret < 0) + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) goto cleanup; - ret = -1; + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + goto cleanup; if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -6700,22 +6683,17 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitorPtr mon, if (!(cmd = qemuMonitorJSONMakeCommand("query-memory-devices", NULL))) return -1; - ret = qemuMonitorJSONCommand(mon, cmd, &reply); + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + goto cleanup; - if (ret == 0) { - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - ret = -2; - goto cleanup; - } - - ret = qemuMonitorJSONCheckError(cmd, reply); + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { + ret = -2; + goto cleanup; } - if (ret < 0) + if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; - ret = -1; - if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-memory-devices reply was missing return data")); -- 2.8.1

On 05/04/2016 08:33 AM, Michal Privoznik wrote:
All these patches will be squashed into one, but I've split them into multiple because of easier review.
Michal Privoznik (4): qemu_monitor_json: Follow refactor qemu_monitor_json: Follow refactor qemu_monitor_json: Follow refactor qemu_monitor_json: Follow refactor
src/qemu/qemu_monitor_json.c | 673 ++++++++++++++++++++++--------------------- 1 file changed, 349 insertions(+), 324 deletions(-)
Not a deal breaker, but most code has: if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup; ret = 0; cleanup: while a few instances [1] have: ret = qemuMonitorJSONCheckError(cmd, reply); cleanup: IDC either way since the result is the same, but I suppose they "could" be all done the same way. I'd probably lean towards the latter, but since you already went with the former in commit '7884d089' it's probably best to keep using that. [1] qemuMonitorJSONSetMigrationCompression qemuMonitorJSONBlockCommit qemuMonitorJSONInjectNMI [2] qemuMonitorJSONSendKey [2] qemuMonitorJSONSetMigrationCapability [2] both instances use the same HMP call attempt to return ret - they could follow the qemuMonitorJSONSetCPU model... Finally, I see the comments move 4 spaces to the right in qemuMonitorJSONSetObjectProperty? (patch 4)... Was that a remnant or expected? ACK series (your call if you want to adjust [2] now or in a followup patch. it's trivial enough as long as you remember to also remove the {} around what would become "one line" if then else statements). John

On 06.05.2016 20:03, John Ferlan wrote:
On 05/04/2016 08:33 AM, Michal Privoznik wrote:
All these patches will be squashed into one, but I've split them into multiple because of easier review.
Michal Privoznik (4): qemu_monitor_json: Follow refactor qemu_monitor_json: Follow refactor qemu_monitor_json: Follow refactor qemu_monitor_json: Follow refactor
src/qemu/qemu_monitor_json.c | 673 ++++++++++++++++++++++--------------------- 1 file changed, 349 insertions(+), 324 deletions(-)
Not a deal breaker, but most code has:
if (qemuMonitorJSONCheckError(cmd, reply) < 0) goto cleanup;
ret = 0; cleanup:
while a few instances [1] have:
ret = qemuMonitorJSONCheckError(cmd, reply);
cleanup:
IDC either way since the result is the same, but I suppose they "could" be all done the same way. I'd probably lean towards the latter, but since you already went with the former in commit '7884d089' it's probably best to keep using that.
[1] qemuMonitorJSONSetMigrationCompression qemuMonitorJSONBlockCommit qemuMonitorJSONInjectNMI [2] qemuMonitorJSONSendKey [2] qemuMonitorJSONSetMigrationCapability
[2] both instances use the same HMP call attempt to return ret - they could follow the qemuMonitorJSONSetCPU model...
Oh. Thanks for catching that. I'll fix those too.
Finally, I see the comments move 4 spaces to the right in qemuMonitorJSONSetObjectProperty? (patch 4)... Was that a remnant or expected?
While working on this I didn't bother with adjusting the code that much initially. After that I've fired up my code alignment key shortcut over each function I've touched. I will leave the comments as they were.
ACK series (your call if you want to adjust [2] now or in a followup patch. it's trivial enough as long as you remember to also remove the {} around what would become "one line" if then else statements).
Thanks, pushed now. Michal
participants (2)
-
John Ferlan
-
Michal Privoznik