[Qemu-devel] [PATCHv3 0/2] block/nfs: add knob to set readahead

[Qemu-devel] [PATCHv3 0/2] block/nfs: add knob to set readahead

[Peter Lieven]

upcoming libnfs will feature internal readahead support.
Add a knob to pass the optional readahead value as a URL
parameter.

This series fixes also the incorrect usage of strncmp and
atoi.

v2->v3: - use parse_uint_full [Eric]
        - split patch [Eric]

v1->v2: use strtol instead of atoi [Eric]

Peter Lieven (2):
  block/nfs: fix url parameter checking
  block/nfs: add knob to set readahead

 block/nfs.c |   22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

-- 
1.7.9.5

[Qemu-devel] [PATCHv3 1/2] block/nfs: fix url parameter checking

[Peter Lieven]

this patch fixes the incorrect usage of strncmp and
adds simple error checking by means of parse_uint_full
instead of atoi for the supplied URL parameters.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 block/nfs.c |   18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/block/nfs.c b/block/nfs.c
index ec43201..0b44483 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -304,17 +304,23 @@ static int64_t nfs_client_open(NFSClient *client, const char *filename,
 
     qp = query_params_parse(uri->query);
     for (i = 0; i < qp->n; i++) {
+        unsigned long long val;
         if (!qp->p[i].value) {
             error_setg(errp, "Value for NFS parameter expected: %s",
                        qp->p[i].name);
             goto fail;
         }
-        if (!strncmp(qp->p[i].name, "uid", 3)) {
-            nfs_set_uid(client->context, atoi(qp->p[i].value));
-        } else if (!strncmp(qp->p[i].name, "gid", 3)) {
-            nfs_set_gid(client->context, atoi(qp->p[i].value));
-        } else if (!strncmp(qp->p[i].name, "tcp-syncnt", 10)) {
-            nfs_set_tcp_syncnt(client->context, atoi(qp->p[i].value));
+        if (parse_uint_full(qp->p[i].value, &val, 0)) {
+            error_setg(errp, "Illegal value for NFS parameter: %s",
+                       qp->p[i].name);
+            goto fail;
+        }
+        if (!strcmp(qp->p[i].name, "uid")) {
+            nfs_set_uid(client->context, val);
+        } else if (!strcmp(qp->p[i].name, "gid")) {
+            nfs_set_gid(client->context, val);
+        } else if (!strcmp(qp->p[i].name, "tcp-syncnt")) {
+            nfs_set_tcp_syncnt(client->context, val);
         } else {
             error_setg(errp, "Unknown NFS parameter name: %s",
                        qp->p[i].name);
-- 
1.7.9.5

Re: [Qemu-devel] [PATCHv3 1/2] block/nfs: fix url parameter checking

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 804 bytes --]

On 06/24/2014 04:06 PM, Peter Lieven wrote:
> this patch fixes the incorrect usage of strncmp and
> adds simple error checking by means of parse_uint_full
> instead of atoi for the supplied URL parameters.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  block/nfs.c |   18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 

> +        if (parse_uint_full(qp->p[i].value, &val, 0)) {
> +            error_setg(errp, "Illegal value for NFS parameter: %s",

I prefer s/Illegal/Invalid/, as this is not a case of breaking a law.

But that's aesthetic.  Whether or not you tweak that choice of wording:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

[Qemu-devel] [PATCHv3 2/2] block/nfs: add knob to set readahead

[Peter Lieven]

upcoming libnfs will feature internal readahead support.
Add a knob to pass the optional readahead value as a URL
parameter.

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 block/nfs.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/block/nfs.c b/block/nfs.c
index 0b44483..8439e0d 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -321,6 +321,10 @@ static int64_t nfs_client_open(NFSClient *client, const char *filename,
             nfs_set_gid(client->context, val);
         } else if (!strcmp(qp->p[i].name, "tcp-syncnt")) {
             nfs_set_tcp_syncnt(client->context, val);
+#ifdef LIBNFS_FEATURE_READAHEAD
+        } else if (!strcmp(qp->p[i].name, "readahead")) {
+            nfs_set_readahead(client->context, val);
+#endif
         } else {
             error_setg(errp, "Unknown NFS parameter name: %s",
                        qp->p[i].name);
-- 
1.7.9.5

Re: [Qemu-devel] [PATCHv3 2/2] block/nfs: add knob to set readahead

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 456 bytes --]

On 06/24/2014 04:06 PM, Peter Lieven wrote:
> upcoming libnfs will feature internal readahead support.
> Add a knob to pass the optional readahead value as a URL
> parameter.
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  block/nfs.c |    4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCHv3 0/2] block/nfs: add knob to set readahead

[Kevin Wolf]

Am 25.06.2014 um 00:05 hat Peter Lieven geschrieben:
> upcoming libnfs will feature internal readahead support.
> Add a knob to pass the optional readahead value as a URL
> parameter.
> 
> This series fixes also the incorrect usage of strncmp and
> atoi.

Thanks, applied to the block branch.

Kevin