* [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup
@ 2017-04-07 20:55 Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 1/8] block: add bdrv_set_read_only() helper function Jeff Cody
` (10 more replies)
0 siblings, 11 replies; 12+ messages in thread
From: Jeff Cody @ 2017-04-07 20:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, stefanha, kwolf, jsnow, eblake
Changes from v1:
Patch 2: Has v1 patch 8 (do not blindly xset bs->read_only) squashed into it
(thanks Stefan)
COW -> "copy-on-read" (Thanks John)
Drop unneeded call in vvfat, and bypass enable_write_target (Stefan)
Patch 5: Rename bdrv_try_... to bdrv_can_set_read_only() (Thanks John, Stefan)
Patch 6: Use "reopen_state->flags" not "reopen_state->bs->open_flags"
(Thanks John)
This series does two things:
1.) Cleans up some of the logic behind setting the read_only flag
for a BDS in the block layer, so that it is done consistently
(and rules are applied consistently), and
2.) Adds .bdrv_reopen_prepare() implementation for RBD, so that block
jobs can be run on backing chains that have rbd protocol nodes.
Jeff Cody (8):
block: add bdrv_set_read_only() helper function
block: do not set BDS read_only if copy_on_read enabled
block: honor BDRV_O_ALLOW_RDWR when clearing bs->read_only
block: code movement
block: introduce bdrv_can_set_read_only()
block: use bdrv_can_set_read_only() during reopen
block/rbd - update variable names to more apt names
block/rbd: Add support for reopen()
block.c | 56 +++++++++++++++++++++++++++++++++++---------
block/bochs.c | 5 +++-
block/cloop.c | 5 +++-
block/dmg.c | 6 ++++-
block/rbd.c | 65 +++++++++++++++++++++++++++++++++++++--------------
block/vvfat.c | 19 +++++++++++----
include/block/block.h | 2 ++
7 files changed, 123 insertions(+), 35 deletions(-)
--
2.9.3
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.10 1/8] block: add bdrv_set_read_only() helper function
2017-04-07 20:55 [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup Jeff Cody
@ 2017-04-07 20:55 ` Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 2/8] block: do not set BDS read_only if copy_on_read enabled Jeff Cody
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Jeff Cody @ 2017-04-07 20:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, stefanha, kwolf, jsnow, eblake
We have a helper wrapper for checking for the BDS read_only flag,
add a helper wrapper to set the read_only flag as well.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block.c | 5 +++++
block/bochs.c | 2 +-
block/cloop.c | 2 +-
block/dmg.c | 2 +-
block/rbd.c | 2 +-
block/vvfat.c | 4 ++--
include/block/block.h | 1 +
7 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/block.c b/block.c
index 927ba89..7b4c7ef 100644
--- a/block.c
+++ b/block.c
@@ -192,6 +192,11 @@ void path_combine(char *dest, int dest_size,
}
}
+void bdrv_set_read_only(BlockDriverState *bs, bool read_only)
+{
+ bs->read_only = read_only;
+}
+
void bdrv_get_full_backing_filename_from_filename(const char *backed,
const char *backing,
char *dest, size_t sz,
diff --git a/block/bochs.c b/block/bochs.c
index 516da56..bdc2831 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -110,7 +110,7 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
return -EINVAL;
}
- bs->read_only = true; /* no write support yet */
+ bdrv_set_read_only(bs, true); /* no write support yet */
ret = bdrv_pread(bs->file, 0, &bochs, sizeof(bochs));
if (ret < 0) {
diff --git a/block/cloop.c b/block/cloop.c
index a6c7b9d..11f17c8 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -72,7 +72,7 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
return -EINVAL;
}
- bs->read_only = true;
+ bdrv_set_read_only(bs, true);
/* read header */
ret = bdrv_pread(bs->file, 128, &s->block_size, 4);
diff --git a/block/dmg.c b/block/dmg.c
index a7d25fc..27ce4a6 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -420,7 +420,7 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
}
block_module_load_one("dmg-bz2");
- bs->read_only = true;
+ bdrv_set_read_only(bs, true);
s->n_chunks = 0;
s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
diff --git a/block/rbd.c b/block/rbd.c
index 1ceeeb5..6ad2904 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -641,7 +641,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
goto failed_open;
}
- bs->read_only = (s->snap != NULL);
+ bdrv_set_read_only(bs, (s->snap != NULL));
qemu_opts_del(opts);
return 0;
diff --git a/block/vvfat.c b/block/vvfat.c
index af5153d..d4ce6d7 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1157,7 +1157,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
s->current_cluster=0xffffffff;
/* read only is the default for safety */
- bs->read_only = true;
+ bdrv_set_read_only(bs, true);
s->qcow = NULL;
s->qcow_filename = NULL;
s->fat2 = NULL;
@@ -1173,7 +1173,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
if (ret < 0) {
goto fail;
}
- bs->read_only = false;
+ bdrv_set_read_only(bs, false);
}
bs->total_sectors = cyls * heads * secs;
diff --git a/include/block/block.h b/include/block/block.h
index 5149260..06c9032 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -426,6 +426,7 @@ int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
int64_t sector_num, int nb_sectors, int *pnum);
bool bdrv_is_read_only(BlockDriverState *bs);
+void bdrv_set_read_only(BlockDriverState *bs, bool read_only);
bool bdrv_is_sg(BlockDriverState *bs);
bool bdrv_is_inserted(BlockDriverState *bs);
int bdrv_media_changed(BlockDriverState *bs);
--
2.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.10 2/8] block: do not set BDS read_only if copy_on_read enabled
2017-04-07 20:55 [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 1/8] block: add bdrv_set_read_only() helper function Jeff Cody
@ 2017-04-07 20:55 ` Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 3/8] block: honor BDRV_O_ALLOW_RDWR when clearing bs->read_only Jeff Cody
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Jeff Cody @ 2017-04-07 20:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, stefanha, kwolf, jsnow, eblake
A few block drivers will set the BDS read_only flag from their
.bdrv_open() function. This means the bs->read_only flag could
be set after we enable copy_on_read, as the BDRV_O_COPY_ON_READ
flag check occurs prior to the call to bdrv->bdrv_open().
This adds an error return to bdrv_set_read_only(), and an error will be
return if we try to set the BDS to read_only while copy_on_read is
enabled.
This patch also changes the behavior of vvfat. Before, vvfat could
override the drive 'readonly' flag with its own, internal 'rw' flag.
For instance, this -drive parameter would result in a writable image:
"-drive format=vvfat,dir=/tmp/vvfat,rw,if=virtio,readonly=on"
This is not correct. Now, attempting to use the above -drive parameter
will result in an error (i.e., 'rw' is incompatible with 'readonly=on').
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block.c | 10 +++++++++-
block/bochs.c | 5 ++++-
block/cloop.c | 5 ++++-
block/dmg.c | 6 +++++-
block/rbd.c | 11 ++++++++++-
block/vvfat.c | 19 +++++++++++++++----
include/block/block.h | 2 +-
7 files changed, 48 insertions(+), 10 deletions(-)
diff --git a/block.c b/block.c
index 7b4c7ef..c9eb613 100644
--- a/block.c
+++ b/block.c
@@ -192,9 +192,17 @@ void path_combine(char *dest, int dest_size,
}
}
-void bdrv_set_read_only(BlockDriverState *bs, bool read_only)
+int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
{
+ /* Do not set read_only if copy_on_read is enabled */
+ if (bs->copy_on_read && read_only) {
+ error_setg(errp, "Can't set node '%s' to r/o with copy-on-read enabled",
+ bdrv_get_device_or_node_name(bs));
+ return -EINVAL;
+ }
+
bs->read_only = read_only;
+ return 0;
}
void bdrv_get_full_backing_filename_from_filename(const char *backed,
diff --git a/block/bochs.c b/block/bochs.c
index bdc2831..a759b6e 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -110,7 +110,10 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
return -EINVAL;
}
- bdrv_set_read_only(bs, true); /* no write support yet */
+ 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) {
diff --git a/block/cloop.c b/block/cloop.c
index 11f17c8..d6597fc 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -72,7 +72,10 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
return -EINVAL;
}
- bdrv_set_read_only(bs, true);
+ 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);
diff --git a/block/dmg.c b/block/dmg.c
index 27ce4a6..900ae5a 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -419,8 +419,12 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
return -EINVAL;
}
+ ret = bdrv_set_read_only(bs, true, errp);
+ if (ret < 0) {
+ return ret;
+ }
+
block_module_load_one("dmg-bz2");
- bdrv_set_read_only(bs, true);
s->n_chunks = 0;
s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
diff --git a/block/rbd.c b/block/rbd.c
index 6ad2904..1c43171 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -635,13 +635,22 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
goto failed_shutdown;
}
+ /* rbd_open is always r/w */
r = rbd_open(s->io_ctx, s->name, &s->image, s->snap);
if (r < 0) {
error_setg_errno(errp, -r, "error reading header from %s", s->name);
goto failed_open;
}
- bdrv_set_read_only(bs, (s->snap != NULL));
+ /* If we are using an rbd snapshot, we must be r/o, otherwise
+ * leave as-is */
+ if (s->snap != NULL) {
+ r = bdrv_set_read_only(bs, true, &local_err);
+ if (r < 0) {
+ error_propagate(errp, local_err);
+ goto failed_open;
+ }
+ }
qemu_opts_del(opts);
return 0;
diff --git a/block/vvfat.c b/block/vvfat.c
index d4ce6d7..b509d55 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1156,8 +1156,6 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
s->current_cluster=0xffffffff;
- /* read only is the default for safety */
- bdrv_set_read_only(bs, true);
s->qcow = NULL;
s->qcow_filename = NULL;
s->fat2 = NULL;
@@ -1169,11 +1167,24 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
s->sector_count = cyls * heads * secs - (s->first_sectors_number - 1);
if (qemu_opt_get_bool(opts, "rw", false)) {
- ret = enable_write_target(bs, errp);
+ if (!bdrv_is_read_only(bs)) {
+ ret = enable_write_target(bs, errp);
+ if (ret < 0) {
+ goto fail;
+ }
+ } else {
+ ret = -EPERM;
+ error_setg(errp,
+ "Unable to set VVFAT to 'rw' when drive is read-only");
+ goto fail;
+ }
+ } else {
+ /* read only is the default for safety */
+ ret = bdrv_set_read_only(bs, true, &local_err);
if (ret < 0) {
+ error_propagate(errp, local_err);
goto fail;
}
- bdrv_set_read_only(bs, false);
}
bs->total_sectors = cyls * heads * secs;
diff --git a/include/block/block.h b/include/block/block.h
index 06c9032..beb563a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -426,7 +426,7 @@ int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
int64_t sector_num, int nb_sectors, int *pnum);
bool bdrv_is_read_only(BlockDriverState *bs);
-void bdrv_set_read_only(BlockDriverState *bs, bool read_only);
+int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp);
bool bdrv_is_sg(BlockDriverState *bs);
bool bdrv_is_inserted(BlockDriverState *bs);
int bdrv_media_changed(BlockDriverState *bs);
--
2.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.10 3/8] block: honor BDRV_O_ALLOW_RDWR when clearing bs->read_only
2017-04-07 20:55 [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 1/8] block: add bdrv_set_read_only() helper function Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 2/8] block: do not set BDS read_only if copy_on_read enabled Jeff Cody
@ 2017-04-07 20:55 ` Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 4/8] block: code movement Jeff Cody
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Jeff Cody @ 2017-04-07 20:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, stefanha, kwolf, jsnow, eblake
The BDRV_O_ALLOW_RDWR flag allows / prohibits the changing of
the BDS 'read_only' state, but there are a few places where it
is ignored. In the bdrv_set_read_only() helper, make sure to
honor the flag.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/block.c b/block.c
index c9eb613..68a18b0 100644
--- a/block.c
+++ b/block.c
@@ -201,6 +201,13 @@ int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
return -EINVAL;
}
+ /* Do not clear read_only if it is prohibited */
+ if (!read_only && !(bs->open_flags & BDRV_O_ALLOW_RDWR)) {
+ error_setg(errp, "Node '%s' is read only",
+ bdrv_get_device_or_node_name(bs));
+ return -EPERM;
+ }
+
bs->read_only = read_only;
return 0;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.10 4/8] block: code movement
2017-04-07 20:55 [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup Jeff Cody
` (2 preceding siblings ...)
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 3/8] block: honor BDRV_O_ALLOW_RDWR when clearing bs->read_only Jeff Cody
@ 2017-04-07 20:55 ` Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 5/8] block: introduce bdrv_can_set_read_only() Jeff Cody
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Jeff Cody @ 2017-04-07 20:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, stefanha, kwolf, jsnow, eblake
Move bdrv_is_read_only() up with its friends.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/block.c b/block.c
index 68a18b0..a05ad48 100644
--- a/block.c
+++ b/block.c
@@ -192,6 +192,11 @@ void path_combine(char *dest, int dest_size,
}
}
+bool bdrv_is_read_only(BlockDriverState *bs)
+{
+ return bs->read_only;
+}
+
int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
{
/* Do not set read_only if copy_on_read is enabled */
@@ -3367,11 +3372,6 @@ void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr)
*nb_sectors_ptr = nb_sectors < 0 ? 0 : nb_sectors;
}
-bool bdrv_is_read_only(BlockDriverState *bs)
-{
- return bs->read_only;
-}
-
bool bdrv_is_sg(BlockDriverState *bs)
{
return bs->sg;
--
2.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.10 5/8] block: introduce bdrv_can_set_read_only()
2017-04-07 20:55 [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup Jeff Cody
` (3 preceding siblings ...)
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 4/8] block: code movement Jeff Cody
@ 2017-04-07 20:55 ` Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 6/8] block: use bdrv_can_set_read_only() during reopen Jeff Cody
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Jeff Cody @ 2017-04-07 20:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, stefanha, kwolf, jsnow, eblake
Introduce check function for setting read_only flags. Will return < 0 on
error, with appropriate Error value set. Does not alter any flags.
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block.c | 14 +++++++++++++-
include/block/block.h | 1 +
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/block.c b/block.c
index a05ad48..1514ae9 100644
--- a/block.c
+++ b/block.c
@@ -197,7 +197,7 @@ bool bdrv_is_read_only(BlockDriverState *bs)
return bs->read_only;
}
-int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
+int bdrv_can_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
{
/* Do not set read_only if copy_on_read is enabled */
if (bs->copy_on_read && read_only) {
@@ -213,6 +213,18 @@ int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
return -EPERM;
}
+ return 0;
+}
+
+int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
+{
+ int ret = 0;
+
+ ret = bdrv_can_set_read_only(bs, read_only, errp);
+ if (ret < 0) {
+ return ret;
+ }
+
bs->read_only = read_only;
return 0;
}
diff --git a/include/block/block.h b/include/block/block.h
index beb563a..dfd8694 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -426,6 +426,7 @@ int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
int64_t sector_num, int nb_sectors, int *pnum);
bool bdrv_is_read_only(BlockDriverState *bs);
+int bdrv_can_set_read_only(BlockDriverState *bs, bool read_only, Error **errp);
int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp);
bool bdrv_is_sg(BlockDriverState *bs);
bool bdrv_is_inserted(BlockDriverState *bs);
--
2.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.10 6/8] block: use bdrv_can_set_read_only() during reopen
2017-04-07 20:55 [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup Jeff Cody
` (4 preceding siblings ...)
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 5/8] block: introduce bdrv_can_set_read_only() Jeff Cody
@ 2017-04-07 20:55 ` Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 7/8] block/rbd - update variable names to more apt names Jeff Cody
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Jeff Cody @ 2017-04-07 20:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, stefanha, kwolf, jsnow, eblake
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/block.c b/block.c
index 1514ae9..5d560f5 100644
--- a/block.c
+++ b/block.c
@@ -2785,6 +2785,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
BlockDriver *drv;
QemuOpts *opts;
const char *value;
+ bool read_only;
assert(reopen_state != NULL);
assert(reopen_state->bs->drv != NULL);
@@ -2813,12 +2814,13 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
qdict_put(reopen_state->options, "driver", qstring_from_str(value));
}
- /* if we are to stay read-only, do not allow permission change
- * to r/w */
- if (!(reopen_state->bs->open_flags & BDRV_O_ALLOW_RDWR) &&
- reopen_state->flags & BDRV_O_RDWR) {
- error_setg(errp, "Node '%s' is read only",
- bdrv_get_device_or_node_name(reopen_state->bs));
+ /* If we are to stay read-only, do not allow permission change
+ * to r/w. Attempting to set to r/w may fail if either BDRV_O_ALLOW_RDWR is
+ * not set, or if the BDS still has copy_on_read enabled */
+ read_only = !(reopen_state->flags & BDRV_O_RDWR);
+ ret = bdrv_can_set_read_only(reopen_state->bs, read_only, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
goto error;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.10 7/8] block/rbd - update variable names to more apt names
2017-04-07 20:55 [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup Jeff Cody
` (5 preceding siblings ...)
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 6/8] block: use bdrv_can_set_read_only() during reopen Jeff Cody
@ 2017-04-07 20:55 ` Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 8/8] block/rbd: Add support for reopen() Jeff Cody
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Jeff Cody @ 2017-04-07 20:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, stefanha, kwolf, jsnow, eblake
Update 'clientname' to be 'user', which tracks better with both
the QAPI and rados variable naming.
Update 'name' to be 'image_name', as it indicates the rbd image.
Naming it 'image' would have been ideal, but we are using that for
the rados_image_t value returned by rbd_open().
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/rbd.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index 1c43171..35853c9 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -94,7 +94,7 @@ typedef struct BDRVRBDState {
rados_t cluster;
rados_ioctx_t io_ctx;
rbd_image_t image;
- char *name;
+ char *image_name;
char *snap;
} BDRVRBDState;
@@ -350,7 +350,7 @@ static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
int64_t bytes = 0;
int64_t objsize;
int obj_order = 0;
- const char *pool, *name, *conf, *clientname, *keypairs;
+ const char *pool, *image_name, *conf, *user, *keypairs;
const char *secretid;
rados_t cluster;
rados_ioctx_t io_ctx;
@@ -393,11 +393,11 @@ static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
*/
pool = qdict_get_try_str(options, "pool");
conf = qdict_get_try_str(options, "conf");
- clientname = qdict_get_try_str(options, "user");
- name = qdict_get_try_str(options, "image");
+ user = qdict_get_try_str(options, "user");
+ image_name = qdict_get_try_str(options, "image");
keypairs = qdict_get_try_str(options, "=keyvalue-pairs");
- ret = rados_create(&cluster, clientname);
+ ret = rados_create(&cluster, user);
if (ret < 0) {
error_setg_errno(errp, -ret, "error initializing");
goto exit;
@@ -434,7 +434,7 @@ static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
goto shutdown;
}
- ret = rbd_create(io_ctx, name, bytes, &obj_order);
+ ret = rbd_create(io_ctx, image_name, bytes, &obj_order);
if (ret < 0) {
error_setg_errno(errp, -ret, "error rbd create");
}
@@ -540,7 +540,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
BDRVRBDState *s = bs->opaque;
- const char *pool, *snap, *conf, *clientname, *name, *keypairs;
+ const char *pool, *snap, *conf, *user, *image_name, *keypairs;
const char *secretid;
QemuOpts *opts;
Error *local_err = NULL;
@@ -567,24 +567,24 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
pool = qemu_opt_get(opts, "pool");
conf = qemu_opt_get(opts, "conf");
snap = qemu_opt_get(opts, "snapshot");
- clientname = qemu_opt_get(opts, "user");
- name = qemu_opt_get(opts, "image");
+ user = qemu_opt_get(opts, "user");
+ image_name = qemu_opt_get(opts, "image");
keypairs = qemu_opt_get(opts, "=keyvalue-pairs");
- if (!pool || !name) {
+ if (!pool || !image_name) {
error_setg(errp, "Parameters 'pool' and 'image' are required");
r = -EINVAL;
goto failed_opts;
}
- r = rados_create(&s->cluster, clientname);
+ r = rados_create(&s->cluster, user);
if (r < 0) {
error_setg_errno(errp, -r, "error initializing");
goto failed_opts;
}
s->snap = g_strdup(snap);
- s->name = g_strdup(name);
+ s->image_name = g_strdup(image_name);
/* try default location when conf=NULL, but ignore failure */
r = rados_conf_read_file(s->cluster, conf);
@@ -636,9 +636,10 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
}
/* rbd_open is always r/w */
- r = rbd_open(s->io_ctx, s->name, &s->image, s->snap);
+ r = rbd_open(s->io_ctx, s->image_name, &s->image, s->snap);
if (r < 0) {
- error_setg_errno(errp, -r, "error reading header from %s", s->name);
+ error_setg_errno(errp, -r, "error reading header from %s",
+ s->image_name);
goto failed_open;
}
@@ -660,7 +661,7 @@ failed_open:
failed_shutdown:
rados_shutdown(s->cluster);
g_free(s->snap);
- g_free(s->name);
+ g_free(s->image_name);
failed_opts:
qemu_opts_del(opts);
g_free(mon_host);
@@ -674,7 +675,7 @@ static void qemu_rbd_close(BlockDriverState *bs)
rbd_close(s->image);
rados_ioctx_destroy(s->io_ctx);
g_free(s->snap);
- g_free(s->name);
+ g_free(s->image_name);
rados_shutdown(s->cluster);
}
--
2.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH v2 for-2.10 8/8] block/rbd: Add support for reopen()
2017-04-07 20:55 [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup Jeff Cody
` (6 preceding siblings ...)
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 7/8] block/rbd - update variable names to more apt names Jeff Cody
@ 2017-04-07 20:55 ` Jeff Cody
2017-04-10 15:38 ` [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup Stefan Hajnoczi
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Jeff Cody @ 2017-04-07 20:55 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, stefanha, kwolf, jsnow, eblake
This adds support for reopen in rbd, for changing between r/w and r/o.
Note, that this is only a flag change, but we will block a change from
r/o to r/w if we are using an RBD internal snapshot.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
---
block/rbd.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/block/rbd.c b/block/rbd.c
index 35853c9..6471f4f 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -668,6 +668,26 @@ failed_opts:
return r;
}
+
+/* Since RBD is currently always opened R/W via the API,
+ * we just need to check if we are using a snapshot or not, in
+ * order to determine if we will allow it to be R/W */
+static int qemu_rbd_reopen_prepare(BDRVReopenState *state,
+ BlockReopenQueue *queue, Error **errp)
+{
+ BDRVRBDState *s = state->bs->opaque;
+ int ret = 0;
+
+ if (s->snap && state->flags & BDRV_O_RDWR) {
+ error_setg(errp,
+ "Cannot change node '%s' to r/w when using RBD snapshot",
+ bdrv_get_device_or_node_name(state->bs));
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+
static void qemu_rbd_close(BlockDriverState *bs)
{
BDRVRBDState *s = bs->opaque;
@@ -1074,6 +1094,7 @@ static BlockDriver bdrv_rbd = {
.bdrv_parse_filename = qemu_rbd_parse_filename,
.bdrv_file_open = qemu_rbd_open,
.bdrv_close = qemu_rbd_close,
+ .bdrv_reopen_prepare = qemu_rbd_reopen_prepare,
.bdrv_create = qemu_rbd_create,
.bdrv_has_zero_init = bdrv_has_zero_init_1,
.bdrv_get_info = qemu_rbd_getinfo,
--
2.9.3
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup
2017-04-07 20:55 [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup Jeff Cody
` (7 preceding siblings ...)
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 8/8] block/rbd: Add support for reopen() Jeff Cody
@ 2017-04-10 15:38 ` Stefan Hajnoczi
2017-04-10 16:04 ` John Snow
2017-04-20 15:05 ` Jeff Cody
10 siblings, 0 replies; 12+ messages in thread
From: Stefan Hajnoczi @ 2017-04-10 15:38 UTC (permalink / raw)
To: Jeff Cody; +Cc: qemu-devel, qemu-block, kwolf, jsnow, eblake
[-- Attachment #1: Type: text/plain, Size: 1810 bytes --]
On Fri, Apr 07, 2017 at 04:55:24PM -0400, Jeff Cody wrote:
>
> Changes from v1:
>
> Patch 2: Has v1 patch 8 (do not blindly xset bs->read_only) squashed into it
> (thanks Stefan)
> COW -> "copy-on-read" (Thanks John)
> Drop unneeded call in vvfat, and bypass enable_write_target (Stefan)
>
> Patch 5: Rename bdrv_try_... to bdrv_can_set_read_only() (Thanks John, Stefan)
>
> Patch 6: Use "reopen_state->flags" not "reopen_state->bs->open_flags"
> (Thanks John)
>
>
>
> This series does two things:
>
> 1.) Cleans up some of the logic behind setting the read_only flag
> for a BDS in the block layer, so that it is done consistently
> (and rules are applied consistently), and
>
> 2.) Adds .bdrv_reopen_prepare() implementation for RBD, so that block
> jobs can be run on backing chains that have rbd protocol nodes.
>
> Jeff Cody (8):
> block: add bdrv_set_read_only() helper function
> block: do not set BDS read_only if copy_on_read enabled
> block: honor BDRV_O_ALLOW_RDWR when clearing bs->read_only
> block: code movement
> block: introduce bdrv_can_set_read_only()
> block: use bdrv_can_set_read_only() during reopen
> block/rbd - update variable names to more apt names
> block/rbd: Add support for reopen()
>
> block.c | 56 +++++++++++++++++++++++++++++++++++---------
> block/bochs.c | 5 +++-
> block/cloop.c | 5 +++-
> block/dmg.c | 6 ++++-
> block/rbd.c | 65 +++++++++++++++++++++++++++++++++++++--------------
> block/vvfat.c | 19 +++++++++++----
> include/block/block.h | 2 ++
> 7 files changed, 123 insertions(+), 35 deletions(-)
>
> --
> 2.9.3
>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup
2017-04-07 20:55 [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup Jeff Cody
` (8 preceding siblings ...)
2017-04-10 15:38 ` [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup Stefan Hajnoczi
@ 2017-04-10 16:04 ` John Snow
2017-04-20 15:05 ` Jeff Cody
10 siblings, 0 replies; 12+ messages in thread
From: John Snow @ 2017-04-10 16:04 UTC (permalink / raw)
To: Jeff Cody, qemu-devel; +Cc: kwolf, stefanha, qemu-block
On 04/07/2017 04:55 PM, Jeff Cody wrote:
> Changes from v1:
>
> Patch 2: Has v1 patch 8 (do not blindly xset bs->read_only) squashed into it
> (thanks Stefan)
> COW -> "copy-on-read" (Thanks John)
> Drop unneeded call in vvfat, and bypass enable_write_target (Stefan)
>
> Patch 5: Rename bdrv_try_... to bdrv_can_set_read_only() (Thanks John, Stefan)
>
> Patch 6: Use "reopen_state->flags" not "reopen_state->bs->open_flags"
> (Thanks John)
>
>
>
> This series does two things:
>
> 1.) Cleans up some of the logic behind setting the read_only flag
> for a BDS in the block layer, so that it is done consistently
> (and rules are applied consistently), and
>
> 2.) Adds .bdrv_reopen_prepare() implementation for RBD, so that block
> jobs can be run on backing chains that have rbd protocol nodes.
>
> Jeff Cody (8):
> block: add bdrv_set_read_only() helper function
> block: do not set BDS read_only if copy_on_read enabled
> block: honor BDRV_O_ALLOW_RDWR when clearing bs->read_only
> block: code movement
> block: introduce bdrv_can_set_read_only()
> block: use bdrv_can_set_read_only() during reopen
> block/rbd - update variable names to more apt names
> block/rbd: Add support for reopen()
>
> block.c | 56 +++++++++++++++++++++++++++++++++++---------
> block/bochs.c | 5 +++-
> block/cloop.c | 5 +++-
> block/dmg.c | 6 ++++-
> block/rbd.c | 65 +++++++++++++++++++++++++++++++++++++--------------
> block/vvfat.c | 19 +++++++++++----
> include/block/block.h | 2 ++
> 7 files changed, 123 insertions(+), 35 deletions(-)
>
Reviewed-by: John Snow <jsnow@redhat.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup
2017-04-07 20:55 [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup Jeff Cody
` (9 preceding siblings ...)
2017-04-10 16:04 ` John Snow
@ 2017-04-20 15:05 ` Jeff Cody
10 siblings, 0 replies; 12+ messages in thread
From: Jeff Cody @ 2017-04-20 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, stefanha, kwolf, jsnow, eblake
On Fri, Apr 07, 2017 at 04:55:24PM -0400, Jeff Cody wrote:
>
> Changes from v1:
>
> Patch 2: Has v1 patch 8 (do not blindly xset bs->read_only) squashed into it
> (thanks Stefan)
> COW -> "copy-on-read" (Thanks John)
> Drop unneeded call in vvfat, and bypass enable_write_target (Stefan)
>
> Patch 5: Rename bdrv_try_... to bdrv_can_set_read_only() (Thanks John, Stefan)
>
> Patch 6: Use "reopen_state->flags" not "reopen_state->bs->open_flags"
> (Thanks John)
>
>
>
> This series does two things:
>
> 1.) Cleans up some of the logic behind setting the read_only flag
> for a BDS in the block layer, so that it is done consistently
> (and rules are applied consistently), and
>
> 2.) Adds .bdrv_reopen_prepare() implementation for RBD, so that block
> jobs can be run on backing chains that have rbd protocol nodes.
>
> Jeff Cody (8):
> block: add bdrv_set_read_only() helper function
> block: do not set BDS read_only if copy_on_read enabled
> block: honor BDRV_O_ALLOW_RDWR when clearing bs->read_only
> block: code movement
> block: introduce bdrv_can_set_read_only()
> block: use bdrv_can_set_read_only() during reopen
> block/rbd - update variable names to more apt names
> block/rbd: Add support for reopen()
>
> block.c | 56 +++++++++++++++++++++++++++++++++++---------
> block/bochs.c | 5 +++-
> block/cloop.c | 5 +++-
> block/dmg.c | 6 ++++-
> block/rbd.c | 65 +++++++++++++++++++++++++++++++++++++--------------
> block/vvfat.c | 19 +++++++++++----
> include/block/block.h | 2 ++
> 7 files changed, 123 insertions(+), 35 deletions(-)
>
> --
> 2.9.3
>
Thanks,
Applied to my block branch:
git://github.com/codyprime/qemu-kvm-jtc block
-Jeff
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-04-20 15:05 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-07 20:55 [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 1/8] block: add bdrv_set_read_only() helper function Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 2/8] block: do not set BDS read_only if copy_on_read enabled Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 3/8] block: honor BDRV_O_ALLOW_RDWR when clearing bs->read_only Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 4/8] block: code movement Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 5/8] block: introduce bdrv_can_set_read_only() Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 6/8] block: use bdrv_can_set_read_only() during reopen Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 7/8] block/rbd - update variable names to more apt names Jeff Cody
2017-04-07 20:55 ` [Qemu-devel] [PATCH v2 for-2.10 8/8] block/rbd: Add support for reopen() Jeff Cody
2017-04-10 15:38 ` [Qemu-devel] [PATCH v2 for-2.10 0/8] RBD reopen, read_only cleanup Stefan Hajnoczi
2017-04-10 16:04 ` John Snow
2017-04-20 15:05 ` Jeff Cody
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.