storage: Add default mount options for fs/netfs storage pools

https://bugzilla.redhat.com/show_bug.cgi?id=1584663

Modify the command generation to add some default options to the
fs/netfs storage pools based on the OS type. For Linux, it'll be
the "nodev, nosuid, noexec". For FreeBSD, it'll be "nosuid, noexec".
For others, just leave the options alone.

Modify the storagepoolxml2argvtest to handle the fact that the
same input XML could generate different output XML based on whether
Linux, FreeBSD, or other was being built.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index a84ee5b60071825d82c7d55048b74582fba2949d..d63237ce8fcfb2f40c4a3320817cdf1c59c5a3cf 100644 (file)
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -34,6 +34,11 @@
 # ifndef FS_NOCOW_FL
 #  define FS_NOCOW_FL                     0x00800000 /* Do not cow file */
 # endif
+# define default_mount_opts "nodev,nosuid,noexec"
+#elif defined(__FreeBSD__)
+# define default_mount_opts "nosuid,noexec"
+#else
+# define default_mount_opts ""
 #endif
 
 #if WITH_BLKID
@@ -4261,12 +4266,34 @@ virStorageBackendFileSystemGetPoolSource(virStoragePoolObjPtr pool)
 }
 
 
+static void
+virStorageBackendFileSystemMountAddOptions(virCommandPtr cmd,
+                                           const char *providedOpts)
+{
+    VIR_AUTOFREE(char *) mountOpts = NULL;
+    virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+    if (*default_mount_opts != '\0')
+        virBufferAsprintf(&buf, "%s,", default_mount_opts);
+
+    if (providedOpts)
+        virBufferAsprintf(&buf, "%s,", providedOpts);
+
+    virBufferTrim(&buf, ",", -1);
+    mountOpts = virBufferContentAndReset(&buf);
+
+    if (mountOpts)
+        virCommandAddArgList(cmd, "-o", mountOpts, NULL);
+}
+
+
 static void
 virStorageBackendFileSystemMountNFSArgs(virCommandPtr cmd,
                                         const char *src,
                                         virStoragePoolDefPtr def)
 {
     virCommandAddArgList(cmd, src, def->target.path, NULL);
+    virStorageBackendFileSystemMountAddOptions(cmd, NULL);
 }
 
 
@@ -4278,8 +4305,8 @@ virStorageBackendFileSystemMountGlusterArgs(virCommandPtr cmd,
     const char *fmt;
 
     fmt = virStoragePoolFormatFileSystemNetTypeToString(def->source.format);
-    virCommandAddArgList(cmd, "-t", fmt, src, "-o", "direct-io-mode=1",
-                         def->target.path, NULL);
+    virCommandAddArgList(cmd, "-t", fmt, src, def->target.path, NULL);
+    virStorageBackendFileSystemMountAddOptions(cmd, "direct-io-mode=1");
 }
 
 
@@ -4291,8 +4318,8 @@ virStorageBackendFileSystemMountCIFSArgs(virCommandPtr cmd,
     const char *fmt;
 
     fmt = virStoragePoolFormatFileSystemNetTypeToString(def->source.format);
-    virCommandAddArgList(cmd, "-t", fmt, src, def->target.path,
-                         "-o", "guest", NULL);
+    virCommandAddArgList(cmd, "-t", fmt, src, def->target.path, NULL);
+    virStorageBackendFileSystemMountAddOptions(cmd, "guest");
 }
 
 
@@ -4308,6 +4335,7 @@ virStorageBackendFileSystemMountDefaultArgs(virCommandPtr cmd,
     else
         fmt = virStoragePoolFormatFileSystemNetTypeToString(def->source.format);
     virCommandAddArgList(cmd, "-t", fmt, src, def->target.path, NULL);
+    virStorageBackendFileSystemMountAddOptions(cmd, NULL);
 }
 
 

diff --git a/tests/storagepoolxml2argvdata/pool-fs-freebsd.argv b/tests/storagepoolxml2argvdata/pool-fs-freebsd.argv
new file mode 100644 (file)
index 0000000..a35d73e
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-fs-freebsd.argv
@@ -0,0 +1 @@
+mount -t ext3 /dev/sda6 /mnt -o nosuid,noexec

