| Message ID | 20210109104254.1077093-5-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/6] dm: use bdev_read_only to check if a device is read-only | expand |
On 1/9/21 11:42 AM, 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@oracle.com> > Signed-off-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com> > --- > block/genhd.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
diff --git a/block/genhd.c b/block/genhd.c index e70bdc9b0893c1..10c76320510fef 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -1658,8 +1658,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);