block: Add auto-read-only option
v3 cover block: Add auto-read-only option
See patch 2 for an explanation of the motivation.
v3:
- Clarified QAPI schema documentation that auto-read-only can only degrade read-write to read-only, not the other way round [Eric]
- Don't refuse to set copy-on-read=on and auto-read-only=on at the same time; only complain when actually trying to degrade to read-only
- Let bdrv_apply_auto_read_only() return -EACCESS on all errors
- Fixed file-posix and gluster implementations [Eric, Niels]
- Added a patch to make auto-read-only=on the default for human user interfaces (-drive/-hda/...)
v2:
- Turn bdrv_set_read_only() into bdrv_apply_auto_read_only()
- Support the option in a lot more block drivers
Kevin Wolf (9):
block: Update flags in bdrv_set_read_only()
block: Add auto-read-only option
block: Require auto-read-only for existing fallbacks
nbd: Support auto-read-only option
file-posix: Support auto-read-only option
curl: Support auto-read-only option
gluster: Support auto-read-only option
iscsi: Support auto-read-only option
block: Make auto-read-only=on default for -drive
qapi/block-core.json | 7 ++++++ include/block/block.h | 5 +++- block.c | 54 +++++++++++++++++++++++++++++++++++-------- block/bochs.c | 17 +++++--------- block/cloop.c | 16 ++++--------- block/curl.c | 8 +++---- block/dmg.c | 16 ++++--------- block/file-posix.c | 19 ++++++++++++--- block/gluster.c | 12 ++++++++-- block/iscsi.c | 8 ++++--- block/nbd-client.c | 10 ++++---- block/rbd.c | 14 ++++------- block/vvfat.c | 11 +++------ blockdev.c | 1 + 14 files changed, 120 insertions(+), 78 deletions(-)
Signature · 1 line
2.19.1
PATCH v3 1/9 block: Update flags in bdrv_set_read_only()
To fully change the read-only state of a node, we must not only change bs->read_only, but also update bs->open_flags.
--- block.c | 7 +++++++ 1 file changed, 7 insertions(+)
block.c +7 -0
@@ -281,6 +281,13 @@ int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp) } bs->read_only = read_only; + +if (read_only) { + bs->open_flags &= ~BDRV_O_RDWR ; + }else { + bs->open_flags |=BDRV_O_RDWR ; + } +return 0 ; }
Signature · 1 line
2.19.1
PATCH v3 2/9 block: Add auto-read-only option
In reply to Kevin WolfIf a management application builds the block graph node by node, the protocol layer doesn't inherit its read-only option from the format layer any more, so it must be set explicitly.
Backing files should work on read-only storage, but at the same time, a block job like commit should be able to reopen them read-write if they are on read-write storage. However, without option inheritance, reopen only changes the read-only option for the root node (typically the format layer), but not the protocol layer, so reopening fails (the format layer wants to get write permissions, but the protocol layer is still read-only).
A simple workaround for the problem in the management tool would be to open the protocol layer always read-write and to make only the format layer read-only for backing files. However, sometimes the file is actually stored on read-only storage and we don't know whether the image can be opened read-write (for example, for NBD it depends on the server we're trying to connect to). This adds an option that makes QEMU try to open the image read-write, but allows it to degrade to a read-only mode without returning an error.
The documentation for this option is consciously phrased in a way that allows QEMU to switch to a better model eventually: Instead of trying when the image is first opened, making the read-only flag dynamic and changing it automatically whenever the first BLK_PERM_WRITE user is attached or the last one is detached would be much more useful behaviour.
Unfortunately, this more useful behaviour is also a lot harder to implement, and libvirt needs a solution now before it can switch to -blockdev, so let's start with this easier approach for now.
Instead of adding a new auto-read-only option, turning the existing read-only into an enum (with a bool alternate for compatibility) was considered, but it complicated the implementation to the point that it didn't seem to be worth it.
--- qapi/block-core.json | 7 +++++++ include/block/block.h | 2 ++ block.c | 17 +++++++++++++++++ block/vvfat.c | 1 + 4 files changed, 27 insertions(+)
qapi/block-core.json +7 -0
@@ -3651,6 +3651,12 @@ # either generally or in certain configurations. In this case, # the default value does not work and the option must be # specified explicitly. +# @auto-read-only: if true and @read-only is false, QEMU may automatically +# decide not to open the image read-write as requested, but +# fall back to read-only instead (and switch between the modes +# later), e.g. depending on whether the image file is writable +# or whether a writing user is attached to the node +# (default: false, since 3.1) # @detect-zeroes: detect and optimize zero writes (Since 2.1) # (default: off) # @force-share: force share all permission on added nodes.
@@ -3666,6 +3672,7 @@ '*discard': 'BlockdevDiscardOptions', '*cache': 'BlockdevCacheOptions', '*read-only': 'bool', + '*auto-read-only': 'bool', '*force-share': 'bool', '*detect-zeroes': 'BlockdevDetectZeroesOptions' }, 'discriminator': 'driver',
include/block/block.h +2 -0
@@ -115,6 +115,7 @@ typedef struct HDGeometry {select an appropriate protocol driver, ignoring the format layer */ #define BDRV_O_NO_IO 0x10000/* don't initialize for I/O */ +#define BDRV_O_AUTO_RDONLY 0x20000/* degrade to read-only if opening read-write fails */ #define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_NO_FLUSH)
@@ -125,6 +126,7 @@ typedef struct HDGeometry {#define BDRV_OPT_CACHE_DIRECT "cache.direct"#define BDRV_OPT_CACHE_NO_FLUSH "cache.no-flush"#define BDRV_OPT_READ_ONLY "read-only" +#define BDRV_OPT_AUTO_READ_ONLY "auto-read-only"#define BDRV_OPT_DISCARD "discard"#define BDRV_OPT_FORCE_SHARE "force-share"
block.c +17 -0
@@ -930,6 +930,7 @@ static void bdrv_inherited_options(int *child_flags, QDict *child_options,/* Inherit the read-only option from the parent if it's not set */ qdict_copy_default (child_options, parent_options,BDRV_OPT_READ_ONLY ); +qdict_copy_default (child_options, parent_options,BDRV_OPT_AUTO_READ_ONLY );/* 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
@@ -1053,6 +1054,7 @@ static void bdrv_backing_options(int *child_flags, QDict *child_options,/* backing files always opened read-only */ qdict_set_default_str (child_options,BDRV_OPT_READ_ONLY ,"on" ); +qdict_set_default_str (child_options,BDRV_OPT_AUTO_READ_ONLY ,"off" ); flags &= ~BDRV_O_COPY_ON_READ ;/* snapshot=on is handled on the top layer */
@@ -1142,6 +1144,10 @@ static void update_flags_from_options(int *flags, QemuOpts *opts) *flags |=BDRV_O_RDWR ; } +assert (qemu_opt_find (opts,BDRV_OPT_AUTO_READ_ONLY )); +if (qemu_opt_get_bool_del (opts,BDRV_OPT_AUTO_READ_ONLY , false)) { + *flags |=BDRV_O_AUTO_RDONLY ; + } }static void update_options_from_flags (QDict *options,int flags)
@@ -1156,6 +1162,10 @@ static void update_options_from_flags(QDict *options, int flags)if (!qdict_haskey (options,BDRV_OPT_READ_ONLY )) {qdict_put_bool (options,BDRV_OPT_READ_ONLY , !(flags &BDRV_O_RDWR )); } +if (!qdict_haskey (options,BDRV_OPT_AUTO_READ_ONLY )) { +qdict_put_bool (options,BDRV_OPT_AUTO_READ_ONLY , + flags &BDRV_O_AUTO_RDONLY ); + } } static void bdrv_assign_node_name(BlockDriverState *bs,
@@ -1328,6 +1338,11 @@ QemuOptsList bdrv_runtime_opts = { .type = QEMU_OPT_BOOL, .help = "Node is opened in read-only mode", }, + { + .name = BDRV_OPT_AUTO_READ_ONLY, + .type = QEMU_OPT_BOOL, + .help = "Node can become read-only if opening read-write fails", + }, { .name = "detect-zeroes", .type = QEMU_OPT_STRING,
@@ -2486,6 +2501,8 @@ BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp)qdict_set_default_str (qdict,BDRV_OPT_CACHE_DIRECT ,"off" );qdict_set_default_str (qdict,BDRV_OPT_CACHE_NO_FLUSH ,"off" );qdict_set_default_str (qdict,BDRV_OPT_READ_ONLY ,"off" ); +qdict_set_default_str (qdict,BDRV_OPT_AUTO_READ_ONLY ,"off" ); + } bs =bdrv_open_inherit (NULL , reference, qdict,0 ,NULL ,NULL , errp);
block/vvfat.c +1 -0
@@ -3130,6 +3130,7 @@ static void vvfat_qcow_options(int *child_flags, QDict *child_options, int parent_flags, QDict *parent_options) { qdict_set_default_str(child_options, BDRV_OPT_READ_ONLY, "off"); +qdict_set_default_str (child_options,BDRV_OPT_AUTO_READ_ONLY ,"off" );qdict_set_default_str (child_options,BDRV_OPT_CACHE_NO_FLUSH ,"on" ); }
Signature · 1 line
2.19.1
Quoted 15 lines · Kevin Wolf
If a management application builds the block graph node by node, the protocol layer doesn't inherit its read-only option from the format layer any more, so it must be set explicitly.
The documentation for this option is consciously phrased in a way that allows QEMU to switch to a better model eventually: Instead of trying when the image is first opened, making the read-only flag dynamic and changing it automatically whenever the first
BLK_PERM_WRITEuser is attached or the last one is detached would be much more useful behaviour.+++
b/qapi/block-core.json
qapi/block-core.json line 3659
Quoted 9 lines
qapi/block-core.json +6
-0@@ -3651,6 +3651,12 @@ # either generally or in certain configurations. In this case, # the default value does not work and the option must be # specified explicitly. +# @auto-read-only: if true and @read-only is false, QEMU may automatically +# decide not to open the image read-write as requested, but +# fall back to read-only instead (and switch between the modes +# later), e.g. depending on whether the image file is writable +# or whether a writing user is attached to the node +# (default: false, since 3.1)
Definitely better wording compared to v2.
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
PATCH v3 3/9 block: Require auto-read-only for existing fallbacks
In reply to Kevin WolfSome block drivers have traditionally changed their node to read-only mode without asking the user. This behaviour has been marked deprecated since 2.11, expecting users to provide an explicit read-only=on option.
Now that we have auto-read-only=on, enable these drivers to make use of the option.
This is the only use of bdrv_set_read_only(), so we can make it a bit more specific and turn it into a bdrv_apply_auto_read_only() that is more convenient for drivers to use.
--- include/block/block.h | 3 ++- block.c | 42 +++++++++++++++++++++++++++--------------- block/bochs.c | 17 ++++++----------- block/cloop.c | 16 +++++----------- block/dmg.c | 16 +++++----------- block/rbd.c | 14 ++++---------- block/vvfat.c | 10 ++-------- 7 files changed, 51 insertions(+), 67 deletions(-)
include/block/block.h +2 -1
@@ -438,7 +438,8 @@ int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,bool bdrv_is_read_only (BlockDriverState *bs);int bdrv_can_set_read_only (BlockDriverState *bs,bool read_only,bool ignore_allow_rdw,Error **errp); -int bdrv_set_read_only (BlockDriverState *bs,bool read_only,Error **errp); +int bdrv_apply_auto_read_only (BlockDriverState *bs,const char *errmsg, +Error **errp);bool bdrv_is_writable (BlockDriverState *bs);bool bdrv_is_sg (BlockDriverState *bs);bool bdrv_is_inserted (BlockDriverState *bs);
block.c +27 -15
56 lines not on this page · raw message
block/bochs.c +6 -11
@@ -105,23 +105,18 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,struct bochs_header bochs;int ret; +/* No write support yet */ + ret =bdrv_apply_auto_read_only (bs,NULL , errp); +if (ret <0 ) { +return ret; + } + bs->file =bdrv_open_child (NULL , options,"file" , bs, &child_file, false, errp);if (!bs->file) {return -EINVAL ; } -if (!bdrv_is_read_only (bs)) { -error_report ("Opening bochs images without an explicit read-only=on " -"option is deprecated. Future versions will refuse to " -"open the image instead of automatically marking the " -"image read-only." ); - ret =bdrv_set_read_only (bs, true, errp);/* no write support yet */ -if (ret <0 ) { -return ret; - } - } - ret =bdrv_pread (bs->file,0 , &bochs,sizeof (bochs));if (ret <0 ) {return ret;
block/cloop.c +5 -11
@@ -67,23 +67,17 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,uint32_t offsets_size, max_compressed_block_size =1 , i;int ret; + ret =bdrv_apply_auto_read_only (bs,NULL , errp); +if (ret <0 ) { +return ret; + } + bs->file =bdrv_open_child (NULL , options,"file" , bs, &child_file, false, errp);if (!bs->file) {return -EINVAL ; } -if (!bdrv_is_read_only (bs)) { -error_report ("Opening cloop images without an explicit read-only=on " -"option is deprecated. Future versions will refuse to " -"open the image instead of automatically marking the " -"image read-only." ); - ret =bdrv_set_read_only (bs, true, errp); -if (ret <0 ) { -return ret; - } - } -/* read header */ ret =bdrv_pread (bs->file,128 , &s->block_size,4 );if (ret <0 ) {
block/dmg.c +5 -11
@@ -413,23 +413,17 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,int64_t offset;int ret; + ret =bdrv_apply_auto_read_only (bs,NULL , errp); +if (ret <0 ) { +return ret; + } + bs->file =bdrv_open_child (NULL , options,"file" , bs, &child_file, false, errp);if (!bs->file) {return -EINVAL ; } -if (!bdrv_is_read_only (bs)) { -error_report ("Opening dmg images without an explicit read-only=on " -"option is deprecated. Future versions will refuse to " -"open the image instead of automatically marking the " -"image read-only." ); - ret =bdrv_set_read_only (bs, true, errp); -if (ret <0 ) { -return ret; - } - } -block_module_load_one ("dmg-bz2" ); s->n_chunks =0 ;
block/rbd.c +4 -10
@@ -780,16 +780,10 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,/* If we are using an rbd snapshot, we must be r/o, otherwise * leave as-is */ if (s->snap !=NULL ) { -if (!bdrv_is_read_only (bs)) { -error_report ("Opening rbd snapshots without an explicit " -"read-only=on option is deprecated. Future versions " -"will refuse to open the image instead of " -"automatically marking the image read-only." ); - r =bdrv_set_read_only (bs, true, &local_err); -if (r <0 ) { -error_propagate (errp, local_err); - goto failed_open; - } + r =bdrv_apply_auto_read_only (bs,"rbd snapshots are read-only" , errp); +if (r <0 ) { +rbd_close (s->image); + goto failed_open; } }
block/vvfat.c +2 -8
@@ -1262,15 +1262,9 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags, "Unable to set VVFAT to 'rw' when drive is read-only"); goto fail; } - }else if (!bdrv_is_read_only (bs)) { -error_report ("Opening non-rw vvfat images without an explicit " -"read-only=on option is deprecated. Future versions " -"will refuse to open the image instead of " -"automatically marking the image read-only." ); -/* read only is the default for safety */ - ret =bdrv_set_read_only (bs, true, &local_err); + }else { + ret =bdrv_apply_auto_read_only (bs,NULL , errp);if (ret <0 ) { -error_propagate (errp, local_err); goto fail; } }
Signature · 1 line
2.19.1
Re: [Qemu-devel] [PATCH v3 3/9] block: Require auto-read-only for existing fallbacks
Quoted 14 lines · Kevin Wolf
Some block drivers have traditionally changed their node to read-only mode without asking the user. This behaviour has been marked deprecated since 2.11, expecting users to provide an explicit read-only=on option.
Now that we have auto-read-only=on, enable these drivers to make use of the option.
This is the only use of
bdrv_set_read_only(), so we can make it a bit more specific and turn it into abdrv_apply_auto_read_only()that is more convenient for drivers to use.Signed-off-by Kevin Wolf <[email protected]>---
Quoted 9 lines
+/*
+ * Called by a driver that can only provide a read-only image.
+ *
+ * Returns 0 if the node is already read-only or it could switch the node to
+ * read-only becauseBDRV_O_AUTO_RDONLYis set.
+ *
+ * Returns-EACCESif the node is read-write andBDRV_O_AUTO_RDONLYis not set
+ * orbdrv_can_set_read_only()forbids making the node read-only. If @errmsg
+ * is not NULL, it is used as the error message for the Error object.
Works for me.
Quoted 9 lines
+++
b/block/rbd.cblock/rbd.c +0
-4@@ -780,16 +780,10 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,/* If we are using an rbd snapshot, we must be r/o, otherwise * leave as-is */ if (s->snap !=NULL ) { -if (!bdrv_is_read_only (bs)) { -error_report ("Opening rbd snapshots without an explicit " -"read-only=on option is deprecated. Future versions " - "will refuse to open the image instead of "
block/rbd.c static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, · line 786
Quoted 10 lines
block/rbd.c +4
-6- "automatically marking the image read-only."); - r =bdrv_set_read_only (bs, true, &local_err); -if (r <0 ) { -error_propagate (errp, local_err); - goto failed_open; - } + r =bdrv_apply_auto_read_only (bs,"rbd snapshots are read-only" , errp); +if (r <0 ) { +rbd_close (s->image); + goto failed_open;
That rbd_close() is an independent bugfix. Should probably be split to a separate commit, or at a minimum called out in the commit message as intentional.
Actually, is it really needed to prevent a leak, or does the existing rados_shutdown() in failed_open already implicitly cover the actions of rbd_close()?
With the RBD issue touched up,
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v3 3/9] block: Require auto-read-only for existing fallbacks
Am 17.10.2018 um 20:53 hat Eric Blake geschrieben:
Quoted 29 lines · Kevin Wolf
Quoted 28 lines
Some block drivers have traditionally changed their node to read-only mode without asking the user. This behaviour has been marked deprecated since 2.11, expecting users to provide an explicit read-only=on option.
Now that we have auto-read-only=on, enable these drivers to make use of the option.
This is the only use of
bdrv_set_read_only(), so we can make it a bit more specific and turn it into abdrv_apply_auto_read_only()that is more convenient for drivers to use.Signed-off-by Kevin Wolf <[email protected]>+++
b/block/rbd.cblock/rbd.c +0
-10@@ -780,16 +780,10 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,/* If we are using an rbd snapshot, we must be r/o, otherwise * leave as-is */ if (s->snap !=NULL ) { -if (!bdrv_is_read_only (bs)) { -error_report ("Opening rbd snapshots without an explicit " -"read-only=on option is deprecated. Future versions " -"will refuse to open the image instead of " -"automatically marking the image read-only." ); - r =bdrv_set_read_only (bs, true, &local_err); -if (r <0 ) { -error_propagate (errp, local_err); - goto failed_open; - }
Quoted 8 lines
block/rbd.c static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, · line 786
Quoted 4 lines
block/rbd.c +4
-0+ r =bdrv_apply_auto_read_only (bs,"rbd snapshots are read-only" , errp); +if (r <0 ) { +rbd_close (s->image); + goto failed_open;That
rbd_close()is an independent bugfix. Should probably be split to a separate commit, or at a minimum called out in the commit message as intentional.
Okay, I'll split it.
Quoted 3 lines
Actually, is it really needed to prevent a leak, or does the existing
rados_shutdown()infailed_openalready implicitly cover the actions ofrbd_close()?
I don't know if rados_shutdown() would implicitly do that as well, but the normal .bdrv_close implementation calls both, too, so I suppose the safe option is to let the error path do the same.
Kevin
PATCH v3 4/9 nbd: Support auto-read-only option
In reply to Kevin WolfIf read-only=off, but auto-read-only=on is given, open a read-write NBD connection if the server provides a read-write export, but instead of erroring out for read-only exports, just degrade to read-only.
--- block/nbd-client.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
block/nbd-client.c +5 -5
@@ -992,11 +992,11 @@ int nbd_client_init(BlockDriverState *bs,logout ("Failed to negotiate with the NBD server\n" );return ret; } -if (client->info.flags &NBD_FLAG_READ_ONLY && - !bdrv_is_read_only (bs)) { -error_setg (errp, -"request for write access conflicts with read-only export" ); -return -EACCES ; +if (client->info.flags &NBD_FLAG_READ_ONLY ) { + ret =bdrv_apply_auto_read_only (bs,"NBD export is read-only" , errp); +if (ret <0 ) { +return ret; + } }if (client->info.flags &NBD_FLAG_SEND_FUA ) { bs->supported_write_flags =BDRV_REQ_FUA ;
Signature · 1 line
2.19.1
PATCH v3 5/9 file-posix: Support auto-read-only option
In reply to Kevin WolfIf read-only=off, but auto-read-only=on is given, open the file read-write if we have the permissions, but instead of erroring out for read-only files, just degrade to read-only.
--- block/file-posix.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
block/file-posix.c +16 -3
@@ -527,9 +527,22 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, s->fd =-1 ; fd =qemu_open (filename, s->open_flags,0644 ); -if (fd <0 ) { - ret = -errno; -error_setg_errno (errp, errno,"Could not open '%s'" , filename); + ret = fd <0 ? -errno :0 ; + +if (ret == -EACCES || ret == -EROFS ) { +/* Try to degrade to read-only, but if it doesn't work, still use the +* normal error message. */ +if (bdrv_apply_auto_read_only (bs,NULL ,NULL ) ==0 ) { + bdrv_flags &= ~BDRV_O_RDWR ; +raw_parse_flags (bdrv_flags, &s->open_flags); +assert (!(s->open_flags &O_CREAT )); + fd =qemu_open (filename, s->open_flags); + ret = fd <0 ? -errno :0 ; + } + } + +if (ret <0 ) { +error_setg_errno (errp, -ret,"Could not open '%s'" , filename);if (ret == -EROFS ) { ret = -EACCES ; }
Signature · 1 line
2.19.1
Re: [Qemu-devel] [PATCH v3 5/9] file-posix: Support auto-read-only option
Quoted 9 lines · Kevin Wolf
If read-only=off, but auto-read-only=on is given, open the file read-write if we have the permissions, but instead of erroring out for read-only files, just degrade to read-only.
Signed-off-by Kevin Wolf <[email protected]>--- block/file-posix.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
PATCH v3 6/9 curl: Support auto-read-only option
In reply to Kevin WolfIf read-only=off, but auto-read-only=on is given, just degrade to read-only.
--- block/curl.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
block/curl.c +4 -4
@@ -684,10 +684,10 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,const char *protocol_delimiter;int ret; - -if (flags &BDRV_O_RDWR ) { -error_setg (errp,"curl block device does not support writes" ); -return -EROFS ; + ret =bdrv_apply_auto_read_only (bs,"curl driver does not support writes" , + errp); +if (ret <0 ) { +return ret; }if (!libcurl_initialized) {
Signature · 1 line
2.19.1
PATCH v3 7/9 gluster: Support auto-read-only option
In reply to Kevin WolfIf read-only=off, but auto-read-only=on is given, open the file read-write if we have the permissions, but instead of erroring out for read-only files, just degrade to read-only.
--- block/gluster.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
block/gluster.c +10 -2
@@ -849,8 +849,16 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options,qemu_gluster_parse_flags (bdrv_flags, &open_flags); s->fd =glfs_open (s->glfs, gconf->path, open_flags); -if (!s->fd) { - ret = -errno; + ret = s->fd ?0 : -errno; + +if (ret == -EACCES || ret == -EROFS ) { +/* Try to degrade to read-only, but if it doesn't work, still use the +* normal error message. */ +if (bdrv_apply_auto_read_only (bs,NULL ,NULL ) ==0 ) { + open_flags = (open_flags & ~O_RDWR ) |O_RDONLY ; + s->fd =glfs_open (s->glfs, gconf->path, open_flags); + ret = s->fd ?0 : -errno; + } } s->supports_seek_data =qemu_gluster_test_seek (s->fd);
Signature · 1 line
2.19.1
Re: [Qemu-devel] [Qemu-block] [PATCH v3 7/9] gluster: Support auto-read-only option
Quoted 23 lines · Kevin Wolf
If read-only=off, but auto-read-only=on is given, open the file read-write if we have the permissions, but instead of erroring out for read-only files, just degrade to read-only.
Signed-off-by Kevin Wolf <[email protected]>--- block/gluster.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)block/gluster.c +3
-2@@ -849,8 +849,16 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options,qemu_gluster_parse_flags (bdrv_flags, &open_flags); s->fd =glfs_open (s->glfs, gconf->path, open_flags); -if (!s->fd) { - ret = -errno; + ret = s->fd ?0 : -errno; + +if (ret == -EACCES || ret == -EROFS ) {
block/gluster.c static int qemu_gluster_open(BlockDriverState *bs, QDict *options, · line 864
Quoted 10 lines
block/gluster.c +7
-0+/* Try to degrade to read-only, but if it doesn't work, still use the +* normal error message. */ +if (bdrv_apply_auto_read_only (bs,NULL ,NULL ) ==0 ) { + open_flags = (open_flags & ~O_RDWR ) |O_RDONLY ; + s->fd =glfs_open (s->glfs, gconf->path, open_flags); + ret = s->fd ?0 : -errno; + } } s->supports_seek_data =qemu_gluster_test_seek (s->fd);
Looks good to me, thanks.
PATCH v3 8/9 iscsi: Support auto-read-only option
In reply to Kevin WolfIf read-only=off, but auto-read-only=on is given, open the volume read-write if we have the permissions, but instead of erroring out for read-only volumes, just degrade to read-only.
--- block/iscsi.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
block/iscsi.c +5 -3
@@ -1878,9 +1878,11 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,/* Check the write protect flag of the LUN if we want to write */ if (iscsilun->type ==TYPE_DISK && (flags &BDRV_O_RDWR ) && iscsilun->write_protected) { -error_setg (errp,"Cannot open a write protected LUN as read-write" ); - ret = -EACCES ; - goto out; + ret =bdrv_apply_auto_read_only (bs,"LUN is write protected" , errp); +if (ret <0 ) { + goto out; + } + flags &= ~BDRV_O_RDWR ; }iscsi_readcapacity_sync (iscsilun, &local_err);
Signature · 1 line
2.19.1
PATCH v3 9/9 block: Make auto-read-only=on default for -drive
In reply to Kevin WolfWhile we want machine interfaces like -blockdev and QMP blockdev-add to add as little auto-detection as possible so that management tools are explicit about their needs, -drive is a convenience option for human users. Enabling auto-read-only=on by default there enables users to use read-only images for read-only guest devices without having to specify read-only=on explicitly. If they try to attach the image to a read-write device, they will still get an error message.
--- blockdev.c | 1 + 1 file changed, 1 insertion(+)
blockdev.c +1 -0
@@ -590,6 +590,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,qdict_set_default_str (bs_opts,BDRV_OPT_CACHE_NO_FLUSH ,"off" );qdict_set_default_str (bs_opts,BDRV_OPT_READ_ONLY , read_only ?"on" :"off" ); +qdict_set_default_str (bs_opts,BDRV_OPT_AUTO_READ_ONLY ,"on" );assert ((bdrv_flags &BDRV_O_CACHE_MASK ) ==0 );if (runstate_check (RUN_STATE_INMIGRATE )) {
Signature · 1 line
2.19.1
Re: [Qemu-devel] [PATCH v3 9/9] block: Make auto-read-only=on default for -drive
Quoted 8 lines · Kevin Wolf
While we want machine interfaces like -blockdev and QMP blockdev-add to add as little auto-detection as possible so that management tools are explicit about their needs, -drive is a convenience option for human users. Enabling auto-read-only=on by default there enables users to use read-only images for read-only guest devices without having to specify read-only=on explicitly. If they try to attach the image to a read-write device, they will still get an error message.
Quoted 5 lines
Signed-off-by Kevin Wolf <[email protected]>--- blockdev.c | 1 + 1 file changed, 1 insertion(+)
However, the obligatory: "iotests coverage?"
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org