| Message ID | 9df6b3723ec30cb749ceaa555d82a29a6d79496d.1589199922.git.lukasstraub2@web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | colo: Introduce resource agent and test suite/CI | expand |
> -----Original Message----- > From: Lukas Straub <lukasstraub2@web.de> > Sent: Monday, May 11, 2020 8:27 PM > To: qemu-devel <qemu-devel@nongnu.org> > Cc: Alberto Garcia <berto@igalia.com>; Dr. David Alan Gilbert > <dgilbert@redhat.com>; Zhang, Chen <chen.zhang@intel.com> > Subject: [PATCH 1/5] block/quorum.c: stable children names > > If we remove the child with the highest index from the quorum, decrement > s->next_child_index. This way we get stable children names as long as we > only remove the last child. > Looks good for me, and it can solve this bug: colo: Can not recover colo after svm failover twice https://bugs.launchpad.net/bugs/1881231 Reviewed-by: Zhang Chen <chen.zhang@intel.com> > Signed-off-by: Lukas Straub <lukasstraub2@web.de> > --- > block/quorum.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/block/quorum.c b/block/quorum.c index 6d7a56bd93..acfa09c2cc > 100644 > --- a/block/quorum.c > +++ b/block/quorum.c > @@ -29,6 +29,8 @@ > > #define HASH_LENGTH 32 > > +#define INDEXSTR_LEN 32 > + > #define QUORUM_OPT_VOTE_THRESHOLD "vote-threshold" > #define QUORUM_OPT_BLKVERIFY "blkverify" > #define QUORUM_OPT_REWRITE "rewrite-corrupted" > @@ -972,9 +974,9 @@ static int quorum_open(BlockDriverState *bs, QDict > *options, int flags, > opened = g_new0(bool, s->num_children); > > for (i = 0; i < s->num_children; i++) { > - char indexstr[32]; > - ret = snprintf(indexstr, 32, "children.%d", i); > - assert(ret < 32); > + char indexstr[INDEXSTR_LEN]; > + ret = snprintf(indexstr, INDEXSTR_LEN, "children.%d", i); > + assert(ret < INDEXSTR_LEN); > > s->children[i] = bdrv_open_child(NULL, options, indexstr, bs, > &child_format, false, &local_err); @@ -1026,7 +1028,7 > @@ static void quorum_add_child(BlockDriverState *bs, BlockDriverState > *child_bs, { > BDRVQuorumState *s = bs->opaque; > BdrvChild *child; > - char indexstr[32]; > + char indexstr[INDEXSTR_LEN]; > int ret; > > if (s->is_blkverify) { > @@ -1041,8 +1043,8 @@ static void quorum_add_child(BlockDriverState *bs, > BlockDriverState *child_bs, > return; > } > > - ret = snprintf(indexstr, 32, "children.%u", s->next_child_index); > - if (ret < 0 || ret >= 32) { > + ret = snprintf(indexstr, INDEXSTR_LEN, "children.%u", s- > >next_child_index); > + if (ret < 0 || ret >= INDEXSTR_LEN) { > error_setg(errp, "cannot generate child name"); > return; > } > @@ -1069,6 +1071,7 @@ static void quorum_del_child(BlockDriverState *bs, > BdrvChild *child, > Error **errp) { > BDRVQuorumState *s = bs->opaque; > + char indexstr[INDEXSTR_LEN]; > int i; > > for (i = 0; i < s->num_children; i++) { @@ -1090,6 +1093,11 @@ static void > quorum_del_child(BlockDriverState *bs, BdrvChild *child, > /* We know now that num_children > threshold, so blkverify must be > false */ > assert(!s->is_blkverify); > > + snprintf(indexstr, INDEXSTR_LEN, "children.%u", s->next_child_index - 1); > + if (!strncmp(child->name, indexstr, INDEXSTR_LEN)) { > + s->next_child_index--; > + } > + > bdrv_drained_begin(bs); > > /* We can safely remove this child now */ > -- > 2.20.1
On Mon 11 May 2020 02:26:54 PM CEST, Lukas Straub wrote: > If we remove the child with the highest index from the quorum, > decrement s->next_child_index. This way we get stable children > names as long as we only remove the last child. > > Signed-off-by: Lukas Straub <lukasstraub2@web.de> Reviewed-by: Alberto Garcia <berto@igalia.com> Berto
diff --git a/block/quorum.c b/block/quorum.c index 6d7a56bd93..acfa09c2cc 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -29,6 +29,8 @@ #define HASH_LENGTH 32 +#define INDEXSTR_LEN 32 + #define QUORUM_OPT_VOTE_THRESHOLD "vote-threshold" #define QUORUM_OPT_BLKVERIFY "blkverify" #define QUORUM_OPT_REWRITE "rewrite-corrupted" @@ -972,9 +974,9 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags, opened = g_new0(bool, s->num_children); for (i = 0; i < s->num_children; i++) { - char indexstr[32]; - ret = snprintf(indexstr, 32, "children.%d", i); - assert(ret < 32); + char indexstr[INDEXSTR_LEN]; + ret = snprintf(indexstr, INDEXSTR_LEN, "children.%d", i); + assert(ret < INDEXSTR_LEN); s->children[i] = bdrv_open_child(NULL, options, indexstr, bs, &child_format, false, &local_err); @@ -1026,7 +1028,7 @@ static void quorum_add_child(BlockDriverState *bs, BlockDriverState *child_bs, { BDRVQuorumState *s = bs->opaque; BdrvChild *child; - char indexstr[32]; + char indexstr[INDEXSTR_LEN]; int ret; if (s->is_blkverify) { @@ -1041,8 +1043,8 @@ static void quorum_add_child(BlockDriverState *bs, BlockDriverState *child_bs, return; } - ret = snprintf(indexstr, 32, "children.%u", s->next_child_index); - if (ret < 0 || ret >= 32) { + ret = snprintf(indexstr, INDEXSTR_LEN, "children.%u", s->next_child_index); + if (ret < 0 || ret >= INDEXSTR_LEN) { error_setg(errp, "cannot generate child name"); return; } @@ -1069,6 +1071,7 @@ static void quorum_del_child(BlockDriverState *bs, BdrvChild *child, Error **errp) { BDRVQuorumState *s = bs->opaque; + char indexstr[INDEXSTR_LEN]; int i; for (i = 0; i < s->num_children; i++) { @@ -1090,6 +1093,11 @@ static void quorum_del_child(BlockDriverState *bs, BdrvChild *child, /* We know now that num_children > threshold, so blkverify must be false */ assert(!s->is_blkverify); + snprintf(indexstr, INDEXSTR_LEN, "children.%u", s->next_child_index - 1); + if (!strncmp(child->name, indexstr, INDEXSTR_LEN)) { + s->next_child_index--; + } + bdrv_drained_begin(bs); /* We can safely remove this child now */
If we remove the child with the highest index from the quorum, decrement s->next_child_index. This way we get stable children names as long as we only remove the last child. Signed-off-by: Lukas Straub <lukasstraub2@web.de> --- block/quorum.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)