[libvirt] [PATCH] Remove invalid parsing of pty path from XML for PTY type chardevs

[Daniel P. Berrange]

On Thu, Nov 28, 2013 at 12:17:31AM +0530, Nehal J Wani wrote:
> While running valgrind on the qemuhotplugtest, one of the many memory leaks
> detected are:
> 
> ==3915== 12 bytes in 1 blocks are definitely lost in loss record 36 of 129
> ==3915==    at 0x4A0887C: malloc (vg_replace_malloc.c:270)
> ==3915==    by 0x34268AF275: xmlStrndup (in /usr/lib64/libxml2.so.2.9.1)
> ==3915==    by 0x4CCC3BB: virDomainChrSourceDefParseXML (domain_conf.c:6957)
> ==3915==    by 0x4CD6A29: virDomainChrDefParseXML (domain_conf.c:7243)
> ==3915==    by 0x4CEC0C9: virDomainDeviceDefParse (domain_conf.c:9616)
> ==3915==    by 0x41D14F: testQemuHotplug (qemuhotplugtest.c:247)
> ==3915==    by 0x41E421: virtTestRun (testutils.c:139)
> ==3915==    by 0x41C6E3: mymain (qemuhotplugtest.c:428)
> ==3915==    by 0x41EAB2: virtTestMain (testutils.c:600)
> ==3915==    by 0x341F421A04: (below main) (libc-start.c:225)
> ==3915==
> 
> The main reason behind this error is that for the PTY type chardevs, it is
> not valid to specify or parse a pty path from the XML. This is an output-only
> attribute, not under user control. Hence, the parsing code inside
> qemuMonitorJSONAttachCharDev() was wrong.
> 
> Refer the discussion: https://www.redhat.com/archives/libvir-list/2013-November/msg01255.html
> 
> tests/qemumonitorjsontest.c
>    * Remove unwanted parsing code inside qemuMonitorJSONAttachCharDev
> 
> src/qemu/qemu_monitor_json.c:
>    * Remove erraneous testcase
> 
> ---
>  src/qemu/qemu_monitor_json.c | 20 --------------------
>  tests/qemumonitorjsontest.c  | 16 ----------------
>  2 files changed, 36 deletions(-)
> 
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index ec3b958..1bfeef0 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -5368,26 +5368,6 @@ qemuMonitorJSONAttachCharDev(qemuMonitorPtr mon,
>      if (qemuMonitorJSONCheckError(cmd, reply) < 0)
>          goto cleanup;
>  
> -    if (chr->type == VIR_DOMAIN_CHR_TYPE_PTY) {
> -        virJSONValuePtr data;
> -        const char *path;
> -
> -        if (!(data = virJSONValueObjectGet(reply, "return"))) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                           _("chardev-add reply was missing return data"));
> -            goto cleanup;
> -        }
> -
> -        if (!(path = virJSONValueObjectGetString(data, "pty"))) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                           _("chardev-add reply was missing pty path"));
> -            goto cleanup;
> -        }
> -
> -        if (VIR_STRDUP(chr->data.file.path, path) < 0)
> -            goto cleanup;
> -    }
> -
>      ret = 0;

NACK, this is completely wrong. I said the *XML parsing* was likely
to be wrong. This is the QEMU monitor code you've deleted.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|