diff mbox series

[29/29] iotests: Test block-export-* QMP interface

Message ID 20200907182011.521007-30-kwolf@redhat.com
State New
Headers show
Series block/export: Add infrastructure and QAPI for block exports | expand

Commit Message

Kevin Wolf Sept. 7, 2020, 6:20 p.m. UTC
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/iotests.py |  11 +++-
 tests/qemu-iotests/307        | 117 ++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/307.out    | 111 ++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group      |   1 +
 4 files changed, 239 insertions(+), 1 deletion(-)
 create mode 100755 tests/qemu-iotests/307
 create mode 100644 tests/qemu-iotests/307.out

Comments

Max Reitz Sept. 10, 2020, 4:11 p.m. UTC | #1
On 07.09.20 20:20, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  tests/qemu-iotests/iotests.py |  11 +++-
>  tests/qemu-iotests/307        | 117 ++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/307.out    | 111 ++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/group      |   1 +
>  4 files changed, 239 insertions(+), 1 deletion(-)
>  create mode 100755 tests/qemu-iotests/307
>  create mode 100644 tests/qemu-iotests/307.out
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 6fed77487e..a842a9f92d 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -67,7 +67,8 @@ 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')]
> +qemu_nbd_prog = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]

It seems counterintuitive to me that something called “*_prog” would be
a list.

> +qemu_nbd_args = qemu_nbd_prog
>  if os.environ.get('QEMU_NBD_OPTIONS'):
>      qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
>  
> @@ -283,6 +284,14 @@ def qemu_nbd_early_pipe(*args: str) -> Tuple[int, str]:
>                                                     *full_args)
>      return returncode, output if returncode else ''
>  
> +def qemu_nbd_list(*args: str) -> str:
> +    '''Run qemu-nbd to list remote exports'''
> +    full_args = qemu_nbd_prog + ['-L'] + list(args)
> +    output, _ = qemu_tool_pipe_and_status('qemu-nbd', *full_args)
> +    log(output, filters=[filter_testfiles])

Not sure whether I like functions “silently” auto-logging the result.
I’d be happier if it were called *_log (i.e. qemu_nbd_list_log) like
most other functions that do.

> +    return output
> +
> +
>  @contextmanager
>  def qemu_nbd_popen(*args):
>      '''Context manager running qemu-nbd within the context'''

This would fit nicely into a dedicated patch.

*shrug*

> diff --git a/tests/qemu-iotests/307 b/tests/qemu-iotests/307
> new file mode 100755
> index 0000000000..06b9eeb15f
> --- /dev/null
> +++ b/tests/qemu-iotests/307
> @@ -0,0 +1,117 @@
> +#!/usr/bin/env python3
> +#
> +# Copyright (C) 2020 Red Hat, Inc.
> +#
> +# 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/>.
> +#
> +# Creator/Owner: Kevin Wolf <kwolf@redhat.com>
> +#
> +# Test the block export QAPI interfaces
> +
> +import iotests
> +import os
> +
> +# Formats that have a request alignment != 1 result in different output for
> +# qemu-nbd -L ("min block")

So?  Why not filter that out?

I wonder anyway why the whole export output is logged.  It seems to me
like it would be sufficient to just log the number of exports available
and their names.

(Sure, logging everything gives us the advantage of perhaps finding NBD
bugs with this test even though it isn’t really an NBD test but intended
largely as a test for the new interface.  But when I see something like
“opt block” or “max block” in the output, I get a tingling sensation of
this potentially causing problems in the future depending on where this
test is running.)

((Which is funny, because now I notice I didn’t have this fear for “min
block”, but that’s where you actually did find an issue.))


Though all in all I don’t even know why it is that you write data at all
and use images, instead of just backing the export by a null-co node.

(I do admit I’m sending mixed signals here – first I ask for whether we
couldn’t make this test run on more formats, then I propose abandoning
physical images altogether.  I’m just emitting the questions that pop
into my head, and my head can be a confusing place from time to time.)

