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(a)redhat.com&gt; --- src/storage/storage_util.c | 36 ++++++++++++-- .../pool-fs-freebsd.argv | 1 + .../pool-fs-linux.argv | 1 + .../pool-netfs-auto-freebsd.argv | 1 + .../pool-netfs-auto-linux.argv | 1 + .../pool-netfs-cifs-freebsd.argv | 1 + .../pool-netfs-cifs-linux.argv | 1 + .../pool-netfs-freebsd.argv | 1 + .../pool-netfs-gluster-freebsd.argv | 2 + .../pool-netfs-gluster-linux.argv | 2 + .../pool-netfs-linux.argv | 1 + tests/storagepoolxml2argvtest.c | 48 +++++++++++++++---- 12 files changed, 84 insertions(+), 12 deletions(-) create mode 100644 tests/storagepoolxml2argvdata/pool-fs-freebsd.argv create mode 100644 tests/storagepoolxml2argvdata/pool-fs-linux.argv 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-cifs-freebsd.argv create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-cifs-linux.argv create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-freebsd.argv create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-gluster-freebsd.argv create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-gluster-linux.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..d63237ce8f 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_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 index 0000000000..a35d73e254 --- /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 index 0000000000..19543f442d --- /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 index 0000000000..39e5c97aed --- /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 index 0000000000..1f82d3d29c --- /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 index 0000000000..d72749a032 --- /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 index 0000000000..85aa9cf23f --- /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 index 0000000000..05c1951f32 --- /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 index 0000000000..700107d78e --- /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 index 0000000000..9535c8a1b9 --- /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 index 0000000000..22fafd7b32 --- /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 2f2d40e027..0331d3497b 100644 --- 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"); -- 2.20.1

Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 5759a9e178..2aa4ed25d6 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -50,6 +50,17 @@ <section title="Improvements"> </section> <section title="Bug fixes"> + <change> + <summary> + storage: Add default mount options for fs/netfs storage pools + </summary> + <description> + Altered the command line generation for fs/netfs storage pools to + add some default options. For Linux based systems, the options + added are "nodev, nosuid, noexec". For FreeBSD based systems, + the options added are "nosuid, noexec". + </description> + </change> </section> </release> <release version="v5.0.0" date="2019-01-15"> -- 2.20.1

If protocolVer present, add the -o nfsvers=# to the command line for the NFS Storage Pool Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/storage/storage_util.c | 19 +++++++++++++------ .../pool-netfs-protocol-ver-freebsd.argv | 1 + .../pool-netfs-protocol-ver-linux.argv | 2 ++ .../pool-netfs-protocol-ver.argv | 1 + tests/storagepoolxml2argvtest.c | 3 +++ 5 files changed, 20 insertions(+), 6 deletions(-) create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-freebsd.argv create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-linux.argv create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-protocol-ver.argv diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index d63237ce8f..a5cb47b4d0 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -4290,10 +4290,11 @@ virStorageBackendFileSystemMountAddOptions(virCommandPtr cmd, static void virStorageBackendFileSystemMountNFSArgs(virCommandPtr cmd, const char *src, - virStoragePoolDefPtr def) + virStoragePoolDefPtr def, + const char *nfsVers) { virCommandAddArgList(cmd, src, def->target.path, NULL); - virStorageBackendFileSystemMountAddOptions(cmd, NULL); + virStorageBackendFileSystemMountAddOptions(cmd, nfsVers); } @@ -4326,7 +4327,8 @@ virStorageBackendFileSystemMountCIFSArgs(virCommandPtr cmd, static void virStorageBackendFileSystemMountDefaultArgs(virCommandPtr cmd, const char *src, - virStoragePoolDefPtr def) + virStoragePoolDefPtr def, + const char *nfsVers) { const char *fmt; @@ -4335,7 +4337,7 @@ virStorageBackendFileSystemMountDefaultArgs(virCommandPtr cmd, else fmt = virStoragePoolFormatFileSystemNetTypeToString(def->source.format); virCommandAddArgList(cmd, "-t", fmt, src, def->target.path, NULL); - virStorageBackendFileSystemMountAddOptions(cmd, NULL); + virStorageBackendFileSystemMountAddOptions(cmd, nfsVers); } @@ -4354,16 +4356,21 @@ virStorageBackendFileSystemMountCmd(const char *cmdstr, bool cifsfs = (def->type == VIR_STORAGE_POOL_NETFS && def->source.format == VIR_STORAGE_POOL_NETFS_CIFS); virCommandPtr cmd = NULL; + VIR_AUTOFREE(char *) nfsVers = NULL; + + if (def->type == VIR_STORAGE_POOL_NETFS && def->source.protocolVer > 0 && + virAsprintf(&nfsVers, "nfsvers=%u", def->source.protocolVer) < 0) + return NULL; cmd = virCommandNew(cmdstr); if (netauto) - virStorageBackendFileSystemMountNFSArgs(cmd, src, def); + virStorageBackendFileSystemMountNFSArgs(cmd, src, def, nfsVers); else if (glusterfs) virStorageBackendFileSystemMountGlusterArgs(cmd, src, def); else if (cifsfs) virStorageBackendFileSystemMountCIFSArgs(cmd, src, def); else - virStorageBackendFileSystemMountDefaultArgs(cmd, src, def); + virStorageBackendFileSystemMountDefaultArgs(cmd, src, def, nfsVers); return cmd; } diff --git a/tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-freebsd.argv b/tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-freebsd.argv new file mode 100644 index 0000000000..59d09d2e5d --- /dev/null +++ b/tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-freebsd.argv @@ -0,0 +1 @@ +mount -t nfs localhost:/var/lib/libvirt/images /mnt -o nosuid,noexec,nfsvers=3 diff --git a/tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-linux.argv b/tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-linux.argv new file mode 100644 index 0000000000..c819a089d2 --- /dev/null +++ b/tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-linux.argv @@ -0,0 +1,2 @@ +mount -t nfs localhost:/var/lib/libvirt/images /mnt -o nodev,nosuid,noexec,\ +nfsvers=3 diff --git a/tests/storagepoolxml2argvdata/pool-netfs-protocol-ver.argv b/tests/storagepoolxml2argvdata/pool-netfs-protocol-ver.argv new file mode 100644 index 0000000000..f26656d5b8 --- /dev/null +++ b/tests/storagepoolxml2argvdata/pool-netfs-protocol-ver.argv @@ -0,0 +1 @@ +mount -t nfs localhost:/var/lib/libvirt/images /mnt -o nfsvers=3 diff --git a/tests/storagepoolxml2argvtest.c b/tests/storagepoolxml2argvtest.c index 0331d3497b..88059cbdfc 100644 --- a/tests/storagepoolxml2argvtest.c +++ b/tests/storagepoolxml2argvtest.c @@ -176,18 +176,21 @@ mymain(void) DO_TEST_LINUX("pool-fs"); DO_TEST_LINUX("pool-netfs"); DO_TEST_LINUX("pool-netfs-auto"); + DO_TEST_LINUX("pool-netfs-protocol-ver"); 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-protocol-ver"); 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-protocol-ver"); DO_TEST("pool-netfs-gluster"); DO_TEST("pool-netfs-cifs"); #endif -- 2.20.1

