diff mbox series

[v10,3/3] iotests: test nbd reconnect

Message ID 20191009084158.15614-4-vsementsov@virtuozzo.com (mailing list archive)
State New, archived
Headers show
Series NBD reconnect | expand

Commit Message

Vladimir Sementsov-Ogievskiy Oct. 9, 2019, 8:41 a.m. UTC
Add test, which starts backup to nbd target and restarts nbd server
during backup.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/264        | 95 +++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/264.out    | 13 +++++
 tests/qemu-iotests/group      |  1 +
 tests/qemu-iotests/iotests.py | 11 ++++
 4 files changed, 120 insertions(+)
 create mode 100755 tests/qemu-iotests/264
 create mode 100644 tests/qemu-iotests/264.out

Comments

Eric Blake Oct. 23, 2019, 1:31 a.m. UTC | #1
On 10/9/19 3:41 AM, Vladimir Sementsov-Ogievskiy wrote:
> Add test, which starts backup to nbd target and restarts nbd server
> during backup.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   tests/qemu-iotests/264        | 95 +++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/264.out    | 13 +++++
>   tests/qemu-iotests/group      |  1 +
>   tests/qemu-iotests/iotests.py | 11 ++++
>   4 files changed, 120 insertions(+)
>   create mode 100755 tests/qemu-iotests/264
>   create mode 100644 tests/qemu-iotests/264.out
> 
> diff --git a/tests/qemu-iotests/264 b/tests/qemu-iotests/264
> new file mode 100755
> index 0000000000..c8cd97ae2b
> --- /dev/null
> +++ b/tests/qemu-iotests/264

> +import iotests
> +from iotests import qemu_img_create, qemu_io_silent_check, file_path, \
> +        qemu_nbd_popen, log
> +
> +disk_a, disk_b, nbd_sock = file_path('disk_a', 'disk_b', 'nbd-sock')
> +nbd_uri = 'nbd+unix:///?socket=' + nbd_sock

Needs rebasing on top of Max's patches to stick sockets in SOCK_DIR:

https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg04201.html

[or, if my pull request lands first, Max needs to add this one to the 
list of affected tests...]


> +vm.qmp_log('blockdev-add', filters=[iotests.filter_qmp_testfiles],
> +           **{'node_name': 'backup0',
> +              'driver': 'raw',
> +              'file': {'driver': 'nbd',
> +                       'server': {'type': 'unix', 'path': nbd_sock},
> +                       'reconnect-delay': 10}})
> +vm.qmp_log('blockdev-backup', device='drive0', sync='full', target='backup0',
> +           speed=(1 * 1024 * 1024))

This starts the job throttled, to give us time...

> +
> +# Wait for some progress
> +t = 0
> +while t < wait_limit:
> +    jobs = vm.qmp('query-block-jobs')['return']
> +    if jobs and jobs[0]['offset'] > 0:
> +        break
> +    time.sleep(wait_step)
> +    t += wait_step
> +
> +if jobs and jobs[0]['offset'] > 0:
> +    log('Backup job is started')
> +
> +log('Kill NBD server')
> +srv.kill()
> +srv.wait()
> +
> +jobs = vm.qmp('query-block-jobs')['return']
> +if jobs and jobs[0]['offset'] < jobs[0]['len']:
> +    log('Backup job is still in progress')
> +
> +vm.qmp_log('block-job-set-speed', device='drive0', speed=0)
> +
> +# Emulate server down time for 1 second
> +time.sleep(1)

...but once we restart,...

> +
> +log('Start NBD server')
> +srv = qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk_b)
> +
> +e = vm.event_wait('BLOCK_JOB_COMPLETED')

...should we unthrottle the job to allow the test to complete slightly 
faster after the reconnect?  But that can be done as an improvement on 
top, if it helps.


> +++ b/tests/qemu-iotests/iotests.py
> @@ -165,6 +165,13 @@ def qemu_io_silent(*args):
>                            (-exitcode, ' '.join(args)))
>       return exitcode
>   
> +def qemu_io_silent_check(*args):
> +    '''Run qemu-io and return the true if subprocess returned 0'''
> +    args = qemu_io_args + list(args)
> +    exitcode = subprocess.call(args, stdout=open('/dev/null', 'w'),
> +                               stderr=subprocess.STDOUT)

