[libvirt] [PATCH] virSecurityManagerNew: Turn array of booleans into flags

Michal Privoznik mprivozn at redhat.com
Tue Oct 6 15:14:04 UTC 2015


So imagine you want to crate new security manager:

  if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false, true)));

Hard to parse, right? What about this:

  if (!(mgr = virSecurityManagerNew("selinux", "QEMU",
                                    VIR_SECURITY_MANAGER_DEFAULT_CONFINED |
                                    VIR_SECURITY_MANAGER_PRIVILEGED)));

Now that's better! This is what the commit does.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/lxc/lxc_controller.c         |  3 +-
 src/lxc/lxc_driver.c             | 14 +++++---
 src/qemu/qemu_driver.c           | 28 ++++++++--------
 src/security/security_manager.c  | 70 +++++++++++++---------------------------
 src/security/security_manager.h  | 25 ++++++++------
 tests/qemuhotplugtest.c          |  3 +-
 tests/seclabeltest.c             |  2 +-
 tests/securityselinuxlabeltest.c |  4 ++-
 tests/securityselinuxtest.c      |  4 ++-
 9 files changed, 72 insertions(+), 81 deletions(-)

diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index b064919..3e5d2b4 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -2645,8 +2645,7 @@ int main(int argc, char *argv[])
     ctrl->handshakeFd = handshakeFd;
 
     if (!(ctrl->securityManager = virSecurityManagerNew(securityDriver,
-                                                        LXC_DRIVER_NAME,
-                                                        false, false, false, false)))
+                                                        LXC_DRIVER_NAME, 0)))
         goto cleanup;
 
     if (ctrl->def->seclabels) {
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index b408be0..1a9550e 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1556,13 +1556,17 @@ static int lxcCheckNetNsSupport(void)
 static virSecurityManagerPtr
 lxcSecurityInit(virLXCDriverConfigPtr cfg)
 {
+    unsigned int flags = VIR_SECURITY_MANAGER_PRIVILEGED;
+
     VIR_INFO("lxcSecurityInit %s", cfg->securityDriverName);
+
+    if (cfg->securityDefaultConfined)
+        flags |= VIR_SECURITY_MANAGER_DEFAULT_CONFINED;
+    if (cfg->securityRequireConfined)
+        flags |= VIR_SECURITY_MANAGER_REQUIRE_CONFINED;
+
     virSecurityManagerPtr mgr = virSecurityManagerNew(cfg->securityDriverName,
-                                                      LXC_DRIVER_NAME,
-                                                      false,
-                                                      cfg->securityDefaultConfined,
-                                                      cfg->securityRequireConfined,
-                                                      true);
+                                                      LXC_DRIVER_NAME, flags);
     if (!mgr)
         goto error;
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index aff1915..598d159 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -389,6 +389,16 @@ qemuSecurityInit(virQEMUDriverPtr driver)
     virSecurityManagerPtr mgr = NULL;
     virSecurityManagerPtr stack = NULL;
     virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    unsigned int flags = 0;
+
+    if (cfg->allowDiskFormatProbing)
+        flags |= VIR_SECURITY_MANAGER_ALLOW_DISK_PROBE;
+    if (cfg->securityDefaultConfined)
+        flags |= VIR_SECURITY_MANAGER_DEFAULT_CONFINED;
+    if (cfg->securityRequireConfined)
+        flags |= VIR_SECURITY_MANAGER_REQUIRE_CONFINED;
+    if (virQEMUDriverIsPrivileged(driver))
+        flags |= VIR_SECURITY_MANAGER_PRIVILEGED;
 
     if (cfg->securityDriverNames &&
         cfg->securityDriverNames[0]) {
@@ -396,10 +406,7 @@ qemuSecurityInit(virQEMUDriverPtr driver)
         while (names && *names) {
             if (!(mgr = virSecurityManagerNew(*names,
                                               QEMU_DRIVER_NAME,
-                                              cfg->allowDiskFormatProbing,
-                                              cfg->securityDefaultConfined,
-                                              cfg->securityRequireConfined,
-                                              virQEMUDriverIsPrivileged(driver))))
+                                              flags)))
                 goto error;
             if (!stack) {
                 if (!(stack = virSecurityManagerNewStack(mgr)))
@@ -414,10 +421,7 @@ qemuSecurityInit(virQEMUDriverPtr driver)
     } else {
         if (!(mgr = virSecurityManagerNew(NULL,
                                           QEMU_DRIVER_NAME,
-                                          cfg->allowDiskFormatProbing,
-                                          cfg->securityDefaultConfined,
-                                          cfg->securityRequireConfined,
-                                          virQEMUDriverIsPrivileged(driver))))
+                                          flags)))
             goto error;
         if (!(stack = virSecurityManagerNewStack(mgr)))
             goto error;
