On 06.05.19 17:34, Vladimir Sementsov-Ogievskiy wrote: > From: Andrey Shinkevich <andrey.shinkev...@virtuozzo.com> > > The bottom node is the intermediate block device that has the base as its > backing image. It is used instead of the base node while a block stream > job is running to avoid dependency on the base that may change due to the > parallel jobs. The change may take place due to a filter node as well that > is inserted between the base and the intermediate bottom node. It occurs > when the base node is the top one for another commit or stream job. > After the introduction of the bottom node, don't freeze its backing child, > that's the base, anymore. > > Suggested-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > Signed-off-by: Andrey Shinkevich <andrey.shinkev...@virtuozzo.com> > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > Reviewed-by: Alberto Garcia <be...@igalia.com> > --- > block/stream.c | 49 +++++++++++++++++++++--------------------- > tests/qemu-iotests/245 | 4 ++-- > 2 files changed, 27 insertions(+), 26 deletions(-) > > diff --git a/block/stream.c b/block/stream.c > index 65b13b27e0..fc97c89f81 100644 > --- a/block/stream.c > +++ b/block/stream.c
[...]
> @@ -248,26 +250,25 @@ void stream_start(const char *job_id, BlockDriverState
> *bs,
> * already have our own plans. Also don't allow resize as the image size
> is
> * queried only at the job start and then cached. */
> s = block_job_create(job_id, &stream_job_driver, NULL, bs,
> - BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED
> |
> - BLK_PERM_GRAPH_MOD,
> - BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED
> |
> - BLK_PERM_WRITE,
> + basic_flags | BLK_PERM_GRAPH_MOD,
> + basic_flags | BLK_PERM_WRITE,
> speed, creation_flags, NULL, NULL, errp);
> if (!s) {
> goto fail;
> }
>
> - /* Block all intermediate nodes between bs and base, because they will
> - * disappear from the chain after this operation. The streaming job reads
> - * every block only once, assuming that it doesn't change, so block
> writes
> - * and resizes. */
> - for (iter = backing_bs(bs); iter && iter != base; iter =
> backing_bs(iter)) {
> - block_job_add_bdrv(&s->common, "intermediate node", iter, 0,
> - BLK_PERM_CONSISTENT_READ |
> BLK_PERM_WRITE_UNCHANGED,
> - &error_abort);
> + /*
> + * Block all intermediate nodes between bs and bottom (inclusive),
> because
> + * they will disappear from the chain after this operation. The streaming
> + * job reads every block only once, assuming that it doesn't change, so
> + * forbid writes and resizes.
> + */
> + for (iter = bs; iter != bottom; iter = backing_bs(iter)) {
> + block_job_add_bdrv(&s->common, "intermediate node", backing_bs(iter),
> + 0, basic_flags, &error_abort);
I don’t understand this change. Isn’t it doing exactly the same as before?
(If so, I just find it harder to read because every iteration isn’t
about @iter but backing_bs(iter).)
The rest looks good to me.
Max
> }
>
> - s->base = base;
> + s->bottom = bottom;
> s->backing_file_str = g_strdup(backing_file_str);
> s->bs_read_only = bs_read_only;
> s->chain_frozen = true;
signature.asc
Description: OpenPGP digital signature
