diff mbox series

[1/9] iotests: Make nbd-fault-injector flush

Message ID 20181015141453.32632-2-mreitz@redhat.com (mailing list archive)
State New, archived
Headers show
Series iotests: Make them work for both Python 2 and 3 | expand

Commit Message

Max Reitz Oct. 15, 2018, 2:14 p.m. UTC
When closing a connection, make the nbd-fault-injector flush the socket.
Without this, the output is a bit unreliable with Python 3.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/083.out               | 9 +++++++++
 tests/qemu-iotests/nbd-fault-injector.py | 1 +
 2 files changed, 10 insertions(+)

Comments

Eduardo Habkost Oct. 15, 2018, 7:42 p.m. UTC | #1
On Mon, Oct 15, 2018 at 04:14:45PM +0200, Max Reitz wrote:
> When closing a connection, make the nbd-fault-injector flush the socket.
> Without this, the output is a bit unreliable with Python 3.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Cleber Rosa Oct. 15, 2018, 8:24 p.m. UTC | #2
On 10/15/18 10:14 AM, Max Reitz wrote:
> When closing a connection, make the nbd-fault-injector flush the socket.
> Without this, the output is a bit unreliable with Python 3.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Cleber Rosa <crosa@redhat.com>
Eric Blake Oct. 16, 2018, 6:07 p.m. UTC | #3
On 10/15/18 9:14 AM, Max Reitz wrote:
> When closing a connection, make the nbd-fault-injector flush the socket.
> Without this, the output is a bit unreliable with Python 3.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   tests/qemu-iotests/083.out               | 9 +++++++++
>   tests/qemu-iotests/nbd-fault-injector.py | 1 +
>   2 files changed, 10 insertions(+)

I already had a complaint that the error message in 083.out should NOT 
be printing a message (whether the server is python 2 and auto-flushes, 
or python 3 and needs an explicit flush, the message itself is 
pointless, and the test is racy as a result).  We may need to revisit 
this patch when that thread is resolved.

https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg01041.html

That said, I'm not opposed to this patch, if it gets iotests to be more 
useful in the meantime.

> 
> diff --git a/tests/qemu-iotests/083.out b/tests/qemu-iotests/083.out
> index be6079d27e..f9af8bb691 100644
> --- a/tests/qemu-iotests/083.out
> +++ b/tests/qemu-iotests/083.out
> @@ -41,6 +41,7 @@ can't open device nbd+tcp://127.0.0.1:PORT/foo
>   
>   === Check disconnect after neg2 ===
>   
> +Unable to read from socket: Connection reset by peer
>   Connection closed
>   read failed: Input/output error
>   
> @@ -54,6 +55,7 @@ can't open device nbd+tcp://127.0.0.1:PORT/foo
>   
>   === Check disconnect before request ===
>   
> +Unable to read from socket: Connection reset by peer
>   Connection closed
>   read failed: Input/output error
>   
> @@ -116,6 +118,7 @@ can't open device nbd+tcp://127.0.0.1:PORT/
>   
>   === Check disconnect after neg-classic ===
>   
> +Unable to read from socket: Connection reset by peer
>   Connection closed
>   read failed: Input/output error
>   
> @@ -161,6 +164,8 @@ can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
>   
>   === Check disconnect after neg2 ===
>   
> +Unable to read from socket: Connection reset by peer
> +Connection closed
>   read failed: Input/output error
>   
>   === Check disconnect 8 neg2 ===
> @@ -173,6 +178,8 @@ can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
>   
>   === Check disconnect before request ===
>   
> +Unable to read from socket: Connection reset by peer
> +Connection closed
>   read failed: Input/output error
>   
>   === Check disconnect after request ===
> @@ -234,6 +241,8 @@ can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
>   
>   === Check disconnect after neg-classic ===
>   
> +Unable to read from socket: Connection reset by peer
> +Connection closed
>   read failed: Input/output error
>   
>   *** done
> diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
> index f9193c0fae..439a090eb6 100755
> --- a/tests/qemu-iotests/nbd-fault-injector.py
> +++ b/tests/qemu-iotests/nbd-fault-injector.py
> @@ -112,6 +112,7 @@ class FaultInjectionSocket(object):
>               if rule.match(event, io):
>                   if rule.when == 0 or bufsize is None:
>                       print('Closing connection on rule match %s' % rule.name)
> +                    self.sock.flush()
>                       sys.exit(0)
>                   if rule.when != -1:
>                       return rule.when
>
Max Reitz Oct. 19, 2018, 9:48 a.m. UTC | #4
On 16.10.18 20:07, Eric Blake wrote:
> On 10/15/18 9:14 AM, Max Reitz wrote:
>> When closing a connection, make the nbd-fault-injector flush the socket.
>> Without this, the output is a bit unreliable with Python 3.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   tests/qemu-iotests/083.out               | 9 +++++++++
>>   tests/qemu-iotests/nbd-fault-injector.py | 1 +
>>   2 files changed, 10 insertions(+)
> 
> I already had a complaint that the error message in 083.out should NOT
> be printing a message