> +iotests.script_initialize(
> +    supported_fmts=['raw', 'qcow2', 'vmdk'],
> +    supported_platforms=['linux'],
> +)
> +
> +with iotests.FilePath('image') as img, \
> +     iotests.FilePath('socket') as socket, \

The second one should use base_dir=iotests.sock_dir.

> +     iotests.VM() as vm:
> +
> +    iotests.qemu_img('create', '-f', iotests.imgfmt, img, '64M')
> +    iotests.qemu_io_log('-f', iotests.imgfmt, '-c', 'write -P 0x11 0 4k', img)
> +
> +    iotests.log('=== Launch VM ===')
> +
> +    virtio_scsi_device = iotests.get_virtio_scsi_device()
> +
> +    vm.add_object('iothread,id=iothread0')
> +    vm.add_blockdev('file,filename=%s,node-name=file' % (img))
> +    vm.add_blockdev('%s,file=file,node-name=fmt' % (iotests.imgfmt))
> +    vm.add_blockdev('raw,file=file,node-name=ro,read-only=on')
> +    vm.add_device('id=scsi0,driver=%s,iothread=iothread0' % virtio_scsi_device)

Format strings would look nicer to me.  Perhaps not to you, though.

> +    vm.launch()
> +
> +    vm.qmp_log('nbd-server-start',
> +               addr={'type': 'unix', 'data': {'path': socket}},
> +               filters=(iotests.filter_qmp_testfiles, ))
> +    vm.qmp_log('query-block-exports')
> +
> +    iotests.log('\n=== Create a read-only NBD export ===')
> +
> +    vm.qmp_log('block-export-add', id='export0', type='nbd', node_name='fmt')
> +    vm.qmp_log('query-block-exports')
> +
> +    iotests.qemu_nbd_list('-k', socket)
> +
> +    iotests.log('\n=== Try a few invalid things ===')
> +
> +    vm.qmp_log('block-export-add', id='#invalid_id', type='nbd', node_name='fmt')
> +    vm.qmp_log('block-export-add', id='export0', type='nbd', node_name='fmt')
> +    vm.qmp_log('block-export-add', id='export1', type='nbd', node_name='ro',
> +               writable=True)
> +    vm.qmp_log('block-export-del', id='export1')
> +    vm.qmp_log('query-block-exports')
> +
> +    iotests.log('\n=== Move export to an iothread ===')
> +
> +    vm.qmp_log('device_add', id='sda', driver='scsi-hd', drive='fmt')
> +    vm.qmp_log('query-block-exports')
> +    iotests.qemu_nbd_list('-k', socket)
> +
> +    iotests.log('\n=== Add a writable export ===')
> +
> +    # This fails because share-rw=off
> +    vm.qmp_log('block-export-add', id='export1', type='nbd', node_name='fmt',
> +               name='export1', writable=True, writethrough=True,
> +               description='This is the writable second export')
> +
> +    vm.qmp_log('device_del', id='sda')
> +    event = vm.event_wait(name="DEVICE_DELETED",
> +                          match={'data': {'device': 'sda'}})
> +    iotests.log(event, filters=[iotests.filter_qmp_event])
> +    vm.qmp_log('device_add', id='sda', driver='scsi-hd', drive='fmt',
> +               share_rw=True)
> +
> +    # Now it should work
> +    vm.qmp_log('block-export-add', id='export1', type='nbd', node_name='fmt',
> +               name='export1', writable=True, writethrough=True,
> +               description='This is the writable second export')
> +
> +    vm.qmp_log('query-block-exports')
> +    iotests.qemu_nbd_list('-k', socket)
> +
> +    iotests.log('\n=== Connect qemu-io to export1, try removing exports ===')
> +
> +    nbd_url = 'nbd+unix:///export1?socket=%s' % socket
> +    qemu_io = iotests.QemuIoInteractive('-f', 'raw', nbd_url)
> +
> +    iotests.log(qemu_io.cmd('read -P 0x11 0 4k'),
> +                filters=[iotests.filter_qemu_io])
> +    iotests.log(qemu_io.cmd('write -P 0x22 4k 4k'),
> +                filters=[iotests.filter_qemu_io])
> +
> +    vm.qmp_log('block-export-del', id='export1')
> +    vm.qmp_log('block-export-del', id='export0')