This discards the stdout data, even on failure. Is there a smarter way 
to grab the output into a variable, but only dump it to the log on 
failure, rather than outright discarding it?

But for the sake of getting this test in before freeze, that can be a 
followup, if at all.

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

I'll send a pull request soon.
Vladimir Sementsov-Ogievskiy Oct. 23, 2019, 8:33 a.m. UTC | #2
23.10.2019 4:31, Eric Blake wrote:
> On 10/9/19 3:41 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Add test, which starts backup to nbd target and restarts nbd server
>> during backup.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/264        | 95 +++++++++++++++++++++++++++++++++++
>>   tests/qemu-iotests/264.out    | 13 +++++
>>   tests/qemu-iotests/group      |  1 +
>>   tests/qemu-iotests/iotests.py | 11 ++++
>>   4 files changed, 120 insertions(+)
>>   create mode 100755 tests/qemu-iotests/264
>>   create mode 100644 tests/qemu-iotests/264.out
>>
>> diff --git a/tests/qemu-iotests/264 b/tests/qemu-iotests/264
>> new file mode 100755
>> index 0000000000..c8cd97ae2b
>> --- /dev/null
>> +++ b/tests/qemu-iotests/264
> 
>> +import iotests
>> +from iotests import qemu_img_create, qemu_io_silent_check, file_path, \
>> +        qemu_nbd_popen, log
>> +
>> +disk_a, disk_b, nbd_sock = file_path('disk_a', 'disk_b', 'nbd-sock')
>> +nbd_uri = 'nbd+unix:///?socket=' + nbd_sock
> 
> Needs rebasing on top of Max's patches to stick sockets in SOCK_DIR:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg04201.html
> 
> [or, if my pull request lands first, Max needs to add this one to the list of affected tests...]
> 
> 
>> +vm.qmp_log('blockdev-add', filters=[iotests.filter_qmp_testfiles],
>> +           **{'node_name': 'backup0',
>> +              'driver': 'raw',
>> +              'file': {'driver': 'nbd',
>> +                       'server': {'type': 'unix', 'path': nbd_sock},
>> +                       'reconnect-delay': 10}})
>> +vm.qmp_log('blockdev-backup', device='drive0', sync='full', target='backup0',
>> +           speed=(1 * 1024 * 1024))
> 
> This starts the job throttled, to give us time...
> 
>> +
>> +# Wait for some progress
>> +t = 0
>> +while t < wait_limit:
>> +    jobs = vm.qmp('query-block-jobs')['return']
>> +    if jobs and jobs[0]['offset'] > 0:
>> +        break
>> +    time.sleep(wait_step)
>> +    t += wait_step
>> +
>> +if jobs and jobs[0]['offset'] > 0:
>> +    log('Backup job is started')
>> +
>> +log('Kill NBD server')
>> +srv.kill()
>> +srv.wait()
>> +
>> +jobs = vm.qmp('query-block-jobs')['return']
>> +if jobs and jobs[0]['offset'] < jobs[0]['len']:
>> +    log('Backup job is still in progress')
>> +
>> +vm.qmp_log('block-job-set-speed', device='drive0', speed=0)
>> +
>> +# Emulate server down time for 1 second
>> +time.sleep(1)
> 
> ...but once we restart,...
> 
>> +
>> +log('Start NBD server')
>> +srv = qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk_b)
>> +
>> +e = vm.event_wait('BLOCK_JOB_COMPLETED')
> 
> ...should we unthrottle the job to allow the test to complete slightly faster after the reconnect?  But that can be done as an improvement on top, if it helps.

It is done above, before time.sleep(1)