You mean the NBD server itself, right?

>                       (whether the server is python 2 and auto-flushes,
> or python 3 and needs an explicit flush, the message itself is
> pointless, and the test is racy as a result).  We may need to revisit
> this patch when that thread is resolved.
> 
> https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg01041.html

Well, it's not like the flush hurts either way. :-)

Max

> That said, I'm not opposed to this patch, if it gets iotests to be more
> useful in the meantime.
> 
>>
>> diff --git a/tests/qemu-iotests/083.out b/tests/qemu-iotests/083.out
>> index be6079d27e..f9af8bb691 100644
>> --- a/tests/qemu-iotests/083.out
>> +++ b/tests/qemu-iotests/083.out
>> @@ -41,6 +41,7 @@ can't open device nbd+tcp://127.0.0.1:PORT/foo
>>     === Check disconnect after neg2 ===
>>   +Unable to read from socket: Connection reset by peer
>>   Connection closed
>>   read failed: Input/output error
>>   @@ -54,6 +55,7 @@ can't open device nbd+tcp://127.0.0.1:PORT/foo
>>     === Check disconnect before request ===
>>   +Unable to read from socket: Connection reset by peer
>>   Connection closed
>>   read failed: Input/output error
>>   @@ -116,6 +118,7 @@ can't open device nbd+tcp://127.0.0.1:PORT/
>>     === Check disconnect after neg-classic ===
>>   +Unable to read from socket: Connection reset by peer
>>   Connection closed
>>   read failed: Input/output error
>>   @@ -161,6 +164,8 @@ can't open device
>> nbd+unix:///foo?socket=TEST_DIR/nbd.sock
>>     === Check disconnect after neg2 ===
>>   +Unable to read from socket: Connection reset by peer
>> +Connection closed
>>   read failed: Input/output error
>>     === Check disconnect 8 neg2 ===
>> @@ -173,6 +178,8 @@ can't open device
>> nbd+unix:///foo?socket=TEST_DIR/nbd.sock
>>     === Check disconnect before request ===
>>   +Unable to read from socket: Connection reset by peer
>> +Connection closed
>>   read failed: Input/output error
>>     === Check disconnect after request ===
>> @@ -234,6 +241,8 @@ can't open device
>> nbd+unix:///?socket=TEST_DIR/nbd.sock
>>     === Check disconnect after neg-classic ===
>>   +Unable to read from socket: Connection reset by peer
>> +Connection closed
>>   read failed: Input/output error
>>     *** done
>> diff --git a/tests/qemu-iotests/nbd-fault-injector.py
>> b/tests/qemu-iotests/nbd-fault-injector.py
>> index f9193c0fae..439a090eb6 100755
>> --- a/tests/qemu-iotests/nbd-fault-injector.py
>> +++ b/tests/qemu-iotests/nbd-fault-injector.py
>> @@ -112,6 +112,7 @@ class FaultInjectionSocket(object):
>>               if rule.match(event, io):
>>                   if rule.when == 0 or bufsize is None:
>>                       print('Closing connection on rule match %s' %
>> rule.name)
>> +                    self.sock.flush()
>>                       sys.exit(0)
>>                   if rule.when != -1:
>>                       return rule.when
>>
>
Eric Blake Oct. 19, 2018, 2:21 p.m. UTC | #5
On 10/19/18 4:48 AM, Max Reitz wrote:
> On 16.10.18 20:07, Eric Blake wrote:
>> On 10/15/18 9:14 AM, Max Reitz wrote:
>>> When closing a connection, make the nbd-fault-injector flush the socket.
>>> Without this, the output is a bit unreliable with Python 3.
>>>
>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>> ---
>>>    tests/qemu-iotests/083.out               | 9 +++++++++
>>>    tests/qemu-iotests/nbd-fault-injector.py | 1 +
>>>    2 files changed, 10 insertions(+)
>>
>> I already had a complaint that the error message in 083.out should NOT
>> be printing a message
> 
> You mean the NBD server itself, right?

