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

[Hanna Reitz]

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.

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.

> [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

> 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')
>