[PATCH] block: fix ioctl return error with GENHD_FL_NO_PART

[PATCH] block: fix ioctl return error with GENHD_FL_NO_PART

[Li Wang]

GENHD_FL_NO_PART means no device(-ENXIO), not parameter error(-EINVAL).

test case with parted command:
@dd if=/dev/zero of=./blk-file bs=1M count=200
@losetup /dev/loop0 ./blk-file
@parted -s /dev/loop0 mklabel MSDOS
Error: Partition(s) 1, ..., 64 on /dev/loop0 have been written,
but we have been unable to inform the kernel of the change,
probably because it/they are in use. As a result,
the old partition(s) will remain in use. You should reboot now
before making further changes.
@echo $?
1

Fixes: 1a721de8489f ("block: don't add or resize partition on the disk with GENHD_FL_NO_PART")
Signed-off-by: Li Wang <li.wang@windriver.com>
---
 block/ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index d5f5cd61efd7..701c64cd67e8 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -21,7 +21,7 @@ static int blkpg_do_ioctl(struct block_device *bdev,
 	long long start, length;
 
 	if (disk->flags & GENHD_FL_NO_PART)
-		return -EINVAL;
+		return -ENXIO;
 	if (!capable(CAP_SYS_ADMIN))
 		return -EACCES;
 	if (copy_from_user(&p, upart, sizeof(struct blkpg_partition)))
-- 
2.25.1

Re: [PATCH] block: fix ioctl return error with GENHD_FL_NO_PART

[Li Lingfeng]

I don't think so.

GENHD_FL_NO_PART means "partition support is disabled". If users try to 
add or resize partition on the disk with this flag, kernel should remind 
them that the parameter of device is wrong.
So I think it's appropriate to return -EINVAL.

Thanks.

在 2023/10/17 16:09, Li Wang 写道:
> GENHD_FL_NO_PART means no device(-ENXIO), not parameter error(-EINVAL).
>
> test case with parted command:
> @dd if=/dev/zero of=./blk-file bs=1M count=200
> @losetup /dev/loop0 ./blk-file
> @parted -s /dev/loop0 mklabel MSDOS
> Error: Partition(s) 1, ..., 64 on /dev/loop0 have been written,
> but we have been unable to inform the kernel of the change,
> probably because it/they are in use. As a result,
> the old partition(s) will remain in use. You should reboot now
> before making further changes.
> @echo $?
> 1
>
> Fixes: 1a721de8489f ("block: don't add or resize partition on the disk with GENHD_FL_NO_PART")
> Signed-off-by: Li Wang <li.wang@windriver.com>
> ---
>   block/ioctl.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/ioctl.c b/block/ioctl.c
> index d5f5cd61efd7..701c64cd67e8 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -21,7 +21,7 @@ static int blkpg_do_ioctl(struct block_device *bdev,
>   	long long start, length;
>   
>   	if (disk->flags & GENHD_FL_NO_PART)
> -		return -EINVAL;
> +		return -ENXIO;
>   	if (!capable(CAP_SYS_ADMIN))
>   		return -EACCES;
>   	if (copy_from_user(&p, upart, sizeof(struct blkpg_partition)))

Re: [PATCH] block: fix ioctl return error with GENHD_FL_NO_PART

[Wang, Li]

refer to previous codes without GENHD_FL_NO_PART, bdev_del_partition() 
return -ENXIO:

Thanks.

vim block/ioctl.c +35

static int blkpg_do_ioctl(struct block_device *bdev,
                           struct blkpg_partition __user *upart, int op)
{
...
         if (disk->flags & GENHD_FL_NO_PART)
                 return -EINVAL;
...
         if (op == BLKPG_DEL_PARTITION)
                 return bdev_del_partition(disk, p.pno);

block/partitions/core.c:479

int bdev_del_partition(struct gendisk *disk, int partno)
{
         struct block_device *part = NULL;
         int ret = -ENXIO;

         mutex_lock(&disk->open_mutex);
         part = xa_load(&disk->part_tbl, partno);
         if (!part)
                 goto out_unlock;

         ret = -EBUSY;
         if (part->bd_openers)
                 goto out_unlock;

         delete_partition(part);
         ret = 0;
out_unlock:
         mutex_unlock(&disk->open_mutex);
         return ret;
}

On 10/17/2023 16:58, Li Lingfeng wrote:
> I don't think so.
>
> GENHD_FL_NO_PART means "partition support is disabled". If users try 
> to add or resize partition on the disk with this flag, kernel should 
> remind them that the parameter of device is wrong.
> So I think it's appropriate to return -EINVAL.
>
> Thanks.
>
> 在 2023/10/17 16:09, Li Wang 写道:
>> GENHD_FL_NO_PART means no device(-ENXIO), not parameter error(-EINVAL).
>>
>> test case with parted command:
>> @dd if=/dev/zero of=./blk-file bs=1M count=200
>> @losetup /dev/loop0 ./blk-file
>> @parted -s /dev/loop0 mklabel MSDOS
>> Error: Partition(s) 1, ..., 64 on /dev/loop0 have been written,
>> but we have been unable to inform the kernel of the change,
>> probably because it/they are in use. As a result,
>> the old partition(s) will remain in use. You should reboot now
>> before making further changes.
>> @echo $?
>> 1
>>
>> Fixes: 1a721de8489f ("block: don't add or resize partition on the 
>> disk with GENHD_FL_NO_PART")
>> Signed-off-by: Li Wang <li.wang@windriver.com>
>> ---
>>   block/ioctl.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/block/ioctl.c b/block/ioctl.c
>> index d5f5cd61efd7..701c64cd67e8 100644
>> --- a/block/ioctl.c
>> +++ b/block/ioctl.c
>> @@ -21,7 +21,7 @@ static int blkpg_do_ioctl(struct block_device *bdev,
>>       long long start, length;
>>         if (disk->flags & GENHD_FL_NO_PART)
>> -        return -EINVAL;
>> +        return -ENXIO;
>>       if (!capable(CAP_SYS_ADMIN))
>>           return -EACCES;
>>       if (copy_from_user(&p, upart, sizeof(struct blkpg_partition)))

Re: [PATCH] block: fix ioctl return error with GENHD_FL_NO_PART

[Li Lingfeng]

"-ENXIO" return by bdev_del_partition() may indicate that the disk being 
operated now do not has the specific partition, and it's different from 
blkpg_do_ioctl().

Thanks.

在 2023/10/17 17:26, Wang, Li 写道:
> refer to previous codes without GENHD_FL_NO_PART, bdev_del_partition() 
> return -ENXIO:
>
> Thanks.
>
> vim block/ioctl.c +35
>
> static int blkpg_do_ioctl(struct block_device *bdev,
>                           struct blkpg_partition __user *upart, int op)
> {
> ...
>         if (disk->flags & GENHD_FL_NO_PART)
>                 return -EINVAL;
> ...
>         if (op == BLKPG_DEL_PARTITION)
>                 return bdev_del_partition(disk, p.pno);
>
> block/partitions/core.c:479
>
> int bdev_del_partition(struct gendisk *disk, int partno)
> {
>         struct block_device *part = NULL;
>         int ret = -ENXIO;
>
>         mutex_lock(&disk->open_mutex);
>         part = xa_load(&disk->part_tbl, partno);
>         if (!part)
>                 goto out_unlock;
>
>         ret = -EBUSY;
>         if (part->bd_openers)
>                 goto out_unlock;
>
>         delete_partition(part);
>         ret = 0;
> out_unlock:
>         mutex_unlock(&disk->open_mutex);
>         return ret;
> }
>
> On 10/17/2023 16:58, Li Lingfeng wrote:
>> I don't think so.
>>
>> GENHD_FL_NO_PART means "partition support is disabled". If users try 
>> to add or resize partition on the disk with this flag, kernel should 
>> remind them that the parameter of device is wrong.
>> So I think it's appropriate to return -EINVAL.
>>
>> Thanks.
>>
>> 在 2023/10/17 16:09, Li Wang 写道:
>>> GENHD_FL_NO_PART means no device(-ENXIO), not parameter error(-EINVAL).
>>>
>>> test case with parted command:
>>> @dd if=/dev/zero of=./blk-file bs=1M count=200
>>> @losetup /dev/loop0 ./blk-file
>>> @parted -s /dev/loop0 mklabel MSDOS
>>> Error: Partition(s) 1, ..., 64 on /dev/loop0 have been written,
>>> but we have been unable to inform the kernel of the change,
>>> probably because it/they are in use. As a result,
>>> the old partition(s) will remain in use. You should reboot now
>>> before making further changes.
>>> @echo $?
>>> 1
>>>
>>> Fixes: 1a721de8489f ("block: don't add or resize partition on the 
>>> disk with GENHD_FL_NO_PART")
>>> Signed-off-by: Li Wang <li.wang@windriver.com>
>>> ---
>>>   block/ioctl.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/block/ioctl.c b/block/ioctl.c
>>> index d5f5cd61efd7..701c64cd67e8 100644
>>> --- a/block/ioctl.c
>>> +++ b/block/ioctl.c
>>> @@ -21,7 +21,7 @@ static int blkpg_do_ioctl(struct block_device *bdev,
>>>       long long start, length;
>>>         if (disk->flags & GENHD_FL_NO_PART)
>>> -        return -EINVAL;
>>> +        return -ENXIO;
>>>       if (!capable(CAP_SYS_ADMIN))
>>>           return -EACCES;
>>>       if (copy_from_user(&p, upart, sizeof(struct blkpg_partition)))