From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KOrtY-0002p6-3h for qemu-devel@nongnu.org; Fri, 01 Aug 2008 06:30:00 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KOrtW-0002na-Sy for qemu-devel@nongnu.org; Fri, 01 Aug 2008 06:29:59 -0400 Received: from [199.232.76.173] (port=36169 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KOrtW-0002mm-JN for qemu-devel@nongnu.org; Fri, 01 Aug 2008 06:29:58 -0400 Received: from mx1.redhat.com ([66.187.233.31]:39376) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KOrtS-0003dm-JG for qemu-devel@nongnu.org; Fri, 01 Aug 2008 06:29:56 -0400 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id m71ATreE014890 for ; Fri, 1 Aug 2008 06:29:53 -0400 Received: from file.fab.redhat.com (file.fab.redhat.com [10.33.63.6]) by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id m71ATqDU020770 for ; Fri, 1 Aug 2008 06:29:53 -0400 Received: (from berrange@localhost) by file.fab.redhat.com (8.13.1/8.13.1/Submit) id m71ATqAW006218 for qemu-devel@nongnu.org; Fri, 1 Aug 2008 11:29:52 +0100 Date: Fri, 1 Aug 2008 11:29:52 +0100 From: "Daniel P. Berrange" Message-ID: <20080801102949.GN23993@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Subject: [Qemu-devel] PATCH: v3 Allow control over drive file open mode Reply-To: "Daniel P. Berrange" , qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org 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 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 cache=@var{cache} @var{cache} is "on" or "off" and allows to disable host cache to access data. +@item mode=@var{mode} +@var{mode} 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 format=@var{format} 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 :|