On Tue, Dec 08, 2020 at 05:28:27PM +0100, Christoph Hellwig wrote:
> Change the policy so that a BLKROSET on the whole device also affects
> partitions. To quote Martin K. Petersen:
>
> It's very common for database folks to twiddle the read-only state of
> block devices and partitions. I know that our users will find it very
> counter-intuitive that setting /dev/sda read-only won't prevent writes
> to /dev/sda1.
>
> The existing behavior is inconsistent in the sense that doing:
>
> # blockdev --setro /dev/sda
> # echo foo > /dev/sda1
>
> permits writes. But:
>
> # blockdev --setro /dev/sda
> <something triggers revalidate>
> # echo foo > /dev/sda1
>
> doesn't.
>
> And a subsequent:
>
> # blockdev --setrw /dev/sda
> # echo foo > /dev/sda1
>
> doesn't work either since sda1's read-only policy has been inherited
> from the whole-disk device.
>
> You need to do:
>
> # blockdev --rereadpt
>
> after setting the whole-disk device rw to effectuate the same change on
> the partitions, otherwise they are stuck being read-only indefinitely.
>
> However, setting the read-only policy on a partition does *not* require
> the revalidate step. As a matter of fact, doing the revalidate will blow
> away the policy setting you just made.
>
> So the user needs to take different actions depending on whether they
> are trying to read-protect a whole-disk device or a partition. Despite
> using the same ioctl. That is really confusing.
>
> I have lost count how many times our customers have had data clobbered
> because of ambiguity of the existing whole-disk device policy. The
> current behavior violates the principle of least surprise by letting the
> user think they write protected the whole disk when they actually
> didn't.
>
> Suggested-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Reviewed-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
> ---
> block/genhd.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/block/genhd.c b/block/genhd.c
> index d9f989c1514123..6e51ecb9280aca 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -1656,8 +1656,7 @@ EXPORT_SYMBOL(set_disk_ro);
>
> int bdev_read_only(struct block_device *bdev)
> {
> - return bdev->bd_read_only ||
> - test_bit(GD_READ_ONLY, &bdev->bd_disk->state);
> + return bdev->bd_read_only || get_disk_ro(bdev->bd_disk);
> }
> EXPORT_SYMBOL(bdev_read_only);
>
> --
> 2.29.2
>
Reviewed-by: Ming Lei <ming.lei@xxxxxxxxxx>
--
Ming
