| Message ID | 20220324183018.2476551-6-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | iotests: add enhanced debugging info to qemu-io failures | expand |
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 <jsnow@redhat.com> > > Signed-off-by: John Snow <jsnow@redhat.com> 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 <eblake@redhat.com>
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') >
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') > > > >
On Thu, Mar 24, 2022 at 9:33 PM Eric Blake <eblake@redhat.com> 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 <jsnow@redhat.com> > > > > Signed-off-by: John Snow <jsnow@redhat.com> > > 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 <eblake@redhat.com> > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3266 > Virtualization: qemu.org | libvirt.org >
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')
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 <jsnow@redhat.com> Signed-off-by: John Snow <jsnow@redhat.com> --- tests/qemu-iotests/040 | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)