Allow the addition of the <protocol ver='n'/> to the provided XML. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- tools/virsh-pool.c | 12 ++++++++++-- tools/virsh.pod | 5 +++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 70ca39bd3d..9514570468 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -136,6 +136,10 @@ {.name = "adapter-parent-fabric-wwn", \ .type = VSH_OT_STRING, \ .help = N_("adapter parent scsi_hostN fabric_wwn to be used for underlying vHBA storage") \ + }, \ + {.name = "source-protocol-ver", \ + .type = VSH_OT_STRING, \ + .help = N_("nfsvers value for NFS pool mount option") \ } virStoragePoolPtr @@ -319,7 +323,7 @@ virshBuildPoolXML(vshControl *ctl, *secretUsage = NULL, *adapterName = NULL, *adapterParent = NULL, *adapterWwnn = NULL, *adapterWwpn = NULL, *secretUUID = NULL, *adapterParentWwnn = NULL, *adapterParentWwpn = NULL, - *adapterParentFabricWwn = NULL; + *adapterParentFabricWwn = NULL, *protoVer = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; VSH_EXCLUSIVE_OPTIONS("secret-usage", "secret-uuid"); @@ -345,7 +349,8 @@ virshBuildPoolXML(vshControl *ctl, vshCommandOptStringReq(ctl, cmd, "adapter-parent", &adapterParent) < 0 || vshCommandOptStringReq(ctl, cmd, "adapter-parent-wwnn", &adapterParentWwnn) < 0 || vshCommandOptStringReq(ctl, cmd, "adapter-parent-wwpn", &adapterParentWwpn) < 0 || - vshCommandOptStringReq(ctl, cmd, "adapter-parent-fabric-wwn", &adapterParentFabricWwn) < 0) + vshCommandOptStringReq(ctl, cmd, "adapter-parent-fabric-wwn", &adapterParentFabricWwn) < 0 || + vshCommandOptStringReq(ctl, cmd, "source-protocol-ver", &protoVer) < 0) goto cleanup; virBufferAsprintf(&buf, "<pool type='%s'>\n", type); @@ -394,6 +399,9 @@ virshBuildPoolXML(vshControl *ctl, if (srcName) virBufferAsprintf(&buf, "<name>%s</name>\n", srcName); + if (protoVer) + virBufferAsprintf(&buf, "<protocol ver='%s'/>\n", protoVer); + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</source>\n"); } diff --git a/tools/virsh.pod b/tools/virsh.pod index 86a4996cae..59a5900162 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3887,6 +3887,7 @@ just I<--build> is provided, then B<pool-build> is called with no flags. [I<--source-name name>] [I<--target path>] [I<--source-format format>] [I<--auth-type authtype> I<--auth-username username> [I<--secret-usage usage> | I<--secret-uuid uuid>]] +[I<--source-protocol-ver ver>] [[I<--adapter-name name>] | [I<--adapter-wwnn> wwnn I<--adapter-wwpn> wwpn] [I<--adapter-parent parent> | I<--adapter-parent-wwnn parent_wwnn> I<adapter-parent-wwpn parent_wwpn> | @@ -3929,6 +3930,10 @@ the storage pool. The I<authtype> is either chap for iscsi I<type> pools or ceph for rbd I<type> pools. Either the secret I<usage> or I<uuid> value may be provided, but not both. +[I<--source-protocol-ver ver>] provides the NFS protocol version number used +to contact the server's NFS service via nfs mount option 'nfsvers=n'. It is +expect the I<ver> value is an unsigned integer. + [I<--adapter-name name>] defines the scsi_hostN adapter name to be used for the scsi_host adapter type pool. -- 2.20.1

Introduce the virStoragePoolFSMountOptionsDef to be used to manage the Storage Pool XML Namespace for mount options. Using a new virStorageBackendNamespaceInit function, set the virStoragePoolXMLNamespace into the _virStoragePoolOptions when the storage backend is loaded. Modify the storagepool.rng to allow for the usage of a different XML namespace to parse the fs_mount_opts to be included with the fs and netfs storage pool definitions. Modify the storagepoolxml2xmltest to utilize a properly modified XML file to parse and format the namespace for a netfs storage pool. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- docs/formatstorage.html.in | 63 +++++++++ docs/schemas/storagepool.rng | 23 +++ src/storage/storage_backend_fs.c | 132 ++++++++++++++++++ src/storage/storage_util.c | 16 +++ src/storage/storage_util.h | 14 ++ tests/Makefile.am | 4 +- .../pool-netfs-ns-mountopts.xml | 25 ++++ .../pool-netfs-ns-mountopts.xml | 25 ++++ tests/storagepoolxml2xmltest.c | 6 + 9 files changed, 307 insertions(+), 1 deletion(-) create mode 100644 tests/storagepoolxml2xmlin/pool-netfs-ns-mountopts.xml create mode 100644 tests/storagepoolxml2xmlout/pool-netfs-ns-mountopts.xml diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 85107c85ae..7a79ec82d8 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -508,6 +508,69 @@ device, measured in bytes. <span class="since">Since 0.4.1</span> </p> + <h3><a id="StoragePoolNamespaces">Storage Pool Namespaces</a></h3> + + <p> + Usage of Storage Pool Namespaces provides a mechanism to provide + pool type specific data in a free form or arbitrary manner via + XML syntax targeted solely for the needs of the specific pool type + which is not otherwise supported in standard XML. For the "fs" and + "netfs" pool types this provides a mechanism to provide additional + mount options on the command line. + </p> + <p> + Usage of namespaces comes with no support guarantees. It is intended + for developers testing out a concept prior to requesting an explicitly + supported XML option in libvirt, and thus should never be used in + production. + </p> + <dl> + <dt><code>fs:mount_opts</code></dt> + <dd>Provides an XML namespace mechanism to optionally utilize + specifically named options for the mount command via the "-o" + option for the <code>fs</code> or <code>netfs</code> type storage + pools. In order to designate that the Storage Pool will be using + the mechanism, the <code>pool</code> element must be modified to + provide the XML namespace attribute syntax as follows: + + <p> + xmlns:fs='http://libvirt.org/schemas/storagepool/source/fs/1.0' + </p> + + <p> + The <code>fs:mount_opts</code> defines the mount options by + specifying multiple <code>fs:option</code> subelements with + the attribute <code>name</code> specifying the mount option to + be added. The value of the named option is not checked since + it's possible options don't exist on all distributions. It is + expected that proper and valid options will be supplied for the + target host. + </p> + + The following XML snippet shows the syntax required in order to + utilize for a netfs pool: + <pre> +&lt;pool type="netfs" xmlns:fs='http://libvirt.org/schemas/storagepool/source/fs/1.0'&a... + &lt;name&gt;nfsimages&lt;/name&gt; +... + &lt;source&gt; +... + &lt;/source&gt; +... + &lt;target&gt; +... + &lt;/target&gt; + &lt;fs:mount_opts&gt; + &lt;fs:option name='sync'/&gt; + &lt;fs:option name='lazytime'/&gt; + &lt;/fs:mount_opts&gt; +&lt;/pool&gt; +...</pre> + + <span class="since">Since 5.1.0.</span></dd> + + </dl> + <h2><a id="StorageVol">Storage volume XML</a></h2> <p> A storage volume will generally be either a file or a device diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index f9a16422cc..0b359669af 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -52,6 +52,9 @@ <ref name='sourcefs'/> <ref name='target'/> </interleave> + <optional> + <ref name='fs_mount_opts'/> + </optional> </define> <define name='poolnetfs'> @@ -64,6 +67,9 @@ <ref name='sourcenetfs'/> <ref name='target'/> </interleave> + <optional> + <ref name='fs_mount_opts'/> + </optional> </define> <define name='poollogical'> @@ -682,4 +688,21 @@ </data> </define> + <!-- + Optional storage pool extensions in their own namespace: + "fs" or "netfs" + --> + + <define name="fs_mount_opts"> + <element name="mount_opts" ns="http://libvirt.org/schemas/storagepool/source/fs/1.0"> + <zeroOrMore> + <element name="option"> + <attribute name='name'> + <text/> + </attribute> + </element> + </zeroOrMore> + </element> + </define> + </grammar> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index dc9869417e..0ec99c60ed 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -41,6 +41,7 @@ VIR_LOG_INIT("storage.storage_backend_fs"); #if WITH_STORAGE_FS +# include <libxml/xpathInternals.h> # include <mntent.h> struct _virNetfsDiscoverState { @@ -559,6 +560,122 @@ virStorageBackendFileSystemBuild(virStoragePoolObjPtr pool, } +#if WITH_STORAGE_FS + +# define STORAGE_POOL_FS_NAMESPACE_HREF "http://libvirt.org/schemas/storagepool/source/fs/1.0" + +/* Backend XML Namespace handling for fs or netfs specific mount options to + * be added to the mount -o {options_list} command line that are not otherwise + * supplied by supported XML. The XML will use the format, such as: + * + * <fs:mount_opts> + * <fs:option name='sync'/> + * <fs:option name='lazytime'/> + * </fs:mount_opts> + * + * and the <pool type='fs'> or <pool type='netfs'> is required to have a + * "xmlns:fs='%s'" attribute using the STORAGE_POOL_FS_NAMESPACE_HREF + */ + +static void +virStoragePoolDefFSNamespaceFree(void *nsdata) +{ + virStoragePoolFSMountOptionsDefPtr cmdopts = nsdata; + size_t i; + + if (!cmdopts) + return; + + for (i = 0; i < cmdopts->noptions; i++) + VIR_FREE(cmdopts->options[i]); + VIR_FREE(cmdopts->options); + + VIR_FREE(cmdopts); +} + + +static int +virStoragePoolDefFSNamespaceParse(xmlXPathContextPtr ctxt, + void **data) +{ + virStoragePoolFSMountOptionsDefPtr cmdopts = NULL; + xmlNodePtr *nodes = NULL; + int nnodes; + size_t i; + int ret = -1; + + if (xmlXPathRegisterNs(ctxt, BAD_CAST "fs", + BAD_CAST STORAGE_POOL_FS_NAMESPACE_HREF) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to register xml namespace '%s'"), + STORAGE_POOL_FS_NAMESPACE_HREF); + return -1; + } + + nnodes = virXPathNodeSet("./fs:mount_opts/fs:option", ctxt, &nodes); + if (nnodes < 0) + return -1; + + if (nnodes == 0) + return 0; + + if (VIR_ALLOC(cmdopts) < 0 || + VIR_ALLOC_N(cmdopts->options, nnodes) < 0) + goto cleanup; + + for (i = 0; i < nnodes; i++) { + if (!(cmdopts->options[cmdopts->noptions] = + virXMLPropString(nodes[i], "name"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("no fs mount option name specified")); + goto cleanup; + } + cmdopts->noptions++; + } + + VIR_STEAL_PTR(*data, cmdopts); + ret = 0; + + cleanup: + VIR_FREE(nodes); + virStoragePoolDefFSNamespaceFree(cmdopts); + return ret; +} + + +static int +virStoragePoolDefFSNamespaceFormatXML(virBufferPtr buf, + void *nsdata) +{ + size_t i; + virStoragePoolFSMountOptionsDefPtr def = nsdata; + + if (!def) + return 0; + + virBufferAddLit(buf, "<fs:mount_opts>\n"); + virBufferAdjustIndent(buf, 2); + + for (i = 0; i < def->noptions; i++) + virBufferEscapeString(buf, "<fs:option name='%s'/>\n", + def->options[i]); + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</fs:mount_opts>\n"); + + return 0; +} + + +static const char * +virStoragePoolDefFSNamespaceHref(void) +{ + return "xmlns:fs='" STORAGE_POOL_FS_NAMESPACE_HREF "'"; +} + +#endif /* WITH_STORAGE_FS */ + + virStorageBackend virStorageBackendDirectory = { .type = VIR_STORAGE_POOL_DIR, @@ -617,6 +734,13 @@ virStorageBackend virStorageBackendNetFileSystem = { .downloadVol = virStorageBackendVolDownloadLocal, .wipeVol = virStorageBackendVolWipeLocal, }; + +static virStoragePoolXMLNamespace virStoragePoolFSXMLNamespace = { + .parse = virStoragePoolDefFSNamespaceParse, + .free = virStoragePoolDefFSNamespaceFree, + .format = virStoragePoolDefFSNamespaceFormatXML, + .href = virStoragePoolDefFSNamespaceHref, +}; #endif /* WITH_STORAGE_FS */ @@ -630,8 +754,16 @@ virStorageBackendFsRegister(void) if (virStorageBackendRegister(&virStorageBackendFileSystem) < 0) return -1; + if (virStorageBackendNamespaceInit(VIR_STORAGE_POOL_FS, + &virStoragePoolFSXMLNamespace) < 0) + return -1; + if (virStorageBackendRegister(&virStorageBackendNetFileSystem) < 0) return -1; + + if (virStorageBackendNamespaceInit(VIR_STORAGE_POOL_NETFS, + &virStoragePoolFSXMLNamespace) < 0) + return -1; #endif /* WITH_STORAGE_FS */ return 0; diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index a5cb47b4d0..4596cd4518 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -82,6 +82,22 @@ VIR_LOG_INIT("storage.storage_util"); + +/* virStorageBackendNamespaceInit: + * @poolType: virStoragePoolType + * @xmlns: Storage Pool specific namespace callback methods + * + * To be called during storage backend registration to configure the + * Storage Pool XML Namespace based on the backend's needs. + */ +int +virStorageBackendNamespaceInit(int poolType, + virStoragePoolXMLNamespacePtr xmlns) +{ + return virStoragePoolOptionsPoolTypeSetXMLNamespace(poolType, xmlns); +} + + #define READ_BLOCK_SIZE_DEFAULT (1024 * 1024) #define WRITE_BLOCK_SIZE_DEFAULT (4 * 1024) diff --git a/src/storage/storage_util.h b/src/storage/storage_util.h index c872468135..8b44b54a3b 100644 --- a/src/storage/storage_util.h +++ b/src/storage/storage_util.h @@ -26,6 +26,20 @@ # include "storage_driver.h" # include "storage_backend.h" +/* Storage Pool Namespace options to share w/ storage_backend_fs.c and + * the virStorageBackendFileSystemMountCmd method */ +typedef struct _virStoragePoolFSMountOptionsDef virStoragePoolFSMountOptionsDef; +typedef virStoragePoolFSMountOptionsDef *virStoragePoolFSMountOptionsDefPtr; +struct _virStoragePoolFSMountOptionsDef { + size_t noptions; + char **options; +}; + +int +virStorageBackendNamespaceInit(int poolType, + virStoragePoolXMLNamespacePtr xmlns); + + /* File creation/cloning functions used for cloning between backends */ int diff --git a/tests/Makefile.am b/tests/Makefile.am index f74d8463b6..ab4c716529 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -937,7 +937,9 @@ storagevolxml2xmltest_LDADD = $(LDADDS) storagepoolxml2xmltest_SOURCES = \ storagepoolxml2xmltest.c \ testutils.c testutils.h -storagepoolxml2xmltest_LDADD = $(LDADDS) +storagepoolxml2xmltest_LDADD = $(LDADDS) \ + ../src/libvirt_driver_storage_impl.la \ + $(GNULIB_LIBS) nodedevxml2xmltest_SOURCES = \ nodedevxml2xmltest.c \ diff --git a/tests/storagepoolxml2xmlin/pool-netfs-ns-mountopts.xml b/tests/storagepoolxml2xmlin/pool-netfs-ns-mountopts.xml new file mode 100644 index 0000000000..0434b16eb7 --- /dev/null +++ b/tests/storagepoolxml2xmlin/pool-netfs-ns-mountopts.xml @@ -0,0 +1,25 @@ +<pool type='netfs' xmlns:fs='http://libvirt.org/schemas/storagepool/source/fs/1.0'> + <name>nfsimages</name> + <uuid>7641d5a8-af11-f730-a34e-0a7dfcede71f</uuid> + <capacity>0</capacity> + <allocation>0</allocation> + <available>0</available> + <source> + <host name='localhost'/> + <dir path='/var/lib/libvirt/images'/> + <format type='nfs'/> + <protocol ver='3'/> + </source> + <target> + <path>/mnt</path> + <permissions> + <mode>0700</mode> + <owner>0</owner> + <group>0</group> + </permissions> + </target> + <fs:mount_opts> + <fs:option name='sync'/> + <fs:option name='lazytime'/> + </fs:mount_opts> +</pool> diff --git a/tests/storagepoolxml2xmlout/pool-netfs-ns-mountopts.xml b/tests/storagepoolxml2xmlout/pool-netfs-ns-mountopts.xml new file mode 100644 index 0000000000..4bd164f220 --- /dev/null +++ b/tests/storagepoolxml2xmlout/pool-netfs-ns-mountopts.xml @@ -0,0 +1,25 @@ +<pool type='netfs' xmlns:fs='http://libvirt.org/schemas/storagepool/source/fs/1.0'> + <name>nfsimages</name> + <uuid>7641d5a8-af11-f730-a34e-0a7dfcede71f</uuid> + <capacity unit='bytes'>0</capacity> + <allocation unit='bytes'>0</allocation> + <available unit='bytes'>0</available> + <source> + <host name='localhost'/> + <dir path='/var/lib/libvirt/images'/> + <format type='nfs'/> + <protocol ver='3'/> + </source> + <target> + <path>/mnt</path> + <permissions> + <mode>0700</mode> + <owner>0</owner> + <group>0</group> + </permissions> + </target> + <fs:mount_opts> + <fs:option name='sync'/> + <fs:option name='lazytime'/> + </fs:mount_opts> +</pool> diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c index d18390034f..aff9ff160c 100644 --- a/tests/storagepoolxml2xmltest.c +++ b/tests/storagepoolxml2xmltest.c @@ -11,6 +11,8 @@ #include "testutilsqemu.h" #include "virstring.h" +#include "storage/storage_util.h" + #define VIR_FROM_THIS VIR_FROM_NONE static int @@ -70,6 +72,9 @@ mymain(void) testCompareXMLToXMLHelper, (name)) < 0) \ ret = -1 + if (storageRegisterAll() < 0) + return EXIT_FAILURE; + DO_TEST("pool-dir"); DO_TEST("pool-dir-naming"); DO_TEST("pool-fs"); @@ -86,6 +91,7 @@ mymain(void) DO_TEST("pool-netfs-protocol-ver"); DO_TEST("pool-netfs-gluster"); DO_TEST("pool-netfs-cifs"); + DO_TEST("pool-netfs-ns-mountopts"); DO_TEST("pool-scsi"); DO_TEST("pool-scsi-type-scsi-host"); DO_TEST("pool-scsi-type-fc-host"); -- 2.20.1

If the Storage Pool Namespace XML data exists, format the mount options on the MOUNT command line and issue a VIR_WARN to indicate that the storage pool was tainted by custom mount_opts. When the pool is started, the options will be generated on the command line along with the options already defined. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/storage/storage_util.c | 22 +++++++++++++++---- .../pool-netfs-ns-mountopts-freebsd.argv | 2 ++ .../pool-netfs-ns-mountopts-linux.argv | 2 ++ .../pool-netfs-ns-mountopts.argv | 1 + tests/storagepoolxml2argvtest.c | 6 +++++ 5 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-ns-mountopts-freebsd.argv create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-ns-mountopts-linux.argv create mode 100644 tests/storagepoolxml2argvdata/pool-netfs-ns-mountopts.argv diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 4596cd4518..37b3d58667 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -4284,6 +4284,7 @@ virStorageBackendFileSystemGetPoolSource(virStoragePoolObjPtr pool) static void virStorageBackendFileSystemMountAddOptions(virCommandPtr cmd, + virStoragePoolDefPtr def, const char *providedOpts) { VIR_AUTOFREE(char *) mountOpts = NULL; @@ -4295,6 +4296,19 @@ virStorageBackendFileSystemMountAddOptions(virCommandPtr cmd, if (providedOpts) virBufferAsprintf(&buf, "%s,", providedOpts); + if (def->namespaceData) { + size_t i; + virStoragePoolFSMountOptionsDefPtr opts = def->namespaceData; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + for (i = 0; i < opts->noptions; i++) + virBufferAsprintf(&buf, "%s,", opts->options[i]); + + virUUIDFormat(def->uuid, uuidstr); + VIR_WARN("Storage Pool name='%s' uuid='%s' is tainted by custom " + "mount_opts from XML", def->name, uuidstr); + } + virBufferTrim(&buf, ",", -1); mountOpts = virBufferContentAndReset(&buf); @@ -4310,7 +4324,7 @@ virStorageBackendFileSystemMountNFSArgs(virCommandPtr cmd, const char *nfsVers) { virCommandAddArgList(cmd, src, def->target.path, NULL); - virStorageBackendFileSystemMountAddOptions(cmd, nfsVers); + virStorageBackendFileSystemMountAddOptions(cmd, def, nfsVers); } @@ -4323,7 +4337,7 @@ virStorageBackendFileSystemMountGlusterArgs(virCommandPtr cmd, fmt = virStoragePoolFormatFileSystemNetTypeToString(def->source.format); virCommandAddArgList(cmd, "-t", fmt, src, def->target.path, NULL); - virStorageBackendFileSystemMountAddOptions(cmd, "direct-io-mode=1"); + virStorageBackendFileSystemMountAddOptions(cmd, def, "direct-io-mode=1"); } @@ -4336,7 +4350,7 @@ virStorageBackendFileSystemMountCIFSArgs(virCommandPtr cmd, fmt = virStoragePoolFormatFileSystemNetTypeToString(def->source.format); virCommandAddArgList(cmd, "-t", fmt, src, def->target.path, NULL); - virStorageBackendFileSystemMountAddOptions(cmd, "guest"); + virStorageBackendFileSystemMountAddOptions(cmd, def, "guest"); } @@ -4353,7 +4367,7 @@ virStorageBackendFileSystemMountDefaultArgs(virCommandPtr cmd, else fmt = virStoragePoolFormatFileSystemNetTypeToString(def->source.format); virCommandAddArgList(cmd, "-t", fmt, src, def->target.path, NULL); - virStorageBackendFileSystemMountAddOptions(cmd, nfsVers); + virStorageBackendFileSystemMountAddOptions(cmd, def, nfsVers); } diff --git a/tests/storagepoolxml2argvdata/pool-netfs-ns-mountopts-freebsd.argv b/tests/storagepoolxml2argvdata/pool-netfs-ns-mountopts-freebsd.argv new file mode 100644 index 0000000000..ac5c0acd00 --- /dev/null +++ b/tests/storagepoolxml2argvdata/pool-netfs-ns-mountopts-freebsd.argv @@ -0,0 +1,2 @@ +mount -t nfs localhost:/var/lib/libvirt/images /mnt -o nosuid,noexec,nfsvers=3,\ +sync,lazytime diff --git a/tests/storagepoolxml2argvdata/pool-netfs-ns-mountopts-linux.argv b/tests/storagepoolxml2argvdata/pool-netfs-ns-mountopts-linux.argv new file mode 100644 index 0000000000..8e10379c04 --- /dev/null +++ b/tests/storagepoolxml2argvdata/pool-netfs-ns-mountopts-linux.argv @@ -0,0 +1,2 @@ +mount -t nfs localhost:/var/lib/libvirt/images /mnt -o nodev,nosuid,noexec,\ +nfsvers=3,sync,lazytime diff --git a/tests/storagepoolxml2argvdata/pool-netfs-ns-mountopts.argv b/tests/storagepoolxml2argvdata/pool-netfs-ns-mountopts.argv new file mode 100644 index 0000000000..a63d6da456 --- /dev/null +++ b/tests/storagepoolxml2argvdata/pool-netfs-ns-mountopts.argv @@ -0,0 +1 @@ +mount -t nfs localhost:/var/lib/libvirt/images /mnt -o nfsvers=3,sync,lazytime diff --git a/tests/storagepoolxml2argvtest.c b/tests/storagepoolxml2argvtest.c index 88059cbdfc..786fb26402 100644 --- a/tests/storagepoolxml2argvtest.c +++ b/tests/storagepoolxml2argvtest.c @@ -162,6 +162,9 @@ mymain(void) #define DO_TEST_FREEBSD(pool, ...) \ DO_TEST_FULL(false, pool, false, true) + if (storageRegisterAll() < 0) + return EXIT_FAILURE; + DO_TEST_FAIL("pool-dir"); DO_TEST_FAIL("pool-dir-naming"); DO_TEST("pool-logical"); @@ -177,6 +180,7 @@ mymain(void) DO_TEST_LINUX("pool-netfs"); DO_TEST_LINUX("pool-netfs-auto"); DO_TEST_LINUX("pool-netfs-protocol-ver"); + DO_TEST_LINUX("pool-netfs-ns-mountopts"); DO_TEST_LINUX("pool-netfs-gluster"); DO_TEST_LINUX("pool-netfs-cifs"); #elif defined(__FreeBSD__) @@ -184,6 +188,7 @@ mymain(void) DO_TEST_FREEBSD("pool-netfs"); DO_TEST_FREEBSD("pool-netfs-auto"); DO_TEST_FREEBSD("pool-netfs-protocol-ver"); + DO_TEST_FREEBSD("pool-netfs-ns-mountopts"); DO_TEST_FREEBSD("pool-netfs-gluster"); DO_TEST_FREEBSD("pool-netfs-cifs"); #else @@ -191,6 +196,7 @@ mymain(void) DO_TEST("pool-netfs"); DO_TEST("pool-netfs-auto"); DO_TEST("pool-netfs-protocol-ver"); + DO_TEST("pool-netfs-ns-mountopts"); DO_TEST("pool-netfs-gluster"); DO_TEST("pool-netfs-cifs"); #endif -- 2.20.1

Allow for adjustment of RBD configuration options via Storage Pool XML Namespace adjustments. When namespace arguments are used to start the pool, add a VIR_WARN to indicate that the startup was tainted by custom config_opts. Based off original patch/concept: https://www.redhat.com/archives/libvir-list/2014-May/msg00940.html Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- docs/formatstorage.html.in | 52 +++++- docs/schemas/storagepool.rng | 23 +++ src/storage/storage_backend_rbd.c | 169 +++++++++++++++++- .../pool-rbd-ns-configopts.xml | 17 ++ .../pool-rbd-ns-configopts.xml | 20 +++ tests/storagepoolxml2xmltest.c | 1 + 6 files changed, 278 insertions(+), 4 deletions(-) create mode 100644 tests/storagepoolxml2xmlin/pool-rbd-ns-configopts.xml create mode 100644 tests/storagepoolxml2xmlout/pool-rbd-ns-configopts.xml diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index 7a79ec82d8..d19bc579a4 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -516,7 +516,8 @@ XML syntax targeted solely for the needs of the specific pool type which is not otherwise supported in standard XML. For the "fs" and "netfs" pool types this provides a mechanism to provide additional - mount options on the command line. + mount options on the command line. For the "rbd" pool this provides + a mechanism to override default settings for RBD configuration options. </p> <p> Usage of namespaces comes with no support guarantees. It is intended @@ -569,6 +570,55 @@ <span class="since">Since 5.1.0.</span></dd> + <dt><code>rbd:config_opts</code></dt> + <dd>Provides an XML namespace mechanism to optionally utilize + specifically named options for the RBD configuration options + via the rados_conf_set API for the <code>rbd</code> type + storage pools. In order to designate that the Storage Pool + will be using the mechanism, the <code>pool</code> element + must be modified to provide the XML namespace attribute + syntax as follows: + + <p> + xmlns:rbd='http://libvirt.org/schemas/storagepool/source/rbd/1.0' + </p> + + <p> + The <code>rbd:config_opts</code> defines the configuration options + by specifying multiple <code>rbd:option</code> subelements with + the attribute <code>name</code> specifying the configuration option + to be added and <code>value</code> specifying the configuration + option value. The name and value for each option is only checked + to be not empty. The name and value provided are not checked since + it's possible options don't exist on all distributions. It is + expected that proper and valid options will be supplied for the + target host. + </p> + + The following XML snippet shows the syntax required in order to + utilize + <pre> +&lt;pool type="rbd" xmlns:rbd='http://libvirt.org/schemas/storagepool/source/rbd/1.0'... + &lt;name&gt;myrbdpool&lt;/name&gt; +... + &lt;source&gt; +... + &lt;/source&gt; +... + &lt;target&gt; +... + &lt;/target&gt; +... + &lt;rbd:config_opts&gt; + &lt;rbd:option name='client_mount_timeout' value='45'/&gt; + &lt;rbd:option name='rados_mon_op_timeout' value='20'/&gt; + &lt;rbd:option name='rados_osd_op_timeout' value='10'/&gt; + &lt;/rbd:config_opts&gt; +&lt;/pool&gt; +</pre> + + <span class="since">Since 5.1.0.</span></dd> + </dl> <h2><a id="StorageVol">Storage volume XML</a></h2> diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 0b359669af..e1944ff8e1 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -156,6 +156,9 @@ <ref name='sizing'/> <ref name='sourcerbd'/> </interleave> + <optional> + <ref name='rbd_config_opts'/> + </optional> </define> <define name='poolsheepdog'> @@ -705,4 +708,24 @@ </element> </define> + <!-- + Optional storage pool extensions in their own namespace: + RBD + --> + + <define name="rbd_config_opts"> + <element name="config_opts" ns="http://libvirt.org/schemas/storagepool/source/rbd/1.0"> + <zeroOrMore> + <element name="option"> + <attribute name='name'> + <text/> + </attribute> + <attribute name='value'> + <text/> + </attribute> + </element> + </zeroOrMore> + </element> + </define> + </grammar> diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 24dd1349ae..2348f80146 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -36,6 +36,7 @@ #include "rbd/librbd.h" #include "secret_util.h" #include "storage_util.h" +#include <libxml/xpathInternals.h> #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -50,6 +51,138 @@ struct _virStorageBackendRBDState { typedef struct _virStorageBackendRBDState virStorageBackendRBDState; typedef virStorageBackendRBDState *virStorageBackendRBDStatePtr; +typedef struct _virStoragePoolRBDConfigOptionsDef virStoragePoolRBDConfigOptionsDef; +typedef virStoragePoolRBDConfigOptionsDef *virStoragePoolRBDConfigOptionsDefPtr; +struct _virStoragePoolRBDConfigOptionsDef { + size_t noptions; + char **names; + char **values; +}; + +#define STORAGE_POOL_RBD_NAMESPACE_HREF "http://libvirt.org/schemas/storagepool/source/rbd/1.0" + +static void +virStoragePoolDefRBDNamespaceFree(void *nsdata) +{ + virStoragePoolRBDConfigOptionsDefPtr cmdopts = nsdata; + size_t i; + + if (!cmdopts) + return; + + for (i = 0; i < cmdopts->noptions; i++) { + VIR_FREE(cmdopts->names[i]); + VIR_FREE(cmdopts->values[i]); + } + VIR_FREE(cmdopts->names); + VIR_FREE(cmdopts->values); + + VIR_FREE(cmdopts); +} + + +static int +virStoragePoolDefRBDNamespaceParse(xmlXPathContextPtr ctxt, + void **data) +{ + virStoragePoolRBDConfigOptionsDefPtr cmdopts = NULL; + xmlNodePtr *nodes = NULL; + int nnodes; + size_t i; + int ret = -1; + + if (xmlXPathRegisterNs(ctxt, BAD_CAST "rbd", + BAD_CAST STORAGE_POOL_RBD_NAMESPACE_HREF) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to register xml namespace '%s'"), + STORAGE_POOL_RBD_NAMESPACE_HREF); + return -1; + } + + nnodes = virXPathNodeSet("./rbd:config_opts/rbd:option", ctxt, &nodes); + if (nnodes < 0) + return -1; + + if (nnodes == 0) + return 0; + + if (VIR_ALLOC(cmdopts) < 0) + goto cleanup; + + if (VIR_ALLOC_N(cmdopts->names, nnodes) < 0 || + VIR_ALLOC_N(cmdopts->values, nnodes) < 0) + goto cleanup; + + for (i = 0; i < nnodes; i++) { + if (!(cmdopts->names[cmdopts->noptions] = + virXMLPropString(nodes[i], "name"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("no rbd option name specified")); + goto cleanup; + } + if (*cmdopts->names[cmdopts->noptions] == '\0') { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("empty rbd option name specified")); + goto cleanup; + } + if (!(cmdopts->values[cmdopts->noptions] = + virXMLPropString(nodes[i], "value"))) { + virReportError(VIR_ERR_XML_ERROR, + _("no rbd option value specified for name '%s'"), + cmdopts->names[cmdopts->noptions]); + goto cleanup; + } + if (*cmdopts->values[cmdopts->noptions] == '\0') { + virReportError(VIR_ERR_XML_ERROR, + _("empty rbd option value specified for name '%s'"), + cmdopts->names[cmdopts->noptions]); + goto cleanup; + } + cmdopts->noptions++; + } + + VIR_STEAL_PTR(*data, cmdopts); + ret = 0; + + cleanup: + VIR_FREE(nodes); + virStoragePoolDefRBDNamespaceFree(cmdopts); + return ret; +} + + +static int +virStoragePoolDefRBDNamespaceFormatXML(virBufferPtr buf, + void *nsdata) +{ + size_t i; + virStoragePoolRBDConfigOptionsDefPtr def = nsdata; + + if (!def) + return 0; + + virBufferAddLit(buf, "<rbd:config_opts>\n"); + virBufferAdjustIndent(buf, 2); + + for (i = 0; i < def->noptions; i++) { + virBufferEscapeString(buf, "<rbd:option name='%s' ", def->names[i]); + virBufferEscapeString(buf, "value='%s'/>\n", def->values[i]); + } + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</rbd:config_opts>\n"); + + return 0; +} + + +static const char * +virStoragePoolDefRBDNamespaceHref(void) +{ + return "xmlns:rbd='" STORAGE_POOL_RBD_NAMESPACE_HREF "'"; +} + + static int virStorageBackendRBDRADOSConfSet(rados_t cluster, const char *option, @@ -69,10 +202,11 @@ virStorageBackendRBDRADOSConfSet(rados_t cluster, static int virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, - virStoragePoolSourcePtr source) + virStoragePoolDefPtr def) { int ret = -1; int r = 0; + virStoragePoolSourcePtr source = &def->source; virStorageAuthDefPtr authdef = source->auth; unsigned char *secret_value = NULL; size_t secret_value_size = 0; @@ -183,6 +317,22 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, rbd_default_format) < 0) goto cleanup; + if (def->namespaceData) { + virStoragePoolRBDConfigOptionsDefPtr cmdopts = def->namespaceData; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + for (i = 0; i < cmdopts->noptions; i++) { + if (virStorageBackendRBDRADOSConfSet(ptr->cluster, + cmdopts->names[i], + cmdopts->values[i]) < 0) + goto cleanup; + } + + virUUIDFormat(def->uuid, uuidstr); + VIR_WARN("Storage Pool name='%s' uuid='%s' is tainted by custom " + "config_opts from XML", def->name, uuidstr); + } + ptr->starttime = time(0); if ((r = rados_connect(ptr->cluster)) < 0) { virReportSystemError(-r, _("failed to connect to the RADOS monitor on: %s"), @@ -256,7 +406,7 @@ virStorageBackendRBDNewState(virStoragePoolObjPtr pool) if (VIR_ALLOC(ptr) < 0) return NULL; - if (virStorageBackendRBDOpenRADOSConn(ptr, &def->source) < 0) + if (virStorageBackendRBDOpenRADOSConn(ptr, def) < 0) goto error; if (virStorageBackendRBDOpenIoCTX(ptr, pool) < 0) @@ -1277,6 +1427,7 @@ virStorageBackendRBDVolWipe(virStoragePoolObjPtr pool, return ret; } + virStorageBackend virStorageBackendRBD = { .type = VIR_STORAGE_POOL_RBD, @@ -1291,8 +1442,20 @@ virStorageBackend virStorageBackendRBD = { }; +static virStoragePoolXMLNamespace virStoragePoolRBDXMLNamespace = { + .parse = virStoragePoolDefRBDNamespaceParse, + .free = virStoragePoolDefRBDNamespaceFree, + .format = virStoragePoolDefRBDNamespaceFormatXML, + .href = virStoragePoolDefRBDNamespaceHref, +}; + + int virStorageBackendRBDRegister(void) { - return virStorageBackendRegister(&virStorageBackendRBD); + if (virStorageBackendRegister(&virStorageBackendRBD) < 0) + return -1; + + return virStorageBackendNamespaceInit(VIR_STORAGE_POOL_RBD, + &virStoragePoolRBDXMLNamespace); } diff --git a/tests/storagepoolxml2xmlin/pool-rbd-ns-configopts.xml b/tests/storagepoolxml2xmlin/pool-rbd-ns-configopts.xml new file mode 100644 index 0000000000..6b62aa6f7e --- /dev/null +++ b/tests/storagepoolxml2xmlin/pool-rbd-ns-configopts.xml @@ -0,0 +1,17 @@ +<pool type='rbd' xmlns:rbd='http://libvirt.org/schemas/storagepool/source/rbd/1.0'... + <name>ceph</name> + <uuid>47c1faee-0207-e741-f5ae-d9b019b98fe2</uuid> + <source> + <name>rbd</name> + <host name='localhost' port='6789'/> + <host name='localhost' port='6790'/> + <auth username='admin' type='ceph'> + <secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/> + </auth> + </source> + <rbd:config_opts> + <rbd:option name='client_mount_timeout' value='45'/> + <rbd:option name='rados_mon_op_timeout' value='10'/> + <rbd:option name='rados_osd_op_timeout' value='20'/> + </rbd:config_opts> +</pool> diff --git a/tests/storagepoolxml2xmlout/pool-rbd-ns-configopts.xml b/tests/storagepoolxml2xmlout/pool-rbd-ns-configopts.xml new file mode 100644 index 0000000000..342a0ff74a --- /dev/null +++ b/tests/storagepoolxml2xmlout/pool-rbd-ns-configopts.xml @@ -0,0 +1,20 @@ +<pool type='rbd' xmlns:rbd='http://libvirt.org/schemas/storagepool/source/rbd/1.0'... + <name>ceph</name> + <uuid>47c1faee-0207-e741-f5ae-d9b019b98fe2</uuid> + <capacity unit='bytes'>0</capacity> + <allocation unit='bytes'>0</allocation> + <available unit='bytes'>0</available> + <source> + <host name='localhost' port='6789'/> + <host name='localhost' port='6790'/> + <name>rbd</name> + <auth type='ceph' username='admin'> + <secret uuid='2ec115d7-3a88-3ceb-bc12-0ac909a6fd87'/> + </auth> + </source> + <rbd:config_opts> + <rbd:option name='client_mount_timeout' value='45'/> + <rbd:option name='rados_mon_op_timeout' value='10'/> + <rbd:option name='rados_osd_op_timeout' value='20'/> + </rbd:config_opts> +</pool> diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c index aff9ff160c..90d00a8d9e 100644 --- a/tests/storagepoolxml2xmltest.c +++ b/tests/storagepoolxml2xmltest.c @@ -106,6 +106,7 @@ mymain(void) DO_TEST("pool-zfs"); DO_TEST("pool-zfs-sourcedev"); DO_TEST("pool-rbd"); + DO_TEST("pool-rbd-ns-configopts"); DO_TEST("pool-vstorage"); DO_TEST("pool-iscsi-direct-auth"); DO_TEST("pool-iscsi-direct"); -- 2.20.1