[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 2/4] Serialize execution of security manager APIs



From: "Daniel P. Berrange" <berrange redhat com>

Add locking to virSecurityManagerXXX APIs, so that use of the
security drivers is internally serialized. This avoids the need
to rely on the global driver locks to achieve serialization

Signed-off-by: Daniel P. Berrange <berrange redhat com>
---
 src/qemu/qemu_conf.h            |   2 +-
 src/security/security_manager.c | 200 +++++++++++++++++++++++++++++++---------
 2 files changed, 157 insertions(+), 45 deletions(-)

diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index d4ec0f7..f0a3da1 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -194,7 +194,7 @@ struct _virQEMUDriver {
     /* Immutable pointer, self-locking APIs */
     virDomainEventStatePtr domainEventState;
 
-    /* Immutable pointer. Unsafe APIs. XXX */
+    /* Immutable pointer. self-locking APIs */
     virSecurityManagerPtr securityManager;
 
     /* Immutable pointers. Requires locks to be held before
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index a3f8669..6f8ddbf 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -216,8 +216,13 @@ virSecurityManagerGetDriver(virSecurityManagerPtr mgr)
 const char *
 virSecurityManagerGetDOI(virSecurityManagerPtr mgr)
 {
-    if (mgr->drv->getDOI)
-        return mgr->drv->getDOI(mgr);
+    if (mgr->drv->getDOI) {
+        const char *ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->getDOI(mgr);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return NULL;
@@ -226,8 +231,13 @@ virSecurityManagerGetDOI(virSecurityManagerPtr mgr)
 const char *
 virSecurityManagerGetModel(virSecurityManagerPtr mgr)
 {
-    if (mgr->drv->getModel)
-        return mgr->drv->getModel(mgr);
+    if (mgr->drv->getModel) {
+        const char *ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->getModel(mgr);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return NULL;
@@ -252,8 +262,13 @@ int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr,
                                         virDomainDefPtr vm,
                                         virDomainDiskDefPtr disk)
 {
-    if (mgr->drv->domainRestoreSecurityImageLabel)
-        return mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, disk);
+    if (mgr->drv->domainRestoreSecurityImageLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainRestoreSecurityImageLabel(mgr, vm, disk);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -262,8 +277,13 @@ int virSecurityManagerRestoreImageLabel(virSecurityManagerPtr mgr,
 int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr,
                                            virDomainDefPtr vm)
 {
-    if (mgr->drv->domainSetSecurityDaemonSocketLabel)
-        return mgr->drv->domainSetSecurityDaemonSocketLabel(mgr, vm);
+    if (mgr->drv->domainSetSecurityDaemonSocketLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainSetSecurityDaemonSocketLabel(mgr, vm);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -272,8 +292,13 @@ int virSecurityManagerSetDaemonSocketLabel(virSecurityManagerPtr mgr,
 int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr,
                                      virDomainDefPtr vm)
 {
-    if (mgr->drv->domainSetSecuritySocketLabel)
-        return mgr->drv->domainSetSecuritySocketLabel(mgr, vm);
+    if (mgr->drv->domainSetSecuritySocketLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainSetSecuritySocketLabel(mgr, vm);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -282,8 +307,13 @@ int virSecurityManagerSetSocketLabel(virSecurityManagerPtr mgr,
 int virSecurityManagerClearSocketLabel(virSecurityManagerPtr mgr,
                                        virDomainDefPtr vm)
 {
-    if (mgr->drv->domainClearSecuritySocketLabel)
-        return mgr->drv->domainClearSecuritySocketLabel(mgr, vm);
+    if (mgr->drv->domainClearSecuritySocketLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainClearSecuritySocketLabel(mgr, vm);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -293,8 +323,13 @@ int virSecurityManagerSetImageLabel(virSecurityManagerPtr mgr,
                                     virDomainDefPtr vm,
                                     virDomainDiskDefPtr disk)
 {
-    if (mgr->drv->domainSetSecurityImageLabel)
-        return mgr->drv->domainSetSecurityImageLabel(mgr, vm, disk);
+    if (mgr->drv->domainSetSecurityImageLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainSetSecurityImageLabel(mgr, vm, disk);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -305,8 +340,13 @@ int virSecurityManagerRestoreHostdevLabel(virSecurityManagerPtr mgr,
                                           virDomainHostdevDefPtr dev,
                                           const char *vroot)
 {
-    if (mgr->drv->domainRestoreSecurityHostdevLabel)
-        return mgr->drv->domainRestoreSecurityHostdevLabel(mgr, vm, dev, vroot);
+    if (mgr->drv->domainRestoreSecurityHostdevLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainRestoreSecurityHostdevLabel(mgr, vm, dev, vroot);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -317,8 +357,13 @@ int virSecurityManagerSetHostdevLabel(virSecurityManagerPtr mgr,
                                       virDomainHostdevDefPtr dev,
                                       const char *vroot)
 {
-    if (mgr->drv->domainSetSecurityHostdevLabel)
-        return mgr->drv->domainSetSecurityHostdevLabel(mgr, vm, dev, vroot);
+    if (mgr->drv->domainSetSecurityHostdevLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainSetSecurityHostdevLabel(mgr, vm, dev, vroot);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -328,8 +373,13 @@ int virSecurityManagerSetSavedStateLabel(virSecurityManagerPtr mgr,
                                          virDomainDefPtr vm,
                                          const char *savefile)
 {
-    if (mgr->drv->domainSetSavedStateLabel)
-        return mgr->drv->domainSetSavedStateLabel(mgr, vm, savefile);
+    if (mgr->drv->domainSetSavedStateLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainSetSavedStateLabel(mgr, vm, savefile);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -339,8 +389,13 @@ int virSecurityManagerRestoreSavedStateLabel(virSecurityManagerPtr mgr,
                                              virDomainDefPtr vm,
                                              const char *savefile)
 {
-    if (mgr->drv->domainRestoreSavedStateLabel)
-        return mgr->drv->domainRestoreSavedStateLabel(mgr, vm, savefile);
+    if (mgr->drv->domainRestoreSavedStateLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainRestoreSavedStateLabel(mgr, vm, savefile);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -360,6 +415,7 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr,
     if ((sec_managers = virSecurityManagerGetNested(mgr)) == NULL)
         return -1;
 
+    virObjectLock(mgr);
     for (i = 0; sec_managers[i]; i++) {
         seclabel = virDomainDefGetSecurityLabelDef(vm,
                                                    sec_managers[i]->drv->name);
@@ -395,6 +451,7 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr,
     }
 
 cleanup:
+    virObjectUnlock(mgr);
     VIR_FREE(sec_managers);
     return rc;
 }
@@ -403,8 +460,13 @@ int virSecurityManagerReserveLabel(virSecurityManagerPtr mgr,
                                    virDomainDefPtr vm,
                                    pid_t pid)
 {
-    if (mgr->drv->domainReserveSecurityLabel)
-        return mgr->drv->domainReserveSecurityLabel(mgr, vm, pid);
+    if (mgr->drv->domainReserveSecurityLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainReserveSecurityLabel(mgr, vm, pid);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -413,8 +475,13 @@ int virSecurityManagerReserveLabel(virSecurityManagerPtr mgr,
 int virSecurityManagerReleaseLabel(virSecurityManagerPtr mgr,
                                    virDomainDefPtr vm)
 {
-    if (mgr->drv->domainReleaseSecurityLabel)
-        return mgr->drv->domainReleaseSecurityLabel(mgr, vm);
+    if (mgr->drv->domainReleaseSecurityLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainReleaseSecurityLabel(mgr, vm);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -424,8 +491,13 @@ int virSecurityManagerSetAllLabel(virSecurityManagerPtr mgr,
                                   virDomainDefPtr vm,
                                   const char *stdin_path)
 {
-    if (mgr->drv->domainSetSecurityAllLabel)
-        return mgr->drv->domainSetSecurityAllLabel(mgr, vm, stdin_path);
+    if (mgr->drv->domainSetSecurityAllLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainSetSecurityAllLabel(mgr, vm, stdin_path);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -435,8 +507,13 @@ int virSecurityManagerRestoreAllLabel(virSecurityManagerPtr mgr,
                                       virDomainDefPtr vm,
                                       int migrated)
 {
-    if (mgr->drv->domainRestoreSecurityAllLabel)
-        return mgr->drv->domainRestoreSecurityAllLabel(mgr, vm, migrated);
+    if (mgr->drv->domainRestoreSecurityAllLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainRestoreSecurityAllLabel(mgr, vm, migrated);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -447,8 +524,13 @@ int virSecurityManagerGetProcessLabel(virSecurityManagerPtr mgr,
                                       pid_t pid,
                                       virSecurityLabelPtr sec)
 {
-    if (mgr->drv->domainGetSecurityProcessLabel)
-        return mgr->drv->domainGetSecurityProcessLabel(mgr, vm, pid, sec);
+    if (mgr->drv->domainGetSecurityProcessLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainGetSecurityProcessLabel(mgr, vm, pid, sec);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -457,8 +539,13 @@ int virSecurityManagerGetProcessLabel(virSecurityManagerPtr mgr,
 int virSecurityManagerSetProcessLabel(virSecurityManagerPtr mgr,
                                       virDomainDefPtr vm)
 {
-    if (mgr->drv->domainSetSecurityProcessLabel)
-        return mgr->drv->domainSetSecurityProcessLabel(mgr, vm);
+    if (mgr->drv->domainSetSecurityProcessLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainSetSecurityProcessLabel(mgr, vm);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -480,8 +567,13 @@ int virSecurityManagerVerify(virSecurityManagerPtr mgr,
     if (secdef == NULL || secdef->model == NULL)
         return 0;
 
-    if (mgr->drv->domainSecurityVerify)
-        return mgr->drv->domainSecurityVerify(mgr, def);
+    if (mgr->drv->domainSecurityVerify) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainSecurityVerify(mgr, def);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -491,8 +583,13 @@ int virSecurityManagerSetImageFDLabel(virSecurityManagerPtr mgr,
                                       virDomainDefPtr vm,
                                       int fd)
 {
-    if (mgr->drv->domainSetSecurityImageFDLabel)
-        return mgr->drv->domainSetSecurityImageFDLabel(mgr, vm, fd);
+    if (mgr->drv->domainSetSecurityImageFDLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainSetSecurityImageFDLabel(mgr, vm, fd);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -502,8 +599,13 @@ int virSecurityManagerSetTapFDLabel(virSecurityManagerPtr mgr,
                                     virDomainDefPtr vm,
                                     int fd)
 {
-    if (mgr->drv->domainSetSecurityTapFDLabel)
-        return mgr->drv->domainSetSecurityTapFDLabel(mgr, vm, fd);
+    if (mgr->drv->domainSetSecurityTapFDLabel) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainSetSecurityTapFDLabel(mgr, vm, fd);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return -1;
@@ -512,8 +614,13 @@ int virSecurityManagerSetTapFDLabel(virSecurityManagerPtr mgr,
 char *virSecurityManagerGetMountOptions(virSecurityManagerPtr mgr,
                                         virDomainDefPtr vm)
 {
-    if (mgr->drv->domainGetSecurityMountOptions)
-        return mgr->drv->domainGetSecurityMountOptions(mgr, vm);
+    if (mgr->drv->domainGetSecurityMountOptions) {
+        char *ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainGetSecurityMountOptions(mgr, vm);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
     return NULL;
@@ -542,8 +649,13 @@ int virSecurityManagerSetHugepages(virSecurityManagerPtr mgr,
                                     virDomainDefPtr vm,
                                     const char *path)
 {
-    if (mgr->drv->domainSetSecurityHugepages)
-        return mgr->drv->domainSetSecurityHugepages(mgr, vm, path);
+    if (mgr->drv->domainSetSecurityHugepages) {
+        int ret;
+        virObjectLock(mgr);
+        ret = mgr->drv->domainSetSecurityHugepages(mgr, vm, path);
+        virObjectUnlock(mgr);
+        return ret;
+    }
 
     return 0;
 }
-- 
1.8.1


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]