7:43 a.m.
On 1/29/19 6:44 AM, Daniel P. Berrangé wrote:
On Thu, Jan 17, 2019 at 04:22:07PM -0500, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1584663
>
> Modify the command generation to add some default options to
> an NFS Storage Pool 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(a)redhat.com>
> ---
> src/storage/storage_util.c | 16 ++++++++
> .../pool-netfs-auto-freebsd.argv | 1 +
> .../pool-netfs-auto-linux.argv | 1 +
> .../pool-netfs-freebsd.argv | 1 +
> .../pool-netfs-linux.argv | 1 +
> tests/storagepoolxml2argvtest.c | 40 +++++++++++++++----
> 6 files changed, 53 insertions(+), 7 deletions(-)
> create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-auto-freebsd.argv
> create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-auto-linux.argv
> create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-freebsd.argv
> create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-linux.argv
>
> diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
> index a84ee5b600..44a95d9fab 100644
> --- 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_nfs_mount_opts "nodev,nosuid,noexec"
> +#elif defined(__FreeBSD__)
> +# define default_nfs_mount_opts "nosuid,noexec"
> +#else
> +# define default_nfs_mount_opts ""
> #endif
>
> #if WITH_BLKID
> @@ -4261,12 +4266,21 @@ virStorageBackendFileSystemGetPoolSource(virStoragePoolObjPtr
pool)
> }
>
>
> +static void
> +virStorageBackendFileSystemMountNFSAddOptions(virCommandPtr cmd)
> +{
> + if (*default_nfs_mount_opts != '\0')
> + virCommandAddArgList(cmd, "-o", default_nfs_mount_opts, NULL);
> +}
> +
> +
> static void
> virStorageBackendFileSystemMountNFSArgs(virCommandPtr cmd,
> const char *src,
> virStoragePoolDefPtr def)
> {
> virCommandAddArgList(cmd, src, def->target.path, NULL);
> + virStorageBackendFileSystemMountNFSAddOptions(cmd);
> }
>
>
> @@ -4308,6 +4322,8 @@ virStorageBackendFileSystemMountDefaultArgs(virCommandPtr cmd,
> else
> fmt = virStoragePoolFormatFileSystemNetTypeToString(def->source.format);
> virCommandAddArgList(cmd, "-t", fmt, src, def->target.path, NULL);
> + if (def->type == VIR_STORAGE_POOL_NETFS)
> + virStorageBackendFileSystemMountNFSAddOptions(cmd);
This doesn't need to be restricted to just NFS. A kind of filesystem we mount
as a storage pool should use these extra options, as they are general purpose
mount options, not FS specific.
Just s/nfs//i in all the method names / constants.
I can modify default_nfs_mount_opts to be just default_mount_opts;
however, as you see going forward the new method ends up being used by
patch4 for "nfsvers=%u," and patch9 for Namespace options, so I thinking
keeping the name virStorageBackendFileSystemMountNFSAddOptions is better.
Fair enough?
John
With diffs squashed in:
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 44a95d9fab..8d78162a5a 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -34,11 +34,11 @@
# ifndef FS_NOCOW_FL
# define FS_NOCOW_FL 0x00800000 /* Do not cow file */
# endif
-# define default_nfs_mount_opts "nodev,nosuid,noexec"
+# define default_mount_opts "nodev,nosuid,noexec"
#elif defined(__FreeBSD__)
-# define default_nfs_mount_opts "nosuid,noexec"
+# define default_mount_opts "nosuid,noexec"
#else
-# define default_nfs_mount_opts ""
+# define default_mount_opts ""
#endif
#if WITH_BLKID
@@ -4269,8 +4269,8 @@
virStorageBackendFileSystemGetPoolSource(virStoragePoolObjPtr pool)
static void
virStorageBackendFileSystemMountNFSAddOptions(virCommandPtr cmd)
{
- if (*default_nfs_mount_opts != '\0')
- virCommandAddArgList(cmd, "-o", default_nfs_mount_opts, NULL);
+ if (*default_mount_opts != '\0')
+ virCommandAddArgList(cmd, "-o", default_mount_opts, NULL);
}