Re: [PATCH v2 05/17] iotests/040: Fix TestCommitWithFilters test

[John Snow <jsnow@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4588 bytes --]

On Fri, Mar 25, 2022, 9:40 AM Hanna Reitz <hreitz@redhat.com> wrote:

> On 24.03.22 19:30, John Snow wrote:
> > Without this change, asserting that qemu_io always returns 0 causes this
> > test to fail in a way we happened not to be catching previously:
> >
> >   qemu.utils.VerboseProcessError: Command
> >    '('/home/jsnow/src/qemu/bin/git/tests/qemu-iotests/../../qemu-io',
> >    '--cache', 'writeback', '--aio', 'threads', '-f', 'qcow2', '-c',
> >    'read -P 4 3M 1M',
> >    '/home/jsnow/src/qemu/bin/git/tests/qemu-iotests/scratch/3.img')'
> >    returned non-zero exit status 1.
> >    ┏━ output ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
> >    ┃ qemu-io: can't open device
> >    ┃ /home/jsnow/src/qemu/bin/git/tests/qemu-iotests/scratch/3.img:
> >    ┃ Could not open backing file: Could not open backing file: Throttle
> >    ┃ group 'tg' does not exist
> >    ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
> >
> > Explicitly provide the backing file so that opening the file outside of
> > QEMU (Where we will not have throttle groups) will succeed.
> >
> > [Patch entirely written by Hanna but I don't have her S-o-B]
>
> Er, well, not sure if this even meets the necessary threshold of
> originality, so a Proposed-by would’ve been fine.
>

I have a dogmatic devotion to crediting others. You diagnosed and fixed the
problem, not me!

(I realize it's a small thing, but still. I can't bring myself to put my
name on something that isn't mine, even if it's tiny.)


> Anyway, here you go:
>
> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
>
> > [My commit message is probably also garbage, sorry]
>
> I don’t find it too bad, but a paragraph describing the actual problem
> might improve it.  E.g. (below the exception output):
>
> The commit jobs changes the backing file string stored in the image file
> header belonging to the node above the commit’s top node to point to the
> commit target (the base node).  QEMU tries to be as accurate as
> possible, and so in these test cases will include the filter that is
> part of the block graph in that backing file string (by virtue of making
> it a json:{} description of the post-commit subgraph).  This makes
> little sense outside of QEMU, though: Specifically, the throttle node in
> that subgraph will dearly miss its supposedly associated throttle group
> object.
>
> When starting the commit job, we can specify a custom backing file
> string to write into said image file, so let’s use that feature to write
> the plain filename of the backing chain’s next actual image file there.
>

Thanks :3


> > [Feel free to suggest a better one]
> > [I hope your day is going well]
>
> Fridays tend to be on the better side of days.
>
> Hanna
>

Up there in my top three kinds of days.


> > Signed-off-by: John Snow <jsnow@redhat.com>
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   tests/qemu-iotests/040 | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
> > index c4a90937dc..30eb97829e 100755
> > --- a/tests/qemu-iotests/040
> > +++ b/tests/qemu-iotests/040
> > @@ -836,7 +836,8 @@ class TestCommitWithFilters(iotests.QMPTestCase):
> >                                job_id='commit',
> >                                device='top-filter',
> >                                top_node='cow-2',
> > -                             base_node='cow-1')
> > +                             base_node='cow-1',
> > +                             backing_file=self.img1)
> >           self.assert_qmp(result, 'return', {})
> >           self.wait_until_completed(drive='commit')
> >
> > @@ -852,7 +853,8 @@ class TestCommitWithFilters(iotests.QMPTestCase):
> >                                job_id='commit',
> >                                device='top-filter',
> >                                top_node='cow-1',
> > -                             base_node='cow-0')
> > +                             base_node='cow-0',
> > +                             backing_file=self.img0)
> >           self.assert_qmp(result, 'return', {})
> >           self.wait_until_completed(drive='commit')
> >
>
>

[-- Attachment #2: Type: text/html, Size: 6676 bytes --]