10:31 a.m.
First half introduces qemuMonitorJSONGetReply and uses it in our code
base, second half introduces virJSONValueArrayToStringList, uses it to
fix a bug and refactor the rest of the usage.
Peter Krempa (12):
qemu: monitor: Introduce qemuMonitorJSONGetReply, a better
qemuMonitorJSONCheckReply
qemu: monitor: Use qemuMonitorJSONGetReply for VIR_JSON_TYPE_OBJECT
qemu: monitor: Use qemuMonitorJSONGetReply for VIR_JSON_TYPE_ARRAY
qemu: monitor: Use qemuMonitorJSONGetReply when the value is extracted
directly
qemu: monitor: Unify and refactor 'PTY' case in
qemuMonitorJSONAttachCharDev
util: json: Split out array->strinlist conversion from
virJSONValueObjectGetStringArray
qemuAgentGetDisks: Don't use virJSONValueObjectGetStringArray for
optional data
qemuMonitorJSONGetCPUDefinitions: Rework lookup of
'unavailable-features'
qemuMonitorJSONGetCPUDefinitions: Avoid double lookup of object
qemu: monitor: Use qemuMonitorJSONGetReply in conjunction with
virJSONValueArrayToStringList
qemuAgentSSHGetAuthorizedKeys: Convert last use
ofvirJSONValueObjectGetStringArray
util: json: Remove unused virJSONValueObjectGetStringArray wrapper
src/libvirt_private.syms | 2 +-
src/qemu/qemu_agent.c | 13 ++-
src/qemu/qemu_monitor_json.c | 189 +++++++++++++++--------------------
src/util/virjson.c | 44 +++-----
src/util/virjson.h | 3 +-
5 files changed, 106 insertions(+), 145 deletions(-)
--
2.38.1
10:31 a.m.
New subject: [PATCH 01/12] qemu: monitor: Introduce qemuMonitorJSONGetReply, a better qemuMonitorJSONCheckReply
Rather than simply checking that the 'return' field is of the expected
type we can directly return it as the caller is very likely going to use
it. Extract the code into the new function and add a wrapper to preserve
old functionality.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index f4e942a32f..e6cfaaf0e9 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -396,15 +396,15 @@ qemuMonitorJSONCheckError(virJSONValue *cmd,
}
-static int
-qemuMonitorJSONCheckReply(virJSONValue *cmd,
- virJSONValue *reply,
- virJSONType type)
+static virJSONValue *
+qemuMonitorJSONGetReply(virJSONValue *cmd,
+ virJSONValue *reply,
+ virJSONType type)
{
virJSONValue *data;
if (qemuMonitorJSONCheckError(cmd, reply) < 0)
- return -1;
+ return NULL;
data = virJSONValueObjectGet(reply, "return");
if (virJSONValueGetType(data) != type) {
@@ -417,9 +417,21 @@ qemuMonitorJSONCheckReply(virJSONValue *cmd,
_("unexpected type returned by QEMU command
'%s'"),
qemuMonitorJSONCommandName(cmd));
- return -1;
+ return NULL;
}
+ return data;
+}
+
+
+static int
+qemuMonitorJSONCheckReply(virJSONValue *cmd,
+ virJSONValue *reply,
+ virJSONType type)
+{
+ if (!qemuMonitorJSONGetReply(cmd, reply, type))
+ return -1;
+
return 0;
}
--
2.38.1
10:31 a.m.
New subject: [PATCH 02/12] qemu: monitor: Use qemuMonitorJSONGetReply for VIR_JSON_TYPE_OBJECT
Replace usage of the following pattern with the new helper:
if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
return -1;
data = virJSONValueObjectGetObject(reply, "return");
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 52 +++++++++---------------------------
1 file changed, 13 insertions(+), 39 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index e6cfaaf0e9..e3fe068c1b 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1481,11 +1481,9 @@ qemuMonitorJSONGetStatus(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT)))
return -1;
- data = virJSONValueObjectGetObject(reply, "return");
-
if (virJSONValueObjectGetBoolean(data, "running", running) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("query-status reply was missing running state"));
@@ -1866,11 +1864,9 @@ qemuMonitorJSONGetBalloonInfo(qemuMonitor *mon,
}
/* See if any other fatal error occurred */
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT)))
return -1;
- data = virJSONValueObjectGetObject(reply, "return");
-
if (virJSONValueObjectGetNumberUlong(data, "actual", &mem) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("info balloon reply was missing balloon data"));
@@ -1964,11 +1960,9 @@ qemuMonitorJSONGetMemoryStats(qemuMonitor *mon,
}
}
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT)))
return got;
- data = virJSONValueObjectGetObject(reply, "return");
-
if (!(statsdata = virJSONValueObjectGet(data, "stats"))) {
VIR_DEBUG("data does not include 'stats'");
return got;
@@ -3165,11 +3159,9 @@ qemuMonitorJSONGetMigrationBlockers(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT)))
return -1;
- data = virJSONValueObjectGetObject(reply, "return");
-
if (!(jblockers = virJSONValueObjectGetArray(data, "blocked-reasons")))
return 0;
@@ -3244,11 +3236,9 @@ qemuMonitorJSONQueryDump(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ if (!(result = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT)))
return -1;
- result = virJSONValueObjectGetObject(reply, "return");
-
return qemuMonitorJSONExtractDumpStats(result, stats);
}
@@ -3270,11 +3260,9 @@ qemuMonitorJSONGetDumpGuestMemoryCapability(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ if (!(caps = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT)))
return -1;
- caps = virJSONValueObjectGetObject(reply, "return");
-
if (!(formats = virJSONValueObjectGetArray(caps, "formats"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("missing supported dump formats"));
@@ -4656,11 +4644,9 @@ int qemuMonitorJSONGetVersion(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT)))
return -1;
- data = virJSONValueObjectGetObject(reply, "return");
-
if (!(qemu = virJSONValueObjectGetObject(data, "qemu"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("query-version reply was missing 'qemu'
data"));
@@ -5155,11 +5141,9 @@ qemuMonitorJSONGetCPUModelBaseline(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT)))
return -1;
- data = virJSONValueObjectGetObject(reply, "return");
-
if (qemuMonitorJSONParseCPUModelData(data, "query-cpu-model-baseline",
false, &cpu_model, &cpu_props,
&cpu_name) < 0)
@@ -5322,11 +5306,9 @@ int qemuMonitorJSONGetKVMState(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT)))
return -1;
- data = virJSONValueObjectGetObject(reply, "return");
-
if (virJSONValueObjectGetBoolean(data, "enabled", enabled) < 0 ||
virJSONValueObjectGetBoolean(data, "present", present) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -5761,11 +5743,9 @@ qemuMonitorJSONGetTargetArch(qemuMonitor *mon)
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return NULL;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT)))
return NULL;
- data = virJSONValueObjectGetObject(reply, "return");
-
if (!(arch = virJSONValueObjectGetString(data, "arch"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("query-target reply was missing arch data"));
@@ -7521,11 +7501,9 @@ qemuMonitorJSONGetRTCTime(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT)))
return -1;
- data = virJSONValueObjectGet(reply, "return");
-
if (virJSONValueObjectGetNumberInt(data, "tm_year", &tm->tm_year)
< 0 ||
virJSONValueObjectGetNumberInt(data, "tm_mon", &tm->tm_mon) <
0 ||
virJSONValueObjectGetNumberInt(data, "tm_mday", &tm->tm_mday)
< 0 ||
@@ -7973,11 +7951,9 @@ qemuMonitorJSONGetSEVMeasurement(qemuMonitor *mon)
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return NULL;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT)))
return NULL;
- data = virJSONValueObjectGetObject(reply, "return");
-
if (!(tmp = virJSONValueObjectGetString(data, "data")))
return NULL;
@@ -8016,11 +7992,9 @@ qemuMonitorJSONGetSEVInfo(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT)))
return -1;
- data = virJSONValueObjectGetObject(reply, "return");
-
if (virJSONValueObjectGetNumberUint(data, "api-major", apiMajor) < 0 ||
virJSONValueObjectGetNumberUint(data, "api-minor", apiMinor) < 0 ||
virJSONValueObjectGetNumberUint(data, "build-id", buildID) < 0 ||
--
2.38.1
10:31 a.m.
New subject: [PATCH 03/12] qemu: monitor: Use qemuMonitorJSONGetReply for VIR_JSON_TYPE_ARRAY
Replace usage of the following pattern with the new helper:
if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
return -1;
data = virJSONValueObjectGetArray(reply, "return");
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 48 +++++++++++-------------------------
1 file changed, 15 insertions(+), 33 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index e3fe068c1b..79729e0a9a 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4702,10 +4702,9 @@ int qemuMonitorJSONGetMachines(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
goto cleanup;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
goto cleanup;
- data = virJSONValueObjectGetArray(reply, "return");
n = virJSONValueArraySize(data);
/* null-terminated list */
@@ -4830,10 +4829,9 @@ qemuMonitorJSONGetCPUDefinitions(qemuMonitor *mon,
if (qemuMonitorJSONHasError(reply, "GenericError"))
return 0;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
return -1;
- data = virJSONValueObjectGetArray(reply, "return");
ncpus = virJSONValueArraySize(data);
if (!(defs = qemuMonitorCPUDefsNew(ncpus)))
@@ -5214,10 +5212,9 @@ int qemuMonitorJSONGetCommands(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
return -1;
- data = virJSONValueObjectGetArray(reply, "return");
n = virJSONValueArraySize(data);
/* null-terminated list */
@@ -5339,10 +5336,9 @@ qemuMonitorJSONGetObjectTypes(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
return -1;
- data = virJSONValueObjectGetArray(reply, "return");
n = virJSONValueArraySize(data);
/* null-terminated list */
@@ -5388,10 +5384,9 @@ int qemuMonitorJSONGetObjectListPaths(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
goto cleanup;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
goto cleanup;
- data = virJSONValueObjectGetArray(reply, "return");
n = virJSONValueArraySize(data);
/* null-terminated list */
@@ -5620,10 +5615,9 @@ qemuMonitorJSONParsePropsList(virJSONValue *cmd,
size_t count = 0;
size_t i;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
return -1;
- data = virJSONValueObjectGetArray(reply, "return");
n = virJSONValueArraySize(data);
/* null-terminated list */
@@ -5776,10 +5770,9 @@ qemuMonitorJSONGetMigrationCapabilities(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(caps = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
return -1;
- caps = virJSONValueObjectGetArray(reply, "return");
n = virJSONValueArraySize(caps);
list = g_new0(char *, n + 1);
@@ -5871,10 +5864,9 @@ qemuMonitorJSONGetGICCapabilities(qemuMonitor *mon,
if (qemuMonitorJSONHasError(reply, "CommandNotFound"))
return 0;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(caps = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
return -1;
- caps = virJSONValueObjectGetArray(reply, "return");
n = virJSONValueArraySize(caps);
/* If the returned array was empty we have to return successfully */
@@ -6754,10 +6746,9 @@ qemuMonitorJSONGetCPUx86Data(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
return -1;
- data = virJSONValueObjectGetArray(reply, "return");
if (!(*cpudata = qemuMonitorJSONParseCPUx86Features(data)))
return -1;
@@ -6795,10 +6786,9 @@ qemuMonitorJSONCheckCPUx86(qemuMonitor *mon,
}
}
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
return -1;
- data = virJSONValueObjectGetArray(reply, "return");
n = virJSONValueArraySize(data);
for (i = 0; i < n; i++) {
@@ -7031,10 +7021,9 @@ qemuMonitorJSONGetIOThreads(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
goto cleanup;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
goto cleanup;
- data = virJSONValueObjectGetArray(reply, "return");
n = virJSONValueArraySize(data);
/* null-terminated list */
@@ -7174,11 +7163,9 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
return -1;
- data = virJSONValueObjectGetArray(reply, "return");
-
for (i = 0; i < virJSONValueArraySize(data); i++) {
virJSONValue *elem = virJSONValueArrayGet(data, i);
g_autofree qemuMonitorMemoryDeviceInfo *meminfo = NULL;
@@ -7659,10 +7646,9 @@ qemuMonitorJSONGetHotpluggableCPUs(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
goto cleanup;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
goto cleanup;
- data = virJSONValueObjectGet(reply, "return");
ninfo = virJSONValueArraySize(data);
info = g_new0(struct qemuMonitorQueryHotpluggableCpusEntry, ninfo);
@@ -8372,11 +8358,9 @@ qemuMonitorJSONGetJobInfo(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
return -1;
- data = virJSONValueObjectGetArray(reply, "return");
-
for (i = 0; i < virJSONValueArraySize(data); i++) {
qemuMonitorJobInfo *job = NULL;
@@ -8783,11 +8767,9 @@ qemuMonitorJSONQueryStatsSchema(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return NULL;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(ret = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
return NULL;
- ret = virJSONValueObjectGetArray(reply, "return");
-
return qemuMonitorJSONExtractQueryStatsSchema(ret);
}
--
2.38.1
10:31 a.m.
New subject: [PATCH 04/12] qemu: monitor: Use qemuMonitorJSONGetReply when the value is extracted directly
Use qemuMonitorJSONGetReply in cases where qemuMonitorJSONCheckReply
is followed by virJSONValueObjectGet*(reply, "return").
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 79729e0a9a..47df4a5160 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5267,6 +5267,7 @@ qemuMonitorJSONGetCommandLineOptions(qemuMonitor *mon)
g_autoptr(GHashTable) ret = virHashNew(virJSONValueHashFree);
g_autoptr(virJSONValue) cmd = NULL;
g_autoptr(virJSONValue) reply = NULL;
+ virJSONValue *data;
if (!(cmd = qemuMonitorJSONMakeCommand("query-command-line-options",
NULL)))
return NULL;
@@ -5274,10 +5275,10 @@ qemuMonitorJSONGetCommandLineOptions(qemuMonitor *mon)
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return NULL;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
return NULL;
- if (virJSONValueArrayForeachSteal(virJSONValueObjectGetArray(reply,
"return"),
+ if (virJSONValueArrayForeachSteal(data,
qemuMonitorJSONGetCommandLineOptionsWorker,
ret) < 0)
return NULL;
@@ -5673,6 +5674,7 @@ qemuMonitorJSONGetDeviceProps(qemuMonitor *mon,
g_autoptr(GHashTable) props = virHashNew(virJSONValueHashFree);
g_autoptr(virJSONValue) cmd = NULL;
g_autoptr(virJSONValue) reply = NULL;
+ virJSONValue *data;
if (!(cmd = qemuMonitorJSONMakeCommand("device-list-properties",
"s:typename", device,
@@ -5686,10 +5688,10 @@ qemuMonitorJSONGetDeviceProps(qemuMonitor *mon,
if (qemuMonitorJSONHasError(reply, "DeviceNotFound"))
return g_steal_pointer(&props);
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
return NULL;
- if (virJSONValueArrayForeachSteal(virJSONValueObjectGetArray(reply,
"return"),
+ if (virJSONValueArrayForeachSteal(data,
qemuMonitorJSONGetDevicePropsWorker,
props) < 0)
return NULL;
@@ -8381,6 +8383,7 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitor *mon,
{
g_autoptr(virJSONValue) cmd = NULL;
g_autoptr(virJSONValue) reply = NULL;
+ virJSONValue *data;
if (!(cmd = qemuMonitorJSONMakeCommand("qom-get",
"s:path", cpuQOMPath,
@@ -8394,11 +8397,10 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitor *mon,
if (qemuMonitorJSONHasError(reply, "GenericError"))
return 1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_BOOLEAN) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_BOOLEAN)))
return -1;
- return virJSONValueGetBoolean(virJSONValueObjectGet(reply, "return"),
- migratable);
+ return virJSONValueGetBoolean(data, migratable);
}
--
2.38.1
10:31 a.m.
New subject: [PATCH 05/12] qemu: monitor: Unify and refactor 'PTY' case in qemuMonitorJSONAttachCharDev
Use qemuMonitorJSONGetReply and unify the two blocks with the same
condition.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 47df4a5160..a5800e182c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -6578,24 +6578,19 @@ qemuMonitorJSONAttachCharDev(qemuMonitor *mon,
return -1;
if (chr->type == VIR_DOMAIN_CHR_TYPE_PTY) {
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0)
- return -1;
- } else {
- if (qemuMonitorJSONCheckError(cmd, reply) < 0)
- return -1;
- }
+ virJSONValue *data;
- if (chr->type == VIR_DOMAIN_CHR_TYPE_PTY) {
- virJSONValue *data = virJSONValueObjectGetObject(reply, "return");
- const char *path;
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT)))
+ return -1;
- if (!(path = virJSONValueObjectGetString(data, "pty"))) {
+ if (!(chr->data.file.path = g_strdup(virJSONValueObjectGetString(data,
"pty")))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("chardev-add reply was missing pty path"));
return -1;
}
-
- chr->data.file.path = g_strdup(path);
+ } else {
+ if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+ return -1;
}
return 0;
--
2.38.1
10:31 a.m.
New subject: [PATCH 06/12] util: json: Split out array->strinlist conversion from virJSONValueObjectGetStringArray
Introduce virJSONValueArrayToStringList which does only the conversion
from an array to a stringlist.
This will allow refactoring the callers to be more careful in case when
they want to handle the existance of the member in the parent object
differently.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virjson.c | 43 ++++++++++++++++++++++------------------
src/util/virjson.h | 2 ++
3 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index ebd7bc61a8..a5bb44a9f8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2564,6 +2564,7 @@ virJSONValueArrayForeachSteal;
virJSONValueArrayGet;
virJSONValueArraySize;
virJSONValueArraySteal;
+virJSONValueArrayToStringList;
virJSONValueCopy;
virJSONValueFree;
virJSONValueFromString;
diff --git a/src/util/virjson.c b/src/util/virjson.c
index ef59b5deb4..5701c09bfd 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1316,10 +1316,7 @@ virJSONValueObjectStealObject(virJSONValue *object,
char **
virJSONValueObjectGetStringArray(virJSONValue *object, const char *key)
{
- g_auto(GStrv) ret = NULL;
virJSONValue *data;
- size_t n;
- size_t i;
data = virJSONValueObjectGetArray(object, key);
if (!data) {
@@ -1329,32 +1326,40 @@ virJSONValueObjectGetStringArray(virJSONValue *object, const char
*key)
return NULL;
}
- n = virJSONValueArraySize(data);
- ret = g_new0(char *, n + 1);
+ return virJSONValueArrayToStringList(data);
+}
+
+
+/**
+ * virJSONValueArrayToStringList:
+ * @data: a JSON array containing strings to convert
+ *
+ * Converts @data a JSON array containing strings to a NULL-terminated string
+ * list. @data must be a JSON array. In case @data is doesn't contain only
+ * strings an error is reported.
+ */
+char **
+virJSONValueArrayToStringList(virJSONValue *data)
+{
+ size_t n = virJSONValueArraySize(data);
+ g_auto(GStrv) ret = g_new0(char *, n + 1);
+ size_t i;
+
for (i = 0; i < n; i++) {
virJSONValue *child = virJSONValueArrayGet(data, i);
- const char *tmp;
- if (!child) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("%s array element is missing item %zu"),
- key, i);
+ if (!child ||
+ !(ret[i] = g_strdup(virJSONValueGetString(child)))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("JSON string array contains non-string
element"));
return NULL;
}
-
- if (!(tmp = virJSONValueGetString(child))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("%s array element does not contain a string"),
- key);
- return NULL;
- }
-
- ret[i] = g_strdup(tmp);
}
return g_steal_pointer(&ret);
}
+
/**
* virJSONValueObjectForeachKeyValue:
* @object: JSON object to iterate
diff --git a/src/util/virjson.h b/src/util/virjson.h
index ce181dcb82..49a89e0910 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -172,6 +172,8 @@ virJSONValueObjectGetString(virJSONValue *object,
char **
virJSONValueObjectGetStringArray(virJSONValue *object,
const char *key);
+char **
+virJSONValueArrayToStringList(virJSONValue *data);
const char *
virJSONValueObjectGetStringOrNumber(virJSONValue *object,
const char *key);
--
2.38.1
10:31 a.m.
New subject: [PATCH 07/12] qemuAgentGetDisks: Don't use virJSONValueObjectGetStringArray for optional data
The 'dependencies' field in the return data may be missing in some
cases. Historically 'virJSONValueObjectGetStringArray' didn't report
error in such case, but later refactor (commit 043b50b948ef3c2 ) added
an error in order to use it in other places too.
Unfortunately this results in the error log being spammed with an
irrelevant error in case when qemuAgentGetDisks is invoked on a VM
running windows.
Replace the use of virJSONValueObjectGetStringArray by fetching the
array first and calling virJSONValueArrayToStringList only when we have
an array.
Fixes: 043b50b948ef3c2a4adf5fa32a93ec2589851ac6
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2149752
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_agent.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index d420b2b901..70d45955b2 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -2545,6 +2545,7 @@ int qemuAgentGetDisks(qemuAgent *agent,
for (i = 0; i < ndata; i++) {
virJSONValue *addr;
virJSONValue *entry = virJSONValueArrayGet(data, i);
+ virJSONValue *dependencies;
qemuAgentDiskInfo *disk;
if (!entry) {
@@ -2570,7 +2571,11 @@ int qemuAgentGetDisks(qemuAgent *agent,
goto error;
}
- disk->dependencies = virJSONValueObjectGetStringArray(entry,
"dependencies");
+ if ((dependencies = virJSONValueObjectGetArray(entry, "dependencies")))
{
+ if (!(disk->dependencies = virJSONValueArrayToStringList(dependencies)))
+ goto error;
+ }
+
disk->alias = g_strdup(virJSONValueObjectGetString(entry,
"alias"));
addr = virJSONValueObjectGetObject(entry, "address");
if (addr) {
--
2.38.1
10:31 a.m.
New subject: [PATCH 08/12] qemuMonitorJSONGetCPUDefinitions: Rework lookup of 'unavailable-features'
Rather than checking that the object has the correct key and then
fetching it again use fetch the array first and then use
virJSONValueArrayToStringList to directly convert it.
Additionally we can avoid the conversion if there are no members
simplifying the surrounding logic.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a5800e182c..cbd1eb6eec 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4841,6 +4841,7 @@ qemuMonitorJSONGetCPUDefinitions(qemuMonitor *mon,
virJSONValue *child = virJSONValueArrayGet(data, i);
const char *tmp;
qemuMonitorCPUDefInfo *cpu = defs->cpus + i;
+ virJSONValue *feat;
if (!(tmp = virJSONValueObjectGetString(child, "name"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -4853,16 +4854,14 @@ qemuMonitorJSONGetCPUDefinitions(qemuMonitor *mon,
if ((tmp = virJSONValueObjectGetString(child, "typename")) &&
*tmp)
cpu->type = g_strdup(tmp);
- if (virJSONValueObjectHasKey(child, "unavailable-features")) {
- if (!(cpu->blockers = virJSONValueObjectGetStringArray(child,
-
"unavailable-features")))
- return -1;
+ if ((feat = virJSONValueObjectGetArray(child, "unavailable-features")))
{
+ if (virJSONValueArraySize(feat) > 0) {
+ if (!(cpu->blockers = virJSONValueArrayToStringList(feat)))
+ return -1;
- if (g_strv_length(cpu->blockers) == 0) {
- cpu->usable = VIR_DOMCAPS_CPU_USABLE_YES;
- g_clear_pointer(&cpu->blockers, g_strfreev);
- } else {
cpu->usable = VIR_DOMCAPS_CPU_USABLE_NO;
+ } else {
+ cpu->usable = VIR_DOMCAPS_CPU_USABLE_YES;
}
}
--
2.38.1
10:31 a.m.
New subject: [PATCH 09/12] qemuMonitorJSONGetCPUDefinitions: Avoid double lookup of object
Using 'virJSONValueObjectHasKey' when we want to access the value
afterwards is wasteful. Fetch the JSON value right away.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index cbd1eb6eec..1deb755130 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4842,6 +4842,7 @@ qemuMonitorJSONGetCPUDefinitions(qemuMonitor *mon,
const char *tmp;
qemuMonitorCPUDefInfo *cpu = defs->cpus + i;
virJSONValue *feat;
+ virJSONValue *deprecated;
if (!(tmp = virJSONValueObjectGetString(child, "name"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -4865,8 +4866,8 @@ qemuMonitorJSONGetCPUDefinitions(qemuMonitor *mon,
}
}
- if (virJSONValueObjectHasKey(child, "deprecated") &&
- virJSONValueObjectGetBoolean(child, "deprecated",
&cpu->deprecated) < 0)
+ if ((deprecated = virJSONValueObjectGet(child, "deprecated"))
&&
+ virJSONValueGetBoolean(deprecated, &cpu->deprecated) < 0)
return -1;
}
--
2.38.1
10:31 a.m.
New subject: [PATCH 10/12] qemu: monitor: Use qemuMonitorJSONGetReply in conjunction with virJSONValueArrayToStringList
In two instances (qemuMonitorJSONGetStringListProperty,
qemuMonitorJSONGetStringArray) the return value is checked by
qemuMonitorJSONCheckReply and extracted by
virJSONValueObjectGetStringArray.
We can use qemuMonitorJSONGetReply which returns it directly and then
virJSONValueArrayToStringList to convert it without the additional
lookup.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 1deb755130..39f313c2af 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5522,6 +5522,7 @@ qemuMonitorJSONGetStringListProperty(qemuMonitor *mon,
{
g_autoptr(virJSONValue) cmd = NULL;
g_autoptr(virJSONValue) reply = NULL;
+ virJSONValue *data;
*strList = NULL;
@@ -5534,10 +5535,10 @@ qemuMonitorJSONGetStringListProperty(qemuMonitor *mon,
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
return -1;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
return -1;
- if (!(*strList = virJSONValueObjectGetStringArray(reply, "return")))
+ if (!(*strList = virJSONValueArrayToStringList(data)))
return -1;
return 0;
@@ -6297,6 +6298,7 @@ qemuMonitorJSONGetStringArray(qemuMonitor *mon,
{
g_autoptr(virJSONValue) cmd = NULL;
g_autoptr(virJSONValue) reply = NULL;
+ virJSONValue *data;
*array = NULL;
@@ -6309,10 +6311,10 @@ qemuMonitorJSONGetStringArray(qemuMonitor *mon,
if (qemuMonitorJSONHasError(reply, "CommandNotFound"))
return 0;
- if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+ if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_ARRAY)))
return -1;
- if (!(*array = virJSONValueObjectGetStringArray(reply, "return")))
+ if (!(*array = virJSONValueArrayToStringList(data)))
return -1;
return 0;
--
2.38.1
10:31 a.m.
New subject: [PATCH 11/12] qemuAgentSSHGetAuthorizedKeys: Convert last use ofvirJSONValueObjectGetStringArray
Use virJSONValueObjectGetArray + virJSONValueArrayToStringList instead
so that the ofvirJSONValueObjectGetStringArray wrapper can be removed.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_agent.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 70d45955b2..fa2c0bf915 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -2413,6 +2413,7 @@ qemuAgentSSHGetAuthorizedKeys(qemuAgent *agent,
g_autoptr(virJSONValue) cmd = NULL;
g_autoptr(virJSONValue) reply = NULL;
virJSONValue *data = NULL;
+ virJSONValue *arr = NULL;
if (!(cmd = qemuAgentMakeCommand("guest-ssh-get-authorized-keys",
"s:username", user,
@@ -2422,13 +2423,14 @@ qemuAgentSSHGetAuthorizedKeys(qemuAgent *agent,
if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0)
return -1;
- if (!(data = virJSONValueObjectGetObject(reply, "return"))) {
+ if (!(data = virJSONValueObjectGetObject(reply, "return")) ||
+ !(arr = virJSONValueObjectGetArray(data, "keys"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("qemu agent didn't return an array of keys"));
return -1;
}
- if (!(*keys = virJSONValueObjectGetStringArray(data, "keys")))
+ if (!(*keys = virJSONValueArrayToStringList(arr)))
return -1;
return g_strv_length(*keys);
--
2.38.1
10:31 a.m.
New subject: [PATCH 12/12] util: json: Remove unused virJSONValueObjectGetStringArray wrapper
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/libvirt_private.syms | 1 -
src/util/virjson.c | 17 -----------------
src/util/virjson.h | 3 ---
3 files changed, 21 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a5bb44a9f8..9389778db7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2616,7 +2616,6 @@ virJSONValueObjectGetNumberUint;
virJSONValueObjectGetNumberUlong;
virJSONValueObjectGetObject;
virJSONValueObjectGetString;
-virJSONValueObjectGetStringArray;
virJSONValueObjectGetValue;
virJSONValueObjectHasKey;
virJSONValueObjectKeysNumber;
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 5701c09bfd..1c3a50440e 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -1313,23 +1313,6 @@ virJSONValueObjectStealObject(virJSONValue *object,
}
-char **
-virJSONValueObjectGetStringArray(virJSONValue *object, const char *key)
-{
- virJSONValue *data;
-
- data = virJSONValueObjectGetArray(object, key);
- if (!data) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("%s is missing or not an array"),
- key);
- return NULL;
- }
-
- return virJSONValueArrayToStringList(data);
-}
-
-
/**
* virJSONValueArrayToStringList:
* @data: a JSON array containing strings to convert
diff --git a/src/util/virjson.h b/src/util/virjson.h
index 49a89e0910..0ea7caad23 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -170,9 +170,6 @@ const char *
virJSONValueObjectGetString(virJSONValue *object,
const char *key);
char **
-virJSONValueObjectGetStringArray(virJSONValue *object,
- const char *key);
-char **
virJSONValueArrayToStringList(virJSONValue *data);
const char *
virJSONValueObjectGetStringOrNumber(virJSONValue *object,
--
2.38.1
3:01 a.m.
On 12/1/22 17:31, Peter Krempa wrote:
First half introduces qemuMonitorJSONGetReply and uses it in our
code
base, second half introduces virJSONValueArrayToStringList, uses it to
fix a bug and refactor the rest of the usage.
Peter Krempa (12):
qemu: monitor: Introduce qemuMonitorJSONGetReply, a better
qemuMonitorJSONCheckReply
qemu: monitor: Use qemuMonitorJSONGetReply for VIR_JSON_TYPE_OBJECT
qemu: monitor: Use qemuMonitorJSONGetReply for VIR_JSON_TYPE_ARRAY
qemu: monitor: Use qemuMonitorJSONGetReply when the value is extracted
directly
qemu: monitor: Unify and refactor 'PTY' case in
qemuMonitorJSONAttachCharDev
util: json: Split out array->strinlist conversion from
virJSONValueObjectGetStringArray
qemuAgentGetDisks: Don't use virJSONValueObjectGetStringArray for
optional data
qemuMonitorJSONGetCPUDefinitions: Rework lookup of
'unavailable-features'
qemuMonitorJSONGetCPUDefinitions: Avoid double lookup of object
qemu: monitor: Use qemuMonitorJSONGetReply in conjunction with
virJSONValueArrayToStringList
qemuAgentSSHGetAuthorizedKeys: Convert last use
ofvirJSONValueObjectGetStringArray
util: json: Remove unused virJSONValueObjectGetStringArray wrapper
src/libvirt_private.syms | 2 +-
src/qemu/qemu_agent.c | 13 ++-
src/qemu/qemu_monitor_json.c | 189 +++++++++++++++--------------------
src/util/virjson.c | 44 +++-----
src/util/virjson.h | 3 +-
5 files changed, 106 insertions(+), 145 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
548
days inactive
549
days old
13 comments
2 participants
participants (2)
-
Michal Prívozník -
Peter Krempa