| Message ID | 1470662013-19785-4-git-send-email-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
Am 08.08.2016 um 15:13 hat Fam Zheng geschrieben: > Respect the locking mode from CLI or QMP, and set the open flags > accordingly. > > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > block.c | 21 +++++++++++++++++++++ > blockdev.c | 9 +++++++++ > include/block/block.h | 1 + > qemu-options.hx | 1 + > 4 files changed, 32 insertions(+) > > diff --git a/block.c b/block.c > index 30d64e6..5f76f60 100644 > --- a/block.c > +++ b/block.c > @@ -705,6 +705,7 @@ static void bdrv_inherited_options(int *child_flags, QDict *child_options, > * the parent. */ > qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT); > qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH); > + qdict_copy_default(child_options, parent_options, BDRV_OPT_LOCK_MODE); > > /* Our block drivers take care to send flushes and respect unmap policy, > * so we can default to enable both on lower layers regardless of the > @@ -757,6 +758,7 @@ static void bdrv_backing_options(int *child_flags, QDict *child_options, > * which is only applied on the top level (BlockBackend) */ > qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT); > qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH); > + qdict_copy_default(child_options, parent_options, BDRV_OPT_LOCK_MODE); > > /* backing files always opened read-only */ > flags &= ~(BDRV_O_RDWR | BDRV_O_COPY_ON_READ); > @@ -880,6 +882,11 @@ static QemuOptsList bdrv_runtime_opts = { > .name = BDRV_OPT_CACHE_NO_FLUSH, > .type = QEMU_OPT_BOOL, > .help = "Ignore flush requests", > + },{ > + .name = BDRV_OPT_LOCK_MODE, > + .type = QEMU_OPT_STRING, > + .help = "how to lock the image (auto, shared, off. " > + "default: auto)", > }, > { /* end of list */ } > }, > @@ -897,6 +904,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file, > const char *filename; > const char *driver_name = NULL; > const char *node_name = NULL; > + const char *lock_mode = NULL; > QemuOpts *opts; > BlockDriver *drv; > Error *local_err = NULL; > @@ -940,6 +948,19 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file, > goto fail_opts; > } > > + lock_mode = qemu_opt_get(opts, BDRV_OPT_LOCK_MODE) ? : "off"; Am I missing some other place that overrides this or does it make the default "off" rather than "auto"? > + if (!strcmp(lock_mode, "auto")) { > + /* Default */ > + } else if (!strcmp(lock_mode, "shared")) { > + bs->open_flags |= BDRV_O_SHARED_LOCK; > + } else if (!strcmp(lock_mode, "off")) { > + bs->open_flags |= BDRV_O_NO_LOCK; > + } else { > + error_setg(errp, "invalid lock mode"); > + ret = -EINVAL; > + goto fail_opts; > + } > + > bs->read_only = !(bs->open_flags & BDRV_O_RDWR); The other thing is that we didn't really finish the discussion whether leaving the configuration on the node level to the user is a good idea when the restrictions that the user can influence rather come from the top level (the guest OS) and only propagate down the BDS tree. Aren't we requiring that the user understands the internal workings of qemu in order to infer which nodes need to be locked in which mode, rather than just saying "my guest is okay with sharing this disk, you can figure out what this means for locking"? Kevin
On Tue, 09/06 18:33, Kevin Wolf wrote: > > + lock_mode = qemu_opt_get(opts, BDRV_OPT_LOCK_MODE) ? : "off"; > > Am I missing some other place that overrides this or does it make the > default "off" rather than "auto"? It is in patch 19, once all tests are fixed (this patch cannot be reordered after tests fixing because it involves specifying the lock-mode option.)
diff --git a/block.c b/block.c index 30d64e6..5f76f60 100644 --- a/block.c +++ b/block.c @@ -705,6 +705,7 @@ static void bdrv_inherited_options(int *child_flags, QDict *child_options, * the parent. */ qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT); qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH); + qdict_copy_default(child_options, parent_options, BDRV_OPT_LOCK_MODE); /* Our block drivers take care to send flushes and respect unmap policy, * so we can default to enable both on lower layers regardless of the @@ -757,6 +758,7 @@ static void bdrv_backing_options(int *child_flags, QDict *child_options, * which is only applied on the top level (BlockBackend) */ qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT); qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH); + qdict_copy_default(child_options, parent_options, BDRV_OPT_LOCK_MODE); /* backing files always opened read-only */ flags &= ~(BDRV_O_RDWR | BDRV_O_COPY_ON_READ); @@ -880,6 +882,11 @@ static QemuOptsList bdrv_runtime_opts = { .name = BDRV_OPT_CACHE_NO_FLUSH, .type = QEMU_OPT_BOOL, .help = "Ignore flush requests", + },{ + .name = BDRV_OPT_LOCK_MODE, + .type = QEMU_OPT_STRING, + .help = "how to lock the image (auto, shared, off. " + "default: auto)", }, { /* end of list */ } }, @@ -897,6 +904,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file, const char *filename; const char *driver_name = NULL; const char *node_name = NULL; + const char *lock_mode = NULL; QemuOpts *opts; BlockDriver *drv; Error *local_err = NULL; @@ -940,6 +948,19 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file, goto fail_opts; } + lock_mode = qemu_opt_get(opts, BDRV_OPT_LOCK_MODE) ? : "off"; + if (!strcmp(lock_mode, "auto")) { + /* Default */ + } else if (!strcmp(lock_mode, "shared")) { + bs->open_flags |= BDRV_O_SHARED_LOCK; + } else if (!strcmp(lock_mode, "off")) { + bs->open_flags |= BDRV_O_NO_LOCK; + } else { + error_setg(errp, "invalid lock mode"); + ret = -EINVAL; + goto fail_opts; + } + bs->read_only = !(bs->open_flags & BDRV_O_RDWR); if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) { diff --git a/blockdev.c b/blockdev.c index 2161400..a9032b9 100644 --- a/blockdev.c +++ b/blockdev.c @@ -545,6 +545,10 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, qdict_put(bs_opts, "driver", qstring_from_str(buf)); } + if ((buf = qemu_opt_get(opts, BDRV_OPT_LOCK_MODE)) != NULL) { + qdict_put(bs_opts, BDRV_OPT_LOCK_MODE, qstring_from_str(buf)); + } + on_write_error = BLOCKDEV_ON_ERROR_ENOSPC; if ((buf = qemu_opt_get(opts, "werror")) != NULL) { on_write_error = parse_block_error_action(buf, 0, &error); @@ -4265,6 +4269,11 @@ QemuOptsList qemu_common_drive_opts = { .type = QEMU_OPT_BOOL, .help = "whether to account for failed I/O operations " "in the statistics", + },{ + .name = BDRV_OPT_LOCK_MODE, + .type = QEMU_OPT_STRING, + .help = "how to lock the image (auto, shared, off. " + "default: auto)", }, { /* end of list */ } }, diff --git a/include/block/block.h b/include/block/block.h index 4f8450b..a14101b 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -108,6 +108,7 @@ typedef struct HDGeometry { #define BDRV_OPT_CACHE_WB "cache.writeback" #define BDRV_OPT_CACHE_DIRECT "cache.direct" #define BDRV_OPT_CACHE_NO_FLUSH "cache.no-flush" +#define BDRV_OPT_LOCK_MODE "lock-mode" #define BDRV_SECTOR_BITS 9 diff --git a/qemu-options.hx b/qemu-options.hx index a71aaf8..0f67b64 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -521,6 +521,7 @@ DEF("drive", HAS_ARG, QEMU_OPTION_drive, " [,werror=ignore|stop|report|enospc][,id=name][,aio=threads|native]\n" " [,readonly=on|off][,copy-on-read=on|off]\n" " [,discard=ignore|unmap][,detect-zeroes=on|off|unmap]\n" + " [,lock-mode=auto|shared|off]\n" " [[,bps=b]|[[,bps_rd=r][,bps_wr=w]]]\n" " [[,iops=i]|[[,iops_rd=r][,iops_wr=w]]]\n" " [[,bps_max=bm]|[[,bps_rd_max=rm][,bps_wr_max=wm]]]\n"
Respect the locking mode from CLI or QMP, and set the open flags accordingly. Signed-off-by: Fam Zheng <famz@redhat.com> --- block.c | 21 +++++++++++++++++++++ blockdev.c | 9 +++++++++ include/block/block.h | 1 + qemu-options.hx | 1 + 4 files changed, 32 insertions(+)