[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] PATCH: v3 Allow control over drive file open mode
From: |
Daniel P. Berrange |
Subject: |
[Qemu-devel] PATCH: v3 Allow control over drive file open mode |
Date: |
Fri, 1 Aug 2008 11:29:52 +0100 |
User-agent: |
Mutt/1.4.1i |
This is version 3 of the patch.
The current block driver code will attempt to open a file backing a drive
for read/write with O_RDWR first, and if that fails, fallback to opening
it readonly with O_RDONLY. So if you set file permissions to readonly on
the underlying drive backing store, QEMU will fallback to opening it read
only, and discard any writes.
Xen has a concept of a read-only disks in its configuration format, and
thus it would be desirable to have an explicit option to request that a
drive operate read-only, regardless of whether underlying file permissions
allow write access or not. We'd like to support this in libvirt too for
QEMU/KVM guests. Finally, in some cases it is desirable to see the failure
if the disk can't be opened read-write, rather than falling back to read
only mode - many guests will be more or less inoperable if their root
filesystem is read-only so there's little point booting.
The current block.h file did already have flags defined
#define BDRV_O_RDONLY 0x0000
#define BDRV_O_RDWR 0x0002
#define BDRV_O_ACCESS 0x0003
However the bdrv_open2() method was treating a 'flags' value of 0, as being
effectively RDWR, and nearly all callers pass in 0 even when they expect
to get a writable file, so the O_RDONLY flag was not usable as is.
So this patch does a couple of things:
- Change the value of the RDONLY flag to be 0x0001 so it can be
distinguished from 0, which all callers assume means read-write
will read-only fallback.
- Adds a new option to the '-drive' command line parameter to specify
how the file should be opened, and any fallback
mode=rw - try to open read-write and return error if this fails.
mode=ro - try to open read-only and return error if this fails
If no 'mode' is provided, it'll try read-write, and fallback to
read-only. This matches existing behaviour
- Extends the monitor 'change' command to allow an optional mode to
be specified, again defaulting to current behaviour. Takes the same
values as the 'mode' param
Signed-off-by: Daniel P. Berrange <address@hidden>
block-raw-posix.c | 6 +++---
block-raw-win32.c | 4 ++--
block.c | 21 ++++++++++++++-------
block.h | 5 ++---
monitor.c | 20 ++++++++++++++------
qemu-doc.texi | 4 ++++
qemu-nbd.c | 3 +++
vl.c | 15 ++++++++++++---
8 files changed, 54 insertions(+), 24 deletions(-)
NB This is against SVN revision 4975, but will likely conflict with
the other patch currently being discussed onlist wrt to qcow encryption
and passwords. I can trivially rebase it if required.
Daniel
Index: block-raw-win32.c
===================================================================
--- block-raw-win32.c (revision 4975)
+++ block-raw-win32.c (working copy)
@@ -90,7 +90,7 @@
s->type = FTYPE_FILE;
- if ((flags & BDRV_O_ACCESS) == O_RDWR) {
+ if (!(flags & BDRV_O_RDONLY)) {
access_flags = GENERIC_READ | GENERIC_WRITE;
} else {
access_flags = GENERIC_READ;
@@ -463,7 +463,7 @@
}
s->type = find_device_type(bs, filename);
- if ((flags & BDRV_O_ACCESS) == O_RDWR) {
+ if (!(flags & BDRV_O_RDONLY)) {
access_flags = GENERIC_READ | GENERIC_WRITE;
} else {
access_flags = GENERIC_READ;
Index: vl.c
===================================================================
--- vl.c (revision 4975)
+++ vl.c (working copy)
@@ -5399,11 +5399,12 @@
int max_devs;
int index;
int cache;
+ int mode = 0; /* Default to writable, with fallback to readonly for
back-compat */
int bdrv_flags;
char *str = arg->opt;
char *params[] = { "bus", "unit", "if", "index", "cyls", "heads",
"secs", "trans", "media", "snapshot", "file",
- "cache", "format", NULL };
+ "cache", "format", "mode", NULL };
if (check_params(buf, sizeof(buf), params, str) < 0) {
fprintf(stderr, "qemu: unknown parameter '%s' in '%s'\n",
@@ -5585,6 +5586,14 @@
}
}
+ if (get_param_value(buf, sizeof(buf), "mode", str)) {
+ if (strcmp(buf, "ro") == 0)
+ mode = BDRV_O_RDONLY;
+ else if (strcmp(buf, "rw") == 0)
+ mode = BDRV_O_RDWR;
+ }
+
+
if (arg->file == NULL)
get_param_value(file, sizeof(file), "file", str);
else
@@ -5682,7 +5691,7 @@
}
if (!file[0])
return 0;
- bdrv_flags = 0;
+ bdrv_flags = mode;
if (snapshot)
bdrv_flags |= BDRV_O_SNAPSHOT;
if (!cache)
@@ -7605,7 +7614,7 @@
"-cdrom file use 'file' as IDE cdrom image (cdrom is ide1
master)\n"
"-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=on|off][,format=f]\n"
+ " [,cache=on|off][,format=f][,mode=ro|rw]\n"
" use 'file' as a drive image\n"
"-mtdblock file use 'file' as on-board Flash memory image\n"
"-sd file use 'file' as SecureDigital card image\n"
Index: block-raw-posix.c
===================================================================
--- block-raw-posix.c (revision 4975)
+++ block-raw-posix.c (working copy)
@@ -103,7 +103,7 @@
s->lseek_err_cnt = 0;
open_flags = O_BINARY;
- if ((flags & BDRV_O_ACCESS) == O_RDWR) {
+ if (!(flags & BDRV_O_RDONLY)) {
open_flags |= O_RDWR;
} else {
open_flags |= O_RDONLY;
@@ -117,7 +117,7 @@
#endif
s->type = FTYPE_FILE;
-
+ fprintf(stderr, "Open raw %d %d %s\n", open_flags, flags, filename);
fd = open(filename, open_flags, 0644);
if (fd < 0) {
ret = -errno;
@@ -896,7 +896,7 @@
}
#endif
open_flags = O_BINARY;
- if ((flags & BDRV_O_ACCESS) == O_RDWR) {
+ if (!(flags & BDRV_O_RDONLY)) {
open_flags |= O_RDWR;
} else {
open_flags |= O_RDONLY;
Index: qemu-nbd.c
===================================================================
--- qemu-nbd.c (revision 4975)
+++ qemu-nbd.c (working copy)
@@ -332,6 +332,9 @@
if (bs == NULL)
return 1;
+ if (readonly)
+ flags |= BDRV_O_RDONLY;
+
if (bdrv_open(bs, argv[optind], flags) == -1)
return 1;
Index: qemu-doc.texi
===================================================================
--- qemu-doc.texi (revision 4975)
+++ qemu-doc.texi (working copy)
@@ -268,6 +268,10 @@
@var{snapshot} is "on" or "off" and allows to enable snapshot for given drive
(see @option{-snapshot}).
@item address@hidden
@var{cache} is "on" or "off" and allows to disable host cache to access data.
address@hidden address@hidden
address@hidden is "rw" to open the file read-write, or "ro" to open the file
read-only.
+If neither is specified, an attempt will be made to open read-write, falling
back
+to read-only if this fails.
@item address@hidden
Specify which disk @var{format} will be used rather than detecting
the format. Can be used to specifiy format=raw to avoid interpreting
Index: monitor.c
===================================================================
--- monitor.c (revision 4975)
+++ monitor.c (working copy)
@@ -396,11 +396,19 @@
eject_device(bs, force);
}
-static void do_change_block(const char *device, const char *filename, const
char *fmt)
+static void do_change_block(const char *device, const char *filename, const
char *fmt, const char *mode)
{
BlockDriverState *bs;
BlockDriver *drv = NULL;
+ int flags = 0;
+ if (mode) {
+ if (strcmp(mode, "ro") == 0)
+ flags = BDRV_O_RDONLY;
+ else if (strcmp(mode, "rw") == 0)
+ flags = BDRV_O_RDWR;
+ }
+
bs = bdrv_find(device);
if (!bs) {
term_printf("device not found\n");
@@ -415,7 +423,7 @@
}
if (eject_device(bs, 0) < 0)
return;
- bdrv_open2(bs, filename, 0, drv);
+ bdrv_open2(bs, filename, flags, drv);
qemu_key_check(bs, filename);
}
@@ -434,12 +442,12 @@
}
}
-static void do_change(const char *device, const char *target, const char *fmt)
+static void do_change(const char *device, const char *target, const char *fmt,
const char *mode)
{
if (strcmp(device, "vnc") == 0) {
do_change_vnc(target);
} else {
- do_change_block(device, target, fmt);
+ do_change_block(device, target, fmt, mode);
}
}
@@ -1374,8 +1382,8 @@
"", "quit the emulator" },
{ "eject", "-fB", do_eject,
"[-f] device", "eject a removable medium (use -f to force it)" },
- { "change", "BFs?", do_change,
- "device filename [format]", "change a removable medium, optional format"
},
+ { "change", "BFs?s?", do_change,
+ "device filename [format] [mode]", "change a removable medium, optional
format and mode" },
{ "screendump", "F", do_screen_dump,
"filename", "save screen into PPM image 'filename'" },
{ "logfile", "F", do_logfile,
Index: block.c
===================================================================
--- block.c (revision 4975)
+++ block.c (working copy)
@@ -381,17 +381,24 @@
bs->opaque = qemu_mallocz(drv->instance_size);
if (bs->opaque == NULL && drv->instance_size > 0)
return -1;
- /* Note: for compatibility, we open disk image files as RDWR, and
- RDONLY as fallback */
+ /* If BDRV_O_RDONLY is set open in read only mode
+ * If BDRV_O_RDWR is set open in read write mode
+ * If neither is set, try read-write and fallback to read only
+ */
if (!(flags & BDRV_O_FILE))
- open_flags = BDRV_O_RDWR | (flags & BDRV_O_DIRECT);
+ open_flags = (flags & (BDRV_O_DIRECT | BDRV_O_RDONLY | BDRV_O_RDWR));
else
open_flags = flags & ~(BDRV_O_FILE | BDRV_O_SNAPSHOT);
ret = drv->bdrv_open(bs, filename, open_flags);
- if (ret == -EACCES && !(flags & BDRV_O_FILE)) {
- ret = drv->bdrv_open(bs, filename, BDRV_O_RDONLY);
+
+ /* Trying fallback to read only mode here.. */
+ if (ret == -EACCES &&
+ !(flags & (BDRV_O_RDWR | BDRV_O_RDONLY))) {
+ open_flags |= BDRV_O_RDONLY;
+ ret = drv->bdrv_open(bs, filename, open_flags);
+ }
+ if (ret == 0 && (flags & BDRV_O_RDONLY))
bs->read_only = 1;
- }
if (ret < 0) {
qemu_free(bs->opaque);
bs->opaque = NULL;
@@ -416,7 +423,7 @@
}
path_combine(backing_filename, sizeof(backing_filename),
filename, bs->backing_file);
- if (bdrv_open(bs->backing_hd, backing_filename, 0) < 0)
+ if (bdrv_open(bs->backing_hd, backing_filename, flags & (BDRV_O_RDONLY
| BDRV_O_RDWR)) < 0)
goto fail;
}
Index: block.h
===================================================================
--- block.h (revision 4975)
+++ block.h (working copy)
@@ -36,9 +36,8 @@
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_RDONLY 0x0001 /* Open as read-only */
+#define BDRV_O_RDWR 0x0002 /* Open as read-write */
#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
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
- [Qemu-devel] PATCH: v3 Allow control over drive file open mode,
Daniel P. Berrange <=
- Re: [Qemu-devel] PATCH: v3 Allow control over drive file open mode, Anthony Liguori, 2008/08/01
- Re: [Qemu-devel] PATCH: v3 Allow control over drive file open mode, Daniel P. Berrange, 2008/08/01
- Re: [Qemu-devel] PATCH: v3 Allow control over drive file open mode, Ian Jackson, 2008/08/01
- Re: [Qemu-devel] PATCH: v3 Allow control over drive file open mode, Anthony Liguori, 2008/08/01
- Re: [Qemu-devel] PATCH: v3 Allow control over drive file open mode, Jamie Lokier, 2008/08/01
- Re: [Qemu-devel] PATCH: v3 Allow control over drive file open mode, Ian Jackson, 2008/08/11
- Re: [Qemu-devel] PATCH: v3 Allow control over drive file open mode, Kevin Wolf, 2008/08/12
- Re: [Qemu-devel] PATCH: v3 Allow control over drive file open mode, Anthony Liguori, 2008/08/12