[PATCH] iotests: Test nbd client reconnect

[PATCH] iotests: Test nbd client reconnect

[Andrey Shinkevich]

The stress test for an NBD client. The NBD server is disconnected after
a client write operation. The NBD client should reconnect and retry the
operation.

Suggested-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/277     | 91 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/277.out |  7 ++++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 99 insertions(+)
 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..46a29b7
--- /dev/null
+++ b/tests/qemu-iotests/277
@@ -0,0 +1,91 @@
+#!/usr/bin/env python
+#
+# Test nbd client reconnect
+#
+# 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 sys
+import io
+import subprocess
+import iotests
+from iotests import file_path, log
+
+
+def start_server_NBD(nbd_sock, conf_file):
+    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: {}'.format(line.rstrip()))
+
+    return srv
+
+
+def check_server_NBD(srv):
+    exitcode = srv.wait(timeout=10)
+
+    if exitcode < 0:
+        sys.stderr.write('NBD server: ERROR %i\n' % (-exitcode))
+        log(srv.communicate()[0])
+    else:
+        line = srv.stdout.readline()
+        log('NBD server: ' + line.rstrip())
+
+    os.remove(nbd_sock)
+    os.remove(conf_file)
+
+
+def make_conf_file(conf_file, event):
+    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))
+
+
+disk, nbd_sock = file_path('disk', 'nbd-sock')
+nbd_uri = 'nbd+unix:///?socket=' + nbd_sock
+if os.path.exists(nbd_sock):
+    os.remove(nbd_sock)
+
+conf_file = os.path.join(iotests.test_dir, "nbd-fault-injector.conf")
+make_conf_file(conf_file, "data")
+srv = start_server_NBD(nbd_sock, conf_file)
+
+log('NBD client: QEMU-IO write')
+args = iotests.qemu_io_args + ['-f', 'raw', '-c', 'write -P 0x7 0 3M', nbd_uri]
+clt = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+
+check_server_NBD(srv)
+
+make_conf_file(conf_file, "reply")
+srv = start_server_NBD(nbd_sock, conf_file)
+
+exitcode = clt.wait(timeout=10)
+if exitcode < 0:
+    sys.stderr.write('qemu-io received signal %i: %s\n' %
+                     (-exitcode, ' '.join(args)))
+
+for line in io.TextIOWrapper(clt.stdout, encoding="utf-8"):
+    if "3 MiB" not in line:
+        log('NBD client: ' + line.rstrip())
+
+check_server_NBD(srv)
diff --git a/tests/qemu-iotests/277.out b/tests/qemu-iotests/277.out
new file mode 100644
index 0000000..07e6e82
--- /dev/null
+++ b/tests/qemu-iotests/277.out
@@ -0,0 +1,7 @@
+NBD server: started
+NBD client: QEMU-IO write
+NBD server: Closing connection on rule match inject-error
+NBD server: started
+NBD client: Connection closed
+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
-- 
1.8.3.1

Re: [PATCH] iotests: Test nbd client reconnect

[Eric Blake]

On 10/27/19 3:48 PM, Andrey Shinkevich wrote:
> The stress test for an NBD client. The NBD server is disconnected after
> a client write operation. The NBD client should reconnect and retry the
> operation.
> 
> Suggested-by: Denis V. Lunev <den@openvz.org>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   tests/qemu-iotests/277     | 91 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/277.out |  7 ++++
>   tests/qemu-iotests/group   |  1 +
>   3 files changed, 99 insertions(+)
>   create mode 100755 tests/qemu-iotests/277
>   create mode 100644 tests/qemu-iotests/277.out

How does this differ from 264?  If it adds anything new, can it be 
merged into the existing test?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH] iotests: Test nbd client reconnect

[Vladimir Sementsov-Ogievskiy]

28.10.2019 10:55, Eric Blake wrote:
> On 10/27/19 3:48 PM, Andrey Shinkevich wrote:
>> The stress test for an NBD client. The NBD server is disconnected after
>> a client write operation. The NBD client should reconnect and retry the
>> operation.
>>
>> Suggested-by: Denis V. Lunev <den@openvz.org>
>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/277     | 91 ++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/277.out |  7 ++++
>>   tests/qemu-iotests/group   |  1 +
>>   3 files changed, 99 insertions(+)
>>   create mode 100755 tests/qemu-iotests/277
>>   create mode 100644 tests/qemu-iotests/277.out
> 
> How does this differ from 264?  If it adds anything new, can it be merged into the existing test?
> 

264 is backup over NBD with reconnect. Here is another thing: check that the only small request works
with reconnect, if disconnect occurs exactly after request was accepted by server and client knows,
that requests is successfully accepted. We want to check that client will not wait reply forever but
resend the request to new started NBD server.

So, I think, they are different enough to keep them in separate. Still, if we want to merge them, it
means that we should rewrite them in unittest style, with test-cases, as I think that huge text-camparing
tests with several test cases are bad thing, I'll write a separate letter about it to discuss a bit later.

-- 
Best regards,
Vladimir

Re: [PATCH] iotests: Test nbd client reconnect

