From patchwork Thu Dec 24 15:02:43 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naphtali Sprei X-Patchwork-Id: 41781 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id AA277B7BF0 for ; Fri, 25 Dec 2009 02:05:16 +1100 (EST) Received: from localhost ([127.0.0.1]:60175 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NNpFV-0002b0-Fy for incoming@patchwork.ozlabs.org; Thu, 24 Dec 2009 10:05:09 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NNpDI-0001MJ-Hy for qemu-devel@nongnu.org; Thu, 24 Dec 2009 10:02:52 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NNpDD-0001K9-PP for qemu-devel@nongnu.org; Thu, 24 Dec 2009 10:02:51 -0500 Received: from [199.232.76.173] (port=33244 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NNpDD-0001K5-Lc for qemu-devel@nongnu.org; Thu, 24 Dec 2009 10:02:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51446) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NNpDC-0004dD-O8 for qemu-devel@nongnu.org; Thu, 24 Dec 2009 10:02:47 -0500 Received: from int-mx05.intmail.prod.int.phx2.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.18]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id nBOF2kBi010092 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 24 Dec 2009 10:02:46 -0500 Received: from [10.35.0.60] (dhcp-0-60.tlv.redhat.com [10.35.0.60]) by int-mx05.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id nBOF2h2w004538 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Thu, 24 Dec 2009 10:02:45 -0500 Message-ID: <4B338293.4000306@redhat.com> Date: Thu, 24 Dec 2009 17:02:43 +0200 From: Naphtali Sprei User-Agent: Thunderbird 2.0.0.23 (X11/20090817) MIME-Version: 1.0 To: qemu-devel@nongnu.org X-Scanned-By: MIMEDefang 2.67 on 10.5.11.18 X-detected-operating-system: by monty-python.gnu.org: Genre and OS details not recognized. Subject: [Qemu-devel] [PATCH v2] Added 'access' option to -drive flag X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org The new option is: access=[rw|ro|auto] rw: open the drive's file with Read and Write permission, don't continue if failed ro: open the file only with Read permission auto: open the file with Read and Write permission, if failed, try only Read permision For compatibility reasons, the default is 'auto'. Should be changed later on. This option replaces the 'readonly' options added lately. Instead of using the field 'readonly' of the BlockDriverState struct for passing the request, pass the request in the flags parameter to the function. The BDRV_O_RDWR/RDONLY becomes just one bit, BDRV_O_RDONLY gone. --- block.c | 27 +++++++++++++++------------ block.h | 6 ++++-- block/raw-posix.c | 2 +- block/raw-win32.c | 4 ++-- block/vmdk.c | 4 ++-- hw/xen_disk.c | 5 +++-- monitor.c | 2 +- qemu-config.c | 4 ++-- qemu-img.c | 14 ++++++++------ qemu-nbd.c | 2 +- qemu-options.hx | 2 +- vl.c | 48 +++++++++++++++++++++++++++++++++++++++--------- 12 files changed, 79 insertions(+), 41 deletions(-) diff --git a/block.c b/block.c index 3f3496e..bdf140a 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, ~BDRV_O_RDWR); 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]; @@ -378,7 +378,7 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags, /* if there is a backing file, use it */ bs1 = bdrv_new(""); - ret = bdrv_open2(bs1, filename, 0, drv); + ret = bdrv_open2(bs1, filename, ~BDRV_O_RDWR, drv); if (ret < 0) { bdrv_delete(bs1); return ret; @@ -445,18 +445,22 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags, 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 + RDONLY as fallback (if flag BDRV_O_RO_FALLBACK is on) */ + 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 */ + open_flags |= BDRV_O_RDWR; + } + } else { open_flags = flags & ~(BDRV_O_FILE | BDRV_O_SNAPSHOT); + } if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv)) ret = -ENOTSUP; else ret = drv->bdrv_open(bs, filename, open_flags); - if ((ret == -EACCES || ret == -EPERM) && !(flags & BDRV_O_FILE)) { + if ((ret == -EACCES || ret == -EPERM) && !(flags & BDRV_O_FILE) && + (flags & BDRV_O_RO_FALLBACK)) { ret = drv->bdrv_open(bs, filename, open_flags & ~BDRV_O_RDWR); bs->read_only = 1; } @@ -481,14 +485,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 fa51ddf..f2501be 100644 --- a/block.h +++ b/block.h @@ -28,8 +28,8 @@ typedef struct QEMUSnapshotInfo { } QEMUSnapshotInfo; #define BDRV_O_RDONLY 0x0000 -#define BDRV_O_RDWR 0x0002 -#define BDRV_O_ACCESS 0x0003 +#define BDRV_O_RDWR 0x0001 +#define BDRV_O_RO_FALLBACK 0x0002 #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 @@ -41,6 +41,8 @@ typedef struct QEMUSnapshotInfo { #define BDRV_O_NATIVE_AIO 0x0080 /* use native AIO instead of the thread pool */ #define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB) +#define BDRV_O_DEFAULT_OPEN (BDRV_O_RDWR | BDRV_O_RO_FALLBACK) +#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..8a24084 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, ~BDRV_O_RDWR) < 0) goto failure; parent_open = 0; } @@ -373,7 +373,7 @@ static int vmdk_open(BlockDriverState *bs, const char *filename, int flags) if (parent_open) // Parent must be opened as RO. - flags = BDRV_O_RDONLY; + 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..0a45ae3 100644 --- a/hw/xen_disk.c +++ b/hw/xen_disk.c @@ -610,10 +610,11 @@ static int blk_init(struct XenDevice *xendev) /* read-only ? */ if (strcmp(blkdev->mode, "w") == 0) { mode = O_RDWR; - qflags = BDRV_O_RDWR; + /* for compatibility, also allow readonly fallback, for now */ + qflags = BDRV_O_RDWR | BDRV_O_RO_FALLBACK; } else { mode = O_RDONLY; - qflags = BDRV_O_RDONLY; + qflags = ~BDRV_O_RDWR; info |= VDISK_READONLY; } diff --git a/monitor.c b/monitor.c index c0dc48e..57b1aba 100644 --- a/monitor.c +++ b/monitor.c @@ -915,7 +915,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-config.c b/qemu-config.c index c3203c8..f7dd4a0 100644 --- a/qemu-config.c +++ b/qemu-config.c @@ -76,8 +76,8 @@ QemuOptsList qemu_drive_opts = { .type = QEMU_OPT_STRING, .help = "pci address (virtio only)", },{ - .name = "readonly", - .type = QEMU_OPT_BOOL, + .name = "access", + .type = QEMU_OPT_STRING, }, { /* end if list */ } }, diff --git a/qemu-img.c b/qemu-img.c index 1d97f2e..6ced3f0 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -201,7 +201,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)) { @@ -406,7 +406,7 @@ static int img_check(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_check(bs); @@ -465,7 +465,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); @@ -884,7 +884,7 @@ static int img_info(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); } bdrv_get_format(bs, fmt_name, sizeof(fmt_name)); @@ -932,10 +932,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; /* but no read-only fallback */ /* Parse commandline parameters */ for(;;) { c = getopt(argc, argv, "la:c:d:h"); @@ -951,6 +952,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) { @@ -988,7 +990,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-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 b8cc375..a83628c 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][,access=rw|ro|auto]\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 e881e45..86836bf 100644 --- a/vl.c +++ b/vl.c @@ -2090,6 +2090,7 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, int *fatal_error) { const char *buf; + const char *access_buf = 0; const char *file = NULL; char devname[128]; const char *serial; @@ -2104,12 +2105,12 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, int index; int cache; int aio = 0; - int ro = 0; int bdrv_flags; int on_read_error, on_write_error; const char *devaddr; DriveInfo *dinfo; int snapshot = 0; + int read_write, ro_fallback; *fatal_error = 1; @@ -2137,7 +2138,6 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, secs = qemu_opt_get_number(opts, "secs", 0); snapshot = qemu_opt_get_bool(opts, "snapshot", 0); - ro = qemu_opt_get_bool(opts, "readonly", 0); file = qemu_opt_get(opts, "file"); serial = qemu_opt_get(opts, "serial"); @@ -2420,6 +2420,31 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, *fatal_error = 0; return NULL; } + + read_write = 1; + ro_fallback = 1; + access_buf = qemu_opt_get(opts, "access"); + if (access_buf) { + if (!strcmp(access_buf, "ro")) { + read_write = 0; + ro_fallback = 0; + } else if (!strcmp(access_buf, "rw")) { + read_write = 1; + ro_fallback = 0; + } else if (!strcmp(access_buf, "auto")) { /* default, but keep it explicit */ + read_write = 1; + ro_fallback = 1; + } else { + fprintf(stderr, "qemu: '%s' invalid 'access' option (valid values: [rw|ro|auto] )\n", access_buf); + return NULL; + } + if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY && + ( !strcmp(access_buf, "ro") || !strcmp(access_buf, "auto") )) { + fprintf(stderr, "qemu: access=%s flag not supported for drive of this interface\n", access_buf); + return NULL; + } + } + bdrv_flags = 0; if (snapshot) { bdrv_flags |= BDRV_O_SNAPSHOT; @@ -2435,17 +2460,22 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, } else { bdrv_flags &= ~BDRV_O_NATIVE_AIO; } + + if (read_write) { + bdrv_flags |= BDRV_O_RDWR; + } else { + bdrv_flags &= ~BDRV_O_RDWR; + } - if (ro == 1) { - if (type == IF_IDE) { - fprintf(stderr, "qemu: readonly flag not supported for drive with ide interface\n"); - return NULL; - } - (void)bdrv_set_read_only(dinfo->bdrv, 1); + if (ro_fallback) { + bdrv_flags |= BDRV_O_RO_FALLBACK; + } else { + bdrv_flags &= ~BDRV_O_RO_FALLBACK; } + if (bdrv_open2(dinfo->bdrv, file, bdrv_flags, drv) < 0) { - fprintf(stderr, "qemu: could not open disk image %s: %s\n", + fprintf(stderr, "qemu: could not open disk image %s with requested permission: %s\n", file, strerror(errno)); return NULL; }