Re: [PATCH v8 6/7] block-stream: freeze link to base node during stream job

[Vladimir Sementsov-Ogievskiy]

04.09.2020 16:21, Max Reitz wrote:
> On 28.08.20 18:52, Andrey Shinkevich wrote:
> > To keep the base node unchanged during the block-stream operation,
> > freeze it as the other part of the backing chain with the intermediate
> > nodes related to the job.
> > This patch revers the change made with the commit c624b015bf as the
> > correct base file name and its format have to be written down to the
> > QCOW2 header on the disk when the backing file is being changed after
> > the stream job completes.
> > This reversion incurs changes in the tests 030, 245 and discards the
> > test 258 where concurrent stream/commit jobs are tested. When the link
> > to a base node is frozen, the concurrent job cannot change the common
> > backing chain.
> >
> > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> > ---
> >   block/stream.c             |  29 ++------
> >   tests/qemu-iotests/030     |  10 +--
> >   tests/qemu-iotests/245     |   2 +-
> >   tests/qemu-iotests/258     | 161 ---------------------------------------------
> >   tests/qemu-iotests/258.out |  33 ----------
> >   5 files changed, 14 insertions(+), 221 deletions(-)
> >   delete mode 100755 tests/qemu-iotests/258
> >   delete mode 100644 tests/qemu-iotests/258.out
>
> Does this need to be in this series?  (I’m not entirely sure, based on
> what I can see in patch 7.)
>
> When doing this, should we introduce a @bottom-node option alongside, so
> that we can make all the tests that are deleted here pass still, just
> with changes?

That's a question to discuss, and I asked Andrey to make this patch in this
simple way to see, how much damage we have with this change.

Honestly, I doubt that we need the new option. Previously, we decided that
we can make this change without a deprecation. If we still going to do it,
we shouldn't care about these use cases. So, if we freeze base again wituhout
a depreaction and:

1. add bottom-node

 - we keep the iotests
 - If (unlikely) someone will came and say: hey, you've broken my working scenario, we 
will say "use bottom-node option, sorry"
 - Most likely we'll have unused option and corresponding unused logic, making code more 
complex for nothing (and we'll have to say "sorry" anyway)

2. without option

 - we loose the iotests. this looks scary, but it is honest: we drop use-cases 
and corresponding iotests
 - If (unlikely) someone will came and say: hey, you've broken my working 
scenario, he will have to wait for next release / package version / or update 
the downstream himself
 - Most likely all will be OK.


Hmm. OK, and the hard-way:

3. Enable all the new logic (filter insertion, freezing base, etc.) only when 
filter-node-name option specified. And immediately deprecate not-specifying the 
option.
 [Note, that in way [3] we don't need bottom-node option]



--
Best regards,
Vladimir