[libvirt] [PATCH v4 08/10] docs, tests: Add schema, description, and tests for NFS namespace

John Ferlan posted 10 patches 7 years ago
[libvirt] [PATCH v4 08/10] docs, tests: Add schema, description, and tests for NFS namespace
Posted by John Ferlan 7 years ago
Modify the storagepool.rng to allow for the usage of a different
XML namespace to parse the netfs_mount_opts to be included with
the netfs storage pool definition.

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@redhat.com>
---
 docs/formatstorage.html.in                    | 57 +++++++++++++++++++
 docs/schemas/storagepool.rng                  | 20 +++++++
 tests/Makefile.am                             |  4 +-
 .../pool-netfs-ns-mountopts.xml               | 25 ++++++++
 .../pool-netfs-ns-mountopts.xml               | 25 ++++++++
 tests/storagepoolxml2xmltest.c                |  6 ++
 6 files changed, 136 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 b1b76a1dda..7a5a38de9e 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -497,6 +497,63 @@
       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 abitrary manner via
+      XML syntax targeted solely for the needs of the specific pool type
+      which is not otherwise supportable via XML. For the "netfs" pool
+      this provides a mechanism to provide additional mount options on the
+      command line.
+    </p>
+    <dl>
+      <dt><code>netfs: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>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:netfs='http://libvirt.org/schemas/storagepool/source/netfs/1.0'
+        </p>
+
+        <p>
+        The <code>netfs:mount_opts</code> defines the mount options by
+        specifying multiple <code>netfs: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
+      <pre>
+&lt;pool type="netfs" xmlns:netfs='http://libvirt.org/schemas/storagepool/source/netfs/1.0'&gt;
+  &lt;name&gt;nfsimages&lt;/name&gt;
+...
+  &lt;source&gt;
+...
+  &lt;/source&gt;
+...
+  &lt;target&gt;
+...
+  &lt;/target&gt;
+  &lt;netfs:mount_opts&gt;
+    &lt;netfs:option name='sync'/&gt;
+    &lt;netfs:option name='lazytime'/&gt;
+  &lt;/netfs: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..cea4bb474a 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -64,6 +64,9 @@
       <ref name='sourcenetfs'/>
       <ref name='target'/>
     </interleave>
+    <optional>
+      <ref name='netfs_mount_opts'/>
+    </optional>
   </define>
 
   <define name='poollogical'>
@@ -682,4 +685,21 @@
     </data>
   </define>
 
+  <!--
+       Optional storage pool extensions in their own namespace:
+         NetFS
+    -->
+
+  <define name="netfs_mount_opts">
+    <element name="mount_opts" ns="http://libvirt.org/schemas/storagepool/source/netfs/1.0">
+      <zeroOrMore>
+        <element name="option">
+          <attribute name='name'>
+            <text/>
+          </attribute>
+        </element>
+      </zeroOrMore>
+    </element>
+  </define>
+
 </grammar>
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..6295edd6ee
--- /dev/null
+++ b/tests/storagepoolxml2xmlin/pool-netfs-ns-mountopts.xml
@@ -0,0 +1,25 @@
+<pool type='netfs' xmlns:netfs='http://libvirt.org/schemas/storagepool/source/netfs/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>
+  <netfs:mount_opts>
+    <netfs:option name='sync'/>
+    <netfs:option name='lazytime'/>
+  </netfs: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..efa7b07f31
--- /dev/null
+++ b/tests/storagepoolxml2xmlout/pool-netfs-ns-mountopts.xml
@@ -0,0 +1,25 @@
+<pool type='netfs' xmlns:netfs='http://libvirt.org/schemas/storagepool/source/netfs/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>
+  <netfs:mount_opts>
+    <netfs:option name='sync'/>
+    <netfs:option name='lazytime'/>
+  </netfs: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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 08/10] docs, tests: Add schema, description, and tests for NFS namespace
Posted by Daniel P. Berrangé 7 years ago
On Thu, Jan 17, 2019 at 04:22:14PM -0500, John Ferlan wrote:
> Modify the storagepool.rng to allow for the usage of a different
> XML namespace to parse the netfs_mount_opts to be included with
> the netfs storage pool definition.
> 
> 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@redhat.com>
> ---
>  docs/formatstorage.html.in                    | 57 +++++++++++++++++++
>  docs/schemas/storagepool.rng                  | 20 +++++++
>  tests/Makefile.am                             |  4 +-
>  .../pool-netfs-ns-mountopts.xml               | 25 ++++++++
>  .../pool-netfs-ns-mountopts.xml               | 25 ++++++++
>  tests/storagepoolxml2xmltest.c                |  6 ++
>  6 files changed, 136 insertions(+), 1 deletion(-)
>  create mode 100644 tests/storagepoolxml2xmlin/pool-netfs-ns-mountopts.xml
>  create mode 100644 tests/storagepoolxml2xmlout/pool-netfs-ns-mountopts.xml

