Re: [PATCH v4 6/8] iotests/300: avoid abnormal shutdown race condition
On 26.10.21 19:07, John Snow wrote:
On Mon, Oct 25, 2021 at 9:20 AM Hanna Reitz wrote:
On 13.10.21 23:57, John Snow wrote:
> Wait for the destination VM to close itself instead of racing to
shut it
> down first, which produces different error log messages from AQMP
> depending on precisely when we tried to shut it down.
>
> (For example: We may try to issue 'quit' immediately prior to
the target
> VM closing its QMP socket, which will cause an ECONNRESET error
to be
> logged. Waiting for the VM to exit itself avoids the race on
shutdown
> behavior.)
>
> Reported-by: Hanna Reitz
> Signed-off-by: John Snow
> ---
> tests/qemu-iotests/300 | 12
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
> index 10f9f2a8da6..bbea7248005 100755
> --- a/tests/qemu-iotests/300
> +++ b/tests/qemu-iotests/300
> @@ -24,8 +24,6 @@ import random
> import re
> from typing import Dict, List, Optional
>
> -from qemu.machine import machine
> -
> import iotests
>
>
> @@ -461,12 +459,10 @@ class
TestBlockBitmapMappingErrors(TestDirtyBitmapMigration):
> f"'{self.src_node_name}': Name is longer
than 255 bytes",
> log)
>
> - # Expect abnormal shutdown of the destination VM because of
> - # the failed migration
> - try:
> - self.vm_b.shutdown()
> - except machine.AbnormalShutdown:
> - pass
> + # Destination VM will terminate w/ error of its own accord
> + # due to the failed migration.
> + self.vm_b.wait()
> + assert self.vm_b.exitcode() > 0
Trying to test, I can see that this fails iotest 297, because
`.exitcode()` is `Optional[int]`...
(I can’t believe how long it took me to figure this out – the message
“300:465: Unsupported operand types for < ("int" and "None")” made me
believe that it was 300 that was failing, because `exitcode()` was
returning `None` for some inconceivable reason. I couldn’t
understand
why my usual test setup failed on every run, but I couldn’t get
300 to
fail manually... Until I noticed that the message came below the
“297”
line, not the “300” line...)
Oops. Is there anything we can do to improve the visual clarity there?
You mean, like, separating iotests from the Python linting tests? :)
I think until then I’ll just need to pay more attention.
Hanna
Re: [PATCH v4 6/8] iotests/300: avoid abnormal shutdown race condition
On Mon, Oct 25, 2021 at 9:20 AM Hanna Reitz wrote:
> On 13.10.21 23:57, John Snow wrote:
> > Wait for the destination VM to close itself instead of racing to shut it
> > down first, which produces different error log messages from AQMP
> > depending on precisely when we tried to shut it down.
> >
> > (For example: We may try to issue 'quit' immediately prior to the target
> > VM closing its QMP socket, which will cause an ECONNRESET error to be
> > logged. Waiting for the VM to exit itself avoids the race on shutdown
> > behavior.)
> >
> > Reported-by: Hanna Reitz
> > Signed-off-by: John Snow
> > ---
> > tests/qemu-iotests/300 | 12
> > 1 file changed, 4 insertions(+), 8 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
> > index 10f9f2a8da6..bbea7248005 100755
> > --- a/tests/qemu-iotests/300
> > +++ b/tests/qemu-iotests/300
> > @@ -24,8 +24,6 @@ import random
> > import re
> > from typing import Dict, List, Optional
> >
> > -from qemu.machine import machine
> > -
> > import iotests
> >
> >
> > @@ -461,12 +459,10 @@ class
> TestBlockBitmapMappingErrors(TestDirtyBitmapMigration):
> > f"'{self.src_node_name}': Name is longer than
> 255 bytes",
> > log)
> >
> > -# Expect abnormal shutdown of the destination VM because of
> > -# the failed migration
> > -try:
> > -self.vm_b.shutdown()
> > -except machine.AbnormalShutdown:
> > -pass
> > +# Destination VM will terminate w/ error of its own accord
> > +# due to the failed migration.
> > +self.vm_b.wait()
> > +assert self.vm_b.exitcode() > 0
>
> Trying to test, I can see that this fails iotest 297, because
> `.exitcode()` is `Optional[int]`...
>
> (I can’t believe how long it took me to figure this out – the message
> “300:465: Unsupported operand types for < ("int" and "None")” made me
> believe that it was 300 that was failing, because `exitcode()` was
> returning `None` for some inconceivable reason. I couldn’t understand
> why my usual test setup failed on every run, but I couldn’t get 300 to
> fail manually... Until I noticed that the message came below the “297”
> line, not the “300” line...)
>
>
Oops. Is there anything we can do to improve the visual clarity there?
> Hanna
>
>
Embarrassing. I scrutinized the other series I sent out, but forgot to
apply the same tests to this one. :(
Fixed, sorry for the noise.
--js
Re: [PATCH v4 6/8] iotests/300: avoid abnormal shutdown race condition
On 13.10.21 23:57, John Snow wrote:
Wait for the destination VM to close itself instead of racing to shut it
down first, which produces different error log messages from AQMP
depending on precisely when we tried to shut it down.
(For example: We may try to issue 'quit' immediately prior to the target
VM closing its QMP socket, which will cause an ECONNRESET error to be
logged. Waiting for the VM to exit itself avoids the race on shutdown
behavior.)
Reported-by: Hanna Reitz
Signed-off-by: John Snow
---
tests/qemu-iotests/300 | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
index 10f9f2a8da6..bbea7248005 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -24,8 +24,6 @@ import random
import re
from typing import Dict, List, Optional
-from qemu.machine import machine
-
import iotests
@@ -461,12 +459,10 @@ class TestBlockBitmapMappingErrors(TestDirtyBitmapMigration):
f"'{self.src_node_name}': Name is longer than 255
bytes",
log)
-# Expect abnormal shutdown of the destination VM because of
-# the failed migration
-try:
-self.vm_b.shutdown()
-except machine.AbnormalShutdown:
-pass
+# Destination VM will terminate w/ error of its own accord
+# due to the failed migration.
+self.vm_b.wait()
+assert self.vm_b.exitcode() > 0
Trying to test, I can see that this fails iotest 297, because
`.exitcode()` is `Optional[int]`...
(I can’t believe how long it took me to figure this out – the message
“300:465: Unsupported operand types for < ("int" and "None")” made me
believe that it was 300 that was failing, because `exitcode()` was
returning `None` for some inconceivable reason. I couldn’t understand
why my usual test setup failed on every run, but I couldn’t get 300 to
fail manually... Until I noticed that the message came below the “297”
line, not the “300” line...)
Hanna
Re: [PATCH v4 6/8] iotests/300: avoid abnormal shutdown race condition
On 13.10.21 23:57, John Snow wrote: Wait for the destination VM to close itself instead of racing to shut it down first, which produces different error log messages from AQMP depending on precisely when we tried to shut it down. (For example: We may try to issue 'quit' immediately prior to the target VM closing its QMP socket, which will cause an ECONNRESET error to be logged. Waiting for the VM to exit itself avoids the race on shutdown behavior.) Reported-by: Hanna Reitz Signed-off-by: John Snow --- tests/qemu-iotests/300 | 12 1 file changed, 4 insertions(+), 8 deletions(-) Reviewed-by: Hanna Reitz
Re: [PATCH v4 6/8] iotests/300: avoid abnormal shutdown race condition
14.10.2021 00:57, John Snow wrote: Wait for the destination VM to close itself instead of racing to shut it down first, which produces different error log messages from AQMP depending on precisely when we tried to shut it down. (For example: We may try to issue 'quit' immediately prior to the target VM closing its QMP socket, which will cause an ECONNRESET error to be logged. Waiting for the VM to exit itself avoids the race on shutdown behavior.) Reported-by: Hanna Reitz Signed-off-by: John Snow Reviewed-by: Vladimir Sementsov-Ogievskiy -- Best regards, Vladimir
[PATCH v4 6/8] iotests/300: avoid abnormal shutdown race condition
Wait for the destination VM to close itself instead of racing to shut it
down first, which produces different error log messages from AQMP
depending on precisely when we tried to shut it down.
(For example: We may try to issue 'quit' immediately prior to the target
VM closing its QMP socket, which will cause an ECONNRESET error to be
logged. Waiting for the VM to exit itself avoids the race on shutdown
behavior.)
Reported-by: Hanna Reitz
Signed-off-by: John Snow
---
tests/qemu-iotests/300 | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
index 10f9f2a8da6..bbea7248005 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -24,8 +24,6 @@ import random
import re
from typing import Dict, List, Optional
-from qemu.machine import machine
-
import iotests
@@ -461,12 +459,10 @@ class
TestBlockBitmapMappingErrors(TestDirtyBitmapMigration):
f"'{self.src_node_name}': Name is longer than 255 bytes",
log)
-# Expect abnormal shutdown of the destination VM because of
-# the failed migration
-try:
-self.vm_b.shutdown()
-except machine.AbnormalShutdown:
-pass
+# Destination VM will terminate w/ error of its own accord
+# due to the failed migration.
+self.vm_b.wait()
+assert self.vm_b.exitcode() > 0
def test_aliased_bitmap_name_too_long(self) -> None:
# Longer than the maximum for bitmap names
--
2.31.1
