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@xxxxxxxxxxx>
>> ---
>> 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 "-"
>>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
