qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH] block/nfs: add knob to set readahead
Date: Mon, 23 Jun 2014 09:11:13 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0

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 <address@hidden>
> ---
>  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: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]