Re: [libvirt] [PATCH 05/12] nodedev: acquire a pidfile in the driver root directory

2019-07-10 Thread Daniel P . Berrangé
On Wed, Jul 10, 2019 at 07:02:08PM +0200, Michal Privoznik wrote:
> On 7/10/19 5:47 PM, Daniel P. Berrangé wrote:
> > When we allow multiple instances of the driver for the same user
> > account, using a separate root directory, we need to ensure mutual
> > exclusion. Use a pidfile to guarantee this.
> > 
> > In privileged libvirtd this ends up locking
> > 
> > /var/run/libvirt/nodedev/driver.pid
> > 
> > In unprivileged libvirtd this ends up locking
> > 
> >/run/user/$UID/libvirt/nodedev/run/driver.pid
> > 
> > NB, the latter can vary depending on $XDG_RUNTIME_DIR
> > 
> > Signed-off-by: Daniel P. Berrangé 
> > ---
> >   src/conf/virnodedeviceobj.h|  5 +
> >   src/node_device/node_device_hal.c  | 31 +
> >   src/node_device/node_device_udev.c | 32 ++
> >   3 files changed, 68 insertions(+)
> 
> Side note, isn't it time to finally kill hal backend? Is somebody still
> using it?

We were wanting Roman's confirmation that its no longer desired
for BSD.

https://www.redhat.com/archives/libvir-list/2019-May/msg00207.html

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 05/12] nodedev: acquire a pidfile in the driver root directory

2019-07-10 Thread Michal Privoznik

On 7/10/19 5:47 PM, Daniel P. Berrangé wrote:

When we allow multiple instances of the driver for the same user
account, using a separate root directory, we need to ensure mutual
exclusion. Use a pidfile to guarantee this.

In privileged libvirtd this ends up locking

/var/run/libvirt/nodedev/driver.pid

In unprivileged libvirtd this ends up locking

   /run/user/$UID/libvirt/nodedev/run/driver.pid

NB, the latter can vary depending on $XDG_RUNTIME_DIR

Signed-off-by: Daniel P. Berrangé 
---
  src/conf/virnodedeviceobj.h|  5 +
  src/node_device/node_device_hal.c  | 31 +
  src/node_device/node_device_udev.c | 32 ++
  3 files changed, 68 insertions(+)


Side note, isn't it time to finally kill hal backend? Is somebody still 
using it?


Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 05/12] nodedev: acquire a pidfile in the driver root directory

2019-07-10 Thread Daniel P . Berrangé
When we allow multiple instances of the driver for the same user
account, using a separate root directory, we need to ensure mutual
exclusion. Use a pidfile to guarantee this.

In privileged libvirtd this ends up locking

   /var/run/libvirt/nodedev/driver.pid

In unprivileged libvirtd this ends up locking

  /run/user/$UID/libvirt/nodedev/run/driver.pid

NB, the latter can vary depending on $XDG_RUNTIME_DIR

Signed-off-by: Daniel P. Berrangé 
---
 src/conf/virnodedeviceobj.h|  5 +
 src/node_device/node_device_hal.c  | 31 +
 src/node_device/node_device_udev.c | 32 ++
 3 files changed, 68 insertions(+)

