src/conf/schemas/storagepool.rng | 4 +--- src/conf/storage_conf.c | 14 +++----------- src/conf/storage_conf.h | 2 +- src/storage/storage_util.c | 4 ++-- .../pool-netfs-protocol-ver-linux.argv | 2 +- .../pool-netfs-protocol-ver.xml | 2 +- .../pool-netfs-protocol-ver.xml | 2 +- 7 files changed, 10 insertions(+), 20 deletions(-)
Treat the 'protocolVer' field as a string so that e.g. '4.1' can be
used.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
src/conf/schemas/storagepool.rng | 4 +---
src/conf/storage_conf.c | 14 +++-----------
src/conf/storage_conf.h | 2 +-
src/storage/storage_util.c | 4 ++--
.../pool-netfs-protocol-ver-linux.argv | 2 +-
.../pool-netfs-protocol-ver.xml | 2 +-
.../pool-netfs-protocol-ver.xml | 2 +-
7 files changed, 10 insertions(+), 20 deletions(-)
diff --git a/src/conf/schemas/storagepool.rng b/src/conf/schemas/storagepool.rng
index bd24b8b8d0..d81ead532a 100644
--- a/src/conf/schemas/storagepool.rng
+++ b/src/conf/schemas/storagepool.rng
@@ -577,9 +577,7 @@
<ref name="sourcefmtnetfs"/>
<optional>
<element name="protocol">
- <attribute name="ver">
- <ref name="unsignedInt"/>
- </attribute>
+ <attribute name="ver"/>
</element>
</optional>
<optional>
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 5da0bf20dd..251fb9f0a2 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -483,6 +483,7 @@ virStoragePoolSourceClear(virStoragePoolSource *source)
virStorageAuthDefFree(source->auth);
VIR_FREE(source->vendor);
VIR_FREE(source->product);
+ VIR_FREE(source->protocolVer);
}
@@ -526,7 +527,6 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
virStoragePoolOptions *options;
int n;
g_autoptr(virStorageAuthDef) authdef = NULL;
- g_autofree char *ver = NULL;
g_autofree xmlNodePtr *nodeset = NULL;
g_autofree char *sourcedir = NULL;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
@@ -634,7 +634,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
}
/* Option protocol version string (NFSvN) */
- if ((ver = virXPathString("string(./protocol/@ver)", ctxt))) {
+ if ((source->protocolVer = virXPathString("string(./protocol/@ver)", ctxt))) {
if ((source->format != VIR_STORAGE_POOL_NETFS_NFS) &&
(source->format != VIR_STORAGE_POOL_NETFS_AUTO)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -643,12 +643,6 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
virStoragePoolFormatFileSystemNetTypeToString(source->format));
return -1;
}
- if (virStrToLong_uip(ver, NULL, 0, &source->protocolVer) < 0) {
- virReportError(VIR_ERR_XML_ERROR,
- _("storage pool protocol ver '%s' is malformed"),
- ver);
- return -1;
- }
}
source->vendor = virXPathString("string(./vendor/@name)", ctxt);
@@ -1099,9 +1093,7 @@ virStoragePoolSourceFormat(virBuffer *buf,
if (src->auth)
virStorageAuthDefFormat(buf, src->auth);
- if (src->protocolVer)
- virBufferAsprintf(buf, "<protocol ver='%u'/>\n", src->protocolVer);
-
+ virBufferEscapeString(buf, "<protocol ver='%s'/>\n", src->protocolVer);
virBufferEscapeString(buf, "<vendor name='%s'/>\n", src->vendor);
virBufferEscapeString(buf, "<product name='%s'/>\n", src->product);
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index de39c3f294..a1bf243935 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -213,7 +213,7 @@ struct _virStoragePoolSource {
int format;
/* Protocol version value for netfs */
- unsigned int protocolVer;
+ char *protocolVer;
};
typedef struct _virStoragePoolTarget virStoragePoolTarget;
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 6ed2078b65..3871718b09 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -4201,8 +4201,8 @@ virStorageBackendFileSystemMountCmd(const char *cmdstr,
virCommand *cmd = NULL;
g_autofree char *nfsVers = NULL;
- if (def->type == VIR_STORAGE_POOL_NETFS && def->source.protocolVer > 0)
- nfsVers = g_strdup_printf("nfsvers=%u", def->source.protocolVer);
+ if (def->type == VIR_STORAGE_POOL_NETFS && def->source.protocolVer)
+ nfsVers = g_strdup_printf("nfsvers=%s", def->source.protocolVer);
cmd = virCommandNew(cmdstr);
if (netauto)
diff --git a/tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-linux.argv b/tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-linux.argv
index dac46a074f..da3e0c5927 100644
--- a/tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-linux.argv
+++ b/tests/storagepoolxml2argvdata/pool-netfs-protocol-ver-linux.argv
@@ -1,5 +1,5 @@
mount \
--o nodev,nosuid,noexec,nfsvers=3 \
+-o nodev,nosuid,noexec,nfsvers=4.1 \
-t nfs \
localhost:/var/lib/libvirt/images \
/mnt
diff --git a/tests/storagepoolxml2xmlin/pool-netfs-protocol-ver.xml b/tests/storagepoolxml2xmlin/pool-netfs-protocol-ver.xml
index 40f3f94e41..f35992e3c8 100644
--- a/tests/storagepoolxml2xmlin/pool-netfs-protocol-ver.xml
+++ b/tests/storagepoolxml2xmlin/pool-netfs-protocol-ver.xml
@@ -8,7 +8,7 @@
<host name='localhost'/>
<dir path='/var/lib/libvirt/images'/>
<format type='nfs'/>
- <protocol ver='3'/>
+ <protocol ver='4.1'/>
</source>
<target>
<path>/mnt</path>
diff --git a/tests/storagepoolxml2xmlout/pool-netfs-protocol-ver.xml b/tests/storagepoolxml2xmlout/pool-netfs-protocol-ver.xml
index 5fcad1305b..74c2f5edfe 100644
--- a/tests/storagepoolxml2xmlout/pool-netfs-protocol-ver.xml
+++ b/tests/storagepoolxml2xmlout/pool-netfs-protocol-ver.xml
@@ -8,7 +8,7 @@
<host name='localhost'/>
<dir path='/var/lib/libvirt/images'/>
<format type='nfs'/>
- <protocol ver='3'/>
+ <protocol ver='4.1'/>
</source>
<target>
<path>/mnt</path>
--
2.36.1On a Thursday in 2022, Peter Krempa wrote:
>Treat the 'protocolVer' field as a string so that e.g. '4.1' can be
>used.
>
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/conf/schemas/storagepool.rng | 4 +---
> src/conf/storage_conf.c | 14 +++-----------
> src/conf/storage_conf.h | 2 +-
> src/storage/storage_util.c | 4 ++--
> .../pool-netfs-protocol-ver-linux.argv | 2 +-
> .../pool-netfs-protocol-ver.xml | 2 +-
> .../pool-netfs-protocol-ver.xml | 2 +-
> 7 files changed, 10 insertions(+), 20 deletions(-)
>
>@@ -643,12 +643,6 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
> virStoragePoolFormatFileSystemNetTypeToString(source->format));
> return -1;
> }
>- if (virStrToLong_uip(ver, NULL, 0, &source->protocolVer) < 0) {
>- virReportError(VIR_ERR_XML_ERROR,
>- _("storage pool protocol ver '%s' is malformed"),
>- ver);
>- return -1;
>- }
This removes any validation of the field...
> }
>
> source->vendor = virXPathString("string(./vendor/@name)", ctxt);
>diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
>index 6ed2078b65..3871718b09 100644
>--- a/src/storage/storage_util.c
>+++ b/src/storage/storage_util.c
>@@ -4201,8 +4201,8 @@ virStorageBackendFileSystemMountCmd(const char *cmdstr,
> virCommand *cmd = NULL;
> g_autofree char *nfsVers = NULL;
>
>- if (def->type == VIR_STORAGE_POOL_NETFS && def->source.protocolVer > 0)
>- nfsVers = g_strdup_printf("nfsvers=%u", def->source.protocolVer);
>+ if (def->type == VIR_STORAGE_POOL_NETFS && def->source.protocolVer)
>+ nfsVers = g_strdup_printf("nfsvers=%s", def->source.protocolVer);
>
... so users can pass arbitrary arguments to mount here.
It would be nice to at least forbid commas.
Jano
> cmd = virCommandNew(cmdstr);
> if (netauto)
On 6/24/22 13:11, Ján Tomko wrote:
> On a Thursday in 2022, Peter Krempa wrote:
>> Treat the 'protocolVer' field as a string so that e.g. '4.1' can be
>> used.
>>
>> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>> ---
>> src/conf/schemas/storagepool.rng | 4 +---
>> src/conf/storage_conf.c | 14 +++-----------
>> src/conf/storage_conf.h | 2 +-
>> src/storage/storage_util.c | 4 ++--
>> .../pool-netfs-protocol-ver-linux.argv | 2 +-
>> .../pool-netfs-protocol-ver.xml | 2 +-
>> .../pool-netfs-protocol-ver.xml | 2 +-
>> 7 files changed, 10 insertions(+), 20 deletions(-)
>>
>> @@ -643,12 +643,6 @@ virStoragePoolDefParseSource(xmlXPathContextPtr
>> ctxt,
>>
>> virStoragePoolFormatFileSystemNetTypeToString(source->format));
>> return -1;
>> }
>> - if (virStrToLong_uip(ver, NULL, 0, &source->protocolVer) < 0) {
>> - virReportError(VIR_ERR_XML_ERROR,
>> - _("storage pool protocol ver '%s' is
>> malformed"),
>> - ver);
>> - return -1;
>> - }
>
> This removes any validation of the field...
Coincidentally, we have virStringParseVersion(). Just my 2 eurocents.
Michal
On Thu, Jun 23, 2022 at 17:18:18 +0200, Peter Krempa wrote: > Treat the 'protocolVer' field as a string so that e.g. '4.1' can be > used. > > Signed-off-by: Peter Krempa <pkrempa@redhat.com> > --- > src/conf/schemas/storagepool.rng | 4 +--- > src/conf/storage_conf.c | 14 +++----------- > src/conf/storage_conf.h | 2 +- > src/storage/storage_util.c | 4 ++-- > .../pool-netfs-protocol-ver-linux.argv | 2 +- > .../pool-netfs-protocol-ver.xml | 2 +- > .../pool-netfs-protocol-ver.xml | 2 +- > 7 files changed, 10 insertions(+), 20 deletions(-) (including the forgotten documentation update) Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
On Thu, Jun 23, 2022 at 17:18:18 +0200, Peter Krempa wrote:
> Treat the 'protocolVer' field as a string so that e.g. '4.1' can be
> used.
>
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> src/conf/schemas/storagepool.rng | 4 +---
> src/conf/storage_conf.c | 14 +++-----------
> src/conf/storage_conf.h | 2 +-
> src/storage/storage_util.c | 4 ++--
> .../pool-netfs-protocol-ver-linux.argv | 2 +-
> .../pool-netfs-protocol-ver.xml | 2 +-
> .../pool-netfs-protocol-ver.xml | 2 +-
> 7 files changed, 10 insertions(+), 20 deletions(-)
Forgot to commit this:
diff --git a/docs/formatstorage.rst b/docs/formatstorage.rst
index ef15c0ac5c..83d7d141ac 100644
--- a/docs/formatstorage.rst
+++ b/docs/formatstorage.rst
@@ -350,7 +350,7 @@ following child elements:
``protocol``
For a ``netfs`` Storage Pool provide a mechanism to define which NFS protocol
version number will be used to contact the server's NFS service. The
- attribute ``ver`` accepts an unsigned integer as the version number to use.
+ attribute ``ver`` accepts the version number to use.
:since:`Since 5.1.0`
``vendor``
Provides optional information about the vendor of the storage device. This
© 2016 - 2026 Red Hat, Inc.