> 
> 
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -165,6 +165,13 @@ def qemu_io_silent(*args):
>>                            (-exitcode, ' '.join(args)))
>>       return exitcode
>> +def qemu_io_silent_check(*args):
>> +    '''Run qemu-io and return the true if subprocess returned 0'''
>> +    args = qemu_io_args + list(args)
>> +    exitcode = subprocess.call(args, stdout=open('/dev/null', 'w'),
>> +                               stderr=subprocess.STDOUT)
> 
> This discards the stdout data, even on failure. Is there a smarter way to grab the output into a variable, but only dump it to the log on failure, rather than outright discarding it?
> 
> But for the sake of getting this test in before freeze, that can be a followup, if at all.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> I'll send a pull request soon.
> 

Thank you!!
Eric Blake Oct. 23, 2019, 11:33 a.m. UTC | #3
On 10/23/19 3:33 AM, Vladimir Sementsov-Ogievskiy wrote:
> 23.10.2019 4:31, Eric Blake wrote:
>> On 10/9/19 3:41 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> Add test, which starts backup to nbd target and restarts nbd server
>>> during backup.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---

>>> +vm.qmp_log('blockdev-backup', device='drive0', sync='full', target='backup0',
>>> +           speed=(1 * 1024 * 1024))
>>
>> This starts the job throttled, to give us time...
>>
>>> +
>>> +# Wait for some progress
>>> +t = 0
>>> +while t < wait_limit:
>>> +    jobs = vm.qmp('query-block-jobs')['return']
>>> +    if jobs and jobs[0]['offset'] > 0:
>>> +        break
>>> +    time.sleep(wait_step)
>>> +    t += wait_step
>>> +
>>> +if jobs and jobs[0]['offset'] > 0:
>>> +    log('Backup job is started')
>>> +
>>> +log('Kill NBD server')
>>> +srv.kill()
>>> +srv.wait()
>>> +
>>> +jobs = vm.qmp('query-block-jobs')['return']
>>> +if jobs and jobs[0]['offset'] < jobs[0]['len']:
>>> +    log('Backup job is still in progress')
>>> +
>>> +vm.qmp_log('block-job-set-speed', device='drive0', speed=0)

Ah, I overlooked this line in my late-night review.

>>> +
>>> +# Emulate server down time for 1 second
>>> +time.sleep(1)
>>
>> ...but once we restart,...
>>
>>> +
>>> +log('Start NBD server')
>>> +srv = qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk_b)
>>> +
>>> +e = vm.event_wait('BLOCK_JOB_COMPLETED')
>>
>> ...should we unthrottle the job to allow the test to complete slightly faster after the reconnect?  But that can be done as an improvement on top, if it helps.
> 
> It is done above, before time.sleep(1)

Yep, so I feel better now.
diff mbox series

Patch