Should we check for the BLOCK_EXPORT_DELETED event?

> +    qemu_io.close()
> +
> +    vm.qmp_log('query-block-exports')
> +    iotests.qemu_nbd_list('-k', socket)

It might be nice to reconnect with qemu-io, and then see whether a hard
del works.

Max

> +
> +    iotests.log('\n=== Shut down QEMU ===')
> +    vm.shutdown()
diff mbox series

Patch

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 6fed77487e..a842a9f92d 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -67,7 +67,8 @@  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')]
+qemu_nbd_prog = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]
+qemu_nbd_args = qemu_nbd_prog
 if os.environ.get('QEMU_NBD_OPTIONS'):
     qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
 
@@ -283,6 +284,14 @@  def qemu_nbd_early_pipe(*args: str) -> Tuple[int, str]:
                                                    *full_args)
     return returncode, output if returncode else ''
 
+def qemu_nbd_list(*args: str) -> str:
+    '''Run qemu-nbd to list remote exports'''
+    full_args = qemu_nbd_prog + ['-L'] + list(args)
+    output, _ = qemu_tool_pipe_and_status('qemu-nbd', *full_args)
+    log(output, filters=[filter_testfiles])
+    return output
+
+
 @contextmanager
 def qemu_nbd_popen(*args):
     '''Context manager running qemu-nbd within the context'''
