Serialize execution of security manager APIs

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: NDaniel P. Berrange <berrange@redhat.com>

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

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;
 }