[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3] iotests: Test NBD client reconnection
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH v3] iotests: Test NBD client reconnection |
Date: |
Fri, 8 Nov 2019 13:49:50 +0000 |
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 <address@hidden>
> Suggested-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> Signed-off-by: Andrey Shinkevich <address@hidden>
> ---
> 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