I would have expected all this stuff to be introduced in the previous
patch really, since its exercising the parser code the previous patch
added

> +    <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 abitrary manner via
> +      XML syntax targeted solely for the needs of the specific pool type
> +      which is not otherwise supportable via XML. For the "netfs" pool
> +      this provides a mechanism to provide additional mount options on the
> +      command line.
> +    </p>

This needs to have text to make it clear that using this feature comes
with *no* support guarantees. It should say it is only 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.

> +    <dl>
> +      <dt><code>netfs: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>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:netfs='http://libvirt.org/schemas/storagepool/source/netfs/1.0'
> +        </p>
> +
> +        <p>
> +        The <code>netfs:mount_opts</code> defines the mount options by
> +        specifying multiple <code>netfs: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
> +      <pre>

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 08/10] docs, tests: Add schema, description, and tests for NFS namespace
Posted by Daniel P. Berrangé 7 years ago
On Thu, Jan 17, 2019 at 04:22:14PM -0500, John Ferlan wrote:
> Modify the storagepool.rng to allow for the usage of a different
> XML namespace to parse the netfs_mount_opts to be included with
> the netfs storage pool definition.
> 
> 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@redhat.com>
> ---
>  docs/formatstorage.html.in                    | 57 +++++++++++++++++++
>  docs/schemas/storagepool.rng                  | 20 +++++++
>  tests/Makefile.am                             |  4 +-
>  .../pool-netfs-ns-mountopts.xml               | 25 ++++++++
>  .../pool-netfs-ns-mountopts.xml               | 25 ++++++++
>  tests/storagepoolxml2xmltest.c                |  6 ++
>  6 files changed, 136 insertions(+), 1 deletion(-)
>  create mode 100644 tests/storagepoolxml2xmlin/pool-netfs-ns-mountopts.xml
>  create mode 100644 tests/storagepoolxml2xmlout/pool-netfs-ns-mountopts.xml


> +    <dl>
> +      <dt><code>netfs: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>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:netfs='http://libvirt.org/schemas/storagepool/source/netfs/1.0'
> +        </p>
> +
> +        <p>
> +        The <code>netfs:mount_opts</code> defines the mount options by
> +        specifying multiple <code>netfs: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
> +      <pre>
> +&lt;pool type="netfs" xmlns:netfs='http://libvirt.org/schemas/storagepool/source/netfs/1.0'&gt;
> +  &lt;name&gt;nfsimages&lt;/name&gt;
> +...
> +  &lt;source&gt;
> +...
> +  &lt;/source&gt;
> +...
> +  &lt;target&gt;
> +...
> +  &lt;/target&gt;
> +  &lt;netfs:mount_opts&gt;
> +    &lt;netfs:option name='sync'/&gt;
> +    &lt;netfs:option name='lazytime'/&gt;
> +  &lt;/netfs:mount_opts&gt;
> +&lt;/pool&gt;
> +...</pre>
> +

Based on my comments in the first patch, we should acutally just
call this  'fs' not 'netfs' and allow it for any filesystem mount
pool, as this is valid for cifs, glusterfs, and local filesystems
too.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list