mbox series

[0/2] block: avoid to drop & re-add partitions if partitions aren't changed

Message ID 20210205021708.1498711-1-ming.lei@redhat.com (mailing list archive)
Headers show
Series block: avoid to drop & re-add partitions if partitions aren't changed | expand

Message

Ming Lei Feb. 5, 2021, 2:17 a.m. UTC 
Hi Guys,

The two patches changes block ioctl(BLKRRPART) for avoiding drop &
re-add partitions if partitions state isn't changed. The current
behavior confuses userspace because partitions can disappear anytime
when ioctl(BLKRRPART).

Ming Lei (2):
  block: move partitions check code into single helper
  block: avoid to drop & re-add partitions if partitions aren't changed

 block/genhd.c            |   2 +
 block/partitions/check.h |   2 +
 block/partitions/core.c  | 101 ++++++++++++++++++++++++++++++++-------
 fs/block_dev.c           |  28 +++++++++--
 include/linux/genhd.h    |   4 ++
 5 files changed, 118 insertions(+), 19 deletions(-)

Cc: Ewan D. Milne <emilne@redhat.com>

Comments

Ming Lei Feb. 15, 2021, 4:03 a.m. UTC | #1 
On Fri, Feb 05, 2021 at 10:17:06AM +0800, Ming Lei wrote:
> Hi Guys,
> 
> The two patches changes block ioctl(BLKRRPART) for avoiding drop &
> re-add partitions if partitions state isn't changed. The current
> behavior confuses userspace because partitions can disappear anytime
> when ioctl(BLKRRPART).
> 
> Ming Lei (2):
>   block: move partitions check code into single helper
>   block: avoid to drop & re-add partitions if partitions aren't changed
> 
>  block/genhd.c            |   2 +
>  block/partitions/check.h |   2 +
>  block/partitions/core.c  | 101 ++++++++++++++++++++++++++++++++-------
>  fs/block_dev.c           |  28 +++++++++--
>  include/linux/genhd.h    |   4 ++
>  5 files changed, 118 insertions(+), 19 deletions(-)
> 
> Cc: Ewan D. Milne <emilne@redhat.com>
> -- 
> 2.29.2
> 

Hello,

Ping...
Christoph Hellwig Feb. 16, 2021, 8:44 a.m. UTC | #2 
On Mon, Feb 15, 2021 at 12:03:41PM +0800, Ming Lei wrote:
> Hello,

I think this is a fundamentally bad idea.  We should not keep the
parsed partition state around forever just to work around some buggy
user space software.
Ming Lei Feb. 17, 2021, 3:07 a.m. UTC | #3 
On Tue, Feb 16, 2021 at 09:44:30AM +0100, Christoph Hellwig wrote:
> On Mon, Feb 15, 2021 at 12:03:41PM +0800, Ming Lei wrote:
> > Hello,
> 
> I think this is a fundamentally bad idea.  We should not keep the
> parsed partition state around forever just to work around some buggy
> user space software.

What is the bug in userspace software?

Do you think it is correct for ioctl(BLKRRPART) to always drop/re-add
partition device node?
Christoph Hellwig Feb. 17, 2021, 7:16 a.m. UTC | #4 
On Wed, Feb 17, 2021 at 11:07:14AM +0800, Ming Lei wrote:
> Do you think it is correct for ioctl(BLKRRPART) to always drop/re-add
> partition device node?

Yes, that is what it is designed to do.  The only reason to call this
ioctl is when userspace software has written new partition table
information to the disk.
Ming Lei Feb. 18, 2021, 7:57 a.m. UTC | #5 
On Wed, Feb 17, 2021 at 08:16:29AM +0100, Christoph Hellwig wrote:
> On Wed, Feb 17, 2021 at 11:07:14AM +0800, Ming Lei wrote:
> > Do you think it is correct for ioctl(BLKRRPART) to always drop/re-add
> > partition device node?
> 
> Yes, that is what it is designed to do.  The only reason to call this
> ioctl is when userspace software has written new partition table
> information to the disk.

I am wondering how userspace can know this design or implication since
this behavior wasn't documented anywhere.

For example, 'blockdev --rereadpt' can do it simply, without updating
partition table at all.

The reality is that almost of all the main userspace consumers of
ioctl(BLKRRPART) didn't follow such 'rule', then partitions node from
'bdev' fs can disappear & re-appear anytime. I believe it is one bug
from userspace view.


Thanks,
Ming