[libvirt] [PATCH 0/2] udp chardev fixes
Ján Tomko (2): conf: fix formatting of udp chardev attributes qemu: fix hotplug of udp device with no connect host src/conf/domain_conf.c | 12 ++++-------- src/qemu/qemu_monitor_json.c | 5 ++++- tests/genericxml2xmloutdata/generic-chardev-udp.xml | 2 +- 3 files changed, 9 insertions(+), 10 deletions(-) -- 2.13.0
It is possible (although possibly not very useful) to leave out the service attribute when using <source mode='bind'/> Fix the formatter bug introduced by commit 4a0da34 and format the host when its present (checked for non-NULL inside virBufferEscapeString) instead of basing it on the presence of the service attribute. https://bugzilla.redhat.com/show_bug.cgi?id=1455825 --- src/conf/domain_conf.c | 12 ++++-------- tests/genericxml2xmloutdata/generic-chardev-udp.xml | 2 +- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 44cfb52b4..3c3db7291 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23307,19 +23307,15 @@ virDomainChrSourceDefFormat(virBufferPtr buf, case VIR_DOMAIN_CHR_TYPE_UDP: if (def->data.udp.bindService || def->data.udp.bindHost) { virBufferAddLit(buf, "<source mode='bind'"); - if (def->data.udp.bindService) - virBufferEscapeString(buf, " host='%s'", def->data.udp.bindHost); - if (def->data.udp.bindService) - virBufferEscapeString(buf, " service='%s'", def->data.udp.bindService); + virBufferEscapeString(buf, " host='%s'", def->data.udp.bindHost); + virBufferEscapeString(buf, " service='%s'", def->data.udp.bindService); virBufferAddLit(buf, "/>\n"); } if (def->data.udp.connectService || def->data.udp.connectHost) { virBufferAddLit(buf, "<source mode='connect'"); - if (def->data.udp.connectService) - virBufferEscapeString(buf, " host='%s'", def->data.udp.connectHost); - if (def->data.udp.connectService) - virBufferEscapeString(buf, " service='%s'", def->data.udp.connectService); + virBufferEscapeString(buf, " host='%s'", def->data.udp.connectHost); + virBufferEscapeString(buf, " service='%s'", def->data.udp.connectService); virBufferAddLit(buf, "/>\n"); } break; diff --git a/tests/genericxml2xmloutdata/generic-chardev-udp.xml b/tests/genericxml2xmloutdata/generic-chardev-udp.xml index c4a719f2f..c9b3e5550 100644 --- a/tests/genericxml2xmloutdata/generic-chardev-udp.xml +++ b/tests/genericxml2xmloutdata/generic-chardev-udp.xml @@ -29,7 +29,7 @@ <target type='virtio' name='test3'/> </channel> <channel type='udp'> - <source mode='bind'/> + <source mode='bind' host='localhost'/> <source mode='connect' service='5678'/> <target type='virtio' name='test4'/> </channel> -- 2.13.0
On Tue, Sep 26, 2017 at 01:58:52PM +0200, Ján Tomko wrote:
It is possible (although possibly not very useful) to leave out the service attribute when using <source mode='bind'/>
Fix the formatter bug introduced by commit 4a0da34 and format the host when its present (checked for non-NULL inside virBufferEscapeString) instead of basing it on the presence of the service attribute.
https://bugzilla.redhat.com/show_bug.cgi?id=1455825 --- src/conf/domain_conf.c | 12 ++++-------- tests/genericxml2xmloutdata/generic-chardev-udp.xml | 2 +- 2 files changed, 5 insertions(+), 9 deletions(-)
NACK to this one, I have already sent it separately: https://www.redhat.com/archives/libvir-list/2017-September/msg00921.html Jan
Use an empty string to let qemu fill out the default. https://bugzilla.redhat.com/show_bug.cgi?id=1454671 --- src/qemu/qemu_monitor_json.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 63b855920..c63d250d3 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6522,7 +6522,10 @@ qemuMonitorJSONAttachCharDevCommand(const char *chrID, case VIR_DOMAIN_CHR_TYPE_UDP: backend_type = "udp"; - addr = qemuMonitorJSONBuildInetSocketAddress(chr->data.udp.connectHost, + host = chr->data.udp.connectHost; + if (!host) + host = ""; + addr = qemuMonitorJSONBuildInetSocketAddress(host, chr->data.udp.connectService); if (!addr || virJSONValueObjectAppend(data, "remote", addr) < 0) -- 2.13.0
On 09/26/2017 07:58 AM, Ján Tomko wrote:
Use an empty string to let qemu fill out the default.
Matches what's done in qemuBuildChrChardevStr
https://bugzilla.redhat.com/show_bug.cgi?id=1454671 --- src/qemu/qemu_monitor_json.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
participants (2)
-
John Ferlan -
Ján Tomko