diff --git a/tests/qemu-iotests/264 b/tests/qemu-iotests/264
new file mode 100755
index 0000000000..c8cd97ae2b
--- /dev/null
+++ b/tests/qemu-iotests/264
@@ -0,0 +1,95 @@ 
+#!/usr/bin/env python
+#
+# Test nbd 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 time
+
+import iotests
+from iotests import qemu_img_create, qemu_io_silent_check, file_path, \
+        qemu_nbd_popen, log
+
+disk_a, disk_b, nbd_sock = file_path('disk_a', 'disk_b', 'nbd-sock')
+nbd_uri = 'nbd+unix:///?socket=' + nbd_sock
+size = 5 * 1024 * 1024
+wait_limit = 3
+wait_step = 0.2
+
+qemu_img_create('-f', iotests.imgfmt, disk_a, str(size))
+qemu_img_create('-f', iotests.imgfmt, disk_b, str(size))
+srv = qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk_b)
+
+# Wait for NBD server availability
+t = 0
+ok = False
+while t < wait_limit:
+    ok = qemu_io_silent_check('-f', 'raw', '-c', 'read 0 512', nbd_uri)
+    if ok:
+        break
+    time.sleep(wait_step)
+    t += wait_step
+
+assert ok
+
+vm = iotests.VM().add_drive(disk_a)
+vm.launch()
+vm.hmp_qemu_io('drive0', 'write 0 {}'.format(size))
+
+vm.qmp_log('blockdev-add', filters=[iotests.filter_qmp_testfiles],
+           **{'node_name': 'backup0',
+              'driver': 'raw',
+              'file': {'driver': 'nbd',
+                       'server': {'type': 'unix', 'path': nbd_sock},
+                       'reconnect-delay': 10}})
+vm.qmp_log('blockdev-backup', device='drive0', sync='full', target='backup0',
+           speed=(1 * 1024 * 1024))
+
+# Wait for some progress
+t = 0
+while t < wait_limit:
+    jobs = vm.qmp('query-block-jobs')['return']
+    if jobs and jobs[0]['offset'] > 0:
+        break
+    time.sleep(wait_step)
+    t += wait_step
+
+if jobs and jobs[0]['offset'] > 0:
+    log('Backup job is started')
+
+log('Kill NBD server')
+srv.kill()
+srv.wait()
+
+jobs = vm.qmp('query-block-jobs')['return']
+if jobs and jobs[0]['offset'] < jobs[0]['len']:
+    log('Backup job is still in progress')
+
+vm.qmp_log('block-job-set-speed', device='drive0', speed=0)
+
+# Emulate server down time for 1 second
+time.sleep(1)
+
+log('Start NBD server')
+srv = qemu_nbd_popen('-k', nbd_sock, '-f', iotests.imgfmt, disk_b)
+
+e = vm.event_wait('BLOCK_JOB_COMPLETED')
+log('Backup completed: {}'.format(e['data']['offset']))
+
+vm.qmp_log('blockdev-del', node_name='backup0')
+srv.kill()
+vm.shutdown()
diff --git a/tests/qemu-iotests/264.out b/tests/qemu-iotests/264.out
new file mode 100644
index 0000000000..3000944b09
--- /dev/null
+++ b/tests/qemu-iotests/264.out
@@ -0,0 +1,13 @@ 
+{"execute": "blockdev-add", "arguments": {"driver": "raw", "file": {"driver": "nbd", "reconnect-delay": 10, "server": {"path": "TEST_DIR/PID-nbd-sock", "type": "unix"}}, "node-name": "backup0"}}
+{"return": {}}
+{"execute": "blockdev-backup", "arguments": {"device": "drive0", "speed": 1048576, "sync": "full", "target": "backup0"}}
+{"return": {}}
+Backup job is started
+Kill NBD server
+Backup job is still in progress
+{"execute": "block-job-set-speed", "arguments": {"device": "drive0", "speed": 0}}
+{"return": {}}
+Start NBD server
+Backup completed: 5242880
+{"execute": "blockdev-del", "arguments": {"node-name": "backup0"}}
+{"return": {}}
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 5805a79d9e..71ff0fc015 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -275,6 +275,7 @@ 
 258 rw quick
 262 rw quick migration
 263 rw quick
+264 rw
 265 rw auto quick
 266 rw quick
 267 rw auto quick snapshot
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 9fb5181c3d..60d1e86b6d 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -165,6 +165,13 @@  def qemu_io_silent(*args):
                          (-exitcode, ' '.join(args)))
     return exitcode
 
+def qemu_io_silent_check(*args):
+    '''Run qemu-io and return the true if subprocess returned 0'''
+    args = qemu_io_args + list(args)
+    exitcode = subprocess.call(args, stdout=open('/dev/null', 'w'),
+                               stderr=subprocess.STDOUT)
+    return exitcode == 0
+
 def get_virtio_scsi_device():
     if qemu_default_machine == 's390-ccw-virtio':
         return 'virtio-scsi-ccw'
@@ -230,6 +237,10 @@  def qemu_nbd_early_pipe(*args):
     else:
         return exitcode, subp.communicate()[0]
 
+def qemu_nbd_popen(*args):
+    '''Run qemu-nbd in daemon mode and return the parent's exit code'''
+    return subprocess.Popen(qemu_nbd_args + ['--persistent'] + list(args))
+
 def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt):
     '''Return True if two image files are identical'''
     return qemu_img('compare', '-f', fmt1,