Yes, the NBD server should not be printing the redundant error messages 
that 083.out exposes.

> 
>>                        (whether the server is python 2 and auto-flushes,
>> or python 3 and needs an explicit flush, the message itself is
>> pointless, and the test is racy as a result).  We may need to revisit
>> this patch when that thread is resolved.
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg01041.html
> 
> Well, it's not like the flush hurts either way. :-)

True. On that grounds:

Reviewed-by: Eric Blake <eblake@redhat.com>
diff mbox series

Patch

diff --git a/tests/qemu-iotests/083.out b/tests/qemu-iotests/083.out
index be6079d27e..f9af8bb691 100644
--- a/tests/qemu-iotests/083.out
+++ b/tests/qemu-iotests/083.out
@@ -41,6 +41,7 @@  can't open device nbd+tcp://127.0.0.1:PORT/foo
 
 === Check disconnect after neg2 ===
 
+Unable to read from socket: Connection reset by peer
 Connection closed
 read failed: Input/output error
 
@@ -54,6 +55,7 @@  can't open device nbd+tcp://127.0.0.1:PORT/foo
 
 === Check disconnect before request ===
 
+Unable to read from socket: Connection reset by peer
 Connection closed
 read failed: Input/output error
 
@@ -116,6 +118,7 @@  can't open device nbd+tcp://127.0.0.1:PORT/
 
 === Check disconnect after neg-classic ===
 
+Unable to read from socket: Connection reset by peer
 Connection closed
 read failed: Input/output error
 
@@ -161,6 +164,8 @@  can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
 
 === Check disconnect after neg2 ===
 
+Unable to read from socket: Connection reset by peer
+Connection closed
 read failed: Input/output error
 
 === Check disconnect 8 neg2 ===
@@ -173,6 +178,8 @@  can't open device nbd+unix:///foo?socket=TEST_DIR/nbd.sock
 
 === Check disconnect before request ===
 
+Unable to read from socket: Connection reset by peer
+Connection closed
 read failed: Input/output error
 
 === Check disconnect after request ===
@@ -234,6 +241,8 @@  can't open device nbd+unix:///?socket=TEST_DIR/nbd.sock
 
 === Check disconnect after neg-classic ===
 
+Unable to read from socket: Connection reset by peer
+Connection closed
 read failed: Input/output error
 
 *** done
diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
index f9193c0fae..439a090eb6 100755
--- a/tests/qemu-iotests/nbd-fault-injector.py
+++ b/tests/qemu-iotests/nbd-fault-injector.py
@@ -112,6 +112,7 @@  class FaultInjectionSocket(object):
             if rule.match(event, io):
                 if rule.when == 0 or bufsize is None:
                     print('Closing connection on rule match %s' % rule.name)
+                    self.sock.flush()
                     sys.exit(0)
                 if rule.when != -1:
                     return rule.when