Re: [PATCH v2 05/17] iotests/040: Fix TestCommitWithFilters test
On Thu, Mar 24, 2022 at 9:33 PM Eric Blake wrote:
>
> On Thu, Mar 24, 2022 at 02:30:06PM -0400, 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]
>
> Yeah, you'll want that.
>
> > [My commit message is probably also garbage, sorry]
>
> No, it was actually decent.
>
> > [Feel free to suggest a better one]
> > [I hope your day is going well]
> > Signed-off-by: John Snow
> >
> > Signed-off-by: John Snow
>
> So giving your S-o-b twice makes up for it, right ;)
This happens when I add a '---' myself into the commit message, and
git-publish sees that the end of the commit message doesn't have a
S-o-B and adds one into the ignored region.
Haven't bothered to fix it yet.
>
> Well, you did say v3 would fix this. But while you're having fun
> fixing it, you can add:
>
> Reviewed-by: Eric Blake
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
>
Re: [PATCH v2 05/17] iotests/040: Fix TestCommitWithFilters test
On Fri, Mar 25, 2022, 9:40 AM Hanna Reitz 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
>
> > [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
> >
> > Signed-off-by: John Snow
> > ---
> > 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')
> >
>
>
Re: [PATCH v2 05/17] iotests/040: Fix TestCommitWithFilters test
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
[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
Signed-off-by: John Snow
---
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')
Re: [PATCH v2 05/17] iotests/040: Fix TestCommitWithFilters test
On Thu, Mar 24, 2022 at 02:30:06PM -0400, 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]
Yeah, you'll want that.
> [My commit message is probably also garbage, sorry]
No, it was actually decent.
> [Feel free to suggest a better one]
> [I hope your day is going well]
> Signed-off-by: John Snow
>
> Signed-off-by: John Snow
So giving your S-o-b twice makes up for it, right ;)
Well, you did say v3 would fix this. But while you're having fun
fixing it, you can add:
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[PATCH v2 05/17] iotests/040: Fix TestCommitWithFilters test
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]
[My commit message is probably also garbage, sorry]
[Feel free to suggest a better one]
[I hope your day is going well]
Signed-off-by: John Snow
Signed-off-by: John Snow
---
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')
--
2.34.1
