[PATCH v2 2/2] qemu: Move channelTargetDir into stateDir

Michal Privoznik mprivozn at redhat.com
Mon Jul 24 10:11:37 UTC 2023


For historical reasons (i.e. unknown reason) we put channel
sockets into a path derived from cfg->libDir which is a path that
survives host reboots (e.g. /var/lib/libvirt/...). This is not
necessary and in fact for session daemon creates a longer prefix:

  XDG_CONFIG_HOME -> /home/user/.config
  XDG_RUNTIME_DIR -> /run/user/1000

Worse, if host is rebooted suddenly (e.g. due to power loss) then
we leave files behind and nobody will ever remove them.

Therefore, place the channel target dir into state dir.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2173980
Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 libvirt.spec.in                               |  1 -
 src/qemu/qemu_conf.c                          |  9 ++--
 src/qemu/qemu_domain.c                        | 52 +++++++++++++++++--
 .../qemuhotplug-qemu-agent-detach.xml         |  2 +-
 ...emuhotplug-base-live+qemu-agent-detach.xml |  2 +-
 .../qemuhotplug-base-live+qemu-agent.xml      |  2 +-
 .../channel-unix-source-path.xml              |  4 ++
 .../channel-unix-source-path-active.xml       |  5 ++
 .../channel-unix-source-path-inactive.xml     |  4 ++
 tests/testutilsqemu.c                         |  2 +-
 10 files changed, 67 insertions(+), 16 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8aee709e42..1f283cec6d 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -2124,7 +2124,6 @@ exit 0
 %ghost %dir %{_rundir}/libvirt/qemu/slirp/
 %ghost %dir %{_rundir}/libvirt/qemu/swtpm/
 %dir %attr(0751, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/
-%dir %attr(0751, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/channel/
 %dir %attr(0751, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/checkpoint/
 %dir %attr(0751, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/dump/
 %dir %attr(0751, %{qemu_user}, %{qemu_group}) %{_localstatedir}/lib/libvirt/qemu/nvram/
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 532fe36be3..3f811d064f 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -143,6 +143,7 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
         cfg->configBaseDir = g_strdup_printf("%s/etc", root);
         cfg->stateDir = g_strdup_printf("%s/run/qemu", root);
         cfg->swtpmStateDir = g_strdup_printf("%s/run/swtpm", root);
+        cfg->channelTargetDir = g_strdup_printf("%s/channel", cfg->stateDir);
         cfg->cacheDir = g_strdup_printf("%s/cache/qemu", root);
         cfg->libDir = g_strdup_printf("%s/lib/qemu", root);
         cfg->swtpmStorageDir = g_strdup_printf("%s/lib/swtpm", root);
@@ -151,7 +152,6 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
         cfg->snapshotDir = g_strdup_printf("%s/snapshot", cfg->libDir);
         cfg->checkpointDir = g_strdup_printf("%s/checkpoint", cfg->libDir);
         cfg->autoDumpPath = g_strdup_printf("%s/dump", cfg->libDir);
-        cfg->channelTargetDir = g_strdup_printf("%s/channel", cfg->libDir);
         cfg->nvramDir = g_strdup_printf("%s/nvram", cfg->libDir);
         cfg->memoryBackingDir = g_strdup_printf("%s/ram", cfg->libDir);
     } else if (privileged) {
@@ -163,8 +163,8 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
         cfg->configBaseDir = g_strdup(SYSCONFDIR "/libvirt");
 
         cfg->stateDir = g_strdup_printf("%s/libvirt/qemu", RUNSTATEDIR);
-
         cfg->swtpmStateDir = g_strdup_printf("%s/libvirt/qemu/swtpm", RUNSTATEDIR);
+        cfg->channelTargetDir = g_strdup_printf("%s/channel", cfg->stateDir);
 
         cfg->cacheDir = g_strdup_printf("%s/cache/libvirt/qemu", LOCALSTATEDIR);
 
@@ -173,7 +173,6 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
         cfg->snapshotDir = g_strdup_printf("%s/snapshot", cfg->libDir);
         cfg->checkpointDir = g_strdup_printf("%s/checkpoint", cfg->libDir);
         cfg->autoDumpPath = g_strdup_printf("%s/dump", cfg->libDir);
-        cfg->channelTargetDir = g_strdup_printf("%s/channel", cfg->libDir);
         cfg->nvramDir = g_strdup_printf("%s/nvram", cfg->libDir);
         cfg->memoryBackingDir = g_strdup_printf("%s/ram", cfg->libDir);
         cfg->swtpmStorageDir = g_strdup_printf("%s/lib/libvirt/swtpm",
@@ -190,8 +189,8 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
 
         rundir = virGetUserRuntimeDirectory();
         cfg->stateDir = g_strdup_printf("%s/qemu/run", rundir);
-
         cfg->swtpmStateDir = g_strdup_printf("%s/swtpm", cfg->stateDir);
+        cfg->channelTargetDir = g_strdup_printf("%s/channel", cfg->stateDir);
 
         cfg->configBaseDir = virGetUserConfigDirectory();
 
@@ -201,8 +200,6 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
         cfg->checkpointDir = g_strdup_printf("%s/qemu/checkpoint",
                                              cfg->configBaseDir);
         cfg->autoDumpPath = g_strdup_printf("%s/qemu/dump", cfg->configBaseDir);
-        cfg->channelTargetDir = g_strdup_printf("%s/qemu/channel",
-                                                cfg->configBaseDir);
         cfg->nvramDir = g_strdup_printf("%s/qemu/nvram", cfg->configBaseDir);
         cfg->memoryBackingDir = g_strdup_printf("%s/qemu/ram", cfg->configBaseDir);
         cfg->swtpmStorageDir = g_strdup_printf("%s/qemu/swtpm",
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index fba5185c1e..1da1356c21 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5392,6 +5392,28 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
 }
 
 
+
+static bool
+qemuDomainChrMatchDefaultPath(const char *prefix,
+                              const char *infix,
+                              const char *target,
+                              const char *path)
+{
+    g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+    g_autofree char *regexp = NULL;
+
+    virBufferEscapeRegex(&buf, "^%s", prefix);
+    if (infix)
+        virBufferEscapeRegex(&buf, "%s", infix);
+    virBufferAddLit(&buf, "/(target/)?([^/]+\\.)|(domain-[^/]+/)|([0-9]+-[^/]+/)");
+    virBufferEscapeRegex(&buf, "%s$", target);
+
+    regexp = virBufferContentAndReset(&buf);
+
+    return virStringMatch(path, regexp);
+}
+
+
 /*
  * Clear auto generated unix socket paths:
  *
@@ -5412,6 +5434,9 @@ qemuDomainDefaultNetModel(const virDomainDef *def,
  *
  * This function clears the path for migration as well, so we need to clear
  * the path even if we are not storing it in the XML.
+ *
+ * Please note, as of libvirt 9.6.0 the channelTargetDir is no longer derived
+ * from cfg->libDir but rather cfg->stateDir.
  */
 static void
 qemuDomainChrDefDropDefaultPath(virDomainChrDef *chr,
@@ -5430,14 +5455,31 @@ qemuDomainChrDefDropDefaultPath(virDomainChrDef *chr,
 
     cfg = virQEMUDriverGetConfig(driver);
 
-    virBufferEscapeRegex(&buf, "^%s", cfg->channelTargetDir);
-    virBufferAddLit(&buf, "/(target/)?([^/]+\\.)|(domain-[^/]+/)|([0-9]+-[^/]+/)");
-    virBufferEscapeRegex(&buf, "%s$", chr->target.name);
+    if (qemuDomainChrMatchDefaultPath(cfg->channelTargetDir,
+                                      NULL,
+                                      chr->target.name,
+                                      chr->source->data.nix.path)) {
+        VIR_FREE(chr->source->data.nix.path);
+        return;
+    }
 
-    regexp = virBufferContentAndReset(&buf);
+    /* Previously, channelTargetDir was derived from cfg->libdir, or
+     * cfg->configBaseDir even. Try them too. */
+    if (qemuDomainChrMatchDefaultPath(cfg->libDir,
+                                      "/channel",
+                                      chr->target.name,
+                                      chr->source->data.nix.path)) {
+        VIR_FREE(chr->source->data.nix.path);
+        return;
+    }
 
-    if (virStringMatch(chr->source->data.nix.path, regexp))
+    if (qemuDomainChrMatchDefaultPath(cfg->configBaseDir,
+                                      "/qemu/channel",
+                                      chr->target.name,
+                                      chr->source->data.nix.path)) {
         VIR_FREE(chr->source->data.nix.path);
+        return;
+    }
 }
 
 
diff --git a/tests/qemuhotplugtestdevices/qemuhotplug-qemu-agent-detach.xml b/tests/qemuhotplugtestdevices/qemuhotplug-qemu-agent-detach.xml
index 7871de59c4..dd4feae855 100644
--- a/tests/qemuhotplugtestdevices/qemuhotplug-qemu-agent-detach.xml
+++ b/tests/qemuhotplugtestdevices/qemuhotplug-qemu-agent-detach.xml
@@ -1,5 +1,5 @@
     <channel type='unix'>
-      <source mode='bind' path='/var/lib/libvirt/qemu/channel/7-hotplug/org.qemu.guest_agent.0'/>
+      <source mode='bind' path='/var/run/libvirt/qemu/channel/7-hotplug/org.qemu.guest_agent.0'/>
       <target type='virtio' name='org.qemu.guest_agent.0'/>
       <address type='virtio-serial' controller='0' bus='0' port='1'/>
     </channel>
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+qemu-agent-detach.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+qemu-agent-detach.xml
index bf2afb67d9..cfa7509485 100644
--- a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+qemu-agent-detach.xml
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+qemu-agent-detach.xml
@@ -39,7 +39,7 @@
       <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
     </controller>
     <channel type='unix'>
-      <source mode='bind' path='/var/lib/libvirt/qemu/channel/7-hotplug/org.qemu.guest_agent.0'/>
+      <source mode='bind' path='/var/run/libvirt/qemu/channel/7-hotplug/org.qemu.guest_agent.0'/>
       <target type='virtio' name='org.qemu.guest_agent.0'/>
       <alias name='channel0'/>
       <address type='virtio-serial' controller='0' bus='0' port='1'/>
diff --git a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+qemu-agent.xml b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+qemu-agent.xml
index 00191a9cb8..eb543dbaf4 100644
--- a/tests/qemuhotplugtestdomains/qemuhotplug-base-live+qemu-agent.xml
+++ b/tests/qemuhotplugtestdomains/qemuhotplug-base-live+qemu-agent.xml
@@ -42,7 +42,7 @@
       <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
     </controller>
     <channel type='unix'>
-      <source mode='bind' path='/var/lib/libvirt/qemu/channel/7-hotplug/org.qemu.guest_agent.0'/>
+      <source mode='bind' path='/var/run/libvirt/qemu/channel/7-hotplug/org.qemu.guest_agent.0'/>
       <target type='virtio' name='org.qemu.guest_agent.0'/>
       <alias name='channel0'/>
       <address type='virtio-serial' controller='0' bus='0' port='1'/>
diff --git a/tests/qemuxml2argvdata/channel-unix-source-path.xml b/tests/qemuxml2argvdata/channel-unix-source-path.xml
index db0e99c3d1..df548d88e7 100644
--- a/tests/qemuxml2argvdata/channel-unix-source-path.xml
+++ b/tests/qemuxml2argvdata/channel-unix-source-path.xml
@@ -28,6 +28,10 @@
       <source mode='bind' path='/var/lib/libvirt/qemu/channel/1-QEMUGuest1/org.qemu.guest_agent.4'/>
       <target type='virtio' name='org.qemu.guest_agent.4'/>
     </channel>
+    <channel type='unix'>
+      <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>
     <memballoon model='none'/>
   </devices>
 </domain>
diff --git a/tests/qemuxml2xmloutdata/channel-unix-source-path-active.xml b/tests/qemuxml2xmloutdata/channel-unix-source-path-active.xml
index 68835ceb15..ec927d37c6 100644
--- a/tests/qemuxml2xmloutdata/channel-unix-source-path-active.xml
+++ b/tests/qemuxml2xmloutdata/channel-unix-source-path-active.xml
@@ -46,6 +46,11 @@
       <target type='virtio' name='org.qemu.guest_agent.4'/>
       <address type='virtio-serial' controller='0' bus='0' port='5'/>
     </channel>
+    <channel type='unix'>
+      <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'/>
+      <address type='virtio-serial' controller='0' bus='0' port='6'/>
+    </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.xml b/tests/qemuxml2xmloutdata/channel-unix-source-path-inactive.xml
index 738c1184c0..c979bedb39 100644
--- a/tests/qemuxml2xmloutdata/channel-unix-source-path-inactive.xml
+++ b/tests/qemuxml2xmloutdata/channel-unix-source-path-inactive.xml
@@ -42,6 +42,10 @@
       <target type='virtio' name='org.qemu.guest_agent.4'/>
       <address type='virtio-serial' controller='0' bus='0' port='5'/>
     </channel>
+    <channel type='unix'>
+      <target type='virtio' name='org.qemu.guest_agent.5'/>
+      <address type='virtio-serial' controller='0' bus='0' port='6'/>
+    </channel>
     <input type='mouse' bus='ps2'/>
     <input type='keyboard' bus='ps2'/>
     <audio id='1' type='none'/>
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index 2d3e04de73..db911d7c90 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -460,7 +460,7 @@ int qemuTestDriverInit(virQEMUDriver *driver)
     VIR_FREE(cfg->libDir);
     cfg->libDir = g_strdup("/var/lib/libvirt/qemu");
     VIR_FREE(cfg->channelTargetDir);
-    cfg->channelTargetDir = g_strdup("/var/lib/libvirt/qemu/channel");
+    cfg->channelTargetDir = g_strdup("/var/run/libvirt/qemu/channel");
     VIR_FREE(cfg->memoryBackingDir);
     cfg->memoryBackingDir = g_strdup("/var/lib/libvirt/qemu/ram");
     VIR_FREE(cfg->nvramDir);
-- 
2.41.0



More information about the libvir-list mailing list