diff --git a/tests/qemu-iotests/307 b/tests/qemu-iotests/307
new file mode 100755
index 0000000000..06b9eeb15f
--- /dev/null
+++ b/tests/qemu-iotests/307
@@ -0,0 +1,117 @@ 
+#!/usr/bin/env python3
+#
+# Copyright (C) 2020 Red Hat, Inc.
+#
+# 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/>.
+#
+# Creator/Owner: Kevin Wolf <kwolf@redhat.com>
+#
+# Test the block export QAPI interfaces
+
+import iotests
+import os
+
+# Formats that have a request alignment != 1 result in different output for
+# qemu-nbd -L ("min block")
+iotests.script_initialize(
+    supported_fmts=['raw', 'qcow2', 'vmdk'],
+    supported_platforms=['linux'],
+)
+
+with iotests.FilePath('image') as img, \
+     iotests.FilePath('socket') as socket, \
+     iotests.VM() as vm:
+
+    iotests.qemu_img('create', '-f', iotests.imgfmt, img, '64M')
+    iotests.qemu_io_log('-f', iotests.imgfmt, '-c', 'write -P 0x11 0 4k', img)
+
+    iotests.log('=== Launch VM ===')
+
+    virtio_scsi_device = iotests.get_virtio_scsi_device()
+
+    vm.add_object('iothread,id=iothread0')
+    vm.add_blockdev('file,filename=%s,node-name=file' % (img))
+    vm.add_blockdev('%s,file=file,node-name=fmt' % (iotests.imgfmt))
+    vm.add_blockdev('raw,file=file,node-name=ro,read-only=on')
+    vm.add_device('id=scsi0,driver=%s,iothread=iothread0' % virtio_scsi_device)
+    vm.launch()
+
+    vm.qmp_log('nbd-server-start',
+               addr={'type': 'unix', 'data': {'path': socket}},
+               filters=(iotests.filter_qmp_testfiles, ))
+    vm.qmp_log('query-block-exports')
+
+    iotests.log('\n=== Create a read-only NBD export ===')
+
+    vm.qmp_log('block-export-add', id='export0', type='nbd', node_name='fmt')
+    vm.qmp_log('query-block-exports')
+
+    iotests.qemu_nbd_list('-k', socket)
+
+    iotests.log('\n=== Try a few invalid things ===')
+
+    vm.qmp_log('block-export-add', id='#invalid_id', type='nbd', node_name='fmt')
+    vm.qmp_log('block-export-add', id='export0', type='nbd', node_name='fmt')
+    vm.qmp_log('block-export-add', id='export1', type='nbd', node_name='ro',
+               writable=True)
+    vm.qmp_log('block-export-del', id='export1')
+    vm.qmp_log('query-block-exports')
+
+    iotests.log('\n=== Move export to an iothread ===')
+
+    vm.qmp_log('device_add', id='sda', driver='scsi-hd', drive='fmt')
+    vm.qmp_log('query-block-exports')
+    iotests.qemu_nbd_list('-k', socket)
+
+    iotests.log('\n=== Add a writable export ===')
+
+    # This fails because share-rw=off
+    vm.qmp_log('block-export-add', id='export1', type='nbd', node_name='fmt',
+               name='export1', writable=True, writethrough=True,
+               description='This is the writable second export')
+
+    vm.qmp_log('device_del', id='sda')
+    event = vm.event_wait(name="DEVICE_DELETED",
+                          match={'data': {'device': 'sda'}})
+    iotests.log(event, filters=[iotests.filter_qmp_event])
+    vm.qmp_log('device_add', id='sda', driver='scsi-hd', drive='fmt',
+               share_rw=True)
+
+    # Now it should work
+    vm.qmp_log('block-export-add', id='export1', type='nbd', node_name='fmt',
+               name='export1', writable=True, writethrough=True,
+               description='This is the writable second export')
+
+    vm.qmp_log('query-block-exports')
+    iotests.qemu_nbd_list('-k', socket)
+
+    iotests.log('\n=== Connect qemu-io to export1, try removing exports ===')
+
+    nbd_url = 'nbd+unix:///export1?socket=%s' % socket
+    qemu_io = iotests.QemuIoInteractive('-f', 'raw', nbd_url)
+
+    iotests.log(qemu_io.cmd('read -P 0x11 0 4k'),
+                filters=[iotests.filter_qemu_io])
+    iotests.log(qemu_io.cmd('write -P 0x22 4k 4k'),
+                filters=[iotests.filter_qemu_io])
+
+    vm.qmp_log('block-export-del', id='export1')
+    vm.qmp_log('block-export-del', id='export0')
+    qemu_io.close()
+
+    vm.qmp_log('query-block-exports')
+    iotests.qemu_nbd_list('-k', socket)
+
+    iotests.log('\n=== Shut down QEMU ===')
+    vm.shutdown()
diff --git a/tests/qemu-iotests/307.out b/tests/qemu-iotests/307.out
new file mode 100644
index 0000000000..d8e572cac0
--- /dev/null
+++ b/tests/qemu-iotests/307.out
@@ -0,0 +1,111 @@ 
+wrote 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Launch VM ===
+{"execute": "nbd-server-start", "arguments": {"addr": {"data": {"path": "TEST_DIR/PID-socket"}, "type": "unix"}}}
+{"return": {}}
+{"execute": "query-block-exports", "arguments": {}}
+{"return": []}
+
+=== Create a read-only NBD export ===
+{"execute": "block-export-add", "arguments": {"id": "export0", "node-name": "fmt", "type": "nbd"}}
+{"return": {}}
+{"execute": "query-block-exports", "arguments": {}}
+{"return": [{"id": "export0", "node-name": "fmt", "shutting-down": false, "type": "nbd"}]}
+exports available: 1
+ export: 'fmt'
+  size:  67108864
+  flags: 0x58f ( readonly flush fua df multi cache )
+  min block: 1
+  opt block: 4096
+  max block: 33554432
+  available meta contexts: 1
+   base:allocation
+
+
+=== Try a few invalid things ===
+{"execute": "block-export-add", "arguments": {"id": "#invalid_id", "node-name": "fmt", "type": "nbd"}}
+{"error": {"class": "GenericError", "desc": "Invalid block export id"}}
+{"execute": "block-export-add", "arguments": {"id": "export0", "node-name": "fmt", "type": "nbd"}}
+{"error": {"class": "GenericError", "desc": "Block export id 'export0' is already in use"}}
+{"execute": "block-export-add", "arguments": {"id": "export1", "node-name": "ro", "type": "nbd", "writable": true}}
+{"error": {"class": "GenericError", "desc": "Cannot export read-only node as writable"}}
+{"execute": "block-export-del", "arguments": {"id": "export1"}}
+{"error": {"class": "GenericError", "desc": "Export 'export1' is not found"}}
+{"execute": "query-block-exports", "arguments": {}}
+{"return": [{"id": "export0", "node-name": "fmt", "shutting-down": false, "type": "nbd"}]}
+
+=== Move export to an iothread ===
+{"execute": "device_add", "arguments": {"drive": "fmt", "driver": "scsi-hd", "id": "sda"}}
+{"return": {}}
+{"execute": "query-block-exports", "arguments": {}}
+{"return": [{"id": "export0", "node-name": "fmt", "shutting-down": false, "type": "nbd"}]}
+exports available: 1
+ export: 'fmt'
+  size:  67108864
+  flags: 0x58f ( readonly flush fua df multi cache )
+  min block: 1
+  opt block: 4096
+  max block: 33554432
+  available meta contexts: 1
+   base:allocation
+
+
+=== Add a writable export ===
+{"execute": "block-export-add", "arguments": {"description": "This is the writable second export", "id": "export1", "name": "export1", "node-name": "fmt", "type": "nbd", "writable": true, "writethrough": true}}
+{"error": {"class": "GenericError", "desc": "Conflicts with use by sda as 'root', which does not allow 'write' on fmt"}}
+{"execute": "device_del", "arguments": {"id": "sda"}}
+{"return": {}}
+{"data": {"device": "sda", "path": "/machine/peripheral/sda"}, "event": "DEVICE_DELETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
+{"execute": "device_add", "arguments": {"drive": "fmt", "driver": "scsi-hd", "id": "sda", "share-rw": true}}
+{"return": {}}
+{"execute": "block-export-add", "arguments": {"description": "This is the writable second export", "id": "export1", "name": "export1", "node-name": "fmt", "type": "nbd", "writable": true, "writethrough": true}}
+{"return": {}}
+{"execute": "query-block-exports", "arguments": {}}
+{"return": [{"id": "export1", "node-name": "fmt", "shutting-down": false, "type": "nbd"}, {"id": "export0", "node-name": "fmt", "shutting-down": false, "type": "nbd"}]}
+exports available: 2
+ export: 'fmt'
+  size:  67108864
+  flags: 0x58f ( readonly flush fua df multi cache )
+  min block: 1
+  opt block: 4096
+  max block: 33554432
+  available meta contexts: 1
+   base:allocation
+ export: 'export1'
+  description: This is the writable second export
+  size:  67108864
+  flags: 0xced ( flush fua trim zeroes df cache fast-zero )
+  min block: 1
+  opt block: 4096
+  max block: 33554432
+  available meta contexts: 1
+   base:allocation
+
+
+=== Connect qemu-io to export1, try removing exports ===
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+wrote 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+{"execute": "block-export-del", "arguments": {"id": "export1"}}
+{"error": {"class": "GenericError", "desc": "export 'export1' still in use"}}
+{"execute": "block-export-del", "arguments": {"id": "export0"}}
+{"return": {}}
+{"execute": "query-block-exports", "arguments": {}}
+{"return": [{"id": "export1", "node-name": "fmt", "shutting-down": false, "type": "nbd"}]}
+exports available: 1
+ export: 'export1'
+  description: This is the writable second export
+  size:  67108864
+  flags: 0xced ( flush fua trim zeroes df cache fast-zero )
+  min block: 1
+  opt block: 4096
+  max block: 33554432
+  available meta contexts: 1
+   base:allocation
+
+
+=== Shut down QEMU ===
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 5cad015231..f0e9030aba 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -313,3 +313,4 @@ 
 302 quick
 303 rw quick
 304 rw quick
+307 rw quick export