Fix error handling in virSecurityManagerGetMountOptions

The impls of virSecurityManagerGetMountOptions had no way to
return errors, since the code was treating 'NULL' as a success
value. This is somewhat pointless, since the calling code did
not want NULL in the first place and has to translate it into
the empty string "". So change the code so that the impls can
return "" directly, allowing use of NULL for error reporting
once again

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

src/lxc/lxc_container.c

@@ -571,7 +571,7 @@ static int lxcContainerMountBasicFS(bool pivotRoot,
          */
 
         ignore_value(virAsprintf(&opts,
-                                 "mode=755,size=65536%s",(sec_mount_options ? sec_mount_options : "")));
+                                 "mode=755,size=65536%s", sec_mount_options));
         if (!opts) {
             virReportOOMError();
             goto cleanup;
@@ -1083,7 +1083,7 @@ static int lxcContainerMountFSTmpfs(virDomainFSDefPtr fs,
     char *data = NULL;
 
     if (virAsprintf(&data,
-                    "size=%lldk%s", fs->usage, (sec_mount_options ? sec_mount_options : "")) < 0) {
+                    "size=%lldk%s", fs->usage, sec_mount_options) < 0) {
         virReportOOMError();
         goto cleanup;
     }
@@ -1456,7 +1456,7 @@ static int lxcContainerMountCGroups(struct lxcContainerCGroup *mounts,
     }
 
     if (virAsprintf(&opts,
-                    "mode=755,size=65536%s",(sec_mount_options ? sec_mount_options : "")) < 0) {
+                    "mode=755,size=65536%s", sec_mount_options) < 0) {
         virReportOOMError();
         return -1;
     }
@@ -1689,7 +1689,9 @@ static int lxcContainerSetupMounts(virDomainDefPtr vmDef,
     if (lxcContainerResolveSymlinks(vmDef) < 0)
         return -1;
 
-    sec_mount_options = virSecurityManagerGetMountOptions(securityDriver, vmDef);
+    if (!(sec_mount_options = virSecurityManagerGetMountOptions(securityDriver, vmDef)))
+        return -1;
+
     if (root && root->src)
         rc =  lxcContainerSetupPivotRoot(vmDef, root, ttyPaths, nttyPaths, sec_mount_options);
     else

src/security/security_apparmor.c

@@ -881,6 +881,21 @@ AppArmorSetTapFDLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
     return 0;
 }
 
+
+static char *
+AppArmorGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
+                        virDomainDefPtr vm ATTRIBUTE_UNUSED)
+{
+    char *opts;
+
+    if (!(opts = strdup(""))) {
+        virReportOOMError();
+        return NULL;
+    }
+    return opts;
+}
+
+
 virSecurityDriver virAppArmorSecurityDriver = {
     .privateDataLen                     = 0,
     .name                               = SECURITY_APPARMOR_NAME,
@@ -918,4 +933,6 @@ virSecurityDriver virAppArmorSecurityDriver = {
 
     .domainSetSecurityImageFDLabel      = AppArmorSetImageFDLabel,
     .domainSetSecurityTapFDLabel        = AppArmorSetTapFDLabel,
+
+    .domainGetSecurityMountOptions      = AppArmorGetMountOptions,
 };

src/security/security_manager.c

@@ -486,10 +486,7 @@ char *virSecurityManagerGetMountOptions(virSecurityManagerPtr mgr,
     if (mgr->drv->domainGetSecurityMountOptions)
         return mgr->drv->domainGetSecurityMountOptions(mgr, vm);
 
-    /*
+    virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
-      I don't think this is an error, these should be optional
-      virReportError(VIR_ERR_NO_SUPPORT, __FUNCTION__);
-    */
     return NULL;
 }
 

src/security/security_nop.c

@@ -21,6 +21,10 @@
 
 #include "security_nop.h"
 
+#include "virterror_internal.h"
+
+#define VIR_FROM_THIS VIR_FROM_SECURITY
+
 static virSecurityDriverStatus virSecurityDriverProbeNop(const char *virtDriver ATTRIBUTE_UNUSED)
 {
     return SECURITY_DRIVER_ENABLE;
@@ -165,8 +169,15 @@ static int virSecurityDomainSetFDLabelNop(virSecurityManagerPtr mgr ATTRIBUTE_UN
 }
 
 static char *virSecurityDomainGetMountOptionsNop(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
-                                                 virDomainDefPtr vm ATTRIBUTE_UNUSED) {
+                                                 virDomainDefPtr vm ATTRIBUTE_UNUSED)
-    return NULL;
+{
+    char *opts;
+
+    if (!(opts = strdup(""))) {
+        virReportOOMError();
+        return NULL;
+    }
+    return opts;
 }
 
 virSecurityDriver virSecurityDriverNop = {

src/security/security_selinux.c

@@ -1974,20 +1974,26 @@ virSecuritySELinuxGetSecurityMountOptions(virSecurityManagerPtr mgr,
     char *opts = NULL;
     virSecurityLabelDefPtr secdef;
 
-    secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
+    if ((secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME))) {
-    if (secdef == NULL)
+        if (!secdef->imagelabel)
-        return NULL;
+            secdef->imagelabel = virSecuritySELinuxGenImageLabel(mgr, def);
 
-    if (! secdef->imagelabel)
+        if (secdef->imagelabel &&
-        secdef->imagelabel = virSecuritySELinuxGenImageLabel(mgr,def);
+            virAsprintf(&opts,
-
+                        ",context=\"%s\"",
-    if (secdef->imagelabel) {
+                        (const char*) secdef->imagelabel) < 0) {
-        virAsprintf(&opts,
+            virReportOOMError();
-                    ",context=\"%s\"",
+            return NULL;
-                    (const char*) secdef->imagelabel);
+        }
     }
 
-    VIR_DEBUG("imageLabel=%s", secdef->imagelabel);
+    if (!opts &&
+        !(opts = strdup(""))) {
+        virReportOOMError();
+        return NULL;
+    }
+
+    VIR_DEBUG("imageLabel=%s opts=%s", secdef->imagelabel, opts);
     return opts;
 }