Re: [PATCH 4/8] qemu: add a DBus daemon helper unit

2020-02-24 Thread Marc-André Lureau 
Hi

On Thu, Feb 20, 2020 at 10:04 AM Michal Privoznik  wrote:
>
> On 1/14/20 2:46 PM, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau 
> >
> > Add a unit to start & stop a private dbus-daemon.
> >
> > The daemon is meant to be started on demand, and associated with a
> > QEMU process. It should be stopped when the QEMU process is stopped.
> >
> > The current policy is permissive like a session bus. Stricter
> > policies can be added later, following recommendations from:
> > https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus.rst
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >   src/qemu/Makefile.inc.am |   4 +
> >   src/qemu/qemu_dbus.c | 299 +++
> >   src/qemu/qemu_dbus.h |  40 ++
> >   src/qemu/qemu_domain.c   |   2 +
> >   src/qemu/qemu_domain.h   |   3 +
> >   tests/Makefile.am|   1 +
> >   6 files changed, 349 insertions(+)
> >   create mode 100644 src/qemu/qemu_dbus.c
> >   create mode 100644 src/qemu/qemu_dbus.h
> >
> > diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
> > index 028ab9043c..833638ec3c 100644
> > --- a/src/qemu/Makefile.inc.am
> > +++ b/src/qemu/Makefile.inc.am
> > @@ -69,6 +69,8 @@ QEMU_DRIVER_SOURCES = \
> >   qemu/qemu_checkpoint.h \
> >   qemu/qemu_backup.c \
> >   qemu/qemu_backup.h \
> > + qemu/qemu_dbus.c \
> > + qemu/qemu_dbus.h \
>
> These can be added where they were just a moment ago ;-)
>

yep

> >   $(NULL)
> >
> >
> > @@ -93,6 +95,7 @@ libvirt_driver_qemu_impl_la_CFLAGS = \
> >   $(LIBNL_CFLAGS) \
> >   $(SELINUX_CFLAGS) \
> >   $(XDR_CFLAGS) \
> > + $(DBUS_CFLAGS) \
> >   -I$(srcdir)/access \
> >   -I$(builddir)/access \
> >   -I$(srcdir)/conf \
> > @@ -105,6 +108,7 @@ libvirt_driver_qemu_impl_la_LIBADD = \
> >   $(GNUTLS_LIBS) \
> >   $(LIBNL_LIBS) \
> >   $(SELINUX_LIBS) \
> > + $(DBUS_LIBS) \
> >   $(LIBXML_LIBS) \
> >   $(NULL)
> >   libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES)
> > diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
> > new file mode 100644
> > index 00..9c8a03c947
> > --- /dev/null
> > +++ b/src/qemu/qemu_dbus.c
> > @@ -0,0 +1,299 @@
> > +/*
> > + * qemu_dbus.c: QEMU dbus daemon
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library.  If not, see
> > + * .
> > + */
> > +
> > +#include 
> > +
> > +#include "qemu_extdevice.h"
> > +#include "qemu_dbus.h"
> > +#include "qemu_security.h"
> > +
> > +#include "viralloc.h"
> > +#include "virlog.h"
> > +#include "virstring.h"
> > +#include "virtime.h"
> > +#include "virpidfile.h"
> > +
> > +#define VIR_FROM_THIS VIR_FROM_NONE
> > +
> > +VIR_LOG_INIT("qemu.dbus");
> > +
> > +
> > +int
> > +qemuDBusPrepareHost(virQEMUDriverPtr driver)
> > +{
> > +virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> > +
> > +return virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
> > +VIR_DIR_CREATE_ALLOW_EXIST);
> > +}
> > +
> > +
> > +static char *
> > +qemuDBusCreatePidFilename(const char *stateDir,
> > +  const char *shortName)
> > +{
> > +g_autofree char *name = g_strdup_printf("%s-dbus", shortName);
> > +
> > +return virPidFileBuildPath(stateDir, name);
>
> Instead of having each caller pass cfg->dbusStateDir, we can accept cfg
> here and deref ourselves.

sure

>
> > +}
> > +
> > +
> > +static char *
> > +qemuDBusCreateFilename(const char *stateDir,
> > +   const char *shortName,
> > +   const char *ext)
> > +{
> > +g_autofree char *name = g_strdup_printf("%s-dbus", shortName);
> > +
> > +return virFileBuildPath(stateDir, name,  ext);
> > +}
> > +
> > +
> > +static char *
> > +qemuDBusCreateSocketPath(virQEMUDriverConfigPtr cfg,
> > + const char *shortName)
> > +{
> > +return qemuDBusCreateFilename(cfg->dbusStateDir, shortName, ".sock");
> > +}
> > +
>
> I'd introduce qemuDBusCreateConfPath() so that nobody else has to call
> qemuDBusCreateFilename() again.

ok

