From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
To: Max Reitz <mreitz@redhat.com>,
"qemu-block@nongnu.org" <qemu-block@nongnu.org>
Cc: Kevin Wolf <kwolf@redhat.com>, Alberto Garcia <berto@igalia.com>,
"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Subject: Re: [PATCH 08/22] quorum: Store children in own structure
Date: Wed, 25 Sep 2019 13:21:05 +0000 [thread overview]
Message-ID: <d27a4ba9-8abe-2eb6-f48c-9fc9609b9d95@virtuozzo.com> (raw)
In-Reply-To: <20190920152804.12875-9-mreitz@redhat.com>
20.09.2019 18:27, Max Reitz wrote:
> This will be useful when we want to store additional attributes for each
> child.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/quorum.c | 58 ++++++++++++++++++++++++++++----------------------
> 1 file changed, 33 insertions(+), 25 deletions(-)
>
> diff --git a/block/quorum.c b/block/quorum.c
> index 17b439056f..cf2171cc74 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -65,9 +65,13 @@ typedef struct QuorumVotes {
> bool (*compare)(QuorumVoteValue *a, QuorumVoteValue *b);
> } QuorumVotes;
>
> +typedef struct QuorumChild {
> + BdrvChild *child;
> +} QuorumChild;
> +
> /* the following structure holds the state of one quorum instance */
> typedef struct BDRVQuorumState {
> - BdrvChild **children; /* children BlockDriverStates */
> + QuorumChild *children;
> int num_children; /* children count */
> unsigned next_child_index; /* the index of the next child that should
> * be added
> @@ -264,7 +268,7 @@ static void quorum_report_bad_versions(BDRVQuorumState *s,
> }
> QLIST_FOREACH(item, &version->items, next) {
> quorum_report_bad(QUORUM_OP_TYPE_READ, acb->offset, acb->bytes,
> - s->children[item->index]->bs->node_name, 0);
> + s->children[item->index].child->bs->node_name, 0);
> }
> }
> }
> @@ -279,7 +283,7 @@ static void quorum_rewrite_entry(void *opaque)
> * corrupted data.
> * Mask out BDRV_REQ_WRITE_UNCHANGED because this overwrites the
> * area with different data from the other children. */
> - bdrv_co_pwritev(s->children[co->idx], acb->offset, acb->bytes,
> + bdrv_co_pwritev(s->children[co->idx].child, acb->offset, acb->bytes,
> acb->qiov, acb->flags & ~BDRV_REQ_WRITE_UNCHANGED);
>
> /* Wake up the caller after the last rewrite */
> @@ -578,8 +582,8 @@ static void read_quorum_children_entry(void *opaque)
> int i = co->idx;
> QuorumChildRequest *sacb = &acb->qcrs[i];
>
> - sacb->bs = s->children[i]->bs;
> - sacb->ret = bdrv_co_preadv(s->children[i], acb->offset, acb->bytes,
> + sacb->bs = s->children[i].child->bs;
> + sacb->ret = bdrv_co_preadv(s->children[i].child, acb->offset, acb->bytes,
> &acb->qcrs[i].qiov, 0);
>
> if (sacb->ret == 0) {
> @@ -605,7 +609,8 @@ static int read_quorum_children(QuorumAIOCB *acb)
>
> acb->children_read = s->num_children;
> for (i = 0; i < s->num_children; i++) {
> - acb->qcrs[i].buf = qemu_blockalign(s->children[i]->bs, acb->qiov->size);
> + acb->qcrs[i].buf = qemu_blockalign(s->children[i].child->bs,
> + acb->qiov->size);
> qemu_iovec_init(&acb->qcrs[i].qiov, acb->qiov->niov);
> qemu_iovec_clone(&acb->qcrs[i].qiov, acb->qiov, acb->qcrs[i].buf);
> }
> @@ -647,8 +652,8 @@ static int read_fifo_child(QuorumAIOCB *acb)
> /* We try to read the next child in FIFO order if we failed to read */
> do {
> n = acb->children_read++;
> - acb->qcrs[n].bs = s->children[n]->bs;
> - ret = bdrv_co_preadv(s->children[n], acb->offset, acb->bytes,
> + acb->qcrs[n].bs = s->children[n].child->bs;
> + ret = bdrv_co_preadv(s->children[n].child, acb->offset, acb->bytes,
> acb->qiov, 0);
> if (ret < 0) {
> quorum_report_bad_acb(&acb->qcrs[n], ret);
> @@ -688,8 +693,8 @@ static void write_quorum_entry(void *opaque)
> int i = co->idx;
> QuorumChildRequest *sacb = &acb->qcrs[i];
>
> - sacb->bs = s->children[i]->bs;
> - sacb->ret = bdrv_co_pwritev(s->children[i], acb->offset, acb->bytes,
> + sacb->bs = s->children[i].child->bs;
> + sacb->ret = bdrv_co_pwritev(s->children[i].child, acb->offset, acb->bytes,
> acb->qiov, acb->flags);
> if (sacb->ret == 0) {
> acb->success_count++;
> @@ -743,12 +748,12 @@ static int64_t quorum_getlength(BlockDriverState *bs)
> int i;
>
> /* check that all file have the same length */
> - result = bdrv_getlength(s->children[0]->bs);
> + result = bdrv_getlength(s->children[0].child->bs);
> if (result < 0) {
> return result;
> }
> for (i = 1; i < s->num_children; i++) {
> - int64_t value = bdrv_getlength(s->children[i]->bs);
> + int64_t value = bdrv_getlength(s->children[i].child->bs);
> if (value < 0) {
> return value;
> }
> @@ -774,10 +779,10 @@ static coroutine_fn int quorum_co_flush(BlockDriverState *bs)
> error_votes.compare = quorum_64bits_compare;
>
> for (i = 0; i < s->num_children; i++) {
> - result = bdrv_co_flush(s->children[i]->bs);
> + result = bdrv_co_flush(s->children[i].child->bs);
> if (result) {
> quorum_report_bad(QUORUM_OP_TYPE_FLUSH, 0, 0,
> - s->children[i]->bs->node_name, result);
> + s->children[i].child->bs->node_name, result);
> result_value.l = result;
> quorum_count_vote(&error_votes, &result_value, i);
> } else {
> @@ -803,7 +808,7 @@ static bool quorum_recurse_is_first_non_filter(BlockDriverState *bs,
> int i;
>
> for (i = 0; i < s->num_children; i++) {
> - bool perm = bdrv_recurse_is_first_non_filter(s->children[i]->bs,
> + bool perm = bdrv_recurse_is_first_non_filter(s->children[i].child->bs,
> candidate);
> if (perm) {
> return true;
> @@ -932,7 +937,7 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
> }
>
> /* allocate the children array */
> - s->children = g_new0(BdrvChild *, s->num_children);
> + s->children = g_new0(QuorumChild, s->num_children);
> opened = g_new0(bool, s->num_children);
>
> for (i = 0; i < s->num_children; i++) {
> @@ -940,8 +945,9 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
> ret = snprintf(indexstr, 32, "children.%d", i);
> assert(ret < 32);
>
> - s->children[i] = bdrv_open_child(NULL, options, indexstr, bs,
> - &child_format, false, &local_err);
> + s->children[i].child = bdrv_open_child(NULL, options, indexstr, bs,
> + &child_format, false,
> + &local_err);
> if (local_err) {
> ret = -EINVAL;
> goto close_exit;
> @@ -962,7 +968,7 @@ close_exit:
> if (!opened[i]) {
> continue;
> }
> - bdrv_unref_child(bs, s->children[i]);
> + bdrv_unref_child(bs, s->children[i].child);
> }
> g_free(s->children);
> g_free(opened);
> @@ -979,7 +985,7 @@ static void quorum_close(BlockDriverState *bs)
> int i;
>
> for (i = 0; i < s->num_children; i++) {
> - bdrv_unref_child(bs, s->children[i]);
> + bdrv_unref_child(bs, s->children[i].child);
> }
>
> g_free(s->children);
> @@ -1022,8 +1028,10 @@ static void quorum_add_child(BlockDriverState *bs, BlockDriverState *child_bs,
> s->next_child_index--;
> goto out;
> }
more context:
assert(s->num_children <= INT_MAX / sizeof(BdrvChild *));
if (s->num_children == INT_MAX / sizeof(BdrvChild *) ||
s->next_child_index == UINT_MAX) {
error_setg(errp, "Too many children");
return;
}
here: s/BdrvChild */QuorumChild
> - s->children = g_renew(BdrvChild *, s->children, s->num_children + 1);
> - s->children[s->num_children++] = child;
> + s->children = g_renew(QuorumChild, s->children, s->num_children + 1);
> + s->children[s->num_children++] = (QuorumChild){
> + .child = child,
> + };
>
> out:
> bdrv_drained_end(bs);
> @@ -1036,7 +1044,7 @@ static void quorum_del_child(BlockDriverState *bs, BdrvChild *child,
> int i;
>
> for (i = 0; i < s->num_children; i++) {
> - if (s->children[i] == child) {
> + if (s->children[i].child == child) {
> break;
> }
> }
> @@ -1059,7 +1067,7 @@ static void quorum_del_child(BlockDriverState *bs, BdrvChild *child,
> /* We can safely remove this child now */
> memmove(&s->children[i], &s->children[i + 1],
> (s->num_children - i - 1) * sizeof(BdrvChild *));
s/BdrvChild */QuorumChild/
> - s->children = g_renew(BdrvChild *, s->children, --s->num_children);
> + s->children = g_renew(QuorumChild, s->children, --s->num_children);
> bdrv_unref_child(bs, child);
>
> bdrv_drained_end(bs);
> @@ -1100,7 +1108,7 @@ static void quorum_gather_child_options(BlockDriverState *bs, QDict *target,
>
> for (i = 0; i < s->num_children; i++) {
> qlist_append(children_list,
> - qobject_ref(s->children[i]->bs->full_open_options));
> + qobject_ref(s->children[i].child->bs->full_open_options));
> }
> }
>
>
with my suggestions:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
--
Best regards,
Vladimir
next prev parent reply other threads:[~2019-09-25 13:31 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-20 15:27 [PATCH 00/22] block: Fix check_to_replace_node() Max Reitz
2019-09-20 15:27 ` [PATCH 01/22] blockdev: Allow external snapshots everywhere Max Reitz
2019-09-25 11:45 ` Vladimir Sementsov-Ogievskiy
2019-09-20 15:27 ` [PATCH 02/22] blockdev: Allow resizing everywhere Max Reitz
2019-09-25 11:46 ` Vladimir Sementsov-Ogievskiy
2019-09-20 15:27 ` [PATCH 03/22] block: Drop bdrv_is_first_non_filter() Max Reitz
2019-09-25 11:46 ` Vladimir Sementsov-Ogievskiy
2019-09-20 15:27 ` [PATCH 04/22] iotests: Let 041 use -blockdev for quorum children Max Reitz
2019-09-25 11:51 ` Vladimir Sementsov-Ogievskiy
2019-09-20 15:27 ` [PATCH 05/22] quorum: Fix child permissions Max Reitz
2019-09-25 11:56 ` Vladimir Sementsov-Ogievskiy
2019-09-26 11:02 ` Max Reitz
2019-09-20 15:27 ` [PATCH 06/22] block: Add bdrv_recurse_can_replace() Max Reitz
2019-09-25 12:39 ` Vladimir Sementsov-Ogievskiy
2019-09-26 11:03 ` Max Reitz
2019-09-20 15:27 ` [PATCH 07/22] blkverify: Implement .bdrv_recurse_can_replace() Max Reitz
2019-09-25 12:56 ` Vladimir Sementsov-Ogievskiy
2019-09-26 11:06 ` Max Reitz
2019-09-20 15:27 ` [PATCH 08/22] quorum: Store children in own structure Max Reitz
2019-09-25 13:21 ` Vladimir Sementsov-Ogievskiy [this message]
2019-09-26 11:07 ` Max Reitz
2019-09-20 15:27 ` [PATCH 09/22] quorum: Add QuorumChild.to_be_replaced Max Reitz
2019-09-25 13:45 ` Vladimir Sementsov-Ogievskiy
2019-09-26 11:13 ` Max Reitz
2019-09-20 15:27 ` [PATCH 10/22] quorum: Implement .bdrv_recurse_can_replace() Max Reitz
2019-09-25 13:39 ` Vladimir Sementsov-Ogievskiy
2019-09-26 11:12 ` Max Reitz
2019-09-25 14:12 ` Vladimir Sementsov-Ogievskiy
2019-09-26 11:17 ` Max Reitz
2019-09-25 14:14 ` Vladimir Sementsov-Ogievskiy
2019-09-20 15:27 ` [PATCH 11/22] block: Use bdrv_recurse_can_replace() Max Reitz
2019-09-20 15:27 ` [PATCH 12/22] block: Remove bdrv_recurse_is_first_non_filter() Max Reitz
2019-09-25 14:16 ` Vladimir Sementsov-Ogievskiy
2019-09-20 15:27 ` [PATCH 13/22] mirror: Double-check immediately before replacing Max Reitz
2019-09-20 15:27 ` [PATCH 14/22] quorum: Stop marking it as a filter Max Reitz
2019-09-26 12:43 ` Vladimir Sementsov-Ogievskiy
2019-09-20 15:27 ` [PATCH 15/22] mirror: Prevent loops Max Reitz
2019-09-26 13:08 ` Vladimir Sementsov-Ogievskiy
2019-10-02 12:36 ` Max Reitz
2019-09-20 15:27 ` [PATCH 16/22] iotests: Use complete_and_wait() in 155 Max Reitz
2019-09-26 13:11 ` Vladimir Sementsov-Ogievskiy
2019-09-20 15:27 ` [PATCH 17/22] iotests: Add VM.assert_block_path() Max Reitz
2019-09-26 14:07 ` Vladimir Sementsov-Ogievskiy
2019-10-02 12:40 ` Max Reitz
2019-10-02 13:51 ` Vladimir Sementsov-Ogievskiy
2019-09-20 15:28 ` [PATCH 18/22] iotests: Resolve TODOs in 041 Max Reitz
2019-09-26 14:09 ` Vladimir Sementsov-Ogievskiy
2019-09-20 15:28 ` [PATCH 19/22] iotests: Use self.image_len in TestRepairQuorum Max Reitz
2019-09-26 14:13 ` Vladimir Sementsov-Ogievskiy
2019-10-02 12:42 ` Max Reitz
2019-09-20 15:28 ` [PATCH 20/22] iotests: Add tests for invalid Quorum @replaces Max Reitz
2019-09-26 14:30 ` Vladimir Sementsov-Ogievskiy
2019-10-02 12:43 ` Max Reitz
2019-09-20 15:28 ` [PATCH 21/22] iotests: Check that @replaces can replace filters Max Reitz
2019-09-26 14:42 ` Vladimir Sementsov-Ogievskiy
2019-09-20 15:28 ` [PATCH 22/22] iotests: Mirror must not attempt to create loops Max Reitz
2019-09-26 15:03 ` Vladimir Sementsov-Ogievskiy
2019-10-02 12:46 ` Max Reitz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d27a4ba9-8abe-2eb6-f48c-9fc9609b9d95@virtuozzo.com \
--to=vsementsov@virtuozzo.com \
--cc=berto@igalia.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).