On 4/20/22 05:58, Damien Le Moal wrote: > On 4/19/22 20:55, Jens Axboe wrote: >> On 4/19/22 5:00 AM, Damien Le Moal wrote: >>> Currently, the directory name used to create a nullb device through >>> sysfs is not used as the device name, potentially causing headaches for >>> users if devices are already created through the modprobe operation >>> withe the nr_device module parameter not set to 0. E.g. a user can do >>> "mkdir /sys/kernel/config/nullb/nullb0" to create a nullb device while >>> /dev/nullb0 wasalready created from modprobe. In this case, the configfs >> ^^^ >> >> space > > Re-sending to fix this. Also realized that using "#define pr_fmt" would > simplify patch 3. Updating that. > >> >>> nullb device will be named nullb1, causing confusion for the user. >>> >>> Simplify this by using the configfs directory name as the nullb device >>> name, always, unless another nullb device is already using the same >>> name. E.g. if modprobe created nullb0, then: >>> >>> $ mkdir /sys/kernel/config/nullb/nullb0 >>> mkdir: cannot create directory '/sys/kernel/config/nullb/nullb0': File >>> exists >>> >>> will be reported to th user. >>> >>> To implement this, the function null_find_dev_by_name() is added to >>> check for the existence of a nullb device with the name used for a new >>> configfs device directory. nullb_group_make_item() uses this new >>> function to check if the directory name can be used as the disk name. >>> Finally, null_add_dev() is modified to use the device config item name >>> as the disk name for new nullb device, for devices created using >>> configfs. The naming of devices created though modprobe remains >>> unchanged. >>> >>> Of note is that it is possible for a user to create through configfs a >>> nullb device with the same name as an existing device. E.g. >> >> This is nice, and solves both the confusing part of having >> pre-configured devices, but also using the actual directory name as the >> device name even if they are not ordered. >> >> Only odd bit is you can create a device name where a special file of >> that name already exists, but I don't think that's solvable in a clean >> way and we just need to ignore that. That's arguably a user error, don't >> pick names that already exist. > > Yes. add_disk() will fail if the device name already exist within the > "block" device class, but will not complain about anything if the existing > device belongs to another class. > > I could add a "block" class wide check for device name existence in > null_find_dev_by_name() instead of only checking the nullb list ? Looked into this, but I do not see any easy ready-to-use way to do it since "struct class block_class" is not exported. And I would not wnat to export this block/dev internal symbol. We could play with vfs_stat() to test for device existence, but that is a little ugly... What about simply enforcing a name pattern like "nullb*" for the configfs directory/device names ? That is simple and the current nullb_find_dev_by_name() will keep working as is. -- Damien Le Moal Western Digital Research
