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

[Dan Mick <dan.mick@inktank.com>]

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)

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