[Andrey Shinkevich]

On 28/10/2019 13:17, Vladimir Sementsov-Ogievskiy wrote:
> 28.10.2019 10:55, Eric Blake wrote:
>> On 10/27/19 3:48 PM, Andrey Shinkevich wrote:
>>> The stress test for an NBD client. The NBD server is disconnected after
>>> a client write operation. The NBD client should reconnect and retry the
>>> operation.
>>>
>>> Suggested-by: Denis V. Lunev <den@openvz.org>
>>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>>> ---
>>>    tests/qemu-iotests/277     | 91 ++++++++++++++++++++++++++++++++++++++++++++++
>>>    tests/qemu-iotests/277.out |  7 ++++
>>>    tests/qemu-iotests/group   |  1 +
>>>    3 files changed, 99 insertions(+)
>>>    create mode 100755 tests/qemu-iotests/277
>>>    create mode 100644 tests/qemu-iotests/277.out
>>
>> How does this differ from 264?  If it adds anything new, can it be merged into the existing test?
>>
> 
> 264 is backup over NBD with reconnect. Here is another thing: check that the only small request works
> with reconnect, if disconnect occurs exactly after request was accepted by server and client knows,
> that requests is successfully accepted. We want to check that client will not wait reply forever but
> resend the request to new started NBD server.
> 
> So, I think, they are different enough to keep them in separate. Still, if we want to merge them, it
> means that we should rewrite them in unittest style, with test-cases, as I think that huge text-camparing
> tests with several test cases are bad thing, I'll write a separate letter about it to discuss a bit later.
> 

Please look at the v2 coming...
-- 
With the best regards,
Andrey Shinkevich

Re: [PATCH] iotests: Test nbd client reconnect

[Andrey Shinkevich]

On 27/10/2019 17:48, Andrey Shinkevich wrote:
> The stress test for an NBD client. The NBD server is disconnected after
> a client write operation. The NBD client should reconnect and retry the
> operation.
> 
> Suggested-by: Denis V. Lunev <den@openvz.org>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   tests/qemu-iotests/277     | 91 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/277.out |  7 ++++
>   tests/qemu-iotests/group   |  1 +
>   3 files changed, 99 insertions(+)
>   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..46a29b7
> --- /dev/null
> +++ b/tests/qemu-iotests/277
> @@ -0,0 +1,91 @@
> +#!/usr/bin/env python
> +#
> +# Test nbd client reconnect
> +#
> +# 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 sys
> +import io
> +import subprocess
> +import iotests
> +from iotests import file_path, log
> +
> +
> +def start_server_NBD(nbd_sock, conf_file):
> +    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: {}'.format(line.rstrip()))
> +
> +    return srv
> +
> +
> +def check_server_NBD(srv):
> +    exitcode = srv.wait(timeout=10)
> +
> +    if exitcode < 0:
> +        sys.stderr.write('NBD server: ERROR %i\n' % (-exitcode))
> +        log(srv.communicate()[0])
> +    else:
> +        line = srv.stdout.readline()
> +        log('NBD server: ' + line.rstrip())
> +
> +    os.remove(nbd_sock)
> +    os.remove(conf_file)
> +
> +
> +def make_conf_file(conf_file, event):
> +    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))
> +
> +
> +disk, nbd_sock = file_path('disk', 'nbd-sock')

The 'disk' is going to be remooved as a construction waste.

> +nbd_uri = 'nbd+unix:///?socket=' + nbd_sock
> +if os.path.exists(nbd_sock):
> +    os.remove(nbd_sock)
> +
> +conf_file = os.path.join(iotests.test_dir, "nbd-fault-injector.conf")
> +make_conf_file(conf_file, "data")
> +srv = start_server_NBD(nbd_sock, conf_file)
> +
> +log('NBD client: QEMU-IO write')
> +args = iotests.qemu_io_args + ['-f', 'raw', '-c', 'write -P 0x7 0 3M', nbd_uri]
> +clt = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
> +
> +check_server_NBD(srv)
> +
> +make_conf_file(conf_file, "reply")
> +srv = start_server_NBD(nbd_sock, conf_file)
> +
> +exitcode = clt.wait(timeout=10)
> +if exitcode < 0:
> +    sys.stderr.write('qemu-io received signal %i: %s\n' %
> +                     (-exitcode, ' '.join(args)))
> +
> +for line in io.TextIOWrapper(clt.stdout, encoding="utf-8"):
> +    if "3 MiB" not in line:
> +        log('NBD client: ' + line.rstrip())
> +
> +check_server_NBD(srv)
> diff --git a/tests/qemu-iotests/277.out b/tests/qemu-iotests/277.out
> new file mode 100644
> index 0000000..07e6e82
> --- /dev/null
> +++ b/tests/qemu-iotests/277.out
> @@ -0,0 +1,7 @@
> +NBD server: started
> +NBD client: QEMU-IO write
> +NBD server: Closing connection on rule match inject-error
> +NBD server: started
> +NBD client: Connection closed
> +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
> 

-- 
With the best regards,
Andrey Shinkevich