@@ -425,14 +429,12 @@ qemuSecurityInit(virQEMUDriverPtr driver)
     }
 
     if (virQEMUDriverIsPrivileged(driver)) {
+        if (cfg->dynamicOwnership)
+            flags |= VIR_SECURITY_MANAGER_DYNAMIC_OWNERSHIP;
         if (!(mgr = virSecurityManagerNewDAC(QEMU_DRIVER_NAME,
                                              cfg->user,
                                              cfg->group,
-                                             cfg->allowDiskFormatProbing,
-                                             cfg->securityDefaultConfined,
-                                             cfg->securityRequireConfined,
-                                             cfg->dynamicOwnership,
-                                             virQEMUDriverIsPrivileged(driver),
+                                             flags,
                                              qemuSecurityChownCallback)))
             goto error;
         if (!stack) {
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 28d7dfd..46b1a25 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -37,10 +37,7 @@ struct _virSecurityManager {
     virObjectLockable parent;
 
     virSecurityDriverPtr drv;
-    bool allowDiskFormatProbing;
-    bool defaultConfined;
-    bool requireConfined;
-    bool privileged;
+    unsigned int flags;
     const char *virtDriver;
     void *privateData;
 };
@@ -77,10 +74,7 @@ VIR_ONCE_GLOBAL_INIT(virSecurityManager);
 static virSecurityManagerPtr
 virSecurityManagerNewDriver(virSecurityDriverPtr drv,
                             const char *virtDriver,
-                            bool allowDiskFormatProbing,
-                            bool defaultConfined,
-                            bool requireConfined,
-                            bool privileged)
+                            unsigned int flags)
 {
     virSecurityManagerPtr mgr;
     char *privateData;
@@ -88,11 +82,10 @@ virSecurityManagerNewDriver(virSecurityDriverPtr drv,
     if (virSecurityManagerInitialize() < 0)
         return NULL;
 
-    VIR_DEBUG("drv=%p (%s) virtDriver=%s allowDiskFormatProbing=%d "
-              "defaultConfined=%d requireConfined=%d privileged=%d",
-              drv, drv->name, virtDriver,
-              allowDiskFormatProbing, defaultConfined,
-              requireConfined, privileged);
+    VIR_DEBUG("drv=%p (%s) virtDriver=%s flags=%x",
+              drv, drv->name, virtDriver, flags);
+
+    virCheckFlags(VIR_SECURITY_MANAGER_NEW_MASK, NULL);
 
     if (VIR_ALLOC_N(privateData, drv->privateDataLen) < 0)
         return NULL;
@@ -103,10 +96,7 @@ virSecurityManagerNewDriver(virSecurityDriverPtr drv,
     }
 
     mgr->drv = drv;
-    mgr->allowDiskFormatProbing = allowDiskFormatProbing;
-    mgr->defaultConfined = defaultConfined;
-    mgr->requireConfined = requireConfined;
-    mgr->privileged = privileged;
+    mgr->flags = flags;
     mgr->virtDriver = virtDriver;
     mgr->privateData = privateData;
 
@@ -125,10 +115,7 @@ virSecurityManagerNewStack(virSecurityManagerPtr primary)
     virSecurityManagerPtr mgr =
         virSecurityManagerNewDriver(&virSecurityDriverStack,
                                     virSecurityManagerGetDriver(primary),
-                                    virSecurityManagerGetAllowDiskFormatProbing(primary),
-                                    virSecurityManagerGetDefaultConfined(primary),
-                                    virSecurityManagerGetRequireConfined(primary),
-                                    virSecurityManagerGetPrivileged(primary));
+                                    primary->flags);
 
     if (!mgr)
         return NULL;
@@ -153,20 +140,13 @@ virSecurityManagerPtr
 virSecurityManagerNewDAC(const char *virtDriver,
                          uid_t user,
                          gid_t group,
-                         bool allowDiskFormatProbing,
-                         bool defaultConfined,
-                         bool requireConfined,
-                         bool dynamicOwnership,
-                         bool privileged,
+                         unsigned int flags,
                          virSecurityManagerDACChownCallback chownCallback)
 {
     virSecurityManagerPtr mgr =
         virSecurityManagerNewDriver(&virSecurityDriverDAC,
                                     virtDriver,
-                                    allowDiskFormatProbing,
-                                    defaultConfined,
-                                    requireConfined,
-                                    privileged);
+                                    flags & VIR_SECURITY_MANAGER_NEW_MASK);
 
     if (!mgr)
         return NULL;
@@ -176,7 +156,7 @@ virSecurityManagerNewDAC(const char *virtDriver,
         return NULL;
     }
 
-    virSecurityDACSetDynamicOwnership(mgr, dynamicOwnership);
+    virSecurityDACSetDynamicOwnership(mgr, flags & VIR_SECURITY_MANAGER_DYNAMIC_OWNERSHIP);
     virSecurityDACSetChownCallback(mgr, chownCallback);
 
     return mgr;
@@ -186,10 +166,7 @@ virSecurityManagerNewDAC(const char *virtDriver,
 virSecurityManagerPtr
 virSecurityManagerNew(const char *name,
                       const char *virtDriver,
-                      bool allowDiskFormatProbing,
-                      bool defaultConfined,
-                      bool requireConfined,
-                      bool privileged)
+                      unsigned int flags)
 {
     virSecurityDriverPtr drv = virSecurityDriverLookup(name, virtDriver);
     if (!drv)
@@ -197,13 +174,13 @@ virSecurityManagerNew(const char *name,
 
     /* driver "none" needs some special handling of *Confined bools */
     if (STREQ(drv->name, "none")) {
-        if (requireConfined) {
+        if (flags & VIR_SECURITY_MANAGER_REQUIRE_CONFINED) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("Security driver \"none\" cannot create confined guests"));
             return NULL;
         }
 
-        if (defaultConfined) {
+        if (flags & VIR_SECURITY_MANAGER_DEFAULT_CONFINED) {
             if (name != NULL) {
                 VIR_WARN("Configured security driver \"none\" disables default"
                          " policy to create confined guests");
@@ -211,16 +188,13 @@ virSecurityManagerNew(const char *name,
                 VIR_DEBUG("Auto-probed security driver is \"none\";"
                           " confined guests will not be created");
             }
-            defaultConfined = false;
+            flags &= ~VIR_SECURITY_MANAGER_DEFAULT_CONFINED;
         }
     }
 
     return virSecurityManagerNewDriver(drv,
                                        virtDriver,
-                                       allowDiskFormatProbing,
-                                       defaultConfined,
-                                       requireConfined,
-                                       privileged);
+                                       flags);
 }
 
 
@@ -323,28 +297,28 @@ virSecurityManagerGetBaseLabel(virSecurityManagerPtr mgr,
 bool
 virSecurityManagerGetAllowDiskFormatProbing(virSecurityManagerPtr mgr)
 {
-    return mgr->allowDiskFormatProbing;
+    return mgr->flags & VIR_SECURITY_MANAGER_ALLOW_DISK_PROBE;
 }
 
 
 bool
 virSecurityManagerGetDefaultConfined(virSecurityManagerPtr mgr)
 {
-    return mgr->defaultConfined;
+    return mgr->flags & VIR_SECURITY_MANAGER_DEFAULT_CONFINED;
 }
 
 
 bool
 virSecurityManagerGetRequireConfined(virSecurityManagerPtr mgr)
 {
-    return mgr->requireConfined;
+    return mgr->flags & VIR_SECURITY_MANAGER_REQUIRE_CONFINED;
 }
 
 
 bool
 virSecurityManagerGetPrivileged(virSecurityManagerPtr mgr)
 {
-    return mgr->privileged;
+    return mgr->flags & VIR_SECURITY_MANAGER_PRIVILEGED;
 }
 
 
@@ -611,7 +585,7 @@ virSecurityManagerGenLabel(virSecurityManagerPtr mgr,
         }
 
         if (seclabel->type == VIR_DOMAIN_SECLABEL_DEFAULT) {
-            if (sec_managers[i]->defaultConfined) {
+            if (virSecurityManagerGetDefaultConfined(sec_managers[i])) {
                 seclabel->type = VIR_DOMAIN_SECLABEL_DYNAMIC;
             } else {
                 seclabel->type = VIR_DOMAIN_SECLABEL_NONE;
@@ -620,7 +594,7 @@ virSecurityManagerGenLabel(virSecurityManagerPtr mgr,
         }
 
         if (seclabel->type == VIR_DOMAIN_SECLABEL_NONE) {
-            if (sec_managers[i]->requireConfined) {
+            if (virSecurityManagerGetRequireConfined(sec_managers[i])) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                                _("Unconfined guests are not allowed on this host"));
                 goto cleanup;
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index 53e56f6..e534e31 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -30,12 +30,23 @@
 typedef struct _virSecurityManager virSecurityManager;
 typedef virSecurityManager *virSecurityManagerPtr;
 
+typedef enum {
+    VIR_SECURITY_MANAGER_ALLOW_DISK_PROBE   = 1 << 0,
+    VIR_SECURITY_MANAGER_DEFAULT_CONFINED   = 1 << 1,
+    VIR_SECURITY_MANAGER_REQUIRE_CONFINED   = 1 << 2,
+    VIR_SECURITY_MANAGER_PRIVILEGED         = 1 << 3,
+    VIR_SECURITY_MANAGER_DYNAMIC_OWNERSHIP  = 1 << 4,
+} virSecurityManagerNewFlags;
+
+# define VIR_SECURITY_MANAGER_NEW_MASK  \
+    (VIR_SECURITY_MANAGER_ALLOW_DISK_PROBE  | \
+     VIR_SECURITY_MANAGER_DEFAULT_CONFINED  | \
+     VIR_SECURITY_MANAGER_REQUIRE_CONFINED  | \
+     VIR_SECURITY_MANAGER_PRIVILEGED)
+
 virSecurityManagerPtr virSecurityManagerNew(const char *name,
                                             const char *virtDriver,
-                                            bool allowDiskFormatProbing,
-                                            bool defaultConfined,
-                                            bool requireConfined,
-                                            bool privileged);
+                                            unsigned int flags);
 
 virSecurityManagerPtr virSecurityManagerNewStack(virSecurityManagerPtr primary);
 int virSecurityManagerStackAddNested(virSecurityManagerPtr stack,
@@ -59,11 +70,7 @@ typedef int
 virSecurityManagerPtr virSecurityManagerNewDAC(const char *virtDriver,
                                                uid_t user,
                                                gid_t group,
-                                               bool allowDiskFormatProbing,
-                                               bool defaultConfined,
-                                               bool requireConfined,
-                                               bool dynamicOwnership,
-                                               bool privileged,
+                                               unsigned int flags,
                                                virSecurityManagerDACChownCallback chownCallback);
 
 int virSecurityManagerPreFork(virSecurityManagerPtr mgr);
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 2135b5a..2c5c48f 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -364,7 +364,8 @@ mymain(void)
     if (!driver.lockManager)
         return EXIT_FAILURE;
 
-    if (!(mgr = virSecurityManagerNew("none", "qemu", false, false, false, true)))
+    if (!(mgr = virSecurityManagerNew("none", "qemu",
+                                      VIR_SECURITY_MANAGER_PRIVILEGED)))
         return EXIT_FAILURE;
     if (!(driver.securityManager = virSecurityManagerNewStack(mgr)))
         return EXIT_FAILURE;
diff --git a/tests/seclabeltest.c b/tests/seclabeltest.c
index 93ddcbb..6b1e789 100644
--- a/tests/seclabeltest.c
+++ b/tests/seclabeltest.c
@@ -17,7 +17,7 @@ main(int argc ATTRIBUTE_UNUSED, char **argv ATTRIBUTE_UNUSED)
     if (virThreadInitialize() < 0)
         return EXIT_FAILURE;
 
-    mgr = virSecurityManagerNew(NULL, "QEMU", false, true, false, false);
+    mgr = virSecurityManagerNew(NULL, "QEMU", VIR_SECURITY_MANAGER_DEFAULT_CONFINED);
     if (mgr == NULL) {
         fprintf(stderr, "Failed to start security driver");
         return EXIT_FAILURE;
diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c
index 4b8fa6f..86660f4 100644
--- a/tests/securityselinuxlabeltest.c
+++ b/tests/securityselinuxlabeltest.c
@@ -351,7 +351,9 @@ mymain(void)
     if (!rc)
         return EXIT_AM_SKIP;
 
-    if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false, true))) {
+    if (!(mgr = virSecurityManagerNew("selinux", "QEMU",
+                                      VIR_SECURITY_MANAGER_DEFAULT_CONFINED |
+                                      VIR_SECURITY_MANAGER_PRIVILEGED))) {
         virErrorPtr err = virGetLastError();
         VIR_TEST_VERBOSE("Unable to initialize security driver: %s\n",
                 err->message);
diff --git a/tests/securityselinuxtest.c b/tests/securityselinuxtest.c
index 3a7862f..49694f3 100644
--- a/tests/securityselinuxtest.c
+++ b/tests/securityselinuxtest.c
@@ -272,7 +272,9 @@ mymain(void)
     int ret = 0;
     virSecurityManagerPtr mgr;
 
-    if (!(mgr = virSecurityManagerNew("selinux", "QEMU", false, true, false, true))) {
+    if (!(mgr = virSecurityManagerNew("selinux", "QEMU",
+                                      VIR_SECURITY_MANAGER_DEFAULT_CONFINED |
+                                      VIR_SECURITY_MANAGER_PRIVILEGED))) {
         virErrorPtr err = virGetLastError();
         fprintf(stderr, "Unable to initialize security driver: %s\n",
                 err->message);
-- 
2.4.9




More information about the libvir-list mailing list