* [PATCH v3] iotests: Test NBD client reconnection
@ 2019-11-01 16:54 Andrey Shinkevich
2019-11-08 13:49 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 6+ messages in thread
From: Andrey Shinkevich @ 2019-11-01 16:54 UTC (permalink / raw)
To: qemu-devel, qemu-block; +Cc: kwolf, vsementsov, mreitz, andrey.shinkevich, den
The test for an NBD client. The NBD server is disconnected after the
client write request. The NBD client should reconnect and complete
the write operation.
Suggested-by: Denis V. Lunev <den@openvz.org>
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
tests/qemu-iotests/277 | 102 +++++++++++++++++++++++++++++++
tests/qemu-iotests/277.out | 6 ++
tests/qemu-iotests/group | 1 +
tests/qemu-iotests/iotests.py | 5 ++
tests/qemu-iotests/nbd-fault-injector.py | 3 +-
5 files changed, 116 insertions(+), 1 deletion(-)
create mode 100755 tests/qemu-iotests/277
create mode 100644 tests/qemu-iotests/277.out
diff --git a/tests/qemu-iotests/277 b/tests/qemu-iotests/277
new file mode 100755
index 0000000..e4e6730
--- /dev/null
+++ b/tests/qemu-iotests/277
@@ -0,0 +1,102 @@
+#!/usr/bin/env python
+#
+# Test NBD client reconnection
+#
+# Copyright (c) 2019 Virtuozzo International GmbH
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+import os
+import subprocess
+import iotests
+from iotests import file_path, log
+
+
+def make_conf_file(event):
+ """
+ Create configuration file for the nbd-fault-injector.py
+
+ :param event: which event the server should close a connection on
+ """
+ if os.path.exists(conf_file):
+ os.remove(conf_file)
+
+ with open(conf_file, "w+") as conff:
+ conff.write("[inject-error]\nevent={}\nwhen=after".format(event))
+
+
+def start_server_NBD(event):
+ make_conf_file(event)
+
+ srv = subprocess.Popen(["nbd-fault-injector.py", "--classic-negotiation",
+ nbd_sock, conf_file], stdout=subprocess.PIPE,
+ stderr=subprocess.STDOUT, universal_newlines=True)
+ line = srv.stdout.readline()
+ if "Listening on " in line:
+ log('NBD server: started')
+ else:
+ log('NBD server: ' + line.rstrip())
+
+ return srv
+
+
+def start_client_NBD():
+ log('NBD client: QEMU-IO write')
+ args = iotests.qemu_io_args_no_fmt + \
+ ['-c', 'write -P 0x7 0 3M', '--image-opts',
+ 'driver=nbd,server.type=unix,server.path={},'
+ 'reconnect-delay=7'.format(nbd_sock)]
+ clt = subprocess.Popen(args, stdout=subprocess.PIPE,
+ stderr=subprocess.STDOUT,
+ universal_newlines=True)
+ return clt
+
+
+def check_proc_NBD(proc, connector):
+ try:
+ exitcode = proc.wait(timeout=10)
+
+ if exitcode < 0:
+ log('NBD {}: EXIT SIGNAL {}\n'.format(connector, -exitcode))
+ log(proc.communicate()[0])
+ else:
+ line = proc.stdout.readline()
+ log('NBD {}: {}'.format(connector, line.rstrip()))
+
+ except subprocess.TimeoutExpired:
+ proc.kill()
+ log('NBD {}: ERROR timeout expired'.format(connector))
+ finally:
+ if connector == 'server':
+ os.remove(nbd_sock)
+ os.remove(conf_file)
+
+
+conf_file = os.path.join(iotests.test_dir, "nbd-fault-injector.conf")
+nbd_sock = file_path('nbd-sock')
+nbd_uri = 'nbd+unix:///?socket=' + nbd_sock
+if os.path.exists(nbd_sock):
+ os.remove(nbd_sock)
+
+srv = start_server_NBD('data')
+clt = start_client_NBD()
+# The server should close the connection after a client write request
+check_proc_NBD(srv, 'server')
+# Start the NBD server again
+srv = start_server_NBD('reply')
+# The client should reconnect and complete the write operation
+check_proc_NBD(clt, 'client')
+# Make it sure that server terminated
+check_proc_NBD(srv, 'server')
diff --git a/tests/qemu-iotests/277.out b/tests/qemu-iotests/277.out
new file mode 100644
index 0000000..45404b3
--- /dev/null
+++ b/tests/qemu-iotests/277.out
@@ -0,0 +1,6 @@
+NBD server: started
+NBD client: QEMU-IO write
+NBD server: Closing connection on rule match inject-error
+NBD server: started
+NBD client: wrote 3145728/3145728 bytes at offset 0
+NBD server: Closing connection on rule match inject-error
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index af322af..22ef1b8 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -282,3 +282,4 @@
267 rw auto quick snapshot
268 rw auto quick
270 rw backing quick
+277 rw
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 709def4..0d16303 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -47,6 +47,11 @@ qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
if os.environ.get('QEMU_IO_OPTIONS'):
qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
+qemu_io_args_no_fmt = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
+if os.environ.get('QEMU_IO_OPTIONS_NO_FMT'):
+ qemu_io_args_no_fmt += \
+ os.environ['QEMU_IO_OPTIONS_NO_FMT'].strip().split(' ')
+
qemu_nbd_args = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]
if os.environ.get('QEMU_NBD_OPTIONS'):
qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
index 6b2d659..7e2dab6 100755
--- a/tests/qemu-iotests/nbd-fault-injector.py
+++ b/tests/qemu-iotests/nbd-fault-injector.py
@@ -115,7 +115,8 @@ 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()
+ self.sock.close()
+ sys.stdout.flush()
sys.exit(0)
if rule.when != -1:
return rule.when
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] iotests: Test NBD client reconnection
2019-11-01 16:54 [PATCH v3] iotests: Test NBD client reconnection Andrey Shinkevich
@ 2019-11-08 13:49 ` Vladimir Sementsov-Ogievskiy
2019-11-08 14:05 ` Roman Kagan
2019-11-11 9:07 ` Andrey Shinkevich
0 siblings, 2 replies; 6+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-11-08 13:49 UTC (permalink / raw)
To: Andrey Shinkevich, qemu-devel, qemu-block; +Cc: kwolf, Denis Lunev, mreitz
01.11.2019 19:54, Andrey Shinkevich wrote:
> The test for an NBD client. The NBD server is disconnected after the
> client write request. The NBD client should reconnect and complete
> the write operation.
>
> Suggested-by: Denis V. Lunev <den@openvz.org>
> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
> tests/qemu-iotests/277 | 102 +++++++++++++++++++++++++++++++
> tests/qemu-iotests/277.out | 6 ++
> tests/qemu-iotests/group | 1 +
> tests/qemu-iotests/iotests.py | 5 ++
> tests/qemu-iotests/nbd-fault-injector.py | 3 +-
> 5 files changed, 116 insertions(+), 1 deletion(-)
> create mode 100755 tests/qemu-iotests/277
> create mode 100644 tests/qemu-iotests/277.out
>
> diff --git a/tests/qemu-iotests/277 b/tests/qemu-iotests/277
> new file mode 100755
> index 0000000..e4e6730
> --- /dev/null
> +++ b/tests/qemu-iotests/277
> @@ -0,0 +1,102 @@
> +#!/usr/bin/env python
> +#
> +# Test NBD client reconnection
> +#
> +# Copyright (c) 2019 Virtuozzo International GmbH
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +import os
> +import subprocess
> +import iotests
> +from iotests import file_path, log
> +
> +
> +def make_conf_file(event):
> + """
> + Create configuration file for the nbd-fault-injector.py
> +
> + :param event: which event the server should close a connection on
> + """
> + if os.path.exists(conf_file):
> + os.remove(conf_file)
a bit strange for my eyes to see global variable not defined higher in the file than used..
> +
> + with open(conf_file, "w+") as conff:
> + conff.write("[inject-error]\nevent={}\nwhen=after".format(event))
better open with "w" and don't need to remove file before it.
> +
> +
> +def start_server_NBD(event):
> + make_conf_file(event)
> +
> + srv = subprocess.Popen(["nbd-fault-injector.py", "--classic-negotiation",
> + nbd_sock, conf_file], stdout=subprocess.PIPE,
> + stderr=subprocess.STDOUT, universal_newlines=True)
> + line = srv.stdout.readline()
> + if "Listening on " in line:
> + log('NBD server: started')
> + else:
> + log('NBD server: ' + line.rstrip())
> +
> + return srv
> +
> +
> +def start_client_NBD():
> + log('NBD client: QEMU-IO write')
> + args = iotests.qemu_io_args_no_fmt + \
> + ['-c', 'write -P 0x7 0 3M', '--image-opts',
> + 'driver=nbd,server.type=unix,server.path={},'
> + 'reconnect-delay=7'.format(nbd_sock)]
> + clt = subprocess.Popen(args, stdout=subprocess.PIPE,
> + stderr=subprocess.STDOUT,
> + universal_newlines=True)
> + return clt
Could you reuse QemuIoInteractive as a client?
> +
> +
> +def check_proc_NBD(proc, connector):
> + try:
> + exitcode = proc.wait(timeout=10)
> +
> + if exitcode < 0:
> + log('NBD {}: EXIT SIGNAL {}\n'.format(connector, -exitcode))
> + log(proc.communicate()[0])
> + else:
> + line = proc.stdout.readline()
could we use proc.communicate() for both cases, what is the difference?
> + log('NBD {}: {}'.format(connector, line.rstrip()))
> +
> + except subprocess.TimeoutExpired:
> + proc.kill()
> + log('NBD {}: ERROR timeout expired'.format(connector))
> + finally:
> + if connector == 'server':
> + os.remove(nbd_sock)
> + os.remove(conf_file)
> +
> +
> +conf_file = os.path.join(iotests.test_dir, "nbd-fault-injector.conf")
use file_path here too.
> +nbd_sock = file_path('nbd-sock')
> +nbd_uri = 'nbd+unix:///?socket=' + nbd_sock
unused variable
> +if os.path.exists(nbd_sock):
> + os.remove(nbd_sock)
I don't think we need this
> +
> +srv = start_server_NBD('data')
> +clt = start_client_NBD()
> +# The server should close the connection after a client write request
> +check_proc_NBD(srv, 'server')
> +# Start the NBD server again
> +srv = start_server_NBD('reply')
> +# The client should reconnect and complete the write operation
> +check_proc_NBD(clt, 'client')
> +# Make it sure that server terminated
> +check_proc_NBD(srv, 'server')
> diff --git a/tests/qemu-iotests/277.out b/tests/qemu-iotests/277.out
> new file mode 100644
> index 0000000..45404b3
> --- /dev/null
> +++ b/tests/qemu-iotests/277.out
> @@ -0,0 +1,6 @@
> +NBD server: started
> +NBD client: QEMU-IO write
> +NBD server: Closing connection on rule match inject-error
> +NBD server: started
> +NBD client: wrote 3145728/3145728 bytes at offset 0
> +NBD server: Closing connection on rule match inject-error
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index af322af..22ef1b8 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -282,3 +282,4 @@
> 267 rw auto quick snapshot
> 268 rw auto quick
> 270 rw backing quick
> +277 rw
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 709def4..0d16303 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -47,6 +47,11 @@ qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
> if os.environ.get('QEMU_IO_OPTIONS'):
> qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
>
> +qemu_io_args_no_fmt = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
> +if os.environ.get('QEMU_IO_OPTIONS_NO_FMT'):
> + qemu_io_args_no_fmt += \
> + os.environ['QEMU_IO_OPTIONS_NO_FMT'].strip().split(' ')
> +
> qemu_nbd_args = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]
> if os.environ.get('QEMU_NBD_OPTIONS'):
> qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
> diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
> index 6b2d659..7e2dab6 100755
> --- a/tests/qemu-iotests/nbd-fault-injector.py
> +++ b/tests/qemu-iotests/nbd-fault-injector.py
> @@ -115,7 +115,8 @@ 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()
> + self.sock.close()
> + sys.stdout.flush()
Why do you need this? It should be mentioned in commit message I think.
> sys.exit(0)
> if rule.when != -1:
> return rule.when
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] iotests: Test NBD client reconnection
2019-11-08 13:49 ` Vladimir Sementsov-Ogievskiy
@ 2019-11-08 14:05 ` Roman Kagan
2019-11-11 9:18 ` Andrey Shinkevich
2019-11-11 9:07 ` Andrey Shinkevich
1 sibling, 1 reply; 6+ messages in thread
From: Roman Kagan @ 2019-11-08 14:05 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: kwolf, Denis Lunev, qemu-block, qemu-devel, mreitz, Andrey Shinkevich
On Fri, Nov 08, 2019 at 01:49:50PM +0000, Vladimir Sementsov-Ogievskiy wrote:
> 01.11.2019 19:54, Andrey Shinkevich wrote:
> > +def check_proc_NBD(proc, connector):
> > + try:
> > + exitcode = proc.wait(timeout=10)
> > +
> > + if exitcode < 0:
> > + log('NBD {}: EXIT SIGNAL {}\n'.format(connector, -exitcode))
> > + log(proc.communicate()[0])
> > + else:
> > + line = proc.stdout.readline()
>
>
> could we use proc.communicate() for both cases, what is the difference?
In fact if proc produces any non-trivial amount of output you are better
off using .communicate() otherwise your child may block on output and
never exit. See
https://docs.python.org/3/library/subprocess.html#subprocess.Popen.communicate
for how to express the above logic correctly. The exit code *after*
.communicate is available in .returncode.
>
> > + log('NBD {}: {}'.format(connector, line.rstrip()))
> > +
> > + except subprocess.TimeoutExpired:
> > + proc.kill()
> > + log('NBD {}: ERROR timeout expired'.format(connector))
> > + finally:
> > + if connector == 'server':
> > + os.remove(nbd_sock)
> > + os.remove(conf_file)
Roman.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] iotests: Test NBD client reconnection
2019-11-08 13:49 ` Vladimir Sementsov-Ogievskiy
2019-11-08 14:05 ` Roman Kagan
@ 2019-11-11 9:07 ` Andrey Shinkevich
1 sibling, 0 replies; 6+ messages in thread
From: Andrey Shinkevich @ 2019-11-11 9:07 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block
Cc: kwolf, Denis Lunev, mreitz
On 08/11/2019 16:49, Vladimir Sementsov-Ogievskiy wrote:
> 01.11.2019 19:54, Andrey Shinkevich wrote:
>> The test for an NBD client. The NBD server is disconnected after the
>> client write request. The NBD client should reconnect and complete
>> the write operation.
>>
>> Suggested-by: Denis V. Lunev <den@openvz.org>
>> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>> tests/qemu-iotests/277 | 102 +++++++++++++++++++++++++++++++
>> tests/qemu-iotests/277.out | 6 ++
>> tests/qemu-iotests/group | 1 +
>> tests/qemu-iotests/iotests.py | 5 ++
>> tests/qemu-iotests/nbd-fault-injector.py | 3 +-
>> 5 files changed, 116 insertions(+), 1 deletion(-)
>> create mode 100755 tests/qemu-iotests/277
>> create mode 100644 tests/qemu-iotests/277.out
>>
>> diff --git a/tests/qemu-iotests/277 b/tests/qemu-iotests/277
>> new file mode 100755
>> index 0000000..e4e6730
>> --- /dev/null
>> +++ b/tests/qemu-iotests/277
>> @@ -0,0 +1,102 @@
>> +#!/usr/bin/env python
>> +#
>> +# Test NBD client reconnection
>> +#
>> +# Copyright (c) 2019 Virtuozzo International GmbH
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
>> +#
>> +
>> +import os
>> +import subprocess
>> +import iotests
>> +from iotests import file_path, log
>> +
>> +
>> +def make_conf_file(event):
>> + """
>> + Create configuration file for the nbd-fault-injector.py
>> +
>> + :param event: which event the server should close a connection on
>> + """
>> + if os.path.exists(conf_file):
>> + os.remove(conf_file)
>
> a bit strange for my eyes to see global variable not defined higher in the file than used..
>
>> +
>> + with open(conf_file, "w+") as conff:
>> + conff.write("[inject-error]\nevent={}\nwhen=after".format(event))
>
> better open with "w" and don't need to remove file before it.
>
>> +
>> +
>> +def start_server_NBD(event):
>> + make_conf_file(event)
>> +
>> + srv = subprocess.Popen(["nbd-fault-injector.py", "--classic-negotiation",
>> + nbd_sock, conf_file], stdout=subprocess.PIPE,
>> + stderr=subprocess.STDOUT, universal_newlines=True)
>> + line = srv.stdout.readline()
>> + if "Listening on " in line:
>> + log('NBD server: started')
>> + else:
>> + log('NBD server: ' + line.rstrip())
>> +
>> + return srv
>> +
>> +
>> +def start_client_NBD():
>> + log('NBD client: QEMU-IO write')
>> + args = iotests.qemu_io_args_no_fmt + \
>> + ['-c', 'write -P 0x7 0 3M', '--image-opts',
>> + 'driver=nbd,server.type=unix,server.path={},'
>> + 'reconnect-delay=7'.format(nbd_sock)]
>> + clt = subprocess.Popen(args, stdout=subprocess.PIPE,
>> + stderr=subprocess.STDOUT,
>> + universal_newlines=True)
>> + return clt
>
> Could you reuse QemuIoInteractive as a client?
>
Thanks for your review.
The class QemuIoInteractive uses the qemu_io_args variable that includes
the format option which conflicts with the option 'image-opts'.
>> +
>> +
>> +def check_proc_NBD(proc, connector):
>> + try:
>> + exitcode = proc.wait(timeout=10)
>> +
>> + if exitcode < 0:
>> + log('NBD {}: EXIT SIGNAL {}\n'.format(connector, -exitcode))
>> + log(proc.communicate()[0])
>> + else:
>> + line = proc.stdout.readline()
>
>
> could we use proc.communicate() for both cases, what is the difference?
>
>> + log('NBD {}: {}'.format(connector, line.rstrip()))
>> +
>> + except subprocess.TimeoutExpired:
>> + proc.kill()
>> + log('NBD {}: ERROR timeout expired'.format(connector))
>> + finally:
>> + if connector == 'server':
>> + os.remove(nbd_sock)
>> + os.remove(conf_file)
>> +
>> +
>> +conf_file = os.path.join(iotests.test_dir, "nbd-fault-injector.conf")
>
> use file_path here too.
>
>> +nbd_sock = file_path('nbd-sock')
>> +nbd_uri = 'nbd+unix:///?socket=' + nbd_sock
>
> unused variable
>
>> +if os.path.exists(nbd_sock):
>> + os.remove(nbd_sock)
>
> I don't think we need this
>
>> +
>> +srv = start_server_NBD('data')
>> +clt = start_client_NBD()
>> +# The server should close the connection after a client write request
>> +check_proc_NBD(srv, 'server')
>> +# Start the NBD server again
>> +srv = start_server_NBD('reply')
>> +# The client should reconnect and complete the write operation
>> +check_proc_NBD(clt, 'client')
>> +# Make it sure that server terminated
>> +check_proc_NBD(srv, 'server')
>> diff --git a/tests/qemu-iotests/277.out b/tests/qemu-iotests/277.out
>> new file mode 100644
>> index 0000000..45404b3
>> --- /dev/null
>> +++ b/tests/qemu-iotests/277.out
>> @@ -0,0 +1,6 @@
>> +NBD server: started
>> +NBD client: QEMU-IO write
>> +NBD server: Closing connection on rule match inject-error
>> +NBD server: started
>> +NBD client: wrote 3145728/3145728 bytes at offset 0
>> +NBD server: Closing connection on rule match inject-error
>> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
>> index af322af..22ef1b8 100644
>> --- a/tests/qemu-iotests/group
>> +++ b/tests/qemu-iotests/group
>> @@ -282,3 +282,4 @@
>> 267 rw auto quick snapshot
>> 268 rw auto quick
>> 270 rw backing quick
>> +277 rw
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 709def4..0d16303 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -47,6 +47,11 @@ qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
>> if os.environ.get('QEMU_IO_OPTIONS'):
>> qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
>>
>> +qemu_io_args_no_fmt = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
>> +if os.environ.get('QEMU_IO_OPTIONS_NO_FMT'):
>> + qemu_io_args_no_fmt += \
>> + os.environ['QEMU_IO_OPTIONS_NO_FMT'].strip().split(' ')
>> +
>> qemu_nbd_args = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]
>> if os.environ.get('QEMU_NBD_OPTIONS'):
>> qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
>> diff --git a/tests/qemu-iotests/nbd-fault-injector.py b/tests/qemu-iotests/nbd-fault-injector.py
>> index 6b2d659..7e2dab6 100755
>> --- a/tests/qemu-iotests/nbd-fault-injector.py
>> +++ b/tests/qemu-iotests/nbd-fault-injector.py
>> @@ -115,7 +115,8 @@ 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()
>> + self.sock.close()
>> + sys.stdout.flush()
>
> Why do you need this? It should be mentioned in commit message I think.
>
The socket object does not have the flush() method. I guess the author
wanted to flush from the stdout.
>> sys.exit(0)
>> if rule.when != -1:
>> return rule.when
>>
>
>
--
With the best regards,
Andrey Shinkevich
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] iotests: Test NBD client reconnection
2019-11-08 14:05 ` Roman Kagan
@ 2019-11-11 9:18 ` Andrey Shinkevich
2019-11-11 9:48 ` Roman Kagan
0 siblings, 1 reply; 6+ messages in thread
From: Andrey Shinkevich @ 2019-11-11 9:18 UTC (permalink / raw)
To: Roman Kagan, Vladimir Sementsov-Ogievskiy
Cc: kwolf, mreitz, qemu-devel, qemu-block, Denis Lunev
On 08/11/2019 17:05, Roman Kagan wrote:
> On Fri, Nov 08, 2019 at 01:49:50PM +0000, Vladimir Sementsov-Ogievskiy wrote:
>> 01.11.2019 19:54, Andrey Shinkevich wrote:
>>> +def check_proc_NBD(proc, connector):
>>> + try:
>>> + exitcode = proc.wait(timeout=10)
>>> +
>>> + if exitcode < 0:
>>> + log('NBD {}: EXIT SIGNAL {}\n'.format(connector, -exitcode))
>>> + log(proc.communicate()[0])
>>> + else:
>>> + line = proc.stdout.readline()
>>
>>
>> could we use proc.communicate() for both cases, what is the difference?
>
> In fact if proc produces any non-trivial amount of output you are better
> off using .communicate() otherwise your child may block on output and
> never exit. See
> https://docs.python.org/3/library/subprocess.html#subprocess.Popen.communicate
> for how to express the above logic correctly. The exit code *after*
> .communicate is available in .returncode.
>
The pattern by the link above does not work (Python3):
proc = subprocess.Popen(...)
try:
outs, errs = proc.communicate(timeout=15)
except TimeoutExpired:
proc.kill()
outs, errs = proc.communicate()
as 'proc' cannot be used for output after being killed. It results in
another exception being raised.
Andrey
>>
>>> + log('NBD {}: {}'.format(connector, line.rstrip()))
>>> +
>>> + except subprocess.TimeoutExpired:
>>> + proc.kill()
>>> + log('NBD {}: ERROR timeout expired'.format(connector))
>>> + finally:
>>> + if connector == 'server':
>>> + os.remove(nbd_sock)
>>> + os.remove(conf_file)
>
> Roman.
>
--
With the best regards,
Andrey Shinkevich
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] iotests: Test NBD client reconnection
2019-11-11 9:18 ` Andrey Shinkevich
@ 2019-11-11 9:48 ` Roman Kagan
0 siblings, 0 replies; 6+ messages in thread
From: Roman Kagan @ 2019-11-11 9:48 UTC (permalink / raw)
To: Andrey Shinkevich
Cc: kwolf, Vladimir Sementsov-Ogievskiy, Denis Lunev, qemu-block,
qemu-devel, mreitz
On Mon, Nov 11, 2019 at 12:18:48PM +0300, Andrey Shinkevich wrote:
>
>
> On 08/11/2019 17:05, Roman Kagan wrote:
> > On Fri, Nov 08, 2019 at 01:49:50PM +0000, Vladimir Sementsov-Ogievskiy wrote:
> >> 01.11.2019 19:54, Andrey Shinkevich wrote:
> >>> +def check_proc_NBD(proc, connector):
> >>> + try:
> >>> + exitcode = proc.wait(timeout=10)
> >>> +
> >>> + if exitcode < 0:
> >>> + log('NBD {}: EXIT SIGNAL {}\n'.format(connector, -exitcode))
> >>> + log(proc.communicate()[0])
> >>> + else:
> >>> + line = proc.stdout.readline()
> >>
> >>
> >> could we use proc.communicate() for both cases, what is the difference?
> >
> > In fact if proc produces any non-trivial amount of output you are better
> > off using .communicate() otherwise your child may block on output and
> > never exit. See
> > https://docs.python.org/3/library/subprocess.html#subprocess.Popen.communicate
> > for how to express the above logic correctly. The exit code *after*
> > .communicate is available in .returncode.
> >
>
> The pattern by the link above does not work (Python3):
>
> proc = subprocess.Popen(...)
> try:
> outs, errs = proc.communicate(timeout=15)
> except TimeoutExpired:
> proc.kill()
> outs, errs = proc.communicate()
>
> as 'proc' cannot be used for output after being killed. It results in
> another exception being raised.
Of course it can't. You need to use the strings returned by
.communicate().
Roman.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-11 9:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-01 16:54 [PATCH v3] iotests: Test NBD client reconnection Andrey Shinkevich
2019-11-08 13:49 ` Vladimir Sementsov-Ogievskiy
2019-11-08 14:05 ` Roman Kagan
2019-11-11 9:18 ` Andrey Shinkevich
2019-11-11 9:48 ` Roman Kagan
2019-11-11 9:07 ` Andrey Shinkevich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).