[PATCH] qemu: Be less aggressive when dropping channel source paths

Michal Privoznik posted 1 patch 3 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/79c920fc64b56ceffe1d6ece4791109fa493de4c.1704810524.git.mprivozn@redhat.com
src/qemu/qemu_domain.c                        |  2 +-
...hannel-unix-source-path.x86_64-latest.args |  8 ++++++++
.../channel-unix-source-path.xml              | 16 +++++++++++++++
...-unix-source-path-active.x86_64-latest.xml | 20 +++++++++++++++++++
...nix-source-path-inactive.x86_64-latest.xml | 20 +++++++++++++++++++
5 files changed, 65 insertions(+), 1 deletion(-)
[PATCH] qemu: Be less aggressive when dropping channel source paths
Posted by Michal Privoznik 3 months, 2 weeks ago
In v9.7.0-rc1~130 I've shortened the path that's generated for
<channel/> source. With that, I had to adjust regex that matches
all versions of paths we have ever generated so that we can drop
them (see comment around qemuDomainChrDefDropDefaultPath()). But
as it is usually the case with regexes - they are write only. And
while I attempted to make one portion of the path optional
("/target/") I accidentally made regex accept more, which
resulted in libvirt dropping the user provided path and
generating our own instead.

Fixes: d3759d3674ab9453e5fb5a27ab6c28b8ff8d5569
Resolves: https://issues.redhat.com/browse/RHEL-20807
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_domain.c                        |  2 +-
 ...hannel-unix-source-path.x86_64-latest.args |  8 ++++++++
 .../channel-unix-source-path.xml              | 16 +++++++++++++++
 ...-unix-source-path-active.x86_64-latest.xml | 20 +++++++++++++++++++
 ...nix-source-path-inactive.x86_64-latest.xml | 20 +++++++++++++++++++
 5 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 003fe7c71b..3a00fb689e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5447,7 +5447,7 @@ qemuDomainChrMatchDefaultPath(const char *prefix,
     virBufferEscapeRegex(&buf, "^%s", prefix);
     if (infix)
         virBufferEscapeRegex(&buf, "%s", infix);
-    virBufferAddLit(&buf, "/(target/)?([^/]+\\.)|(domain-[^/]+/)|([0-9]+-[^/]+/)");
+    virBufferAddLit(&buf, "/(target/)?(([^/]+\\.)|(domain-[^/]+/)|([0-9]+-[^/]+/))");
     virBufferEscapeRegex(&buf, "%s$", target);
 
     regexp = virBufferContentAndReset(&buf);
diff --git a/tests/qemuxml2argvdata/channel-unix-source-path.x86_64-latest.args b/tests/qemuxml2argvdata/channel-unix-source-path.x86_64-latest.args
index 821a508a40..845236d286 100644
--- a/tests/qemuxml2argvdata/channel-unix-source-path.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/channel-unix-source-path.x86_64-latest.args
@@ -40,6 +40,14 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \
 -device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":5,"chardev":"charchannel4","id":"channel4","name":"org.qemu.guest_agent.4"}' \
 -chardev socket,id=charchannel5,fd=1729,server=on,wait=off \
 -device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":6,"chardev":"charchannel5","id":"channel5","name":"org.qemu.guest_agent.5"}' \
+-chardev socket,id=charchannel6,fd=1729,server=on,wait=off \
+-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":7,"chardev":"charchannel6","id":"channel6","name":"org.qemu.guest_agent.6"}' \
+-chardev socket,id=charchannel7,fd=1729,server=on,wait=off \
+-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":8,"chardev":"charchannel7","id":"channel7","name":"org.qemu.guest_agent.7"}' \
+-chardev socket,id=charchannel8,fd=1729,server=on,wait=off \
+-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":9,"chardev":"charchannel8","id":"channel8","name":"org.qemu.guest_agent.8"}' \
+-chardev socket,id=charchannel9,fd=1729,server=on,wait=off \
+-device '{"driver":"virtserialport","bus":"virtio-serial0.0","nr":10,"chardev":"charchannel9","id":"channel9","name":"org.qemu.guest_agent.9"}' \
 -audiodev '{"id":"audio1","driver":"none"}' \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxml2argvdata/channel-unix-source-path.xml b/tests/qemuxml2argvdata/channel-unix-source-path.xml
index df548d88e7..265971eb94 100644
--- a/tests/qemuxml2argvdata/channel-unix-source-path.xml
+++ b/tests/qemuxml2argvdata/channel-unix-source-path.xml
@@ -32,6 +32,22 @@
       <source mode='bind' path='/var/run/libvirt/qemu/channel/1-QEMUGuest1/org.qemu.guest_agent.5'/>
       <target type='virtio' name='org.qemu.guest_agent.5'/>
     </channel>
+    <channel type="unix">
+      <source mode="bind" path="/1-QEMUGuest1/org.qemu.guest_agent.6"/>
+      <target type="virtio" name="org.qemu.guest_agent.6" state="connected"/>
+    </channel>
+    <channel type="unix">
+      <source mode="bind" path="/tmp/1-QEMUGuest1/org.qemu.guest_agent.7"/>
+      <target type="virtio" name="org.qemu.guest_agent.7" state="connected"/>
+    </channel>
+    <channel type="unix">
+      <source mode="bind" path="/tmp/libvirt/qemu/channel/1-QEMUGuest1/org.qemu.guest_agent.8"/>
+      <target type="virtio" name="org.qemu.guest_agent.8" state="connected"/>
+    </channel>
+    <channel type="unix">
+      <source mode="bind" path="/tmp/org.qemu.guest_agent.9"/>
+      <target type="virtio" name="org.qemu.guest_agent.9" state="connected"/>
+    </channel>
     <memballoon model='none'/>
   </devices>
 </domain>
