Re: [RFC PATCH 2/2] lightnvm: Append device name to target name

[Wenwei Tao]

2016-05-23 17:16 GMT+08:00 Matias BjÃrling <mb@xxxxxxxxxxx>:
> On 05/23/2016 11:13 AM, Wenwei Tao wrote:
>>
>> From: Wenwei Tao <ww.tao0320@xxxxxxxxx>
>>
>> We may create targets with same name on different
>> backend devices, this is not what we want, so append
>> the device name to target name to make the new target
>> name unique in the system.
>>
>> Signed-off-by: Wenwei Tao <ww.tao0320@xxxxxxxxx>
>> ---
>>   drivers/lightnvm/gennvm.c | 13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/lightnvm/gennvm.c b/drivers/lightnvm/gennvm.c
>> index 39ff0af..ecb09cb 100644
>> --- a/drivers/lightnvm/gennvm.c
>> +++ b/drivers/lightnvm/gennvm.c
>> @@ -43,9 +43,18 @@ static int gen_create_tgt(struct nvm_dev *dev, struct
>> nvm_ioctl_create *create)
>>         struct gendisk *tdisk;
>>         struct nvm_tgt_type *tt;
>>         struct nvm_target *t;
>> +       char tgtname[DISK_NAME_LEN];
>>         void *targetdata;
>>         int ret = -ENOMEM;
>>
>> +       if (strlen(dev->name) + strlen(create->tgtname) + 1 >
>> DISK_NAME_LEN) {
>> +               pr_err("nvm: target name too long. %s:%s\n",
>> +                               dev->name, create->tgtname);
>> +               return -EINVAL;
>> +       }
>> +
>> +       sprintf(tgtname, "%s%s", dev->name, create->tgtname);
>> +
>>         tt = nvm_find_target_type(create->tgttype, 1);
>>         if (!tt) {
>>                 pr_err("nvm: target type %s not found\n",
>> create->tgttype);
>> @@ -53,7 +62,7 @@ static int gen_create_tgt(struct nvm_dev *dev, struct
>> nvm_ioctl_create *create)
>>         }
>>
>>         mutex_lock(&gn->lock);
>> -       t = gen_find_target(gn, create->tgtname);
>> +       t = gen_find_target(gn, tgtname);
>>         if (t) {
>>                 pr_err("nvm: target name already exists.\n");
>>                 ret = -EINVAL;
>> @@ -73,7 +82,7 @@ static int gen_create_tgt(struct nvm_dev *dev, struct
>> nvm_ioctl_create *create)
>>         if (!tdisk)
>>                 goto err_queue;
>>
>> -       sprintf(tdisk->disk_name, "%s", create->tgtname);
>> +       sprintf(tdisk->disk_name, "%s", tgtname);
>>         tdisk->flags = GENHD_FL_EXT_DEVT;
>>         tdisk->major = 0;
>>         tdisk->first_minor = 0;
>>
>
> Hi Wenwei, what about the case where a target instance has multiple devices
> associated?
>
You mean a target may be build on multiple backend devices ?

> I am okay with having the user choosing a unique name for the target to be
> exposed.
You mean user should check the name before create the target?
Before move target mgmt into media mgr, that would be okay(after apply
lightnvm: hold lock until finish the target creation), since all
targets are in the global list.
Now consider below case:
There are two users A and B. A want to create target test0 upon
device0 B want to create test0 upon device1,
before creation they both check whether test0 is exist (e.g. by list
/dev/test0) , they all find test0 is not exist now, and they continue
their
creation. Both of them use disk name test0 to call add_disk, that
would cause panic.