diff mbox

[v2] block/raw-posix: detect readonly Linux block devices using BLKROGET

Message ID 1360063713-10687-1-git-send-email-stefanha@redhat.com
State New
Headers show

Commit Message

Stefan Hajnoczi Feb. 5, 2013, 11:28 a.m. UTC
Linux block devices can be set read-only with "blockdev --setro
<device>".  The same thing can be done for LVM volumes using "lvchange
--permission r <volume>".  This read-only setting is independent of
device node permissions.  Therefore the device can still be opened
O_RDWR but actual writes will fail.

This results in odd behavior for QEMU.  bdrv_open() is supposed to fail
if a read-only image is being opened with BDRV_O_RDWR.  By not failing
for Linux block devices, the guest boots up but every write produces an
I/O error.

This patch checks whether the block device is read-only so that Linux
block devices behave like regular files.

Reported-by: Sibiao Luo <sluo@redhat.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/raw-posix.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

Comments

Markus Armbruster Feb. 5, 2013, 3:12 p.m. UTC | #1
Stefan Hajnoczi <stefanha@redhat.com> writes:

> Linux block devices can be set read-only with "blockdev --setro
> <device>".  The same thing can be done for LVM volumes using "lvchange
> --permission r <volume>".  This read-only setting is independent of
> device node permissions.  Therefore the device can still be opened
> O_RDWR but actual writes will fail.
>
> This results in odd behavior for QEMU.  bdrv_open() is supposed to fail
> if a read-only image is being opened with BDRV_O_RDWR.  By not failing
> for Linux block devices, the guest boots up but every write produces an
> I/O error.
>
> This patch checks whether the block device is read-only so that Linux
> block devices behave like regular files.
>
> Reported-by: Sibiao Luo <sluo@redhat.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Kevin Wolf Feb. 6, 2013, 10:30 a.m. UTC | #2
Am 05.02.2013 16:12, schrieb Markus Armbruster:
> Stefan Hajnoczi <stefanha@redhat.com> writes:
> 
>> Linux block devices can be set read-only with "blockdev --setro
>> <device>".  The same thing can be done for LVM volumes using "lvchange
>> --permission r <volume>".  This read-only setting is independent of
>> device node permissions.  Therefore the device can still be opened
>> O_RDWR but actual writes will fail.
>>
>> This results in odd behavior for QEMU.  bdrv_open() is supposed to fail
>> if a read-only image is being opened with BDRV_O_RDWR.  By not failing
>> for Linux block devices, the guest boots up but every write produces an
>> I/O error.
>>
>> This patch checks whether the block device is read-only so that Linux
>> block devices behave like regular files.
>>
>> Reported-by: Sibiao Luo <sluo@redhat.com>
>> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Thanks, applied to the block branch.

Kevin
diff mbox

Patch

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 8b6b926..4dfdf98 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1257,9 +1257,43 @@  static int hdev_probe_device(const char *filename)
     return 0;
 }
 
+static int check_hdev_writable(BDRVRawState *s)
+{
+#if defined(BLKROGET)
+    /* Linux block devices can be configured "read-only" using blockdev(8).
+     * This is independent of device node permissions and therefore open(2)
+     * with O_RDWR succeeds.  Actual writes fail with EPERM.
+     *
+     * bdrv_open() is supposed to fail if the disk is read-only.  Explicitly
+     * check for read-only block devices so that Linux block devices behave
+     * properly.
+     */
+    struct stat st;
+    int readonly = 0;
+
+    if (fstat(s->fd, &st)) {
+        return -errno;
+    }
+
+    if (!S_ISBLK(st.st_mode)) {
+        return 0;
+    }
+
+    if (ioctl(s->fd, BLKROGET, &readonly) < 0) {
+        return -errno;
+    }
+
+    if (readonly) {
+        return -EACCES;
+    }
+#endif /* defined(BLKROGET) */
+    return 0;
+}
+
 static int hdev_open(BlockDriverState *bs, const char *filename, int flags)
 {
     BDRVRawState *s = bs->opaque;
+    int ret;
 
 #if defined(__APPLE__) && defined(__MACH__)
     if (strstart(filename, "/dev/cdrom", NULL)) {
@@ -1300,7 +1334,20 @@  static int hdev_open(BlockDriverState *bs, const char *filename, int flags)
     }
 #endif
 
-    return raw_open_common(bs, filename, flags, 0);
+    ret = raw_open_common(bs, filename, flags, 0);
+    if (ret < 0) {
+        return ret;
+    }
+
+    if (flags & BDRV_O_RDWR) {
+        ret = check_hdev_writable(s);
+        if (ret < 0) {
+            raw_close(bs);
+            return ret;
+        }
+    }
+
+    return ret;
 }
 
 #if defined(__linux__)