From e352a692a7129ef5ef40f0c98e496c61f563edc5 Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Fri, 13 Sep 2024 10:33:47 +0200 Subject: [PATCH] qemu: Use the new chardev backend JSON props generator also in the monitor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that we have a unified generator of chardev backend which is also validated against the QMP schema we can replace the old generator with it. This patch modifies the monitor code to take virJSONValue 'props' instead of the chardev definition and adds the conversion from the chardev definition to JSON on higher levels. The monitor code now also attempts to extract the returned 'pty' if returned from qemu, so higher level code needs to report the error if the path is needed and missing. The current monitor generator is for now abandoned in place and will be removed later. Signed-off-by: Peter Krempa Reviewed-by: Ján Tomko --- src/qemu/qemu_block.c | 9 ++++++++- src/qemu/qemu_hotplug.c | 22 +++++++++++++++++++++- src/qemu/qemu_monitor.c | 8 +++----- src/qemu/qemu_monitor.h | 4 ++-- src/qemu/qemu_monitor_json.c | 32 ++++++++++---------------------- src/qemu/qemu_monitor_json.h | 4 ++-- tests/qemumonitorjsontest.c | 17 +++++++++++------ 7 files changed, 57 insertions(+), 39 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 6e90bae9f2..9bdec92697 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -24,6 +24,7 @@ #include "qemu_alias.h" #include "qemu_security.h" #include "qemu_process.h" +#include "qemu_chardev.h" #include "storage_source.h" #include "viralloc.h" @@ -1682,7 +1683,13 @@ qemuBlockStorageSourceAttachApply(qemuMonitor *mon, return -1; if (data->chardevDef) { - if (qemuMonitorAttachCharDev(mon, data->chardevAlias, data->chardevDef) < 0) + g_autoptr(virJSONValue) props = NULL; + + if (qemuChardevGetBackendProps(data->chardevDef, false, + data->chardevAlias, NULL, &props) < 0) + return -1; + + if (qemuMonitorAttachCharDev(mon, &props, NULL) < 0) return -1; data->chardevAdded = true; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index a58c1446e9..c1070d2562 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -37,6 +37,7 @@ #include "qemu_block.h" #include "qemu_snapshot.h" #include "qemu_virtiofs.h" +#include "qemu_chardev.h" #include "domain_audit.h" #include "domain_cgroup.h" #include "domain_interface.h" @@ -243,6 +244,9 @@ qemuHotplugChardevAttach(qemuMonitor *mon, const char *alias, virDomainChrSourceDef *def) { + g_autoptr(virJSONValue) props = NULL; + g_autofree char *ptypath = NULL; + switch ((virDomainChrType) def->type) { case VIR_DOMAIN_CHR_TYPE_NULL: case VIR_DOMAIN_CHR_TYPE_VC: @@ -272,7 +276,23 @@ qemuHotplugChardevAttach(qemuMonitor *mon, return -1; } - return qemuMonitorAttachCharDev(mon, alias, def); + if (qemuChardevGetBackendProps(def, false, alias, NULL, &props) < 0) + return -1; + + if (qemuMonitorAttachCharDev(mon, &props, &ptypath) < 0) + return -1; + + if (def->type == VIR_DOMAIN_CHR_TYPE_PTY) { + if (!ptypath) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("chardev-add reply was missing pty path")); + return -1; + } + + def->data.file.path = g_steal_pointer(&ptypath); + } + + return 0; } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 7f65c23748..ada3de474f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3523,14 +3523,12 @@ qemuMonitorGetTPMTypes(qemuMonitor *mon, int qemuMonitorAttachCharDev(qemuMonitor *mon, - const char *chrID, - virDomainChrSourceDef *chr) + virJSONValue **props, + char **ptypath) { - VIR_DEBUG("chrID=%s chr=%p", chrID, chr); - QEMU_CHECK_MONITOR(mon); - return qemuMonitorJSONAttachCharDev(mon, chrID, chr); + return qemuMonitorJSONAttachCharDev(mon, props, ptypath); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 57d1b45bf5..2bb64dd53f 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1212,8 +1212,8 @@ int qemuMonitorGetTPMTypes(qemuMonitor *mon, char ***tpmtypes); int qemuMonitorAttachCharDev(qemuMonitor *mon, - const char *chrID, - virDomainChrSourceDef *chr); + virJSONValue **props, + char **ptypath); int qemuMonitorDetachCharDev(qemuMonitor *mon, const char *chrID); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 2db38c1007..f3a353a13b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6342,7 +6342,7 @@ int qemuMonitorJSONGetTPMTypes(qemuMonitor *mon, } -static virJSONValue * +static G_GNUC_UNUSED virJSONValue * qemuMonitorJSONAttachCharDevGetProps(const char *chrID, const virDomainChrSourceDef *chr) { @@ -6571,41 +6571,29 @@ qemuMonitorJSONAttachCharDevGetProps(const char *chrID, int qemuMonitorJSONAttachCharDev(qemuMonitor *mon, - const char *chrID, - virDomainChrSourceDef *chr) + virJSONValue **props, + char **ptypath) { g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; - g_autoptr(virJSONValue) props = NULL; + virJSONValue *data; - if (!(props = qemuMonitorJSONAttachCharDevGetProps(chrID, chr))) - return -1; - - if (!(cmd = qemuMonitorJSONMakeCommandInternal("chardev-add", &props))) + if (!(cmd = qemuMonitorJSONMakeCommandInternal("chardev-add", props))) return -1; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) return -1; - if (chr->type == VIR_DOMAIN_CHR_TYPE_PTY) { - virJSONValue *data; + if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT))) + return -1; - if (!(data = qemuMonitorJSONGetReply(cmd, reply, VIR_JSON_TYPE_OBJECT))) - return -1; - - 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; - } - } else { - if (qemuMonitorJSONCheckError(cmd, reply) < 0) - return -1; - } + if (ptypath) + *ptypath = g_strdup(virJSONValueObjectGetString(data, "pty")); return 0; } + int qemuMonitorJSONDetachCharDev(qemuMonitor *mon, const char *chrID) diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 921dd34ed2..fef81fd911 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -584,8 +584,8 @@ qemuMonitorJSONGetTPMTypes(qemuMonitor *mon, int qemuMonitorJSONAttachCharDev(qemuMonitor *mon, - const char *chrID, - virDomainChrSourceDef *chr); + virJSONValue **props, + char **ptypath); int qemuMonitorJSONDetachCharDev(qemuMonitor *mon, const char *chrID); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 2249787ba7..ab2c445289 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -28,6 +28,7 @@ #include "qemu/qemu_monitor_json.h" #include "qemu/qemu_qapi.h" #include "qemu/qemu_alias.h" +#include "qemu/qemu_chardev.h" #include "virerror.h" #include "cpu/cpu.h" #include "qemu/qemu_monitor.h" @@ -553,6 +554,8 @@ testQemuMonitorJSONAttachChardev(const void *opaque) { const struct qemuMonitorJSONTestAttachChardevData *data = opaque; g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewSchema(data->xmlopt, data->schema); + g_autoptr(virJSONValue) props = NULL; + g_autofree char *ptypath = NULL; int rc; if (!test) @@ -575,20 +578,20 @@ testQemuMonitorJSONAttachChardev(const void *opaque) return -1; } - if ((rc = qemuMonitorAttachCharDev(qemuMonitorTestGetMonitor(test), - "alias", data->chr)) < 0) + if (qemuChardevGetBackendProps(data->chr, false, "alias", NULL, &props) < 0) + return -1; + + if ((rc = qemuMonitorAttachCharDev(qemuMonitorTestGetMonitor(test), &props, &ptypath)) < 0) goto cleanup; if (data->chr->type == VIR_DOMAIN_CHR_TYPE_PTY) { - if (STRNEQ_NULLABLE(data->expectPty, data->chr->data.file.path)) { + if (STRNEQ_NULLABLE(data->expectPty, ptypath)) { virReportError(VIR_ERR_INTERNAL_ERROR, "expected PTY path: %s got: %s", NULLSTR(data->expectPty), NULLSTR(data->chr->data.file.path)); rc = -1; } - - VIR_FREE(data->chr->data.file.path); } cleanup: @@ -653,7 +656,9 @@ qemuMonitorJSONTestAttachChardev(virDomainXMLOption *xmlopt, "'data':{'type':'vdagent'}}}"); chr->type = VIR_DOMAIN_CHR_TYPE_PTY; - CHECK("pty missing path", true, + /* Higher level code regards the missing path as error, but we simply + * check here what we've parsed */ + CHECK("pty missing path", false, "{'id':'alias','backend':{'type':'pty','data':{}}}"); if (qemuMonitorJSONTestAttachOneChardev(xmlopt, schema, "pty", chr, "{'id':'alias',"