From da888d37b0b85fc23e4ea55ab8b0c482d4918afb Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 5 Feb 2013 12:28:33 +0100 Subject: [PATCH 1/2] block/raw-posix: detect readonly Linux block devices using BLKROGET Linux block devices can be set read-only with "blockdev --setro ". The same thing can be done for LVM volumes using "lvchange --permission r ". 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 Suggested-by: Paolo Bonzini Signed-off-by: Stefan Hajnoczi Reviewed-by: Markus Armbruster Signed-off-by: Kevin Wolf --- block/raw-posix.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 8b6b92608b..4dfdf985b0 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__) From 33ccf6675faa3c56f30399e184064fd126904515 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 12 Feb 2013 12:25:15 +0100 Subject: [PATCH 2/2] Revert "block/vpc: Fix size calculation" This reverts commit f880defbb06708d30a38ce9f2667067626acdd38. Jeff Cody's testing revealed that the interpretation of size differs even between VirtualPC and HyperV. Revert this so there is time to consider the impact of any backwards incompatible behavior this change creates. Signed-off-by: Stefan Hajnoczi --- block/vpc.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/block/vpc.c b/block/vpc.c index b4ff5646a2..82229ef5a0 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -34,8 +34,6 @@ #define HEADER_SIZE 512 -#define VHD_SECTOR_SIZE 512 - //#define CACHE enum vhd_type { @@ -206,13 +204,11 @@ static int vpc_open(BlockDriverState *bs, int flags) /* Write 'checksum' back to footer, or else will leave it with zero. */ footer->checksum = be32_to_cpu(checksum); - /* The visible size of a image in Virtual PC depends on the guest: - * QEMU and other emulators report the real size (here in sectors). - * All modern operating systems use this real size. - * Very old operating systems use CHS values to calculate the total size. - * This calculated size is usually smaller than the real size. - */ - bs->total_sectors = be64_to_cpu(footer->size) / VHD_SECTOR_SIZE; + // The visible size of a image in Virtual PC depends on the geometry + // rather than on the size stored in the footer (the size in the footer + // is too large usually) + bs->total_sectors = (int64_t) + be16_to_cpu(footer->cyls) * footer->heads * footer->secs_per_cyl; /* Allow a maximum disk size of approximately 2 TB */ if (bs->total_sectors >= 65535LL * 255 * 255) {