Skip to content

Commit bca5283

Browse files
committed
file-posix: Fix check_hdev_writable() with auto-read-only
For Linux block devices, being able to open the device read-write doesn't necessarily mean that the device is actually writable (one example is a read-only LV, as you get with lvchange -pr <device>). We have check_hdev_writable() to check this condition and fail opening the image read-write if it's not actually writable. However, this check doesn't take auto-read-only into account, but results in a hard failure instead of downgrading to read-only where possible. Fix this and do the writable check not based on BDRV_O_RDWR, but only when this actually results in opening the file read-write. A second check is inserted in raw_reconfigure_getfd() to have the same check when dynamic auto-read-only upgrades an image file from read-only to read-write. Signed-off-by: Kevin Wolf <[email protected]> Message-Id: <[email protected]> Reviewed-by: Max Reitz <[email protected]> Signed-off-by: Kevin Wolf <[email protected]>
1 parent 20eaf1b commit bca5283

File tree

1 file changed

+21
-12
lines changed

1 file changed

+21
-12
lines changed

block/file-posix.c

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp)
425425
}
426426
}
427427

428-
static int check_hdev_writable(BDRVRawState *s)
428+
static int check_hdev_writable(int fd)
429429
{
430430
#if defined(BLKROGET)
431431
/* Linux block devices can be configured "read-only" using blockdev(8).
@@ -439,15 +439,15 @@ static int check_hdev_writable(BDRVRawState *s)
439439
struct stat st;
440440
int readonly = 0;
441441

442-
if (fstat(s->fd, &st)) {
442+
if (fstat(fd, &st)) {
443443
return -errno;
444444
}
445445

446446
if (!S_ISBLK(st.st_mode)) {
447447
return 0;
448448
}
449449

450-
if (ioctl(s->fd, BLKROGET, &readonly) < 0) {
450+
if (ioctl(fd, BLKROGET, &readonly) < 0) {
451451
return -errno;
452452
}
453453

@@ -642,6 +642,15 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
642642
}
643643
s->fd = fd;
644644

645+
/* Check s->open_flags rather than bdrv_flags due to auto-read-only */
646+
if (s->open_flags & O_RDWR) {
647+
ret = check_hdev_writable(s->fd);
648+
if (ret < 0) {
649+
error_setg_errno(errp, -ret, "The device is not writable");
650+
goto fail;
651+
}
652+
}
653+
645654
s->perm = 0;
646655
s->shared_perm = BLK_PERM_ALL;
647656

@@ -1034,6 +1043,15 @@ static int raw_reconfigure_getfd(BlockDriverState *bs, int flags,
10341043
}
10351044
}
10361045

1046+
if (fd != -1 && (*open_flags & O_RDWR)) {
1047+
ret = check_hdev_writable(fd);
1048+
if (ret < 0) {
1049+
qemu_close(fd);
1050+
error_setg_errno(errp, -ret, "The device is not writable");
1051+
return -1;
1052+
}
1053+
}
1054+
10371055
return fd;
10381056
}
10391057

@@ -3478,15 +3496,6 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
34783496
/* Since this does ioctl the device must be already opened */
34793497
bs->sg = hdev_is_sg(bs);
34803498

3481-
if (flags & BDRV_O_RDWR) {
3482-
ret = check_hdev_writable(s);
3483-
if (ret < 0) {
3484-
raw_close(bs);
3485-
error_setg_errno(errp, -ret, "The device is not writable");
3486-
return ret;
3487-
}
3488-
}
3489-
34903499
return ret;
34913500
}
34923501

0 commit comments

Comments
 (0)