Re: [libvirt] [PATCH 4/5] conf: make sure that chardev reconnect is formatted only for connect mode

30 Aug 2017

On Wed, Aug 30, 2017 at 02:32:50PM +0200, Michal Privoznik wrote:
...
On 08/30/2017 01:40 PM, Pavel Hrdina wrote:
...
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/conf/domain_conf.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f7574d77b6..7f443e5b4d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23257,8 +23257,9 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
             virBufferAsprintf(&attrBuf, " tlsFromConfig='%d'",
                               def->data.tcp.tlsFromConfig);
-        virDomainChrSourceReconnectDefFormat(&childBuf,
-                                             &def->data.tcp.reconnect);
+        if (!def->data.tcp.listen)
+            virDomainChrSourceReconnectDefFormat(&childBuf,
+                                                 &def->data.tcp.reconnect);
if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0)
             goto error;
@@ -23276,8 +23277,9 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
             virDomainSourceDefFormatSeclabel(&childBuf, def->nseclabels,
                                              def->seclabels, flags);
-            virDomainChrSourceReconnectDefFormat(&childBuf,
-                                                 &def->data.nix.reconnect);
+            if (!def->data.nix.listen)
+                virDomainChrSourceReconnectDefFormat(&childBuf,
+                                                     &def->data.nix.reconnect);
if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0)
                 goto error;
This looks like a workaround. Because def->data.tcp.listen shouldn't be
set if reconnect is enabled and vice versa. And
virDomainChrSourceReconnectDefFormat() shortcuts out. Or you want the
following:
<channel type='tcp'>
      <source mode='bind' host='localhost' service='5678'>
        <reconnect enabled='no'/>
      </source>
      <target type='virtio' name='test2'/>
    </channel>
to be turned into:
<channel type='tcp'>
      <source mode='bind' host='localhost' service='5678'/>
      <target type='virtio' name='test2'/>
    </channel>
Michal
Yes, it's kind of workaround for the case where you will set

  <channel type='unix'>
    <source mode='connect' path='/var/lib/libvirt/qemu/channel/target/domain-test/test2'>
      <reconnect enabled='no'/>
    </source>
    <target type='virtio' name='test2'/>
  </channel>

Without this patch it would lead to:

  <channel type='unix'>
    <source mode='bind' path='/var/lib/libvirt/qemu/channel/target/domain-5-test/test2'>
      <reconnect enabled='no'/>
    </source>
    <target type='virtio' name='test2'/>
  </channel>

because we remove the auto-generated path and while starting a guest
we generate a new one and sets the mode to "bind".

This patch makes sure that in this case the XML of live guest is
correct.

The proper fix would be to update _virDomainChrSourceDef by changing
'bool listen' into 'virDomainChrSourceModeType mode' and the parse would
properly store three different values: default, connect, bind.  This
would require rewrite a lot of code which is not suitable for a freeze,
therefore this workaround.  I'm planning to fix it properly so that
workaround is not required anymore.

Pavel

    