Am 23.06.2014 um 17:11 schrieb Eric Blake <ebl...@redhat.com>:
> 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).
Thanks for that hint I was not aware and somehow everybody
mist it when it first went in. I will respin for that.
>
>>
>> Signed-off-by: Peter Lieven <p...@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.
I would like to leave this for a follow-up and I promise to take care of this.
If you could give me a pointer of an existing driver that does this as a
reference
I would be grateful.
For the read ahead parameter its likely not needed as (at least in my use case)
the read ahead makes only sense when converting a compressed QCOW2 Template File
which lives on an NFS Share to a RAW Device. So I use it with qemu-img.
For a Container File that is used as a VM hard drive I would likely not use
read ahead
as the operating system itself will implement some sort of read ahead already.
Peter
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