diff --git a/src/conf/virnodedeviceobj.h b/src/conf/virnodedeviceobj.h
index 1abfcb9af4..c4d3c55d73 100644
--- a/src/conf/virnodedeviceobj.h
+++ b/src/conf/virnodedeviceobj.h
@@ -37,6 +37,11 @@ typedef virNodeDeviceDriverState 
*virNodeDeviceDriverStatePtr;
 struct _virNodeDeviceDriverState {
 virMutex lock;
 
+/* pid file FD, ensures two copies of the driver can't use the same root */
+int lockFD;
+
+char *stateDir;
+
 virNodeDeviceObjListPtr devs;   /* currently-known devices */
 void *privateData;  /* driver-specific private data */
 bool privileged;/* whether we run in privileged mode */
diff --git a/src/node_device/node_device_hal.c 
b/src/node_device/node_device_hal.c
index d1eb6c7851..876e808dce 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -33,10 +33,13 @@
 #include "viralloc.h"
 #include "viruuid.h"
 #include "virpci.h"
+#include "virpidfile.h"
 #include "virlog.h"
 #include "virdbus.h"
 #include "virstring.h"
 
+#include "configmake.h"
+
 #define VIR_FROM_THIS VIR_FROM_NODEDEV
 
 VIR_LOG_INIT("node_device.node_device_hal");
@@ -606,12 +609,36 @@ nodeStateInitialize(bool privileged ATTRIBUTE_UNUSED,
 if (VIR_ALLOC(driver) < 0)
 return -1;
 
+driver->lockFD = -1;
 if (virMutexInit(>lock) < 0) {
 VIR_FREE(driver);
 return -1;
 }
 nodeDeviceLock();
 
+if (privileged) {
+if (virAsprintf(>stateDir,
+"%s/run/libvirt/nodedev", LOCALSTATEDIR) < 0)
+goto failure;
+} else {
+VIR_AUTOFREE(char *) rundir = NULL;
+
+if (!(rundir = virGetUserRuntimeDirectory()))
+goto failure;
+if (virAsprintf(>stateDir, "%s/nodedev/run", rundir) < 0)
+goto failure;
+}
+
+if (virFileMakePathWithMode(driver->stateDir, S_IRWXU) < 0) {
+virReportSystemError(errno, _("cannot create state directory '%s'"),
+ driver->stateDir);
+goto failure;
+}
+
+if ((driver->lockFD =
+ virPidFileAcquire(driver->stateDir, "driver", true, getpid())) < 0)
+goto failure;
+
 if (!(driver->devs = virNodeDeviceObjListNew()))
 goto failure;
 
@@ -708,6 +735,10 @@ nodeStateCleanup(void)
 virNodeDeviceObjListFree(driver->devs);
 (void)libhal_ctx_shutdown(hal_ctx, NULL);
 (void)libhal_ctx_free(hal_ctx);
+if (driver->lockFD != -1)
+virPidFileRelease(driver->stateDir, "driver", driver->lockFD);
+
+VIR_FREE(driver->stateDir);
 nodeDeviceUnlock();
 virMutexDestroy(>lock);
 VIR_FREE(driver);
diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 276bf3dd99..d883462948 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -38,10 +38,13 @@
 #include "virbuffer.h"
 #include "virfile.h"
 #include "virpci.h"
+#include "virpidfile.h"
 #include "virstring.h"
 #include "virnetdev.h"
 #include "virmdev.h"
 
+#include "configmake.h"
+
 #define VIR_FROM_THIS VIR_FROM_NODEDEV
 
 VIR_LOG_INIT("node_device.node_device_udev");
@@ -1494,6 +1497,11 @@ nodeStateCleanup(void)
 virObjectUnref(driver->nodeDeviceEventState);
 
 virNodeDeviceObjListFree(driver->devs);
+
+if (driver->lockFD != -1)
+virPidFileRelease(driver->stateDir, "driver", driver->lockFD);
+
+VIR_FREE(driver->stateDir);
 virMutexDestroy(>lock);
 VIR_FREE(driver);
 
@@ -1810,6 +1818,7 @@ nodeStateInitialize(bool privileged,
 if (VIR_ALLOC(driver) < 0)
 return -1;
 
+driver->lockFD = -1;
 if (virMutexInit(>lock) < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Unable to initialize mutex"));
@@ -1819,6 +1828,29 @@ nodeStateInitialize(bool privileged,
 
 driver->privileged = privileged;
 
+if (privileged) {
+if (virAsprintf(>stateDir,
+"%s/run/libvirt/nodedev", LOCALSTATEDIR) < 0)
+goto cleanup;
+} else {
+VIR_AUTOFREE(char *) rundir = NULL;
+
+if (!(rundir = virGetUserRuntimeDirectory()))
+goto cleanup;
+if (virAsprintf(>stateDir, "%s/nodedev/run", rundir) < 0)
+goto cleanup;