diff --git a/tests/storagepoolxml2argvdata/pool-fs-linux.argv b/tests/storagepoolxml2argvdata/pool-fs-linux.argv
new file mode 100644 (file)
index 0000000..19543f4
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-fs-linux.argv
@@ -0,0 +1 @@
+mount -t ext3 /dev/sda6 /mnt -o nodev,nosuid,noexec

diff --git a/tests/storagepoolxml2argvdata/pool-netfs-auto-freebsd.argv b/tests/storagepoolxml2argvdata/pool-netfs-auto-freebsd.argv
new file mode 100644 (file)
index 0000000..39e5c97
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs-auto-freebsd.argv
@@ -0,0 +1 @@
+mount localhost:/var/lib/libvirt/images /mnt -o nosuid,noexec

diff --git a/tests/storagepoolxml2argvdata/pool-netfs-auto-linux.argv b/tests/storagepoolxml2argvdata/pool-netfs-auto-linux.argv
new file mode 100644 (file)
index 0000000..1f82d3d
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs-auto-linux.argv
@@ -0,0 +1 @@
+mount localhost:/var/lib/libvirt/images /mnt -o nodev,nosuid,noexec

diff --git a/tests/storagepoolxml2argvdata/pool-netfs-cifs-freebsd.argv b/tests/storagepoolxml2argvdata/pool-netfs-cifs-freebsd.argv
new file mode 100644 (file)
index 0000000..d72749a
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs-cifs-freebsd.argv
@@ -0,0 +1 @@
+mount -t cifs //example.com/samba_share /mnt/cifs -o nosuid,noexec,guest

diff --git a/tests/storagepoolxml2argvdata/pool-netfs-cifs-linux.argv b/tests/storagepoolxml2argvdata/pool-netfs-cifs-linux.argv
new file mode 100644 (file)
index 0000000..85aa9cf
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs-cifs-linux.argv
@@ -0,0 +1 @@
+mount -t cifs //example.com/samba_share /mnt/cifs -o nodev,nosuid,noexec,guest

diff --git a/tests/storagepoolxml2argvdata/pool-netfs-freebsd.argv b/tests/storagepoolxml2argvdata/pool-netfs-freebsd.argv
new file mode 100644 (file)
index 0000000..05c1951
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs-freebsd.argv
@@ -0,0 +1 @@
+mount -t nfs localhost:/var/lib/libvirt/images /mnt -o nosuid,noexec

diff --git a/tests/storagepoolxml2argvdata/pool-netfs-gluster-freebsd.argv b/tests/storagepoolxml2argvdata/pool-netfs-gluster-freebsd.argv
new file mode 100644 (file)
index 0000000..700107d
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs-gluster-freebsd.argv
@@ -0,0 +1,2 @@
+mount -t glusterfs example.com:/volume /mnt/gluster -o nosuid,noexec,\
+direct-io-mode=1

diff --git a/tests/storagepoolxml2argvdata/pool-netfs-gluster-linux.argv b/tests/storagepoolxml2argvdata/pool-netfs-gluster-linux.argv
new file mode 100644 (file)
index 0000000..9535c8a
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs-gluster-linux.argv
@@ -0,0 +1,2 @@
+mount -t glusterfs example.com:/volume /mnt/gluster -o nodev,nosuid,noexec,\
+direct-io-mode=1

diff --git a/tests/storagepoolxml2argvdata/pool-netfs-linux.argv b/tests/storagepoolxml2argvdata/pool-netfs-linux.argv
new file mode 100644 (file)
index 0000000..22fafd7
--- /dev/null
+++ b/tests/storagepoolxml2argvdata/pool-netfs-linux.argv
@@ -0,0 +1 @@
+mount -t nfs localhost:/var/lib/libvirt/images /mnt -o nodev,nosuid,noexec

diff --git a/tests/storagepoolxml2argvtest.c b/tests/storagepoolxml2argvtest.c
index 2f2d40e0277b9e3f79a371e28ad3e8052c8d581f..0331d3497b90d478463ea1a7a1ed7bda09b0ba43 100644 (file)
--- a/tests/storagepoolxml2argvtest.c
+++ b/tests/storagepoolxml2argvtest.c
@@ -96,6 +96,8 @@ testCompareXMLToArgvFiles(bool shouldFail,
 struct testInfo {
     bool shouldFail;
     const char *pool;
+    bool linuxOut;
+    bool freebsdOut;
 };
 
 static int
@@ -110,9 +112,19 @@ testCompareXMLToArgvHelper(const void *data)
                     abs_srcdir, info->pool) < 0)
         goto cleanup;
 
