[libvirt PATCH] qemu_monitor_json: fix JSON generator for VC chardev
The correct backend type is 'vc', same as in qemuBuildChrChardevStr() where we generate qemu command line. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_monitor_json.c | 5 ++++- tests/qemumonitorjsontest.c | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 97c5e5b36c..231f0ded32 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -7438,10 +7438,13 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID, switch ((virDomainChrType)chr->type) { case VIR_DOMAIN_CHR_TYPE_NULL: - case VIR_DOMAIN_CHR_TYPE_VC: backend_type = "null"; break; + case VIR_DOMAIN_CHR_TYPE_VC: + backend_type = "vc"; + break; + case VIR_DOMAIN_CHR_TYPE_PTY: backend_type = "pty"; break; diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 29c396891b..956423f10f 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -718,7 +718,7 @@ qemuMonitorJSONTestAttachChardev(virDomainXMLOptionPtr xmlopt, chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_VC }; CHECK("vc", false, - "{'id':'alias','backend':{'type':'null','data':{}}}"); + "{'id':'alias','backend':{'type':'vc','data':{}}}"); chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PTY }; if (qemuMonitorJSONTestAttachOneChardev(xmlopt, schema, "pty", &chr, -- 2.29.2
On 2/3/21 7:01 PM, Pavel Hrdina wrote:
The correct backend type is 'vc', same as in qemuBuildChrChardevStr() where we generate qemu command line.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_monitor_json.c | 5 ++++- tests/qemumonitorjsontest.c | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 97c5e5b36c..231f0ded32 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -7438,10 +7438,13 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID,
switch ((virDomainChrType)chr->type) { case VIR_DOMAIN_CHR_TYPE_NULL: - case VIR_DOMAIN_CHR_TYPE_VC: backend_type = "null"; break;
+ case VIR_DOMAIN_CHR_TYPE_VC: + backend_type = "vc"; + break; + case VIR_DOMAIN_CHR_TYPE_PTY: backend_type = "pty"; break; diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 29c396891b..956423f10f 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -718,7 +718,7 @@ qemuMonitorJSONTestAttachChardev(virDomainXMLOptionPtr xmlopt,
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_VC }; CHECK("vc", false, - "{'id':'alias','backend':{'type':'null','data':{}}}"); + "{'id':'alias','backend':{'type':'vc','data':{}}}");
chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PTY }; if (qemuMonitorJSONTestAttachOneChardev(xmlopt, schema, "pty", &chr,
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> But maybe we could deduplicate some code? I mean, qemuBuildChrChardevStr() and some parts of qemuMonitorJSONAttachCharDevCommand() look very similar and we could use virQEMUBuildCommandLineJSON() or something. But merging the patch as is is okay too. Michal
On Thu, Feb 04, 2021 at 01:00:51PM +0100, Michal Privoznik wrote:
On 2/3/21 7:01 PM, Pavel Hrdina wrote:
The correct backend type is 'vc', same as in qemuBuildChrChardevStr() where we generate qemu command line.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_monitor_json.c | 5 ++++- tests/qemumonitorjsontest.c | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 97c5e5b36c..231f0ded32 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -7438,10 +7438,13 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID, switch ((virDomainChrType)chr->type) { case VIR_DOMAIN_CHR_TYPE_NULL: - case VIR_DOMAIN_CHR_TYPE_VC: backend_type = "null"; break; + case VIR_DOMAIN_CHR_TYPE_VC: + backend_type = "vc"; + break; + case VIR_DOMAIN_CHR_TYPE_PTY: backend_type = "pty"; break; diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 29c396891b..956423f10f 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -718,7 +718,7 @@ qemuMonitorJSONTestAttachChardev(virDomainXMLOptionPtr xmlopt, chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_VC }; CHECK("vc", false, - "{'id':'alias','backend':{'type':'null','data':{}}}"); + "{'id':'alias','backend':{'type':'vc','data':{}}}"); chr = (virDomainChrSourceDef) { .type = VIR_DOMAIN_CHR_TYPE_PTY }; if (qemuMonitorJSONTestAttachOneChardev(xmlopt, schema, "pty", &chr,
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Thanks
But maybe we could deduplicate some code? I mean, qemuBuildChrChardevStr() and some parts of qemuMonitorJSONAttachCharDevCommand() look very similar and we could use virQEMUBuildCommandLineJSON() or something. But merging the patch as is is okay too.
That was a plan but it is not that trivial as the JSON and cmdline syntax are different. Pavel
On 2/4/21 1:06 PM, Pavel Hrdina wrote:
On Thu, Feb 04, 2021 at 01:00:51PM +0100, Michal Privoznik wrote:
On 2/3/21 7:01 PM, Pavel Hrdina wrote:
The correct backend type is 'vc', same as in qemuBuildChrChardevStr() where we generate qemu command line.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_monitor_json.c | 5 ++++- tests/qemumonitorjsontest.c | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-)
But maybe we could deduplicate some code? I mean, qemuBuildChrChardevStr() and some parts of qemuMonitorJSONAttachCharDevCommand() look very similar and we could use virQEMUBuildCommandLineJSON() or something. But merging the patch as is is okay too.
That was a plan but it is not that trivial as the JSON and cmdline syntax are different.
Oh, are they? I only quickly skimmed through both functions and it appeared similar. If it isn't then it may be more confusing to merge them together. Michal
participants (2)
-
Michal Privoznik -
Pavel Hrdina