diff --git a/tests/qemuxml2xmloutdata/channel-unix-source-path-active.x86_64-latest.xml b/tests/qemuxml2xmloutdata/channel-unix-source-path-active.x86_64-latest.xml
index 022ad7025a..12a981438c 100644
--- a/tests/qemuxml2xmloutdata/channel-unix-source-path-active.x86_64-latest.xml
+++ b/tests/qemuxml2xmloutdata/channel-unix-source-path-active.x86_64-latest.xml
@@ -54,6 +54,26 @@
       <target type='virtio' name='org.qemu.guest_agent.5'/>
       <address type='virtio-serial' controller='0' bus='0' port='6'/>
     </channel>
+    <channel type='unix'>
+      <source mode='bind' path='/1-QEMUGuest1/org.qemu.guest_agent.6'/>
+      <target type='virtio' name='org.qemu.guest_agent.6'/>
+      <address type='virtio-serial' controller='0' bus='0' port='7'/>
+    </channel>
+    <channel type='unix'>
+      <source mode='bind' path='/tmp/1-QEMUGuest1/org.qemu.guest_agent.7'/>
+      <target type='virtio' name='org.qemu.guest_agent.7'/>
+      <address type='virtio-serial' controller='0' bus='0' port='8'/>
+    </channel>
+    <channel type='unix'>
+      <source mode='bind' path='/tmp/libvirt/qemu/channel/1-QEMUGuest1/org.qemu.guest_agent.8'/>
+      <target type='virtio' name='org.qemu.guest_agent.8'/>
+      <address type='virtio-serial' controller='0' bus='0' port='9'/>
+    </channel>
+    <channel type='unix'>
+      <source mode='bind' path='/tmp/org.qemu.guest_agent.9'/>
+      <target type='virtio' name='org.qemu.guest_agent.9'/>
+      <address type='virtio-serial' controller='0' bus='0' port='10'/>
+    </channel>
     <input type='mouse' bus='ps2'/>
     <input type='keyboard' bus='ps2'/>
     <audio id='1' type='none'/>
diff --git a/tests/qemuxml2xmloutdata/channel-unix-source-path-inactive.x86_64-latest.xml b/tests/qemuxml2xmloutdata/channel-unix-source-path-inactive.x86_64-latest.xml
index 3b013e9f87..2315a572e4 100644
--- a/tests/qemuxml2xmloutdata/channel-unix-source-path-inactive.x86_64-latest.xml
+++ b/tests/qemuxml2xmloutdata/channel-unix-source-path-inactive.x86_64-latest.xml
@@ -49,6 +49,26 @@
       <target type='virtio' name='org.qemu.guest_agent.5'/>
       <address type='virtio-serial' controller='0' bus='0' port='6'/>
     </channel>
+    <channel type='unix'>
+      <source mode='bind' path='/1-QEMUGuest1/org.qemu.guest_agent.6'/>
+      <target type='virtio' name='org.qemu.guest_agent.6'/>
+      <address type='virtio-serial' controller='0' bus='0' port='7'/>
+    </channel>
+    <channel type='unix'>
+      <source mode='bind' path='/tmp/1-QEMUGuest1/org.qemu.guest_agent.7'/>
+      <target type='virtio' name='org.qemu.guest_agent.7'/>
+      <address type='virtio-serial' controller='0' bus='0' port='8'/>
+    </channel>
+    <channel type='unix'>
+      <source mode='bind' path='/tmp/libvirt/qemu/channel/1-QEMUGuest1/org.qemu.guest_agent.8'/>
+      <target type='virtio' name='org.qemu.guest_agent.8'/>
+      <address type='virtio-serial' controller='0' bus='0' port='9'/>
+    </channel>
+    <channel type='unix'>
+      <source mode='bind' path='/tmp/org.qemu.guest_agent.9'/>
+      <target type='virtio' name='org.qemu.guest_agent.9'/>
+      <address type='virtio-serial' controller='0' bus='0' port='10'/>
+    </channel>
     <input type='mouse' bus='ps2'/>
     <input type='keyboard' bus='ps2'/>
     <audio id='1' type='none'/>
-- 
2.41.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH] qemu: Be less aggressive when dropping channel source paths
Posted by Andrea Bolognani 3 months, 2 weeks ago
On Tue, Jan 09, 2024 at 03:28:44PM +0100, Michal Privoznik wrote:
> In v9.7.0-rc1~130 I've shortened the path that's generated for
> <channel/> source. With that, I had to adjust regex that matches
> all versions of paths we have ever generated so that we can drop
> them (see comment around qemuDomainChrDefDropDefaultPath()). But
> as it is usually the case with regexes - they are write only.

Sounds like a skill issue to be honest :P

> And
> while I attempted to make one portion of the path optional
> ("/target/") I accidentally made regex accept more, which
> resulted in libvirt dropping the user provided path and
> generating our own instead.
>
> Fixes: d3759d3674ab9453e5fb5a27ab6c28b8ff8d5569
> Resolves: https://issues.redhat.com/browse/RHEL-20807
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_domain.c                        |  2 +-
>  ...hannel-unix-source-path.x86_64-latest.args |  8 ++++++++
>  .../channel-unix-source-path.xml              | 16 +++++++++++++++
>  ...-unix-source-path-active.x86_64-latest.xml | 20 +++++++++++++++++++
>  ...nix-source-path-inactive.x86_64-latest.xml | 20 +++++++++++++++++++
>  5 files changed, 65 insertions(+), 1 deletion(-)

Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org