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

[Eric Blake <eblake@redhat.com>]

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

On 06/23/2014 08:30 AM, Peter Lieven wrote:
> upcoming libnfs will feature internal readahead support.
> Add a knob to pass the optional readahead value as a URL
> parameter.
> 
> This patch fixes also the incorrect usage of strncmp.

But not the incorrect usage of atoi() (hint - atoi cannot detect
overflow; it should NEVER be used on user-provided input; instead
strtol() should be preferred).

> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  block/nfs.c |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/block/nfs.c b/block/nfs.c
> index ec43201..a5c0577 100644
> --- a/block/nfs.c
> +++ b/block/nfs.c
> @@ -309,12 +309,16 @@ static int64_t nfs_client_open(NFSClient *client, const char *filename,
>                         qp->p[i].name);
>              goto fail;
>          }
> -        if (!strncmp(qp->p[i].name, "uid", 3)) {
> +        if (!strcmp(qp->p[i].name, "uid")) {
>              nfs_set_uid(client->context, atoi(qp->p[i].value));

Of course, the use of atoi was pre-existing, so it doesn't necessarily
need to be done in _this_ patch.

> -        } else if (!strncmp(qp->p[i].name, "gid", 3)) {
> +        } else if (!strcmp(qp->p[i].name, "gid")) {
>              nfs_set_gid(client->context, atoi(qp->p[i].value));
> -        } else if (!strncmp(qp->p[i].name, "tcp-syncnt", 10)) {
> +        } else if (!strcmp(qp->p[i].name, "tcp-syncnt")) {
>              nfs_set_tcp_syncnt(client->context, atoi(qp->p[i].value));
> +#ifdef LIBNFS_FEATURE_READAHEAD
> +        } else if (!strcmp(qp->p[i].name, "readahead")) {
> +            nfs_set_readahead(client->context, atoi(qp->p[i].value));
> +#endif

I'm not a fan of adding even more special-casing to the file-name URI
without also adding it to the QAPI schema for use in the blockdev-add
command.  Of course, we don't have structured nfs options for
blockdev-add yet, but maybe it's time to start thinking about that
addition before we do this addition.

-- 
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 --]