-    if (virAsprintf(&cmdline, "%s/storagepoolxml2argvdata/%s.argv",
-                    abs_srcdir, info->pool) < 0 && !info->shouldFail)
-        goto cleanup;
+    if (info->linuxOut) {
+        if (virAsprintf(&cmdline, "%s/storagepoolxml2argvdata/%s-linux.argv",
+                        abs_srcdir, info->pool) < 0 && !info->shouldFail)
+            goto cleanup;
+    } else if (info->freebsdOut) {
+        if (virAsprintf(&cmdline, "%s/storagepoolxml2argvdata/%s-freebsd.argv",
+                        abs_srcdir, info->pool) < 0 && !info->shouldFail)
+            goto cleanup;
+    } else {
+        if (virAsprintf(&cmdline, "%s/storagepoolxml2argvdata/%s.argv",
+                        abs_srcdir, info->pool) < 0 && !info->shouldFail)
+            goto cleanup;
+    }
 
     result = testCompareXMLToArgvFiles(info->shouldFail, poolxml, cmdline);
 
@@ -129,9 +141,9 @@ mymain(void)
 {
     int ret = 0;
 
-#define DO_TEST_FULL(shouldFail, pool) \
+#define DO_TEST_FULL(shouldFail, pool, linuxOut, freebsdOut) \
     do { \
-        struct testInfo info = { shouldFail, pool }; \
+        struct testInfo info = { shouldFail, pool, linuxOut, freebsdOut }; \
         if (virTestRun("Storage Pool XML-2-argv " pool, \
                        testCompareXMLToArgvHelper, &info) < 0) \
             ret = -1; \
@@ -139,14 +151,19 @@ mymain(void)
     while (0);
 
 #define DO_TEST(pool, ...) \
-    DO_TEST_FULL(false, pool)
+    DO_TEST_FULL(false, pool, false, false)
 
 #define DO_TEST_FAIL(pool, ...) \
-    DO_TEST_FULL(true, pool)
+    DO_TEST_FULL(true, pool, false, false)
+
+#define DO_TEST_LINUX(pool, ...) \
+    DO_TEST_FULL(false, pool, true, false)
+
+#define DO_TEST_FREEBSD(pool, ...) \
+    DO_TEST_FULL(false, pool, false, true)
 
     DO_TEST_FAIL("pool-dir");
     DO_TEST_FAIL("pool-dir-naming");
-    DO_TEST("pool-fs");
     DO_TEST("pool-logical");
     DO_TEST("pool-logical-nopath");
     DO_TEST("pool-logical-create");
@@ -155,10 +172,25 @@ mymain(void)
     DO_TEST_FAIL("pool-disk-device-nopartsep");
     DO_TEST_FAIL("pool-iscsi");
     DO_TEST_FAIL("pool-iscsi-auth");
+#ifdef __linux__
+    DO_TEST_LINUX("pool-fs");
+    DO_TEST_LINUX("pool-netfs");
+    DO_TEST_LINUX("pool-netfs-auto");
+    DO_TEST_LINUX("pool-netfs-gluster");
+    DO_TEST_LINUX("pool-netfs-cifs");
+#elif defined(__FreeBSD__)
+    DO_TEST_FREEBSD("pool-fs");
+    DO_TEST_FREEBSD("pool-netfs");
+    DO_TEST_FREEBSD("pool-netfs-auto");
+    DO_TEST_FREEBSD("pool-netfs-gluster");
+    DO_TEST_FREEBSD("pool-netfs-cifs");
+#else
+    DO_TEST("pool-fs");
     DO_TEST("pool-netfs");
     DO_TEST("pool-netfs-auto");
     DO_TEST("pool-netfs-gluster");
     DO_TEST("pool-netfs-cifs");
+#endif
     DO_TEST_FAIL("pool-scsi");
     DO_TEST_FAIL("pool-scsi-type-scsi-host");
     DO_TEST_FAIL("pool-scsi-type-fc-host");