Currently while parsing domain XML we clear the UNIX path if it matches
one of the auto-generated paths by libvirt.  After that when the guest
is started new path is generated but the mode is also changed to "bind".

In the real-world use-case the mode should not change, it only happens
if a user provides a mode='connect' and path that matches one of the
auto-generated path or not provides a path at all.

Before *reconnect* feature was introduced there was no issue, but with
the new feature we need to make sure that it's used only with "connect"
mode, therefore we need to move the mode change into parsing in order
to have a proper error reported by validation code.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/conf/domain_conf.c                             |  8 ++++++++
 src/qemu/qemu_domain.c                             |  6 +++---
 ...muxml2argv-chardev-reconnect-generated-path.xml | 23 ++++++++++++++++++++++
 tests/qemuxml2argvtest.c                           |  3 +++
 4 files changed, 37 insertions(+), 3 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect-generated-path.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f7574d77b6..9fdd0f90fc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4421,6 +4421,14 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev,
 
             chr->target.port = maxport + 1;
         }
+
+        /* For UNIX chardev if no path is provided we generate one.
+         * This also implies that the mode is 'bind'. */
+        if (chr->source &&
+            chr->source->type == VIR_DOMAIN_CHR_TYPE_UNIX &&
+            !chr->source->data.nix.path) {
+            chr->source->data.nix.listen = true;
+        }
     }
 
     /* set default path for virtio-rng "random" backend to /dev/random */
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bf57f94a50..ecad68d3bb 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3489,8 +3489,10 @@ qemuDomainChrDefDropDefaultPath(virDomainChrDefPtr chr,
 
     regexp = virBufferContentAndReset(&buf);
 
-    if (virStringMatch(chr->source->data.nix.path, regexp))
+    if (virStringMatch(chr->source->data.nix.path, regexp)) {
         VIR_FREE(chr->source->data.nix.path);
+        chr->source->data.nix.listen = true;
+    }
 
     ret = 0;
  cleanup:
@@ -7441,8 +7443,6 @@ qemuDomainPrepareChannel(virDomainChrDefPtr channel,
             return -1;
     }
 
-    channel->source->data.nix.listen = true;
-
     return 0;
 }
 
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect-generated-path.xml b/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect-generated-path.xml
new file mode 100644
index 0000000000..3cb67ecdcc
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect-generated-path.xml
@@ -0,0 +1,23 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <memory unit='KiB'>219136</memory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <devices>
+    <emulator>/usr/bin/qemu-system-i686</emulator>
+    <controller type='virtio-serial' index='1'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/>
+    </controller>
+    <channel type='unix'>
+      <source mode='connect' path='/tmp/channel/domain-oldname/asdf'>
+        <reconnect enabled='yes' timeout='10'/>
+      </source>
+      <target type='virtio' name='asdf'/>
+    </channel>
+    <memballoon model='none'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 0c0bd16f94..18f06e5aa5 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1351,6 +1351,9 @@ mymain(void)
     DO_TEST_PARSE_ERROR("chardev-reconnect-invalid-timeout",
                         QEMU_CAPS_NODEFCONFIG,
                         QEMU_CAPS_CHARDEV_RECONNECT);
+    DO_TEST_PARSE_ERROR("chardev-reconnect-generated-path",
+                        QEMU_CAPS_NODEFCONFIG,
+                        QEMU_CAPS_CHARDEV_RECONNECT);
 
     DO_TEST("usb-controller",
             QEMU_CAPS_NODEFCONFIG);
-- 
2.13.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 08/30/2017 04:49 PM, Pavel Hrdina wrote:
> Currently while parsing domain XML we clear the UNIX path if it matches
> one of the auto-generated paths by libvirt.  After that when the guest
> is started new path is generated but the mode is also changed to "bind".
> 
> In the real-world use-case the mode should not change, it only happens
> if a user provides a mode='connect' and path that matches one of the
> auto-generated path or not provides a path at all.
> 
> Before *reconnect* feature was introduced there was no issue, but with
> the new feature we need to make sure that it's used only with "connect"
> mode, therefore we need to move the mode change into parsing in order
> to have a proper error reported by validation code.
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/conf/domain_conf.c                             |  8 ++++++++
>  src/qemu/qemu_domain.c                             |  6 +++---
>  ...muxml2argv-chardev-reconnect-generated-path.xml | 23 ++++++++++++++++++++++
>  tests/qemuxml2argvtest.c                           |  3 +++
>  4 files changed, 37 insertions(+), 3 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-chardev-reconnect-generated-path.xml
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index f7574d77b6..9fdd0f90fc 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4421,6 +4421,14 @@ virDomainDeviceDefPostParseCommon(virDomainDeviceDefPtr dev,
>  
>              chr->target.port = maxport + 1;
>          }
> +
> +        /* For UNIX chardev if no path is provided we generate one.
> +         * This also implies that the mode is 'bind'. */
> +        if (chr->source &&
> +            chr->source->type == VIR_DOMAIN_CHR_TYPE_UNIX &&
> +            !chr->source->data.nix.path) {
> +            chr->source->data.nix.listen = true;
> +        }
>      }

This hunk should go somewhere under src/qemu/ because leaving it here
enables this for all the drivers. qemuDomainDeviceDefPostParse() looks
like a good candidate.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list