From: Naphtali Sprei <nsprei@redhat.com>
To: qemu-devel@nongnu.org
Subject: [Qemu-devel] [PATCH 2/3] Switch to bit-flags based read-only drive option implementation
Date: Thu, 14 Jan 2010 16:33:22 +0200 [thread overview]
Message-ID: <4B4F2B32.6030704@redhat.com> (raw)
Clean-up a little bit the RW related bits of BDRV_O_FLAGS.
BDRV_O_RDONLY gone (and so is BDRV_O_ACCESS). Default value for bdrv_flags (0/zero) is READ-ONLY.
Need to explicitly request READ-WRITE.
Instead of using the field 'readonly' of the BlockDriverState struct for passing the request,
pass the request in the flags parameter to the function.
Signed-off-by: Naphtali Sprei <nsprei@redhat.com>
---
block.c | 33 +++++++++++++++++----------------
block.h | 4 ++--
block/raw-posix.c | 2 +-
block/raw-win32.c | 4 ++--
block/vmdk.c | 9 +++++----
hw/xen_disk.c | 2 +-
monitor.c | 2 +-
qemu-img.c | 10 ++++++----
qemu-io.c | 14 ++++++--------
qemu-nbd.c | 2 +-
qemu-options.hx | 2 +-
vl.c | 14 +++++++++-----
12 files changed, 52 insertions(+), 46 deletions(-)
diff --git a/block.c b/block.c
index 115e591..cbd72cc 100644
--- a/block.c
+++ b/block.c
@@ -310,7 +310,7 @@ static BlockDriver *find_image_format(const char *filename)
if (drv && strcmp(drv->format_name, "vvfat") == 0)
return drv;
- ret = bdrv_file_open(&bs, filename, BDRV_O_RDONLY);
+ ret = bdrv_file_open(&bs, filename, 0);
if (ret < 0)
return NULL;
ret = bdrv_pread(bs, 0, buf, sizeof(buf));
@@ -356,7 +356,7 @@ int bdrv_open(BlockDriverState *bs, const char *filename, int flags)
int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
BlockDriver *drv)
{
- int ret, open_flags, try_rw;
+ int ret, open_flags;
char tmp_filename[PATH_MAX];
char backing_filename[PATH_MAX];
@@ -444,21 +444,23 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
if (flags & (BDRV_O_CACHE_WB|BDRV_O_NOCACHE))
bs->enable_write_cache = 1;
- /* Note: for compatibility, we open disk image files as RDWR, and
- RDONLY as fallback */
- try_rw = !bs->read_only || bs->is_temporary;
- if (!(flags & BDRV_O_FILE))
- open_flags = (try_rw ? BDRV_O_RDWR : 0) |
- (flags & (BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO));
- else
+ bs->read_only = BDRV_FLAGS_IS_RO(flags);
+ if (!(flags & BDRV_O_FILE)) {
+ open_flags = (flags & (BDRV_O_RDWR | BDRV_O_CACHE_MASK|BDRV_O_NATIVE_AIO));
+ if (bs->is_temporary) { /* snapshot should be writeable */
+ open_flags |= BDRV_O_RDWR;
+ }
+ } else {
open_flags = flags & ~(BDRV_O_FILE | BDRV_O_SNAPSHOT);
- if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv))
+ }
+ if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv)) {
ret = -ENOTSUP;
- else
+ } else {
ret = drv->bdrv_open(bs, filename, open_flags);
- if ((ret == -EACCES || ret == -EPERM) && !(flags & BDRV_O_FILE)) {
- ret = drv->bdrv_open(bs, filename, open_flags & ~BDRV_O_RDWR);
- bs->read_only = 1;
+ if ((ret == -EACCES || ret == -EPERM) && !(flags & BDRV_O_FILE)) {
+ ret = drv->bdrv_open(bs, filename, open_flags & ~BDRV_O_RDWR);
+ bs->read_only = 1;
+ }
}
if (ret < 0) {
qemu_free(bs->opaque);
@@ -481,14 +483,13 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
/* if there is a backing file, use it */
BlockDriver *back_drv = NULL;
bs->backing_hd = bdrv_new("");
- /* pass on read_only property to the backing_hd */
- bs->backing_hd->read_only = bs->read_only;
path_combine(backing_filename, sizeof(backing_filename),
filename, bs->backing_file);
if (bs->backing_format[0] != '\0')
back_drv = bdrv_find_format(bs->backing_format);
ret = bdrv_open2(bs->backing_hd, backing_filename, open_flags,
back_drv);
+ bs->backing_hd->read_only = BDRV_FLAGS_IS_RO(open_flags);
if (ret < 0) {
bdrv_close(bs);
return ret;
diff --git a/block.h b/block.h
index bee9ec5..a37a605 100644
--- a/block.h
+++ b/block.h
@@ -27,9 +27,7 @@ typedef struct QEMUSnapshotInfo {
uint64_t vm_clock_nsec; /* VM clock relative to boot */
} QEMUSnapshotInfo;
-#define BDRV_O_RDONLY 0x0000
#define BDRV_O_RDWR 0x0002
-#define BDRV_O_ACCESS 0x0003
#define BDRV_O_CREAT 0x0004 /* create an empty file */
#define BDRV_O_SNAPSHOT 0x0008 /* open the file read only and save writes in a snapshot */
#define BDRV_O_FILE 0x0010 /* open as a raw file (do not try to
@@ -42,6 +40,8 @@ typedef struct QEMUSnapshotInfo {
#define BDRV_O_NO_BACKING 0x0100 /* don't open the backing file */
#define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB)
+#define BDRV_O_DEFAULT_OPEN (BDRV_O_RDWR)
+#define BDRV_FLAGS_IS_RO(flags) ((flags & BDRV_O_RDWR) == 0)
#define BDRV_SECTOR_BITS 9
#define BDRV_SECTOR_SIZE (1 << BDRV_SECTOR_BITS)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 5a6a22b..b427211 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -138,7 +138,7 @@ static int raw_open_common(BlockDriverState *bs, const char *filename,
s->open_flags = open_flags | O_BINARY;
s->open_flags &= ~O_ACCMODE;
- if ((bdrv_flags & BDRV_O_ACCESS) == BDRV_O_RDWR) {
+ if (bdrv_flags & BDRV_O_RDWR) {
s->open_flags |= O_RDWR;
} else {
s->open_flags |= O_RDONLY;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 72acad5..01a6d25 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -81,7 +81,7 @@ static int raw_open(BlockDriverState *bs, const char *filename, int flags)
s->type = FTYPE_FILE;
- if ((flags & BDRV_O_ACCESS) == O_RDWR) {
+ if (flags & BDRV_O_RDWR) {
access_flags = GENERIC_READ | GENERIC_WRITE;
} else {
access_flags = GENERIC_READ;
@@ -337,7 +337,7 @@ static int hdev_open(BlockDriverState *bs, const char *filename, int flags)
}
s->type = find_device_type(bs, filename);
- if ((flags & BDRV_O_ACCESS) == O_RDWR) {
+ if (flags & BDRV_O_RDWR) {
access_flags = GENERIC_READ | GENERIC_WRITE;
} else {
access_flags = GENERIC_READ;
diff --git a/block/vmdk.c b/block/vmdk.c
index 4e48622..7f866f9 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -357,7 +357,7 @@ static int vmdk_parent_open(BlockDriverState *bs, const char * filename)
return -1;
}
parent_open = 1;
- if (bdrv_open(bs->backing_hd, parent_img_name, BDRV_O_RDONLY) < 0)
+ if (bdrv_open(bs->backing_hd, parent_img_name, 0) < 0)
goto failure;
parent_open = 0;
}
@@ -371,9 +371,10 @@ static int vmdk_open(BlockDriverState *bs, const char *filename, int flags)
uint32_t magic;
int l1_size, i, ret;
- if (parent_open)
- // Parent must be opened as RO.
- flags = BDRV_O_RDONLY;
+ if (parent_open) {
+ /* Parent must be opened as RO, turn off RDWR bit. */
+ flags &= ~BDRV_O_RDWR;
+ }
ret = bdrv_file_open(&s->hd, filename, flags);
if (ret < 0)
diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index 5c55251..beadf90 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -613,7 +613,7 @@ static int blk_init(struct XenDevice *xendev)
qflags = BDRV_O_RDWR;
} else {
mode = O_RDONLY;
- qflags = BDRV_O_RDONLY;
+ qflags = 0;
info |= VDISK_READONLY;
}
diff --git a/monitor.c b/monitor.c
index b824e7c..0f2bdca 100644
--- a/monitor.c
+++ b/monitor.c
@@ -916,7 +916,7 @@ static void do_change_block(Monitor *mon, const char *device,
}
if (eject_device(mon, bs, 0) < 0)
return;
- bdrv_open2(bs, filename, 0, drv);
+ bdrv_open2(bs, filename, BDRV_O_DEFAULT_OPEN, drv);
monitor_read_bdrv_key_start(mon, bs, NULL, NULL);
}
diff --git a/qemu-img.c b/qemu-img.c
index 48b9315..ccfdc30 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -204,7 +204,7 @@ static BlockDriverState *bdrv_new_open(const char *filename,
} else {
drv = NULL;
}
- if (bdrv_open2(bs, filename, BRDV_O_FLAGS, drv) < 0) {
+ if (bdrv_open2(bs, filename, BRDV_O_FLAGS | BDRV_O_DEFAULT_OPEN, drv) < 0) {
error("Could not open '%s'", filename);
}
if (bdrv_is_encrypted(bs)) {
@@ -468,7 +468,7 @@ static int img_commit(int argc, char **argv)
} else {
drv = NULL;
}
- if (bdrv_open2(bs, filename, BRDV_O_FLAGS, drv) < 0) {
+ if (bdrv_open2(bs, filename, BRDV_O_FLAGS | BDRV_O_RDWR, drv) < 0) {
error("Could not open '%s'", filename);
}
ret = bdrv_commit(bs);
@@ -966,10 +966,11 @@ static int img_snapshot(int argc, char **argv)
BlockDriverState *bs;
QEMUSnapshotInfo sn;
char *filename, *snapshot_name = NULL;
- int c, ret;
+ int c, ret, bdrv_oflags;
int action = 0;
qemu_timeval tv;
+ bdrv_oflags = BDRV_O_RDWR;
/* Parse commandline parameters */
for(;;) {
c = getopt(argc, argv, "la:c:d:h");
@@ -985,6 +986,7 @@ static int img_snapshot(int argc, char **argv)
return 0;
}
action = SNAPSHOT_LIST;
+ bdrv_oflags &= ~BDRV_O_RDWR; /* no need for RW */
break;
case 'a':
if (action) {
@@ -1022,7 +1024,7 @@ static int img_snapshot(int argc, char **argv)
if (!bs)
error("Not enough memory");
- if (bdrv_open2(bs, filename, 0, NULL) < 0) {
+ if (bdrv_open2(bs, filename, bdrv_oflags, NULL) < 0) {
error("Could not open '%s'", filename);
}
diff --git a/qemu-io.c b/qemu-io.c
index 1c19b92..b159bc9 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -1359,10 +1359,9 @@ open_f(int argc, char **argv)
}
}
- if (readonly)
- flags |= BDRV_O_RDONLY;
- else
- flags |= BDRV_O_RDWR;
+ if (!readonly) {
+ flags |= BDRV_O_RDWR;
+ }
if (optind != argc - 1)
return command_usage(&open_cmd);
@@ -1506,10 +1505,9 @@ int main(int argc, char **argv)
add_check_command(init_check_command);
/* open the device */
- if (readonly)
- flags |= BDRV_O_RDONLY;
- else
- flags |= BDRV_O_RDWR;
+ if (!readonly) {
+ flags |= BDRV_O_RDWR;
+ }
if ((argc - optind) == 1)
openfile(argv[optind], flags, growable);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 6707ea5..0f0e2fc 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -213,7 +213,7 @@ int main(int argc, char **argv)
int opt_ind = 0;
int li;
char *end;
- int flags = 0;
+ int flags = BDRV_O_DEFAULT_OPEN;
int partition = -1;
int ret;
int shared = 1;
diff --git a/qemu-options.hx b/qemu-options.hx
index e2edd71..4617867 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -103,7 +103,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive,
"-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n"
" [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n"
" [,cache=writethrough|writeback|none][,format=f][,serial=s]\n"
- " [,addr=A][,id=name][,aio=threads|native]\n"
+ " [,addr=A][,id=name][,aio=threads|native][,readonly=on|off]\n"
" use 'file' as a drive image\n")
DEF("set", HAS_ARG, QEMU_OPTION_set,
"-set group.id.arg=value\n"
diff --git a/vl.c b/vl.c
index 4f19505..6573bb9 100644
--- a/vl.c
+++ b/vl.c
@@ -2210,6 +2210,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
*fatal_error = 0;
return NULL;
}
+
bdrv_flags = 0;
if (snapshot) {
bdrv_flags |= BDRV_O_SNAPSHOT;
@@ -2227,16 +2228,19 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
}
if (ro == 1) {
- if (type == IF_IDE) {
- fprintf(stderr, "qemu: readonly flag not supported for drive with ide interface\n");
+ if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY) {
+ fprintf(stderr, "qemu: readonly flag not supported for drive with this interface\n");
return NULL;
}
- (void)bdrv_set_read_only(dinfo->bdrv, 1);
}
-
+ /*
+ * cdrom is read-only. Set it now, after above interface checking
+ * since readonly attribute not explicitly required, so no error.
+ */
if (media == MEDIA_CDROM) {
- (void)bdrv_set_read_only(dinfo->bdrv, 1);
+ ro = 1;
}
+ bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
if (bdrv_open2(dinfo->bdrv, file, bdrv_flags, drv) < 0) {
fprintf(stderr, "qemu: could not open disk image %s: %s\n",
--
1.6.3.3
next reply other threads:[~2010-01-14 14:33 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-01-14 14:33 Naphtali Sprei [this message]
2010-01-14 15:29 ` [Qemu-devel] [PATCH 2/3] Switch to bit-flags based read-only drive option implementation Kevin Wolf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4B4F2B32.6030704@redhat.com \
--to=nsprei@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.