Re: [PATCH REPOST 2/4] rbd: add warning messages for missing arguments

[Alex Elder <elder@inktank.com>]

On 01/03/2013 07:10 PM, Dan Mick wrote:
> Do you want to include in the message some kind of indication which
> operation/function is involved?  (this is definitely better, but even
> better might be to add "rbd add" or "rbd_add_parse_args" to the msgs)

Sure.  This comment really applies to the previous patch.  I'm
sure I thought of doing that and I'm not sure any more why I did
not.  Maybe worried about lines getting too long?  Or maybe
I bumped into varargs problems?  I don't know.

I'll rename the rbd_warn() function _rbd_warn(), and will add a new
first argument which is the function name.  Then I'll define a new
macro rbd_warn() that just calls _rbd_warn(), inserting __func__ as
a first argument.

					-Alex

> On 01/03/2013 11:11 AM, Alex Elder wrote:
>> Tell the user (via dmesg) what was wrong with the arguments provided
>> via /sys/bus/rbd/add.
>>
>> Signed-off-by: Alex Elder <elder@inktank.com>
>> ---
>>   drivers/block/rbd.c |   24 ++++++++++++++++--------
>>   1 file changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
>> index 635b81d..31da8c5 100644
>> --- a/drivers/block/rbd.c
>> +++ b/drivers/block/rbd.c
>> @@ -3244,8 +3244,10 @@ static int rbd_add_parse_args(const char *buf,
>>       /* The first four tokens are required */
>>
>>       len = next_token(&buf);
>> -    if (!len)
>> -        return -EINVAL;    /* Missing monitor address(es) */
>> +    if (!len) {
>> +        rbd_warn(NULL, "no monitor address(es) provided");
>> +        return -EINVAL;
>> +    }
>>       mon_addrs = buf;
>>       mon_addrs_size = len + 1;
>>       buf += len;
>> @@ -3254,8 +3256,10 @@ static int rbd_add_parse_args(const char *buf,
>>       options = dup_token(&buf, NULL);
>>       if (!options)
>>           return -ENOMEM;
>> -    if (!*options)
>> -        goto out_err;    /* Missing options */
>> +    if (!*options) {
>> +        rbd_warn(NULL, "no options provided");
>> +        goto out_err;
>> +    }
>>
>>       spec = rbd_spec_alloc();
>>       if (!spec)
>> @@ -3264,14 +3268,18 @@ static int rbd_add_parse_args(const char *buf,
>>       spec->pool_name = dup_token(&buf, NULL);
>>       if (!spec->pool_name)
>>           goto out_mem;
>> -    if (!*spec->pool_name)
>> -        goto out_err;    /* Missing pool name */
>> +    if (!*spec->pool_name) {
>> +        rbd_warn(NULL, "no pool name provided");
>> +        goto out_err;
>> +    }
>>
>>       spec->image_name = dup_token(&buf, NULL);
>>       if (!spec->image_name)
>>           goto out_mem;
>> -    if (!*spec->image_name)
>> -        goto out_err;    /* Missing image name */
>> +    if (!*spec->image_name) {
>> +        rbd_warn(NULL, "no image name provided");
>> +        goto out_err;
>> +    }
>>
>>       /*
>>        * Snapshot name is optional; default is to use "-"
>>