[PATCH 13/19] qemu/dbus: keep a connection to the VM D-Bus

marcandre.lureau@redhat.com posted 19 patches 7 months, 1 week ago
There is a newer version of this series
[PATCH 13/19] qemu/dbus: keep a connection to the VM D-Bus
Posted by marcandre.lureau@redhat.com 7 months, 1 week ago
From: Marc-André Lureau <marcandre.lureau@redhat.com>

The following changes are going to communicate with the qemu-rdp server
through the VM D-Bus bus, keep a connection for that and further usage.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 src/qemu/qemu_dbus.c   | 22 +++++++++++++++++++---
 src/qemu/qemu_domain.h |  2 ++
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
index 06b655d870..4a019ae092 100644
--- a/src/qemu/qemu_dbus.c
+++ b/src/qemu/qemu_dbus.c
@@ -85,7 +85,7 @@ qemuDBusGetAddress(virQEMUDriver *driver,
 
 
 static int
-qemuDBusWriteConfig(const char *filename, const char *path)
+qemuDBusWriteConfig(const char *filename, const char *address)
 {
     g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
     g_autofree char *config = NULL;
@@ -96,7 +96,7 @@ qemuDBusWriteConfig(const char *filename, const char *path)
     virBufferAdjustIndent(&buf, 2);
 
     virBufferAddLit(&buf, "<type>org.libvirt.qemu</type>\n");
-    virBufferAsprintf(&buf, "<listen>unix:path=%s</listen>\n", path);
+    virBufferAsprintf(&buf, "<listen>%s</listen>\n", address);
     virBufferAddLit(&buf, "<auth>EXTERNAL</auth>\n");
 
     virBufferAddLit(&buf, "<policy context='default'>\n");
@@ -140,6 +140,8 @@ qemuDBusStop(virQEMUDriver *driver,
     } else {
         priv->dbusDaemonRunning = false;
     }
+
+    g_clear_object(&priv->dbusConnection);
 }
 
 
@@ -175,11 +177,13 @@ qemuDBusStart(virQEMUDriver *driver,
     g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
     qemuDomainObjPrivate *priv = vm->privateData;
     g_autoptr(virCommand) cmd = NULL;
+    g_autoptr(GError) gerr = NULL;
     g_autofree char *dbusDaemonPath = NULL;
     g_autofree char *shortName = NULL;
     g_autofree char *pidfile = NULL;
     g_autofree char *configfile = NULL;
     g_autofree char *sockpath = NULL;
+    g_autofree char *address = NULL;
     virTimeBackOffVar timebackoff;
     const unsigned long long timeout = 500 * 1000; /* ms */
     VIR_AUTOCLOSE errfd = -1;
@@ -205,8 +209,9 @@ qemuDBusStart(virQEMUDriver *driver,
     pidfile = qemuDBusCreatePidFilename(cfg, shortName);
     configfile = qemuDBusCreateConfPath(cfg, shortName);
     sockpath = qemuDBusCreateSocketPath(cfg, shortName);
+    address = g_strdup_printf("unix:path=%s", sockpath);
 
-    if (qemuDBusWriteConfig(configfile, sockpath) < 0) {
+    if (qemuDBusWriteConfig(configfile, address) < 0) {
         virReportSystemError(errno, _("Failed to write '%1$s'"), configfile);
         return -1;
     }
@@ -264,6 +269,17 @@ qemuDBusStart(virQEMUDriver *driver,
     if (qemuSecurityDomainSetPathLabel(driver, vm, sockpath, false) < 0)
         goto cleanup;
 
+    priv->dbusConnection =
+        g_dbus_connection_new_for_address_sync(address,
+                                               G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT|
+                                               G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION,
+                                               NULL, NULL, &gerr);
+    if (!priv->dbusConnection) {
+        virReportError(VIR_ERR_OPERATION_FAILED,
+                      _("Failed to connect to dbus-daemon: %1$s"), gerr->message);
+        goto cleanup;
+    }
+
     priv->dbusDaemonRunning = true;
     ret = 0;
  cleanup:
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 04577f1297..03cf84695f 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -39,6 +39,7 @@
 #include "qemu_fd.h"
 #include "virchrdev.h"
 #include "virobject.h"
+#include "virgdbus.h"
 #include "virdomainmomentobjlist.h"
 #include "virenum.h"
 #include "vireventthread.h"
@@ -240,6 +241,7 @@ struct _qemuDomainObjPrivate {
     /* running backup job */
     virDomainBackupDef *backup;
 
+    GDBusConnection *dbusConnection;
     bool dbusDaemonRunning;
 
     /* list of Ids to migrate */
-- 
2.47.0
Re: [PATCH 13/19] qemu/dbus: keep a connection to the VM D-Bus
Posted by Daniel P. Berrangé 6 months, 4 weeks ago
On Wed, Jan 29, 2025 at 05:40:35PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> The following changes are going to communicate with the qemu-rdp server
> through the VM D-Bus bus, keep a connection for that and further usage.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  src/qemu/qemu_dbus.c   | 22 +++++++++++++++++++---
>  src/qemu/qemu_domain.h |  2 ++
>  2 files changed, 21 insertions(+), 3 deletions(-)


> 
> diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
> index 06b655d870..4a019ae092 100644
> --- a/src/qemu/qemu_dbus.c
> +++ b/src/qemu/qemu_dbus.c

> @@ -175,11 +177,13 @@ qemuDBusStart(virQEMUDriver *driver,
>      g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
>      qemuDomainObjPrivate *priv = vm->privateData;
>      g_autoptr(virCommand) cmd = NULL;
> +    g_autoptr(GError) gerr = NULL;
>      g_autofree char *dbusDaemonPath = NULL;
>      g_autofree char *shortName = NULL;
>      g_autofree char *pidfile = NULL;
>      g_autofree char *configfile = NULL;
>      g_autofree char *sockpath = NULL;
> +    g_autofree char *address = NULL;
>      virTimeBackOffVar timebackoff;
>      const unsigned long long timeout = 500 * 1000; /* ms */
>      VIR_AUTOCLOSE errfd = -1;
> @@ -205,8 +209,9 @@ qemuDBusStart(virQEMUDriver *driver,
>      pidfile = qemuDBusCreatePidFilename(cfg, shortName);
>      configfile = qemuDBusCreateConfPath(cfg, shortName);
>      sockpath = qemuDBusCreateSocketPath(cfg, shortName);
> +    address = g_strdup_printf("unix:path=%s", sockpath);
>  
> -    if (qemuDBusWriteConfig(configfile, sockpath) < 0) {
> +    if (qemuDBusWriteConfig(configfile, address) < 0) {
>          virReportSystemError(errno, _("Failed to write '%1$s'"), configfile);
>          return -1;
>      }
> @@ -264,6 +269,17 @@ qemuDBusStart(virQEMUDriver *driver,
>      if (qemuSecurityDomainSetPathLabel(driver, vm, sockpath, false) < 0)
>          goto cleanup;
>  
> +    priv->dbusConnection =
> +        g_dbus_connection_new_for_address_sync(address,
> +                                               G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT|
> +                                               G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION,
> +                                               NULL, NULL, &gerr);

We have just started dbus-daemon above here. We've proved the PID is running
but can you confirm this guarantees the listener socket is accepting
connections... that we're just about to establish.

> +    if (!priv->dbusConnection) {
> +        virReportError(VIR_ERR_OPERATION_FAILED,
> +                      _("Failed to connect to dbus-daemon: %1$s"), gerr->message);
> +        goto cleanup;
> +    }
> +
>      priv->dbusDaemonRunning = true;
>      ret = 0;
>   cleanup:

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH 13/19] qemu/dbus: keep a connection to the VM D-Bus
Posted by Marc-André Lureau 6 months, 3 weeks ago
Hi

On Wed, Feb 12, 2025 at 8:16 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Wed, Jan 29, 2025 at 05:40:35PM +0400, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > The following changes are going to communicate with the qemu-rdp server
> > through the VM D-Bus bus, keep a connection for that and further usage.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  src/qemu/qemu_dbus.c   | 22 +++++++++++++++++++---
> >  src/qemu/qemu_domain.h |  2 ++
> >  2 files changed, 21 insertions(+), 3 deletions(-)
>
>
> >
> > diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
> > index 06b655d870..4a019ae092 100644
> > --- a/src/qemu/qemu_dbus.c
> > +++ b/src/qemu/qemu_dbus.c
>
> > @@ -175,11 +177,13 @@ qemuDBusStart(virQEMUDriver *driver,
> >      g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
> >      qemuDomainObjPrivate *priv = vm->privateData;
> >      g_autoptr(virCommand) cmd = NULL;
> > +    g_autoptr(GError) gerr = NULL;
> >      g_autofree char *dbusDaemonPath = NULL;
> >      g_autofree char *shortName = NULL;
> >      g_autofree char *pidfile = NULL;
> >      g_autofree char *configfile = NULL;
> >      g_autofree char *sockpath = NULL;
> > +    g_autofree char *address = NULL;
> >      virTimeBackOffVar timebackoff;
> >      const unsigned long long timeout = 500 * 1000; /* ms */
> >      VIR_AUTOCLOSE errfd = -1;
> > @@ -205,8 +209,9 @@ qemuDBusStart(virQEMUDriver *driver,
> >      pidfile = qemuDBusCreatePidFilename(cfg, shortName);
> >      configfile = qemuDBusCreateConfPath(cfg, shortName);
> >      sockpath = qemuDBusCreateSocketPath(cfg, shortName);
> > +    address = g_strdup_printf("unix:path=%s", sockpath);
> >
> > -    if (qemuDBusWriteConfig(configfile, sockpath) < 0) {
> > +    if (qemuDBusWriteConfig(configfile, address) < 0) {
> >          virReportSystemError(errno, _("Failed to write '%1$s'"), configfile);
> >          return -1;
> >      }
> > @@ -264,6 +269,17 @@ qemuDBusStart(virQEMUDriver *driver,
> >      if (qemuSecurityDomainSetPathLabel(driver, vm, sockpath, false) < 0)
> >          goto cleanup;
> >
> > +    priv->dbusConnection =
> > +        g_dbus_connection_new_for_address_sync(address,
> > +                                               G_DBUS_CONNECTION_FLAGS_AUTHENTICATION_CLIENT|
> > +                                               G_DBUS_CONNECTION_FLAGS_MESSAGE_BUS_CONNECTION,
> > +                                               NULL, NULL, &gerr);
>
> We have just started dbus-daemon above here. We've proved the PID is running
> but can you confirm this guarantees the listener socket is accepting
> connections... that we're just about to establish.

Just above, we wait & check for the existence of the socket path.

>
> > +    if (!priv->dbusConnection) {
> > +        virReportError(VIR_ERR_OPERATION_FAILED,
> > +                      _("Failed to connect to dbus-daemon: %1$s"), gerr->message);
> > +        goto cleanup;
> > +    }
> > +
> >      priv->dbusDaemonRunning = true;
> >      ret = 0;
> >   cleanup:
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>