From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KOWNZ-0001rB-8W for qemu-devel@nongnu.org; Thu, 31 Jul 2008 07:31:33 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KOWNX-0001pf-SD for qemu-devel@nongnu.org; Thu, 31 Jul 2008 07:31:32 -0400 Received: from [199.232.76.173] (port=50471 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KOWNX-0001pR-Kw for qemu-devel@nongnu.org; Thu, 31 Jul 2008 07:31:31 -0400 Received: from mx1.redhat.com ([66.187.233.31]:47114) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KOWNX-0005i2-3V for qemu-devel@nongnu.org; Thu, 31 Jul 2008 07:31:31 -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 m6VBVM4J017429 for ; Thu, 31 Jul 2008 07:31:22 -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 m6VBVLpm017187 for ; Thu, 31 Jul 2008 07:31:21 -0400 Received: (from berrange@localhost) by file.fab.redhat.com (8.13.1/8.13.1/Submit) id m6VBVLOm016271 for qemu-devel@nongnu.org; Thu, 31 Jul 2008 12:31:21 +0100 Date: Thu, 31 Jul 2008 12:31:20 +0100 From: "Daniel P. Berrange" Message-ID: <20080731113120.GJ23888@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Subject: [Qemu-devel] PATCH: Control over drive open modes for backing file 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 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 useless as is. So this patch does a couple of things: - Change the access flags to be #define BDRV_O_RDONLY 0x0001 /* Force read-only */ #define BDRV_O_WRONLY 0x0002 /* Force writeable, no fallback */ #define BDRV_O_RDWR 0x0003 /* Try writeable, fallback to read-only */ - Change all callers of bdrv_open/open2/file_open to pass the correct access flags they desire instead of 0. - 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 first, and fallback to read-only if getting EACCESS. This is the default, and matches current behaviour. mode=wo - try to open read-write and exit if this fails. mode=ro - try to open read-only and exit if this fails - 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 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_WRONLY) { 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_WRONLY) { 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 = BDRV_O_RDWR; /* 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, "wo") == 0) + mode = BDRV_O_WRONLY; + } + + 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|wo|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_WRONLY) { open_flags |= O_RDWR; } else { open_flags |= O_RDONLY; @@ -896,7 +896,7 @@ } #endif open_flags = O_BINARY; - if ((flags & BDRV_O_ACCESS) == O_RDWR) { + if (flags & BDRV_O_WRONLY) { 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,11 @@ if (bs == NULL) return 1; + if (readonly) + flags |= BDRV_O_RDONLY; + else + flags |= BDRV_O_RDWR; + 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, falling back to read-only on failure, +"wo" to open the file read-write, with no fallback or "ro" to open the file read-only. +The default is "rw". @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 = BDRV_O_RDWR; + if (mode) { + if (strcmp(mode, "ro") == 0) + flags = BDRV_O_RDONLY; + else if (strcmp(mode, "wo") == 0) + flags = BDRV_O_WRONLY; + } + 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) @@ -348,7 +348,7 @@ if (!bs1) { return -ENOMEM; } - if (bdrv_open(bs1, filename, 0) < 0) { + if (bdrv_open(bs1, filename, BDRV_O_RDWR) < 0) { bdrv_delete(bs1); return -1; } @@ -381,15 +381,20 @@ 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 O_WRONLY and O_RDONLY are set, try read-write and fallback + * to readonly + * If only O_WRONLY is set, then try read-write with no fallback + * If only O_RDONLY is set, then try read-only with no fallback + */ if (!(flags & BDRV_O_FILE)) - open_flags = BDRV_O_RDWR | (flags & BDRV_O_DIRECT); + open_flags = (flags & (BDRV_O_DIRECT | 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); + if (ret == -EACCES && !(flags & BDRV_O_FILE) && + ((flags & BDRV_O_RDWR) == BDRV_O_RDWR)) { + open_flags = (flags & (BDRV_O_DIRECT | BDRV_O_RDONLY)); + ret = drv->bdrv_open(bs, filename, open_flags); bs->read_only = 1; } if (ret < 0) { @@ -416,7 +421,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_RDWR) < 0) goto fail; } Index: block.h =================================================================== --- block.h (revision 4975) +++ block.h (working copy) @@ -36,9 +36,9 @@ 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 /* Force read-only */ +#define BDRV_O_WRONLY 0x0002 /* Force writeable, no fallback */ +#define BDRV_O_RDWR 0x0003 /* Try writeable, fallback to read-only */ #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 Index: hw/usb-msd.c =================================================================== --- hw/usb-msd.c (revision 4975) +++ hw/usb-msd.c (working copy) @@ -523,7 +523,7 @@ return NULL; bdrv = bdrv_new("usb"); - if (bdrv_open(bdrv, filename, 0) < 0) + if (bdrv_open(bdrv, filename, BDRV_O_RDWR) < 0) goto fail; if (qemu_key_check(bdrv, filename)) goto fail; Index: qemu-img.c =================================================================== --- qemu-img.c (revision 4975) +++ qemu-img.c (working copy) @@ -186,7 +186,7 @@ } else { drv = NULL; } - if (bdrv_open2(bs, filename, 0, drv) < 0) { + if (bdrv_open2(bs, filename, BDRV_O_RDWR, drv) < 0) { error("Could not open '%s'", filename); } if (bdrv_is_encrypted(bs)) { @@ -317,7 +317,7 @@ } else { drv = NULL; } - if (bdrv_open2(bs, filename, 0, drv) < 0) { + if (bdrv_open2(bs, filename, BDRV_O_RDWR, drv) < 0) { error("Could not open '%s'", filename); } ret = bdrv_commit(bs); @@ -691,7 +691,7 @@ } else { drv = NULL; } - if (bdrv_open2(bs, filename, 0, drv) < 0) { + if (bdrv_open2(bs, filename, BDRV_O_RDWR, drv) < 0) { error("Could not open '%s'", filename); } bdrv_get_format(bs, fmt_name, sizeof(fmt_name)); -- |: 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 :|