[PATCH 00/10] qemu: Refactor typed parameter handling in qemuDomainGetGuestInfo
Use virTypedParam list to construct the returned data. Note that this applies on top of two patch(sets) that were ACK'd upstream: [PATCH v2 0/4] Add support for getting load averages from guest agent [PATCH] qemu: Report disk bus as reported by agent in virDomainGetGuestInfo You can fetch the whole thing via: git fetch https://gitlab.com/pipo.sk/libvirt.git agent-data-refactor Peter Krempa (10): qemuDomainGetGuestInfo: Prepare for refactor to virTypedParamList qemuDomainGetGuestInfo: Convert load code to virTypedParamList virDomainInterfaceFormatParams: Convert interface code to virTypedParamList qemuAgentDiskInfoFormatParams: Convert interface code to virTypedParamList qemuAgentFSInfoFormatParams: Convert interface code to virTypedParamList qemuAgentGetTimezone: Convert to virTypedParamList qemuAgentGetOSInfo: Convert to virTypedParamList qemuAgentGetUsers: Convert to virTypedParamList qemuDomainGetGuestInfo: Convert hostname code to virTypedParamList qemuDomainGetGuestInfo: Remove temporary infrastructure src/qemu/qemu_agent.c | 52 ++------- src/qemu/qemu_agent.h | 12 +- src/qemu/qemu_driver.c | 249 ++++++++++++----------------------------- tests/qemuagenttest.c | 148 +++++++++++------------- 4 files changed, 152 insertions(+), 309 deletions(-) -- 2.48.1
Use of raw typed param APIs is very clunky. Prepare qemuDomainGetGuestInfo for step-by-step refactor to virTypedParamList. The two lists will coexist until the refactor is complete. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 86b945d9b9..3d2ebd7719 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19483,6 +19483,8 @@ qemuDomainGetGuestInfo(virDomainPtr dom, double load15m = 0; bool format_load = false; size_t i; + g_autoptr(virTypedParamList) list = virTypedParamListNew(); + g_autoptr(virTypedParamList) tmplist = NULL; virCheckFlags(0, -1); @@ -19592,6 +19594,14 @@ qemuDomainGetGuestInfo(virDomainPtr dom, virTypedParamsAddDouble(params, nparams, &maxparams, "load.15m", load15m); } + /* temporarily allow the old and new construction style to coexist */ + tmplist = virTypedParamListFromParams(params, *nparams); + virTypedParamListConcat(tmplist, &list); + list = g_steal_pointer(&tmplist); + + if (virTypedParamListSteal(list, params, nparams) < 0) + goto cleanup; + ret = 0; cleanup: -- 2.48.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3d2ebd7719..3eabb0e1ed 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19589,9 +19589,9 @@ qemuDomainGetGuestInfo(virDomainPtr dom, } if (format_load) { - virTypedParamsAddDouble(params, nparams, &maxparams, "load.1m", load1m); - virTypedParamsAddDouble(params, nparams, &maxparams, "load.5m", load5m); - virTypedParamsAddDouble(params, nparams, &maxparams, "load.15m", load15m); + virTypedParamListAddDouble(list, load1m, "load.1m"); + virTypedParamListAddDouble(list, load5m, "load.5m"); + virTypedParamListAddDouble(list, load15m, "load.15m"); } /* temporarily allow the old and new construction style to coexist */ -- 2.48.1
Also deletes pre-existing broken formatting. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 60 ++++++++++-------------------------------- 1 file changed, 14 insertions(+), 46 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3eabb0e1ed..3ebc712f9a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19393,66 +19393,34 @@ qemuAgentFSInfoFormatParams(qemuAgentFSInfo **fsinfo, static void virDomainInterfaceFormatParams(virDomainInterfacePtr *ifaces, int nifaces, - virTypedParameterPtr *params, - int *nparams, int *maxparams) + virTypedParamList *list) { size_t i; - size_t j; - if (virTypedParamsAddUInt(params, nparams, maxparams, - "if.count", nifaces) < 0) - return; + virTypedParamListAddUInt(list, nifaces, "if.count"); for (i = 0; i < nifaces; i++) { - char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; - - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "if.%zu.name", i); - if (virTypedParamsAddString(params, nparams, maxparams, - param_name, ifaces[i]->name) < 0) - return; + size_t j; - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "if.%zu.hwaddr", i); - if (virTypedParamsAddString(params, nparams, maxparams, - param_name, ifaces[i]->hwaddr) < 0) - return; - - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "if.%zu.addr.count", i); - if (virTypedParamsAddUInt(params, nparams, maxparams, - param_name, ifaces[i]->naddrs) < 0) - return; + virTypedParamListAddString(list, ifaces[i]->name, "if.%zu.name", i); + virTypedParamListAddString(list, ifaces[i]->hwaddr, "if.%zu.hwaddr", i); + virTypedParamListAddUInt(list, ifaces[i]->naddrs, "if.%zu.addr.count", i); for (j = 0; j < ifaces[i]->naddrs; j++) { - const char *type = NULL; - switch (ifaces[i]->addrs[j].type) { case VIR_IP_ADDR_TYPE_IPV4: - type = "ipv4"; + virTypedParamListAddString(list, "ipv4", "if.%zu.addr.%zu.type", i, j); break; + case VIR_IP_ADDR_TYPE_IPV6: - type = "ipv6"; + virTypedParamListAddString(list, "ipv6", "if.%zu.addr.%zu.type", i, j); break; } - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "if.%zu.addr.%zu.type", i, j); - if (virTypedParamsAddString(params, nparams, maxparams, - param_name, type) < 0) - return; - - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "if.%zu.addr.%zu.addr", i, j); - if (virTypedParamsAddString(params, nparams, maxparams, - param_name, ifaces[i]->addrs[j].addr) < 0) - return; - - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "if.%zu.addr.%zu.prefix", i, j); - if (virTypedParamsAddUInt(params, nparams, maxparams, - param_name, ifaces[i]->addrs[j].prefix) < 0) - return; + virTypedParamListAddString(list, ifaces[i]->addrs[j].addr, + "if.%zu.addr.%zu.addr", i, j); + virTypedParamListAddUInt(list, ifaces[i]->addrs[j].prefix, + "if.%zu.addr.%zu.prefix", i, j); } } } @@ -19585,7 +19553,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom, } if (nifaces > 0) { - virDomainInterfaceFormatParams(ifaces, nifaces, params, nparams, &maxparams); + virDomainInterfaceFormatParams(ifaces, nifaces, list); } if (format_load) { -- 2.48.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 81 +++++++++++------------------------------- 1 file changed, 21 insertions(+), 60 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3ebc712f9a..515577b71a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19210,44 +19210,26 @@ static void qemuAgentDiskInfoFormatParams(qemuAgentDiskInfo **info, int ndisks, virDomainDef *vmdef, - virTypedParameterPtr *params, - int *nparams, int *maxparams) + virTypedParamList *list) { - size_t i, j, ndeps; + size_t i; - if (virTypedParamsAddUInt(params, nparams, maxparams, - "disk.count", ndisks) < 0) - return; + virTypedParamListAddUInt(list, ndisks, "disk.count"); for (i = 0; i < ndisks; i++) { - char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; + virTypedParamListAddString(list, info[i]->name, "disk.%zu.name", i); + virTypedParamListAddBoolean(list, info[i]->partition, "disk.%zu.partition", i); - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "disk.%zu.name", i); - if (virTypedParamsAddString(params, nparams, maxparams, - param_name, info[i]->name) < 0) - return; + if (info[i]->dependencies) { + size_t ndeps = g_strv_length(info[i]->dependencies); + size_t j; - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "disk.%zu.partition", i); - if (virTypedParamsAddBoolean(params, nparams, maxparams, - param_name, info[i]->partition) < 0) - return; + if (ndeps > 0) + virTypedParamListAddUInt(list, ndeps, "disk.%zu.dependency.count", i); - if (info[i]->dependencies) { - ndeps = g_strv_length(info[i]->dependencies); - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "disk.%zu.dependency.count", i); - if (ndeps && - virTypedParamsAddUInt(params, nparams, maxparams, - param_name, ndeps) < 0) - return; for (j = 0; j < ndeps; j++) { - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "disk.%zu.dependency.%zu.name", i, j); - if (virTypedParamsAddString(params, nparams, maxparams, - param_name, info[i]->dependencies[j]) < 0) - return; + virTypedParamListAddString(list, info[i]->dependencies[j], + "disk.%zu.dependency.%zu.name", i, j); } } @@ -19255,13 +19237,8 @@ qemuAgentDiskInfoFormatParams(qemuAgentDiskInfo **info, qemuAgentDiskAddress *address = info[i]->address; virDomainDiskDef *diskdef = NULL; - if (address->serial) { - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "disk.%zu.serial", i); - if (virTypedParamsAddString(params, nparams, maxparams, - param_name, address->serial) < 0) - return; - } + if (address->serial) + virTypedParamListAddString(list, address->serial, "disk.%zu.serial", i); /* match the disk to the target in the vm definition */ diskdef = virDomainDiskByAddress(vmdef, @@ -19270,32 +19247,16 @@ qemuAgentDiskInfoFormatParams(qemuAgentDiskInfo **info, address->bus, address->target, address->unit); - if (diskdef) { - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "disk.%zu.alias", i); - if (diskdef->dst && - virTypedParamsAddString(params, nparams, maxparams, - param_name, diskdef->dst) < 0) - return; - } - if (address->bus_type) { - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "disk.%zu.guest_bus", i); + if (diskdef && diskdef->dst) + virTypedParamListAddString(list, diskdef->dst, "disk.%zu.alias", i); - if (virTypedParamsAddString(params, nparams, maxparams, - param_name, address->bus_type) < 0) - return; - } + if (address->bus_type) + virTypedParamListAddString(list, address->bus_type, "disk.%zu.guest_bus", i); } - if (info[i]->alias) { - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "disk.%zu.guest_alias", i); - if (virTypedParamsAddString(params, nparams, maxparams, - param_name, info[i]->alias) < 0) - return; - } + if (info[i]->alias) + virTypedParamListAddString(list, info[i]->alias, "disk.%zu.guest_alias", i); } } @@ -19546,7 +19507,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom, qemuAgentFSInfoFormatParams(agentfsinfo, nfs, vm->def, params, nparams, &maxparams); if (ndisks > 0) - qemuAgentDiskInfoFormatParams(agentdiskinfo, ndisks, vm->def, params, nparams, &maxparams); + qemuAgentDiskInfoFormatParams(agentdiskinfo, ndisks, vm->def, list); endjob: virDomainObjEndJob(vm); -- 2.48.1
Also remove stale TODO comment as we already report disk target. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 86 +++++++++++------------------------------- 1 file changed, 23 insertions(+), 63 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 515577b71a..0d88f182b8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19265,59 +19265,32 @@ static void qemuAgentFSInfoFormatParams(qemuAgentFSInfo **fsinfo, int nfs, virDomainDef *vmdef, - virTypedParameterPtr *params, - int *nparams, int *maxparams) + virTypedParamList *list) { - size_t i, j; - - /* FIXME: get disk target */ + size_t i; - if (virTypedParamsAddUInt(params, nparams, maxparams, - "fs.count", nfs) < 0) - return; + virTypedParamListAddUInt(list, nfs, "fs.count"); for (i = 0; i < nfs; i++) { - char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.name", i); - if (virTypedParamsAddString(params, nparams, maxparams, - param_name, fsinfo[i]->name) < 0) - return; - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.mountpoint", i); - if (virTypedParamsAddString(params, nparams, maxparams, - param_name, fsinfo[i]->mountpoint) < 0) - return; - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.fstype", i); - if (virTypedParamsAddString(params, nparams, maxparams, - param_name, fsinfo[i]->fstype) < 0) - return; + size_t j; + + virTypedParamListAddString(list, fsinfo[i]->name, "fs.%zu.name", i); + virTypedParamListAddString(list, fsinfo[i]->mountpoint, "fs.%zu.mountpoint", i); + virTypedParamListAddString(list, fsinfo[i]->fstype, "fs.%zu.fstype", i); /* disk usage values are not returned by older guest agents, so * only add the params if the value is set */ - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.total-bytes", i); - if (fsinfo[i]->total_bytes != -1 && - virTypedParamsAddULLong(params, nparams, maxparams, - param_name, fsinfo[i]->total_bytes) < 0) - return; + if (fsinfo[i]->total_bytes != -1) + virTypedParamListAddULLong(list, fsinfo[i]->total_bytes, "fs.%zu.total-bytes", i); + if (fsinfo[i]->used_bytes != -1) + virTypedParamListAddULLong(list, fsinfo[i]->used_bytes, "fs.%zu.used-bytes", i); - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.used-bytes", i); - if (fsinfo[i]->used_bytes != -1 && - virTypedParamsAddULLong(params, nparams, maxparams, - param_name, fsinfo[i]->used_bytes) < 0) - return; + virTypedParamListAddUInt(list, fsinfo[i]->ndisks, "fs.%zu.disk.count", i); - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.disk.count", i); - if (virTypedParamsAddUInt(params, nparams, maxparams, - param_name, fsinfo[i]->ndisks) < 0) - return; for (j = 0; j < fsinfo[i]->ndisks; j++) { virDomainDiskDef *diskdef = NULL; qemuAgentDiskAddress *d = fsinfo[i]->disks[j]; + /* match the disk to the target in the vm definition */ diskdef = virDomainDiskByAddress(vmdef, &d->pci_controller, @@ -19325,28 +19298,15 @@ qemuAgentFSInfoFormatParams(qemuAgentFSInfo **fsinfo, d->bus, d->target, d->unit); - if (diskdef) { - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.disk.%zu.alias", i, j); - if (diskdef->dst && - virTypedParamsAddString(params, nparams, maxparams, - param_name, diskdef->dst) < 0) - return; - } + if (diskdef && diskdef->dst) + virTypedParamListAddString(list, diskdef->dst, + "fs.%zu.disk.%zu.alias", i, j); + + if (d->serial) + virTypedParamListAddString(list, d->serial, "fs.%zu.disk.%zu.serial", i, j); - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.disk.%zu.serial", i, j); - if (d->serial && - virTypedParamsAddString(params, nparams, maxparams, - param_name, d->serial) < 0) - return; - - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "fs.%zu.disk.%zu.device", i, j); - if (d->devnode && - virTypedParamsAddString(params, nparams, maxparams, - param_name, d->devnode) < 0) - return; + if (d->devnode) + virTypedParamListAddString(list, d->devnode, "fs.%zu.disk.%zu.device", i, j); } } } @@ -19504,7 +19464,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom, /* we need to convert the agent fsinfo struct to parameters and match * it to the vm disk target */ if (nfs > 0) - qemuAgentFSInfoFormatParams(agentfsinfo, nfs, vm->def, params, nparams, &maxparams); + qemuAgentFSInfoFormatParams(agentfsinfo, nfs, vm->def, list); if (ndisks > 0) qemuAgentDiskInfoFormatParams(agentdiskinfo, ndisks, vm->def, list); -- 2.48.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 14 ++++---------- src/qemu/qemu_agent.h | 4 +--- src/qemu/qemu_driver.c | 2 +- tests/qemuagenttest.c | 39 ++++++++++++++++----------------------- 4 files changed, 22 insertions(+), 37 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 27efb4b389..e158b3d7ab 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2311,9 +2311,7 @@ qemuAgentGetOSInfo(qemuAgent *agent, */ int qemuAgentGetTimezone(qemuAgent *agent, - virTypedParameterPtr *params, - int *nparams, - int *maxparams, + virTypedParamList *list, bool report_unsupported) { g_autoptr(virJSONValue) cmd = NULL; @@ -2336,10 +2334,8 @@ qemuAgentGetTimezone(qemuAgent *agent, return -1; } - if ((name = virJSONValueObjectGetString(data, "zone")) && - virTypedParamsAddString(params, nparams, maxparams, - "timezone.name", name) < 0) - return -1; + if ((name = virJSONValueObjectGetString(data, "zone"))) + virTypedParamListAddString(list, name, "timezone.name"); if ((virJSONValueObjectGetNumberInt(data, "offset", &offset)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -2347,9 +2343,7 @@ qemuAgentGetTimezone(qemuAgent *agent, return -1; } - if (virTypedParamsAddInt(params, nparams, maxparams, - "timezone.offset", offset) < 0) - return -1; + virTypedParamListAddInt(list, offset, "timezone.offset"); return 0; } diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index cd17a98d39..3f25f0e5a6 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -169,9 +169,7 @@ int qemuAgentGetOSInfo(qemuAgent *mon, bool report_unsupported); int qemuAgentGetTimezone(qemuAgent *mon, - virTypedParameterPtr *params, - int *nparams, - int *maxparams, + virTypedParamList *list, bool report_unsupported); void qemuAgentSetResponseTimeout(qemuAgent *mon, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0d88f182b8..53be46ce0a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19408,7 +19408,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom, goto exitagent; if (supportedTypes & VIR_DOMAIN_GUEST_INFO_TIMEZONE && - qemuAgentGetTimezone(agent, params, nparams, &maxparams, report_unsupported) == -1) + qemuAgentGetTimezone(agent, list, report_unsupported) == -1) goto exitagent; if (supportedTypes & VIR_DOMAIN_GUEST_INFO_HOSTNAME && diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 566571cf11..5fd4d70a70 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -1291,57 +1291,54 @@ testQemuAgentTimezone(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); - virTypedParameterPtr params = NULL; - int nparams = 0; - int ret = -1; if (!test) return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; #define VALIDATE_TIMEZONE(response_, expected_name_, expected_offset_) \ do { \ - int maxparams_ = 0; \ + g_autoptr(virTypedParamList) list = virTypedParamListNew(); \ + virTypedParameterPtr params; \ + size_t nparams; \ const char *name_ = NULL; \ int offset_; \ if (qemuMonitorTestAddItem(test, "guest-get-timezone", \ response_) < 0) \ - goto cleanup; \ - virTypedParamsFree(params, nparams); \ - params = NULL; \ - nparams = 0; \ - if (qemuAgentGetTimezone(qemuMonitorTestGetAgent(test), \ - ¶ms, &nparams, &maxparams_, true) < 0) \ - goto cleanup; \ + return -1; \ + if (qemuAgentGetTimezone(qemuMonitorTestGetAgent(test), list, true) < 0) \ + return -1; \ + if (virTypedParamListFetch(list, ¶ms, &nparams) < 0) \ + return -1; \ if (nparams != 2) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ - "Expected 2 params, got %d", nparams); \ - goto cleanup; \ + "Expected 2 params, got %zu", nparams); \ + return -1; \ } \ if (virTypedParamsGetString(params, nparams, \ "timezone.name", &name_) < 0) { \ virReportError(VIR_ERR_INTERNAL_ERROR, "missing param '%s'", \ "tiemzone.name"); \ - goto cleanup; \ + return -1; \ } \ if (STRNEQ(name_, expected_name_)) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ "Expected name '%s', got '%s'", expected_name_, name_); \ - goto cleanup; \ + return -1; \ } \ if (virTypedParamsGetInt(params, nparams, \ "timezone.offset", &offset_) < 0) { \ virReportError(VIR_ERR_INTERNAL_ERROR, "missing param '%s'", \ "tiemzone.offset"); \ - goto cleanup; \ + return -1; \ } \ if (offset_ != expected_offset_) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ "Expected offset '%i', got '%i'", offset_, \ expected_offset_); \ - goto cleanup; \ + return -1; \ } \ } while (0) @@ -1350,11 +1347,7 @@ testQemuAgentTimezone(const void *data) VALIDATE_TIMEZONE(testQemuAgentTimezoneResponse3, "NDT", -9000); VALIDATE_TIMEZONE(testQemuAgentTimezoneResponse4, "PDT", -25200); - ret = 0; - - cleanup: - virTypedParamsFree(params, nparams); - return ret; + return 0; } -- 2.48.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 9 ++----- src/qemu/qemu_agent.h | 4 +--- src/qemu/qemu_driver.c | 2 +- tests/qemuagenttest.c | 54 ++++++++++++++++++++---------------------- 4 files changed, 30 insertions(+), 39 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index e158b3d7ab..fcfe50dd9e 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2257,9 +2257,7 @@ qemuAgentGetUsers(qemuAgent *agent, */ int qemuAgentGetOSInfo(qemuAgent *agent, - virTypedParameterPtr *params, - int *nparams, - int *maxparams, + virTypedParamList *list, bool report_unsupported) { g_autoptr(virJSONValue) cmd = NULL; @@ -2284,10 +2282,7 @@ qemuAgentGetOSInfo(qemuAgent *agent, do { \ const char *result; \ if ((result = virJSONValueObjectGetString(data, agent_string_))) { \ - if (virTypedParamsAddString(params, nparams, maxparams, \ - param_string_, result) < 0) { \ - return -1; \ - } \ + virTypedParamListAddString(list, result, param_string_); \ } \ } while (0) OSINFO_ADD_PARAM("id", "os.id"); diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 3f25f0e5a6..ac2c8506a2 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -163,9 +163,7 @@ int qemuAgentGetUsers(qemuAgent *mon, bool report_unsupported); int qemuAgentGetOSInfo(qemuAgent *mon, - virTypedParameterPtr *params, - int *nparams, - int *maxparams, + virTypedParamList *list, bool report_unsupported); int qemuAgentGetTimezone(qemuAgent *mon, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 53be46ce0a..201ac89de6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19404,7 +19404,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom, goto exitagent; if (supportedTypes & VIR_DOMAIN_GUEST_INFO_OS && - qemuAgentGetOSInfo(agent, params, nparams, &maxparams, report_unsupported) == -1) + qemuAgentGetOSInfo(agent, list, report_unsupported) == -1) goto exitagent; if (supportedTypes & VIR_DOMAIN_GUEST_INFO_TIMEZONE && diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 5fd4d70a70..93f88083fe 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -1191,30 +1191,31 @@ testQemuAgentOSInfo(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); - virTypedParameterPtr params = NULL; - int nparams = 0; - int maxparams = 0; - int ret = -1; + g_autoptr(virTypedParamList) list = virTypedParamListNew(); + virTypedParameterPtr params; + size_t nparams; if (!test) return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItem(test, "guest-get-osinfo", testQemuAgentOSInfoResponse) < 0) - goto cleanup; + return -1; /* get osinfo */ - if (qemuAgentGetOSInfo(qemuMonitorTestGetAgent(test), - ¶ms, &nparams, &maxparams, true) < 0) - goto cleanup; + if (qemuAgentGetOSInfo(qemuMonitorTestGetAgent(test), list, true) < 0) + return -1; + + if (virTypedParamListFetch(list, ¶ms, &nparams) < 0) + return -1; if (nparams != 8) { virReportError(VIR_ERR_INTERNAL_ERROR, - "Expected 8 params, got %d", nparams); - goto cleanup; + "Expected 8 params, got %zu", nparams); + return -1; } #define VALIDATE_PARAM(param_name_, expected_) \ do { \ @@ -1222,12 +1223,12 @@ testQemuAgentOSInfo(const void *data) if (virTypedParamsGetString(params, nparams, param_name_, &value_) < 0 || \ value_ == NULL) { \ virReportError(VIR_ERR_INTERNAL_ERROR, "missing param '%s'", param_name_); \ - goto cleanup; \ + return -1; \ } \ if (STRNEQ(value_, expected_)) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ "Expected name '%s', got '%s'", expected_, value_); \ - goto cleanup; \ + return -1; \ } \ } while (0) @@ -1239,24 +1240,25 @@ testQemuAgentOSInfo(const void *data) VALIDATE_PARAM("os.kernel-release", "3.10.0-862.14.4.el7.x86_64"); VALIDATE_PARAM("os.kernel-version", "#1 SMP Wed Sep 26 15:12:11 UTC 2018"); VALIDATE_PARAM("os.machine", "x86_64"); - virTypedParamsFree(params, nparams); - params = NULL; - nparams = 0; - maxparams = 0; + + g_clear_pointer(&list, virTypedParamListFree); + list = virTypedParamListNew(); if (qemuMonitorTestAddItem(test, "guest-get-osinfo", testQemuAgentOSInfoResponse2) < 0) - goto cleanup; + return -1; /* get users with domain */ - if (qemuAgentGetOSInfo(qemuMonitorTestGetAgent(test), - ¶ms, &nparams, &maxparams, true) < 0) - goto cleanup; + if (qemuAgentGetOSInfo(qemuMonitorTestGetAgent(test), list, true) < 0) + return -1; + + if (virTypedParamListFetch(list, ¶ms, &nparams) < 0) + return -1; if (nparams != 10) { virReportError(VIR_ERR_INTERNAL_ERROR, - "Expected 10 params, got %d", nparams); - goto cleanup; + "Expected 10 params, got %zu", nparams); + return -1; } VALIDATE_PARAM("os.id", "mswindows"); @@ -1270,11 +1272,7 @@ testQemuAgentOSInfo(const void *data) VALIDATE_PARAM("os.kernel-version", "6.1"); VALIDATE_PARAM("os.machine", "x86_64"); - ret = 0; - - cleanup: - virTypedParamsFree(params, nparams); - return ret; + return 0; } static const char testQemuAgentTimezoneResponse1[] = -- 2.48.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 29 +++++----------------- src/qemu/qemu_agent.h | 4 +-- src/qemu/qemu_driver.c | 2 +- tests/qemuagenttest.c | 55 +++++++++++++++++++----------------------- 4 files changed, 33 insertions(+), 57 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index fcfe50dd9e..606600451d 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2172,9 +2172,7 @@ qemuAgentSetUserPassword(qemuAgent *agent, */ int qemuAgentGetUsers(qemuAgent *agent, - virTypedParameterPtr *params, - int *nparams, - int *maxparams, + virTypedParamList *list, bool report_unsupported) { g_autoptr(virJSONValue) cmd = NULL; @@ -2199,13 +2197,10 @@ qemuAgentGetUsers(qemuAgent *agent, ndata = virJSONValueArraySize(data); - if (virTypedParamsAddUInt(params, nparams, maxparams, - "user.count", ndata) < 0) - return -1; + virTypedParamListAddUInt(list, ndata, "user.count"); for (i = 0; i < ndata; i++) { virJSONValue *entry = virJSONValueArrayGet(data, i); - char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; const char *strvalue; double logintime; @@ -2221,30 +2216,18 @@ qemuAgentGetUsers(qemuAgent *agent, return -1; } - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, "user.%zu.name", i); - if (virTypedParamsAddString(params, nparams, maxparams, - param_name, strvalue) < 0) - return -1; + virTypedParamListAddString(list, strvalue, "user.%zu.name", i); /* 'domain' is only present for windows guests */ - if ((strvalue = virJSONValueObjectGetString(entry, "domain"))) { - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "user.%zu.domain", i); - if (virTypedParamsAddString(params, nparams, maxparams, - param_name, strvalue) < 0) - return -1; - } + if ((strvalue = virJSONValueObjectGetString(entry, "domain"))) + virTypedParamListAddString(list, strvalue, "user.%zu.domain", i); if (virJSONValueObjectGetNumberDouble(entry, "login-time", &logintime) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("'login-time' missing in reply of guest-get-users")); return -1; } - g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, - "user.%zu.login-time", i); - if (virTypedParamsAddULLong(params, nparams, maxparams, - param_name, logintime * 1000) < 0) - return -1; + virTypedParamListAddULLong(list, logintime * 1000, "user.%zu.login-time", i); } return 0; diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index ac2c8506a2..860f19b6bd 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -157,9 +157,7 @@ int qemuAgentSetUserPassword(qemuAgent *mon, bool crypted); int qemuAgentGetUsers(qemuAgent *mon, - virTypedParameterPtr *params, - int *nparams, - int *maxparams, + virTypedParamList *list, bool report_unsupported); int qemuAgentGetOSInfo(qemuAgent *mon, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 201ac89de6..cd07d3eac5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19400,7 +19400,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom, * 'unsupported' errors and gather as much information as we can. In all * other cases, abort on error. */ if (supportedTypes & VIR_DOMAIN_GUEST_INFO_USERS && - qemuAgentGetUsers(agent, params, nparams, &maxparams, report_unsupported) == -1) + qemuAgentGetUsers(agent, list, report_unsupported) == -1) goto exitagent; if (supportedTypes & VIR_DOMAIN_GUEST_INFO_OS && diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 93f88083fe..9c64ed3c5f 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -1095,69 +1095,64 @@ testQemuAgentUsers(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); - virTypedParameterPtr params = NULL; - int nparams = 0; - int maxparams = 0; - int ret = -1; + g_autoptr(virTypedParamList) list = virTypedParamListNew(); + virTypedParameterPtr params; + size_t nparams = 0; unsigned int count; if (!test) return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItem(test, "guest-get-users", testQemuAgentUsersResponse) < 0) - goto cleanup; + return -1; - /* get users */ - if (qemuAgentGetUsers(qemuMonitorTestGetAgent(test), - ¶ms, &nparams, &maxparams, true) < 0) - goto cleanup; + if (qemuAgentGetUsers(qemuMonitorTestGetAgent(test), list, true) < 0) + return -1; + + if (virTypedParamListFetch(list, ¶ms, &nparams) < 0) + return -1; if (virTypedParamsGetUInt(params, nparams, "user.count", &count) < 0) - goto cleanup; + return -1; if (count != 2) { virReportError(VIR_ERR_INTERNAL_ERROR, "Expected '2' users, got '%u'", count); - goto cleanup; + return -1; } if (checkUserInfo(params, nparams, 0, "test", NULL, 1561739203584) < 0 || checkUserInfo(params, nparams, 1, "test2", NULL, 1561739229190) < 0) - goto cleanup; + return -1; + + g_clear_pointer(&list, virTypedParamListFree); + list = virTypedParamListNew(); if (qemuMonitorTestAddItem(test, "guest-get-users", testQemuAgentUsersResponse2) < 0) - goto cleanup; + return -1; - virTypedParamsFree(params, nparams); - params = NULL; - nparams = 0; - maxparams = 0; + if (qemuAgentGetUsers(qemuMonitorTestGetAgent(test), list, true) < 0) + return -1; - /* get users with domain */ - if (qemuAgentGetUsers(qemuMonitorTestGetAgent(test), - ¶ms, &nparams, &maxparams, true) < 0) - goto cleanup; + if (virTypedParamListFetch(list, ¶ms, &nparams) < 0) + return -1; if (virTypedParamsGetUInt(params, nparams, "user.count", &count) < 0) - goto cleanup; + return -1; if (count != 1) { virReportError(VIR_ERR_INTERNAL_ERROR, "Expected '1' user, got '%u'", count); - goto cleanup; + return -1; } if (checkUserInfo(params, nparams, 0, "test", "DOMAIN", 1561739203584) < 0) - goto cleanup; - - ret = 0; + return -1; - cleanup: - virTypedParamsFree(params, nparams); - return ret; + return 0; } static const char testQemuAgentOSInfoResponse[] = -- 2.48.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cd07d3eac5..5d91bd1a93 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19356,7 +19356,6 @@ qemuDomainGetGuestInfo(virDomainPtr dom, virDomainObj *vm = NULL; qemuAgent *agent; int ret = -1; - int maxparams = 0; g_autofree char *hostname = NULL; unsigned int supportedTypes; bool report_unsupported = types != 0; @@ -19415,9 +19414,8 @@ qemuDomainGetGuestInfo(virDomainPtr dom, qemuAgentGetHostname(agent, &hostname, report_unsupported) == -1) goto exitagent; - if (hostname && - virTypedParamsAddString(params, nparams, &maxparams, "hostname", hostname) < 0) - goto exitagent; + if (hostname) + virTypedParamListAddString(list, hostname, "hostname"); if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) { rc = qemuAgentGetFSInfo(agent, &agentfsinfo, report_unsupported); -- 2.48.1
Now that the refactor was completed the helper infrastructure can be removed. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5d91bd1a93..c69074766b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19372,7 +19372,6 @@ qemuDomainGetGuestInfo(virDomainPtr dom, bool format_load = false; size_t i; g_autoptr(virTypedParamList) list = virTypedParamListNew(); - g_autoptr(virTypedParamList) tmplist = NULL; virCheckFlags(0, -1); @@ -19481,11 +19480,6 @@ qemuDomainGetGuestInfo(virDomainPtr dom, virTypedParamListAddDouble(list, load15m, "load.15m"); } - /* temporarily allow the old and new construction style to coexist */ - tmplist = virTypedParamListFromParams(params, *nparams); - virTypedParamListConcat(tmplist, &list); - list = g_steal_pointer(&tmplist); - if (virTypedParamListSteal(list, params, nparams) < 0) goto cleanup; -- 2.48.1
On a Wednesday in 2025, Peter Krempa wrote:
Use virTypedParam list to construct the returned data.
Note that this applies on top of two patch(sets) that were ACK'd upstream:
[PATCH v2 0/4] Add support for getting load averages from guest agent [PATCH] qemu: Report disk bus as reported by agent in virDomainGetGuestInfo
You can fetch the whole thing via:
git fetch https://gitlab.com/pipo.sk/libvirt.git agent-data-refactor
Peter Krempa (10): qemuDomainGetGuestInfo: Prepare for refactor to virTypedParamList qemuDomainGetGuestInfo: Convert load code to virTypedParamList virDomainInterfaceFormatParams: Convert interface code to virTypedParamList qemuAgentDiskInfoFormatParams: Convert interface code to virTypedParamList qemuAgentFSInfoFormatParams: Convert interface code to virTypedParamList qemuAgentGetTimezone: Convert to virTypedParamList qemuAgentGetOSInfo: Convert to virTypedParamList qemuAgentGetUsers: Convert to virTypedParamList qemuDomainGetGuestInfo: Convert hostname code to virTypedParamList qemuDomainGetGuestInfo: Remove temporary infrastructure
src/qemu/qemu_agent.c | 52 ++------- src/qemu/qemu_agent.h | 12 +- src/qemu/qemu_driver.c | 249 ++++++++++++----------------------------- tests/qemuagenttest.c | 148 +++++++++++------------- 4 files changed, 152 insertions(+), 309 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa