[libvirt PATCH v2] rpc: don't try to spawn non-existant daemon

Daniel P. Berrangé posted 1 patch 5 months, 4 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20231101120155.3179397-1-berrange@redhat.com
src/rpc/virnetsocket.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
[libvirt PATCH v2] rpc: don't try to spawn non-existant daemon
Posted by Daniel P. Berrangé 5 months, 4 weeks ago
If libvirt is built in client only mode, the libvirtd/virtqemud/etc
daemons won't exist. If the client is told to connect to a local
hypervisor, it'll see the socket doesn't exist, try to spawn the
daemon and then re-try connecting to the socket for a few seconds.
Ultimately this will fail because the daemon doesn't exist and the
user gets an error message

  error: Failed to connect socket to '/run/user/1000/libvirt/virtqemud-sock': No such file or directory

technically this is accurate, but it doesn't help identify the root
cause. With this change it will now report

  error: binary 'virtqemud' does not exist in $PATH: No such file or directory

and will skip all the socket connect retries

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---

Last time it was suggested that virCommandGetBinaryPath could be
extended to always check whether the binary exists. I started
doing that and realized it was a bad idea as this method runs in
a context which might not have permission to access to the binary
we are about to run, as we've not changed user/group ID yet. So
I'm re-posting this targetted fix, with format string fixup.

 src/rpc/virnetsocket.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index b58f7a6b8f..0215c99c73 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -123,9 +123,19 @@ VIR_ONCE_GLOBAL_INIT(virNetSocket);
 #ifndef WIN32
 static int virNetSocketForkDaemon(const char *binary)
 {
-    g_autoptr(virCommand) cmd = virCommandNewArgList(binary,
-                                                     "--timeout=120",
-                                                     NULL);
+    g_autofree char *binarypath = virFindFileInPath(binary);
+    g_autoptr(virCommand) cmd = NULL;
+
+    if (!binarypath) {
+        virReportSystemError(ENOENT,
+                             _("binary '%1$s' does not exist in $PATH"),
+                             binary);
+        return -1;
+    }
+
+    cmd = virCommandNewArgList(binarypath,
+                               "--timeout=120",
+                               NULL);
 
     virCommandAddEnvPassCommon(cmd);
     virCommandAddEnvPass(cmd, "XDG_CACHE_HOME");
-- 
2.41.0
Re: [libvirt PATCH v2] rpc: don't try to spawn non-existant daemon
Posted by Michal Prívozník 4 months, 3 weeks ago
On 11/1/23 13:01, Daniel P. Berrangé wrote:
> If libvirt is built in client only mode, the libvirtd/virtqemud/etc
> daemons won't exist. If the client is told to connect to a local
> hypervisor, it'll see the socket doesn't exist, try to spawn the
> daemon and then re-try connecting to the socket for a few seconds.
> Ultimately this will fail because the daemon doesn't exist and the
> user gets an error message
> 
>   error: Failed to connect socket to '/run/user/1000/libvirt/virtqemud-sock': No such file or directory
> 
> technically this is accurate, but it doesn't help identify the root
> cause. With this change it will now report
> 
>   error: binary 'virtqemud' does not exist in $PATH: No such file or directory
> 
> and will skip all the socket connect retries
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> 
> Last time it was suggested that virCommandGetBinaryPath could be
> extended to always check whether the binary exists. I started
> doing that and realized it was a bad idea as this method runs in
> a context which might not have permission to access to the binary
> we are about to run, as we've not changed user/group ID yet. So
> I'm re-posting this targetted fix, with format string fixup.
> 
>  src/rpc/virnetsocket.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org