From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46819) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gKNSZ-00040c-6B for qemu-devel@nongnu.org; Wed, 07 Nov 2018 08:01:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gKNSC-0002JJ-8r for qemu-devel@nongnu.org; Wed, 07 Nov 2018 08:01:22 -0500 From: Alberto Garcia Date: Wed, 7 Nov 2018 14:59:46 +0200 Message-Id: <37b20c1e4ca5ef29ec7fbba2fa62dbf7acc292a5.1541595424.git.berto@igalia.com> In-Reply-To: References: In-Reply-To: References: Subject: [Qemu-devel] [PATCH v4 11/15] block: Clean up reopen_backing_file() in block/replication.c List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Alberto Garcia , qemu-block@nongnu.org, Kevin Wolf , Max Reitz This function is used to put the hidden and secondary disks in read-write mode before launching the backup job, and back in read-only mode afterwards. This patch does the following changes: - Use an options QDict with the "read-only" option instead of passing the changes as flags only. - Simplify the code (it was unnecessarily complicated and verbose). - Fix a bug due to which the secondary disk was not being put back in read-only mode when writable=false (because in this case orig_secondary_flags always had the BDRV_O_RDWR flag set). - Stop clearing the BDRV_O_INACTIVE flag. The flags parameter to bdrv_reopen_queue() becomes redundant and we'll be able to get rid of it in a subsequent patch. Signed-off-by: Alberto Garcia --- block/replication.c | 45 +++++++++++++++++++++------------------------ 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/block/replication.c b/block/replication.c index 6349d6958e..481a225924 100644 --- a/block/replication.c +++ b/block/replication.c @@ -20,6 +20,7 @@ #include "block/block_backup.h" #include "sysemu/block-backend.h" #include "qapi/error.h" +#include "qapi/qmp/qdict.h" #include "replication.h" typedef enum { @@ -39,8 +40,8 @@ typedef struct BDRVReplicationState { char *top_id; ReplicationState *rs; Error *blocker; - int orig_hidden_flags; - int orig_secondary_flags; + bool orig_hidden_read_only; + bool orig_secondary_read_only; int error; } BDRVReplicationState; @@ -371,44 +372,40 @@ static void secondary_do_checkpoint(BDRVReplicationState *s, Error **errp) } } +/* This function is supposed to be called twice: + * first with writable = true, then with writable = false. + * The first call puts s->hidden_disk and s->secondary_disk in + * r/w mode, and the second puts them back in their original state. + */ static void reopen_backing_file(BlockDriverState *bs, bool writable, Error **errp) { BDRVReplicationState *s = bs->opaque; BlockReopenQueue *reopen_queue = NULL; - int orig_hidden_flags, orig_secondary_flags; - int new_hidden_flags, new_secondary_flags; Error *local_err = NULL; if (writable) { - orig_hidden_flags = s->orig_hidden_flags = - bdrv_get_flags(s->hidden_disk->bs); - new_hidden_flags = (orig_hidden_flags | BDRV_O_RDWR) & - ~BDRV_O_INACTIVE; - orig_secondary_flags = s->orig_secondary_flags = - bdrv_get_flags(s->secondary_disk->bs); - new_secondary_flags = (orig_secondary_flags | BDRV_O_RDWR) & - ~BDRV_O_INACTIVE; - } else { - orig_hidden_flags = (s->orig_hidden_flags | BDRV_O_RDWR) & - ~BDRV_O_INACTIVE; - new_hidden_flags = s->orig_hidden_flags; - orig_secondary_flags = (s->orig_secondary_flags | BDRV_O_RDWR) & - ~BDRV_O_INACTIVE; - new_secondary_flags = s->orig_secondary_flags; + s->orig_hidden_read_only = bdrv_is_read_only(s->hidden_disk->bs); + s->orig_secondary_read_only = bdrv_is_read_only(s->secondary_disk->bs); } bdrv_subtree_drained_begin(s->hidden_disk->bs); bdrv_subtree_drained_begin(s->secondary_disk->bs); - if (orig_hidden_flags != new_hidden_flags) { - reopen_queue = bdrv_reopen_queue(reopen_queue, s->hidden_disk->bs, NULL, - new_hidden_flags); + if (s->orig_hidden_read_only) { + int flags = bdrv_get_flags(s->hidden_disk->bs); + QDict *opts = qdict_new(); + qdict_put_bool(opts, BDRV_OPT_READ_ONLY, !writable); + reopen_queue = bdrv_reopen_queue(reopen_queue, s->hidden_disk->bs, + opts, flags); } - if (!(orig_secondary_flags & BDRV_O_RDWR)) { + if (s->orig_secondary_read_only) { + int flags = bdrv_get_flags(s->secondary_disk->bs); + QDict *opts = qdict_new(); + qdict_put_bool(opts, BDRV_OPT_READ_ONLY, !writable); reopen_queue = bdrv_reopen_queue(reopen_queue, s->secondary_disk->bs, - NULL, new_secondary_flags); + opts, flags); } if (reopen_queue) { -- 2.11.0