>
> > +
> > +char *
> > +qemuDBusGetAddress(virQEMUDriverPtr driver,
> > +   virDomainObjPtr vm)
> > +{
> > +virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> > +g_autofree char *shortName =

Re: [PATCH 4/8] qemu: add a DBus daemon helper unit

2020-02-20 Thread Michal Privoznik 

On 1/14/20 2:46 PM, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

Add a unit to start & stop a private dbus-daemon.

The daemon is meant to be started on demand, and associated with a
QEMU process. It should be stopped when the QEMU process is stopped.

The current policy is permissive like a session bus. Stricter
policies can be added later, following recommendations from:
https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus.rst

Signed-off-by: Marc-André Lureau 
---
  src/qemu/Makefile.inc.am |   4 +
  src/qemu/qemu_dbus.c | 299 +++
  src/qemu/qemu_dbus.h |  40 ++
  src/qemu/qemu_domain.c   |   2 +
  src/qemu/qemu_domain.h   |   3 +
  tests/Makefile.am|   1 +
  6 files changed, 349 insertions(+)
  create mode 100644 src/qemu/qemu_dbus.c
  create mode 100644 src/qemu/qemu_dbus.h

diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index 028ab9043c..833638ec3c 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -69,6 +69,8 @@ QEMU_DRIVER_SOURCES = \
qemu/qemu_checkpoint.h \
qemu/qemu_backup.c \
qemu/qemu_backup.h \
+   qemu/qemu_dbus.c \
+   qemu/qemu_dbus.h \


These can be added where they were just a moment ago ;-)


$(NULL)
  
  
@@ -93,6 +95,7 @@ libvirt_driver_qemu_impl_la_CFLAGS = \

$(LIBNL_CFLAGS) \
$(SELINUX_CFLAGS) \
$(XDR_CFLAGS) \
+   $(DBUS_CFLAGS) \
-I$(srcdir)/access \
-I$(builddir)/access \
-I$(srcdir)/conf \
@@ -105,6 +108,7 @@ libvirt_driver_qemu_impl_la_LIBADD = \
$(GNUTLS_LIBS) \
$(LIBNL_LIBS) \
$(SELINUX_LIBS) \
+   $(DBUS_LIBS) \
$(LIBXML_LIBS) \
$(NULL)
  libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES)
diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
new file mode 100644
index 00..9c8a03c947
--- /dev/null
+++ b/src/qemu/qemu_dbus.c
@@ -0,0 +1,299 @@
+/*
+ * qemu_dbus.c: QEMU dbus daemon
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ */
+
+#include 
+
+#include "qemu_extdevice.h"
+#include "qemu_dbus.h"
+#include "qemu_security.h"
+
+#include "viralloc.h"
+#include "virlog.h"
+#include "virstring.h"
+#include "virtime.h"
+#include "virpidfile.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+VIR_LOG_INIT("qemu.dbus");
+
+
+int
+qemuDBusPrepareHost(virQEMUDriverPtr driver)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+
+return virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
+VIR_DIR_CREATE_ALLOW_EXIST);
+}
+
+
+static char *
+qemuDBusCreatePidFilename(const char *stateDir,
+  const char *shortName)
+{
+g_autofree char *name = g_strdup_printf("%s-dbus", shortName);
+
+return virPidFileBuildPath(stateDir, name);


Instead of having each caller pass cfg->dbusStateDir, we can accept cfg 
here and deref ourselves.



+}
+
+
+static char *
+qemuDBusCreateFilename(const char *stateDir,
+   const char *shortName,
+   const char *ext)
+{
+g_autofree char *name = g_strdup_printf("%s-dbus", shortName);
+
+return virFileBuildPath(stateDir, name,  ext);
+}
+
+
+static char *
+qemuDBusCreateSocketPath(virQEMUDriverConfigPtr cfg,
+ const char *shortName)
+{
+return qemuDBusCreateFilename(cfg->dbusStateDir, shortName, ".sock");
+}
+


I'd introduce qemuDBusCreateConfPath() so that nobody else has to call 
qemuDBusCreateFilename() again.



+
+char *
+qemuDBusGetAddress(virQEMUDriverPtr driver,
+   virDomainObjPtr vm)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+g_autofree char *shortName = virDomainDefGetShortName(vm->def);
+g_autofree char *path = qemuDBusCreateSocketPath(cfg, shortName);
+
+return g_strdup_printf("unix:path=%s", path);
+}
+
+
+static int
+qemuDBusGetPid(const char *binPath,
+   const char *stateDir,
+   const char *shortName,
+   pid_t *pid)
+{
+g_autofree char *pidfile = qemuDBusCreatePidFilename(stateDir, shortName);
+
+return virPidFileReadPathIfAlive(pidfile, pid, binPath);
+}


After the daemon startup is reworked, this function is no longer needed.


+
+
+static int
+qemuDBusWriteConfig(const char *filename,

[libvirt] [PATCH 4/8] qemu: add a DBus daemon helper unit

2020-01-14 Thread marcandre . lureau 
From: Marc-André Lureau 

Add a unit to start & stop a private dbus-daemon.

The daemon is meant to be started on demand, and associated with a
QEMU process. It should be stopped when the QEMU process is stopped.

The current policy is permissive like a session bus. Stricter
policies can be added later, following recommendations from:
https://git.qemu.org/?p=qemu.git;a=blob;f=docs/interop/dbus.rst

Signed-off-by: Marc-André Lureau 
---
 src/qemu/Makefile.inc.am |   4 +
 src/qemu/qemu_dbus.c | 299 +++
 src/qemu/qemu_dbus.h |  40 ++
 src/qemu/qemu_domain.c   |   2 +
 src/qemu/qemu_domain.h   |   3 +
 tests/Makefile.am|   1 +
 6 files changed, 349 insertions(+)
 create mode 100644 src/qemu/qemu_dbus.c
 create mode 100644 src/qemu/qemu_dbus.h

diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index 028ab9043c..833638ec3c 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -69,6 +69,8 @@ QEMU_DRIVER_SOURCES = \
qemu/qemu_checkpoint.h \
qemu/qemu_backup.c \
qemu/qemu_backup.h \
+   qemu/qemu_dbus.c \
+   qemu/qemu_dbus.h \
$(NULL)
 
 
@@ -93,6 +95,7 @@ libvirt_driver_qemu_impl_la_CFLAGS = \
$(LIBNL_CFLAGS) \
$(SELINUX_CFLAGS) \
$(XDR_CFLAGS) \
+   $(DBUS_CFLAGS) \
-I$(srcdir)/access \
-I$(builddir)/access \
-I$(srcdir)/conf \
@@ -105,6 +108,7 @@ libvirt_driver_qemu_impl_la_LIBADD = \
$(GNUTLS_LIBS) \
$(LIBNL_LIBS) \
$(SELINUX_LIBS) \
+   $(DBUS_LIBS) \
$(LIBXML_LIBS) \
$(NULL)
 libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES)
diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
new file mode 100644
index 00..9c8a03c947
--- /dev/null
+++ b/src/qemu/qemu_dbus.c
@@ -0,0 +1,299 @@
+/*
+ * qemu_dbus.c: QEMU dbus daemon
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ */
+
+#include 
+
+#include "qemu_extdevice.h"
+#include "qemu_dbus.h"
+#include "qemu_security.h"
+
+#include "viralloc.h"
+#include "virlog.h"
+#include "virstring.h"
+#include "virtime.h"
+#include "virpidfile.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+VIR_LOG_INIT("qemu.dbus");
+
+
+int
+qemuDBusPrepareHost(virQEMUDriverPtr driver)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+
+return virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
+VIR_DIR_CREATE_ALLOW_EXIST);
+}
+
+
+static char *
+qemuDBusCreatePidFilename(const char *stateDir,
+  const char *shortName)
+{
+g_autofree char *name = g_strdup_printf("%s-dbus", shortName);
+
+return virPidFileBuildPath(stateDir, name);
+}
+
+
+static char *
+qemuDBusCreateFilename(const char *stateDir,
+   const char *shortName,
+   const char *ext)
+{
+g_autofree char *name = g_strdup_printf("%s-dbus", shortName);
+
+return virFileBuildPath(stateDir, name,  ext);
+}
+
+
+static char *
+qemuDBusCreateSocketPath(virQEMUDriverConfigPtr cfg,
+ const char *shortName)
+{
+return qemuDBusCreateFilename(cfg->dbusStateDir, shortName, ".sock");
+}
+
+
+char *
+qemuDBusGetAddress(virQEMUDriverPtr driver,
+   virDomainObjPtr vm)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+g_autofree char *shortName = virDomainDefGetShortName(vm->def);
+g_autofree char *path = qemuDBusCreateSocketPath(cfg, shortName);
+
+return g_strdup_printf("unix:path=%s", path);
+}
+
+
+static int
+qemuDBusGetPid(const char *binPath,
+   const char *stateDir,
+   const char *shortName,
+   pid_t *pid)
+{
+g_autofree char *pidfile = qemuDBusCreatePidFilename(stateDir, shortName);
+
+return virPidFileReadPathIfAlive(pidfile, pid, binPath);
+}
+
+
+static int
+qemuDBusWriteConfig(const char *filename, const char *path)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+g_autofree char *config = NULL;
+
+virBufferAddLit(, "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd\;>\n");
+virBufferAddLit(, "\n");
+virBufferAdjustIndent(, 2);
+
+virBufferAddLit(, "org.libvirt.qemu\n");
+virBufferAsprintf(, "unix:path=%s\n", path);
+virBufferAddLit(, "EXTERNAL\n");
+
+virBufferAddLit(,