Commit Graph

1717 Commits

Author SHA1 Message Date
Max Reitz
d2a839ede8 iotests: Check whether luks works
Whenever running an iotest for the luks format, we should check whether
luks actually really works.

Tests that try to create luks-encrypted qcow2 images should do the same.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200625125548.870061-7-mreitz@redhat.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
2020-07-06 08:49:28 +02:00
Max Reitz
6649f4bd29 iotests.py: Add (verify|has)_working_luks()
Similar to _require_working_luks for bash tests, these functions can be
used to check whether our luks driver can actually create images.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200625125548.870061-6-mreitz@redhat.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
2020-07-06 08:49:28 +02:00
Max Reitz
d849acab41 iotests.py: Add qemu_img_pipe_and_status()
This function will be used by the next patch, which intends to check
both the exit code and qemu-img's output.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200625125548.870061-5-mreitz@redhat.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
[mreitz: Rebased on 49438972b8]
Signed-off-by: Max Reitz <mreitz@redhat.com>
2020-07-06 08:49:02 +02:00
Max Reitz
dc4ab02919 iotests/common.rc: Add _require_working_luks
That the luks driver is present is little indication on whether it is
actually working.  Without the crypto libraries linked in, it does not
work.  So add this function, which tries to create a luks image to see
whether that actually works.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200625125548.870061-4-mreitz@redhat.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
2020-07-06 08:33:06 +02:00
Maxim Levitsky
cbb32e79dd iotests: filter few more luks specific create options
This allows more tests to be able to have same output on both qcow2 luks encrypted images
and raw luks images

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Message-Id: <20200625125548.870061-3-mreitz@redhat.com>
2020-07-06 08:33:06 +02:00
Max Reitz
57ee95ed4e iotests: Make _filter_img_create more active
Right now, _filter_img_create just filters out everything that looks
format-dependent, and applies some filename filters.  That means that we
have to add another filter line every time some format gets a new
creation option.  This can be avoided by instead discarding everything
and just keeping what we know is format-independent (format, size,
backing file, encryption information[1], preallocation) or just
interesting to have in the reference output (external data file path).

Furthermore, we probably want to sort these options.  Format drivers are
not required to define them in any specific order, so the output is
effectively random (although this has never bothered us until now).  We
need a specific order for our reference outputs, though.  Unfortunately,
just using a plain "sort" would change a lot of existing reference
outputs, so we have to pre-filter the option keys to keep our existing
order (fmt, size, backing*, data, encryption info, preallocation).

Finally, this makes it difficult for _filter_img_create to automagically
work for QMP output.  Thus, this patch adds a separate
_filter_img_create_for_qmp function that echos every line verbatim that
does not start with "Formatting", and pipes those "Formatting" lines to
_filter_img_create.

[1] Actually, the only thing that is really important is whether
    encryption is enabled or not.  A patch by Maxim thus removes all
    other "encrypt.*" options from the output:
    https://lists.nongnu.org/archive/html/qemu-block/2020-06/msg00339.html
    But that patch needs to come later so we can get away with changing
    as few reference outputs in this patch here as possible.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200625125548.870061-2-mreitz@redhat.com>
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
2020-07-06 08:33:06 +02:00
Alberto Garcia
a5675f3901 qcow2: Fix preallocation on images with unaligned sizes
When resizing an image with qcow2_co_truncate() using the falloc or
full preallocation modes the code assumes that both the old and new
sizes are cluster-aligned.

There are two problems with this:

  1) The calculation of how many clusters are involved does not always
     get the right result.

     Example: creating a 60KB image and resizing it (with
     preallocation=full) to 80KB won't allocate the second cluster.

  2) No copy-on-write is performed, so in the previous example if
     there is a backing file then the first 60KB of the first cluster
     won't be filled with data from the backing file.

This patch fixes both issues.

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-Id: <20200617140036.20311-1-berto@igalia.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
2020-07-06 08:33:06 +02:00
Peter Maydell
7b75157020 Block layer patches:
- qemu-img convert: Don't pre-zero images (removes nowadays
   counterproductive optimisation)
 - qemu-storage-daemon: Fix object-del, cleaner shutdown
 - vvfat: Check that the guest doesn't escape the given host directory
   with read-write vvfat drives
 - vvfat: Fix crash by out-of-bounds array writes for read-write drives
 - iotests fixes
 -----BEGIN PGP SIGNATURE-----
 
 iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAl7++G4RHGt3b2xmQHJl
 ZGhhdC5jb20ACgkQfwmycsiPL9Yn1A/+JKzLEHRGt8VGnR6zPsnHfQvhnRsqk5kF
 G9z/c68Jk+Q/sg2mdMZNs1Jkb1oJPTlGSMkNmWJTah5zEPsbfoBS5wvVMhK8OcQR
 mGgy8r2Abht6rYbtDqkFARldNtBG1T6V3uHiXw1a1qIqNEpp2ogUO/iw2L+wIxcH
 i3wsX4idjrGZG+/eEfzXIo9wqy6QF6TRwR7bEPyFIo6ywRF5u4/mcDF1ujH8WogS
 vJ1GF1JnSchXgY1rKyyBa45aUPS7s0hE7c0qkMZF4d41qBYE1+P5lMbAoKtB5ZW4
 EEnqZyv+Pt0Kf8iXWZ5eEDBXItN/eFam5AeiibzIBhb8IXJUJ6aU9S0bpYaQmHBJ
 YCcOHdE5FPo+Dj4G9MMhTidJ2KKAHrTZZJwVxhs/SXbVp2+z7odycNZBQRepjlNX
 EJ/HzYoaJiBkXK/g4Zc0mYisKEAidYOriyb6Kf8MDZxk3Up6ZcaOY7DWukK2rwjg
 9/YPJ8EIRqP6RSgVQvHscIhrasngw49ENiDigCLByNQsVJeE5m3kP7GU5yaS+kN2
 tUSdtvUzASvoLHtGY35fjVZL6OIQWEAYAWHYPzhJTX9fwCwXlMmVkYMwiTtxjjDD
 KLwFyqKUop0DUUfXlCrjVk9GK92HHx9IDEOrcTeooQHj2lNXvLdg8BpGDTBZ98b0
 ZRUP2w9IHZA=
 =RCvS
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging

Block layer patches:

- qemu-img convert: Don't pre-zero images (removes nowadays
  counterproductive optimisation)
- qemu-storage-daemon: Fix object-del, cleaner shutdown
- vvfat: Check that the guest doesn't escape the given host directory
  with read-write vvfat drives
- vvfat: Fix crash by out-of-bounds array writes for read-write drives
- iotests fixes

# gpg: Signature made Fri 03 Jul 2020 10:20:46 BST
# gpg:                using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6
# gpg:                issuer "kwolf@redhat.com"
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full]
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74  56FE 7F09 B272 C88F 2FD6

* remotes/kevin/tags/for-upstream:
  iotests: Fix 051 output after qdev_init_nofail() removal
  iotests.py: Do not wait() before communicate()
  vvfat: Fix array_remove_slice()
  vvfat: Check that updated filenames are valid
  qemu-storage-daemon: add missing cleanup calls
  qemu-storage-daemon: remember to add qemu_object_opts
  qemu-img convert: Don't pre-zero images

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-07-03 17:55:31 +01:00
Peter Maydell
4abf70a661 Block patches:
- Two iotest fixes
 -----BEGIN PGP SIGNATURE-----
 
 iQFGBAABCAAwFiEEkb62CjDbPohX0Rgp9AfbAGHVz0AFAl7zCDMSHG1yZWl0ekBy
 ZWRoYXQuY29tAAoJEPQH2wBh1c9Ayo8H/jn4CVqPFvy4n9/hgm8xkiUjV7YTSaqm
 0OzbdcCqrA2D8ZhkEvwVLxZ+F06zE6qevOJGn4Ic926VmMbwS8LTLHH2DJnWWwkV
 wYGSJg+BMsRlYYzjcAYxhh3nSwr61U05ShYP5h33iZUzgOyfrJr1WNndBnxoUztf
 4iQ0BppYsjFSVVEZvYuza5hAzfPTyOuBtVH72UnyCSs9YOZelsH93Kg4UXD/3wKH
 IX+bw7cuEk6dCn2hsUFk3dXa+SlUEcqs2aC2RAE/1T+4cRMkE5EvPV56pKMUOpWQ
 HsxdiUf1skpjDd1ECSKAdU2I4q+bfVcGaPU93mamxjF/bS1JtPW7WW0=
 =73iL
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2020-06-24' into staging

Block patches:
- Two iotest fixes

# gpg: Signature made Wed 24 Jun 2020 09:00:51 BST
# gpg:                using RSA key 91BEB60A30DB3E8857D11829F407DB0061D5CF40
# gpg:                issuer "mreitz@redhat.com"
# gpg: Good signature from "Max Reitz <mreitz@redhat.com>" [full]
# Primary key fingerprint: 91BE B60A 30DB 3E88 57D1  1829 F407 DB00 61D5 CF40

* remotes/maxreitz/tags/pull-block-2020-06-24:
  iotests: don't test qcow2.py inside 291
  iotests: Fix 051 output after qdev_init_nofail() removal

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-07-03 15:34:45 +01:00
Philippe Mathieu-Daudé
4f071a9460 iotests: Fix 051 output after qdev_init_nofail() removal
Commit 96927c744 replaced qdev_init_nofail() call by
isa_realize_and_unref() which has a different error
message. Update the test output accordingly.

Gitlab CI error after merging b77b5b3dc7:
https://gitlab.com/qemu-project/qemu/-/jobs/597414772#L4375

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20200616154949.6586-1-philmd@redhat.com>
Message-Id: <20200624140446.15380-2-alex.bennee@linaro.org>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-07-03 10:06:29 +02:00
Max Reitz
49438972b8 iotests.py: Do not wait() before communicate()
Waiting on a process for which we have a pipe will stall if the process
outputs more data than fits into the OS-provided buffer.  We must use
communicate() before wait(), and in fact, communicate() perfectly
replaces wait() already.

We have to drop the stderr=subprocess.STDOUT parameter from
subprocess.Popen() in qemu_nbd_early_pipe(), because stderr is passed on
to the child process, so if we do not drop this parameter, communicate()
will hang (because the pipe is not closed).

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200630083711.40567-1-mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-07-03 09:54:07 +02:00
Vladimir Sementsov-Ogievskiy
24b861c038 iotests: don't test qcow2.py inside 291
820c6bee53 added testing of qcow2.py into 291, and it breaks 291
with external data file. Actually, 291 is bad place for qcow2.py
testing, better add a separate test.

For now, drop qcow2.py testing from 291 to fix the regression.

Fixes: 820c6bee53
Reported-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200618154052.8629-1-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
2020-06-24 10:00:04 +02:00
Philippe Mathieu-Daudé
e11543c53f iotests: Fix 051 output after qdev_init_nofail() removal
Commit 96927c744 replaced qdev_init_nofail() call by
isa_realize_and_unref() which has a different error
message. Update the test output accordingly.

Gitlab CI error after merging b77b5b3dc7:
https://gitlab.com/qemu-project/qemu/-/jobs/597414772#L4375

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200616154949.6586-1-philmd@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
2020-06-24 10:00:04 +02:00
Markus Armbruster
84b0475ced qdev: Reject drive property override
qdev_prop_set_drive() screws up when the property already has a
non-null value: it neglects to release the old value.  Both the old
and the new backend become attached to the same device.

Example (taken from iotest 172): -fda ... -drive if=none,... -global
floppy.drive=none0.

Special case: attempting to use the same backend both times fails.
Example (also from iotest 172): -fda ... -global floppy.drive=floppy0.

Yet another example: -device with multiple drive=... (but not
device_add, which silently drops all but the last duplicate property).

Perhaps drive property override could be made to work.  Perhaps it
should.  I can't afford the time to figure this out now.  What I can
do is reject usage that leaves backends in unhealthy states.  For what
it's worth, we've long done the same for netdev properties.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200622094227.1271650-12-armbru@redhat.com>
2020-06-23 16:07:07 +02:00
Markus Armbruster
4a27a638e7 fdc: Deprecate configuring floppies with -global isa-fdc
Deprecate

    -global isa-fdc.driveA=...
    -global isa-fdc.driveB=...

in favour of

    -device floppy,unit=0,drive=...
    -device floppy,unit=1,drive=...

Same for the other floppy controller devices.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Acked-by: John Snow <jsnow@redhat.com>
Message-Id: <20200622094227.1271650-7-armbru@redhat.com>
2020-06-23 16:07:07 +02:00
Markus Armbruster
6172e067a4 fdc: Reject clash between -drive if=floppy and -global isa-fdc
The floppy controller devices desugar their drive properties into
floppy devices (since commit a92bd191a4 "fdc: Move qdev properties to
FloppyDrive", v2.8.0).  This involves some bad magic in
fdctrl_connect_drives(), and exists for backward compatibility.

The functions for boards to create floppy controller devices
fdctrl_init_isa(), fdctrl_init_sysbus(), and sun4m_fdctrl_init()
desugar -drive if=floppy to these floppy controller drive properties.

If you use both -drive if=floppy (or its -fda / -fdb sugar) and
-global isa-fdc for the same floppy device, -global silently loses the
conflict, and both backends involved end up with the floppy device
frontend attached, as demonstrated by iotest 172 (see commit before
previous).  This is wrong.

Desugar -drive if=floppy straight to floppy devices instead, with
helper fdctrl_init_drives().  The conflict now gets rejected cleanly:
first, fdctrl_connect_drives() creates the floppy for the controller's
property, then fdctrl_init_drives() attempts to create the floppy for
-drive if=floppy, but fails because the unit is already in use.

Output of iotest 172 changes in three ways:

1. The clash gets rejected.

2. In one test case, "info qtree" has the floppy devices swapped, and
   "info block" has their QOM paths swapped.  This is because the
   floppy device for -fda now gets created after the one for -global
   isa-fdc.driveB.

3. The error message for -global floppy.drive=floppy0 changes.  Before
   the patch, we set isa-fdc.driveA to -fda's block backend, then
   create the floppy device for it, then move the backend from
   isa-fdc.driveA to floppy.drive.  Floppy creation fails when
   applying -global floppy.drive=floppy0, because floppy0 is still
   attached to isa-fdc.  After the patch, we create the floppy for
   -fda, then set its drive property to floppy0.  Now floppy creation
   succeeds, but setting the drive property fails, because -global
   already set it.  Yes, this is exasperatingly complicated.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200622094227.1271650-5-armbru@redhat.com>
2020-06-23 16:07:07 +02:00
Markus Armbruster
02b83f7d7c iotests/172: Cover -global floppy.drive=...
Use of -global to set a default backend for non-singleton devices is a
bad idea.  But as long as we permit it, we better test it.

Test output demonstrates we screw up when -global floppy clashes with
-fda or with -device floppy: according to "info qtree", only the
latter backend is attached, but according to "info block", both are.
Here's the clash with -device:

    Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global floppy.drive=none0 -device floppy,drive=none1,unit=0

              dev: isa-fdc, id ""
                [...]
                driveA = ""
                driveB = ""
                [...]
                bus: floppy-bus.0
                  type floppy-bus
                  dev: floppy, id ""
                    unit = 0 (0x0)
--->                drive = "none1"
    [...]
    none0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
--->    Attached to:      /machine/peripheral-anon/device[0]
        Cache mode:       writeback

    none1 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
--->    Attached to:      /machine/peripheral-anon/device[0]
        Removable device: not locked, tray closed
        Cache mode:       writeback

/machine/peripheral-anon/device[0] is the floppy created with -device.

Test output further demonstrates the "Drive 'FOO' is already in use
because it has been automatically connected to another device" error
message can be misleading.  With '-fda "" -global
floppy.drive=floppy0', it's in use because -global reuses -fda's
backend.  There is no other device involved.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200622094227.1271650-4-armbru@redhat.com>
2020-06-23 16:07:07 +02:00
Markus Armbruster
2017173968 iotests/172: Cover empty filename and multiple use of drives
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200622094227.1271650-3-armbru@redhat.com>
2020-06-23 16:07:07 +02:00
Markus Armbruster
6a1a643301 iotests/172: Include "info block" in test output
The additional output demonstrates we screw up when -global isa-fdc
clashes with -drive if=floppy or its sugared forms: according to "info
qtree", only the latter backend is attached, but according to "info
block", both are.  For instance:

    Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -global isa-fdc.driveA=none0

	      dev: isa-fdc, id ""
	        [...]
		driveA = ""
		driveB = ""
                [...]
                bus: floppy-bus.0
                  type floppy-bus
                  dev: floppy, id ""
                    unit = 0 (0x0)
--->                drive = "floppy0"
    [...]
    floppy0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
--->    Attached to:      /machine/unattached/device[15]
        Removable device: not locked, tray closed
        Cache mode:       writeback

    none0 (NODE_NAME): TEST_DIR/t.qcow2.2 (qcow2)
--->    Attached to:      /machine/unattached/device[14]
        Cache mode:       writeback

/machine/unattached/device[15] is floppy, and
/machine/unattached/device[14] is isa-fdc.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200622094227.1271650-2-armbru@redhat.com>
2020-06-23 16:07:07 +02:00
Eric Blake
3419ec713f iotests: Add copyright line in qcow2.py
The file qcow2.py was originally contributed in 2012 by Kevin Wolf,
but was not given traditional boilerplate headers at the time.  The
missing license was just rectified (commit 16306a7b39) using the
project-default GPLv2+, but as Vladimir is not at Red Hat, he did not
add a Copyright line.  All earlier contributions have come from CC'd
authors, where all but Stefan used a Red Hat address at the time of
the contribution, and that copyright carries over to the split to
qcow2_format.py (d5262c7124).

CC: Kevin Wolf <kwolf@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Eduardo Habkost <ehabkost@redhat.com>
CC: Max Reitz <mreitz@redhat.com>
CC: Philippe Mathieu-Daudé <philmd@redhat.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200609205944.3549240-1-eblake@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-06-17 16:21:21 +02:00
Max Reitz
2e3becf9d7 iotests/{190,291}: compat=0.10 is unsupported
Fixes: 5d72c68b49
Fixes: cf2d1203dc
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200617104822.27525-6-mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-06-17 16:21:21 +02:00
Max Reitz
73b2b7b5ca iotests/229: data_file is unsupported
Fixes: d89ac3cf30
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200617104822.27525-5-mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-06-17 16:21:21 +02:00
Max Reitz
e6de31bcad iotests/292: data_file is unsupported
Fixes: e4d7019e1a
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200617104822.27525-4-mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-06-17 16:21:21 +02:00
Max Reitz
c7070942c7 iotests/041: Skip test_small_target for qed
qed does not support shrinking images, so the test_small_target method
should be skipped to keep 041 passing.

Fixes: 16cea4ee1c
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200617104822.27525-3-mreitz@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-06-17 16:21:21 +02:00
Max Reitz
ff3caf5af0 iotests.py: Add skip_for_formats() decorator
Sometimes, we want to skip some test methods for certain formats.  This
decorator allows that.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200617104822.27525-2-mreitz@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-06-17 16:21:21 +02:00
Roman Kagan
031ffd9a61 qdev-properties: add getter for size32 and blocksize
Add getter for size32, and use it for blocksize, too.

In its human-readable branch, it reports approximate size in
human-readable units next to the exact byte value, like the getter for
64bit size does.

Adjust the expected test output accordingly.

Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200528225516.1676602-8-rvkagan@yandex-team.ru>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-06-17 14:53:40 +02:00
Roman Kagan
c56ee92fcb block: consolidate blocksize properties consistency checks
Several block device properties related to blocksize configuration must
be in certain relationship WRT each other: physical block must be no
smaller than logical block; min_io_size, opt_io_size, and
discard_granularity must be a multiple of a logical block.

To ensure these requirements are met, add corresponding consistency
checks to blkconf_blocksizes, adjusting its signature to communicate
possible error to the caller.  Also remove the now redundant consistency
checks from the specific devices.

Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Message-Id: <20200528225516.1676602-3-rvkagan@yandex-team.ru>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-06-17 14:53:40 +02:00
Peter Maydell
9f1f264edb NBD patches for 2020-06-09
- fix iotest 194 race
 - fix CVE-2020-10761: server DoS from assertion on long NBD error messages
 -----BEGIN PGP SIGNATURE-----
 
 iQEzBAABCAAdFiEEccLMIrHEYCkn0vOqp6FrSiUnQ2oFAl7hH3cACgkQp6FrSiUn
 Q2qPwAf+Le4m5AhAv9rLT+B9LGZFdD17dd7Dqj0CBeUyfVJKD9RtmcWIoVOsnI9Z
 RspYZwRgbYLZQZxKjqTKq1d1BNhK/73suGklkGQC554dik9QJOsHOmkcdK4KPwSD
 L0UG9muBKsmwUueGQusKFLixx39IkhQgLwLdno0wLGCao2PZUd1Z+4f/QmgLhxzI
 /cHzqqPtM97PFjf/lPWHvAZBcQVYmsf6SNMEqrSR30Tff5Lb5vsDFlEoaoPviEWA
 T2Yv1AQJwKcOrMuzmzbGeAIYeqip/WzH5mC4b8ZcKeSZ0pRcG4KoJRjuKIH78D8i
 iA34mc+fyUoctoyLSEFNA/v5Zdde3w==
 =m3k2
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2020-06-09-v2' into staging

NBD patches for 2020-06-09

- fix iotest 194 race
- fix CVE-2020-10761: server DoS from assertion on long NBD error messages

# gpg: Signature made Wed 10 Jun 2020 18:59:19 BST
# gpg:                using RSA key 71C2CC22B1C4602927D2F3AAA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <eblake@redhat.com>" [full]
# gpg:                 aka "Eric Blake (Free Software Programmer) <ebb9@byu.net>" [full]
# gpg:                 aka "[jpeg image of size 6874]" [full]
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2  F3AA A7A1 6B4A 2527 436A

* remotes/ericb/tags/pull-nbd-2020-06-09-v2:
  block: Call attention to truncation of long NBD exports
  nbd/server: Avoid long error message assertions CVE-2020-10761
  iotests: 194: wait for migration completion on target too

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-06-11 21:19:29 +01:00
Eric Blake
5c4fe018c0 nbd/server: Avoid long error message assertions CVE-2020-10761
Ever since commit 36683283 (v2.8), the server code asserts that error
strings sent to the client are well-formed per the protocol by not
exceeding the maximum string length of 4096.  At the time the server
first started sending error messages, the assertion could not be
triggered, because messages were completely under our control.
However, over the years, we have added latent scenarios where a client
could trigger the server to attempt an error message that would
include the client's information if it passed other checks first:

- requesting NBD_OPT_INFO/GO on an export name that is not present
  (commit 0cfae925 in v2.12 echoes the name)

- requesting NBD_OPT_LIST/SET_META_CONTEXT on an export name that is
  not present (commit e7b1948d in v2.12 echoes the name)

At the time, those were still safe because we flagged names larger
than 256 bytes with a different message; but that changed in commit
93676c88 (v4.2) when we raised the name limit to 4096 to match the NBD
string limit.  (That commit also failed to change the magic number
4096 in nbd_negotiate_send_rep_err to the just-introduced named
constant.)  So with that commit, long client names appended to server
text can now trigger the assertion, and thus be used as a denial of
service attack against a server.  As a mitigating factor, if the
server requires TLS, the client cannot trigger the problematic paths
unless it first supplies TLS credentials, and such trusted clients are
less likely to try to intentionally crash the server.

We may later want to further sanitize the user-supplied strings we
place into our error messages, such as scrubbing out control
characters, but that is less important to the CVE fix, so it can be a
later patch to the new nbd_sanitize_name.

Consideration was given to changing the assertion in
nbd_negotiate_send_rep_verr to instead merely log a server error and
truncate the message, to avoid leaving a latent path that could
trigger a future CVE DoS on any new error message.  However, this
merely complicates the code for something that is already (correctly)
flagging coding errors, and now that we are aware of the long message
pitfall, we are less likely to introduce such errors in the future,
which would make such error handling dead code.

Reported-by: Xueqiang Wei <xuwei@redhat.com>
CC: qemu-stable@nongnu.org
Fixes: https://bugzilla.redhat.com/1843684 CVE-2020-10761
Fixes: 93676c88d7
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200610163741.3745251-2-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
2020-06-10 12:58:59 -05:00
Vladimir Sementsov-Ogievskiy
93d487807b iotests: 194: wait for migration completion on target too
It is possible, that shutdown on target occurs earlier than migration
finish. In this case we crash in bdrv_release_dirty_bitmap_locked()
on assertion "assert(!bdrv_dirty_bitmap_busy(bitmap));" as we do have
busy bitmap, as bitmap migration is ongoing.

We'll fix bitmap migration to gracefully cancel on early shutdown soon.
Now let's fix iotest 194 to wait migration completion before shutdown.

Note that in this test dest_vm.shutdown() is called implicitly, as vms
used as context-providers, see __exit__() method of QEMUMachine class.

Actually, not waiting migration finish is a wrong thing, but the test
started to crash after commit ae00aa2398
"iotests: 194: test also migration of dirty bitmap", which added dirty
bitmaps here. So, Fixes: tag won't hurt.

Fixes: ae00aa2398
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Tested-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: grammar tweak]
Message-Id: <20200604083341.26978-1-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09 17:05:50 -05:00
Eric Blake
adf92f4645 iotests: Fix 291 across more file systems
Depending on the granularity of holes and amount of metadata consumed
by a file, the 'disk size:' number of 'qemu-img info' is not reliable.
Adjust our test to use a different set of filters to avoid spurious
failures.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Fixes: cf2d1203dc
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200608195629.3299649-1-eblake@redhat.com>
Acked-by: Kevin Wolf <kwolf@redhat.com>
[eblake: fix merge conflict]
Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09 15:48:00 -05:00
Vladimir Sementsov-Ogievskiy
820c6bee53 qcow2_format.py: dump bitmaps header extension
Add class for bitmap extension and dump its fields. Further work is to
dump bitmap directory.

Test new functionality inside 291 iotest.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <20200606081806.23897-14-vsementsov@virtuozzo.com>
[eblake: fix iotest output]
Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09 15:47:10 -05:00
Vladimir Sementsov-Ogievskiy
aef87784f9 qcow2: QcowHeaderExtension print names for extension magics
Suggested-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200606081806.23897-13-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09 15:47:10 -05:00
Vladimir Sementsov-Ogievskiy
a9e750e1ce qcow2_format: refactor QcowHeaderExtension as a subclass of Qcow2Struct
Only two fields we can parse by generic code, but that is better than
nothing. Keep further refactoring of variable-length fields for another
day.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <20200606081806.23897-12-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09 15:47:09 -05:00
Vladimir Sementsov-Ogievskiy
0931fcc7be qcow2_format.py: QcowHeaderExtension: add dump method
Obviously, for-loop body in dump_extensions should be the dump method
of extension.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <20200606081806.23897-11-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09 15:47:09 -05:00
Vladimir Sementsov-Ogievskiy
860543f055 qcow2_format.py: add field-formatting class
Allow formatter class in structure definition instead of hacking with
'mask'. This will simplify further introduction of new formatters.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <20200606081806.23897-10-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09 15:47:09 -05:00
Vladimir Sementsov-Ogievskiy
0903e3b371 qcow2_format.py: separate generic functionality of structure classes
We are going to introduce more Qcow2 structure types, defined like
QcowHeader. Move generic functionality into base class to be reused for
further structure classes.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <20200606081806.23897-9-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09 15:47:09 -05:00
Vladimir Sementsov-Ogievskiy
5432a0db52 qcow2_format.py: use strings to specify c-type of struct fields
We are going to move field-parsing to super-class, this will be simpler
with simple string specifiers instead of variables.

For some reason, python doesn't allow the definition of ctypes variable
in the class alongside fields: it would not be available then for use
by the 'for' operator. Don't worry: ctypes will be moved to metaclass
soon.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <20200606081806.23897-8-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09 15:47:09 -05:00
Vladimir Sementsov-Ogievskiy
621ca4988a qcow2_format.py: use modern string formatting
Use .format and f-strings instead of old %style. Also, the file uses
both '' and "" quotes, for consistency let's use '', except for cases
when we need '' inside the string (use "" to avoid extra escaping).

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <20200606081806.23897-7-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09 15:47:09 -05:00
Vladimir Sementsov-Ogievskiy
b2f1415444 qcow2_format.py: use tuples instead of lists for fields
No need in lists: it's a constant variable.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <20200606081806.23897-6-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09 15:47:09 -05:00
Vladimir Sementsov-Ogievskiy
eeafed5f6e qcow2_format.py: drop new line printing at end of dump()
This will simplify further conversion. To compensate, print this empty
line directly in cmd_dump_header().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <20200606081806.23897-5-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09 15:47:09 -05:00
Vladimir Sementsov-Ogievskiy
d5262c7124 qcow2.py: move qcow2 format classes to separate module
We are going to enhance qcow2 format parsing by adding more structure
classes. Let's split format parsing from utility code.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200606081806.23897-4-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09 15:47:09 -05:00
Vladimir Sementsov-Ogievskiy
16306a7b39 qcow2.py: add licensing blurb
Add classic heading, which is missing here. Keep copyright place empty,
prior authors may add a line later.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200606081806.23897-3-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: tweak commit message]
Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09 15:47:09 -05:00
Vladimir Sementsov-Ogievskiy
02756054e1 qcow2.py: python style fixes
Fix flake8 complaints.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200606081806.23897-2-vsementsov@virtuozzo.com>
Tested-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: commit message improved]
Signed-off-by: Eric Blake <eblake@redhat.com>
2020-06-09 15:47:09 -05:00
Peter Maydell
b73f417aae Python queue:
* migration acceptance test fix
 * introduce pylintrc & flake8 config
 * various cleanups (Python3, style)
 * vm-test can set QEMU_LOCAL=1 to use locally built binaries
 * refactored BootLinuxBase & LinuxKernelTest acceptance classes
 
 https://gitlab.com/philmd/qemu/pipelines/151323210
 https://travis-ci.org/github/philmd/qemu/builds/693157969
 -----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCAAdFiEE+qvnXhKRciHc/Wuy4+MsLN6twN4FAl7T3U8ACgkQ4+MsLN6t
 wN4JnhAAif0Lw06lEl+ZksD0d4YLyBp079BNJEUWvflsivHY9RIn4e+eXdL3Q8m9
 roBzlsV71C7Ufbp26LFthcnvLaq0JH7RhBUVUoQAI7XZ46HAr3KRyOBJQP7LCr5N
 4Z97y8hqfSdchwpYbxkEbPy58caCRIneqIvg0sp8XuyXpDpVDqP11rXTg4fgqi7i
 1+D1yjr+wgaa7Vvf4sYzOw4D5zD2Mh+zMyDFI9d7yajs/4RH9k+iZteV7baLRQ5Q
 xkC0yqHDGp+uzEF4mk+5VUiZDvDUUxnkuFYKc6mFcahKzhrxLpEsvhnPFZ+vr4ib
 1DDmSr6ihf37wBzowHgAkmTwAiGmVEobu/2h93JXJesWw0TKRT74w1ftZKEIY1v4
 1Hka38gV0LULOAOjiy+aKNJqpJ/eipds94MvllRLHCgbB4H9VKBd4ts6linn+xsM
 CUebvUOgiVzH+hYbLJ1EBLFhbsmQ+yvopbQtLIlyFpKTFhdE1dA3vfb9NV0iqfOL
 fxaP/WaibKEFF5H40H7Ro+H7cT2+hF8MyByBT6q/UzoDURkZxeswqd2ww2VcUw2M
 X6h3/Hzek8PtZ+md3G6Hb1mJccfBHElrSgXAjrZ0WLOy4ZV7Y+/QrE8ooJwIKGKZ
 NinXrUocDl8xfRNWjynImzqma5TdaLW5tOmx6yTSK1R3lQh2z7A=
 =7xHS
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/philmd-gitlab/tags/python-next-20200531' into staging

Python queue:

* migration acceptance test fix
* introduce pylintrc & flake8 config
* various cleanups (Python3, style)
* vm-test can set QEMU_LOCAL=1 to use locally built binaries
* refactored BootLinuxBase & LinuxKernelTest acceptance classes

https://gitlab.com/philmd/qemu/pipelines/151323210
https://travis-ci.org/github/philmd/qemu/builds/693157969

# gpg: Signature made Sun 31 May 2020 17:37:35 BST
# gpg:                using RSA key FAABE75E12917221DCFD6BB2E3E32C2CDEADC0DE
# gpg: Good signature from "Philippe Mathieu-Daudé (F4BUG) <f4bug@amsat.org>" [full]
# Primary key fingerprint: FAAB E75E 1291 7221 DCFD  6BB2 E3E3 2C2C DEAD C0DE

* remotes/philmd-gitlab/tags/python-next-20200531: (25 commits)
  tests/acceptance: refactor boot_linux to allow code reuse
  tests/acceptance: refactor boot_linux_console test to allow code reuse
  tests/acceptance: allow console interaction with specific VMs
  tests/acceptance/migration.py: Wait for both sides
  tests/migration/guestperf: Use Python 3 interpreter
  tests/vm: allow wait_ssh() to specify command
  tests/vm: Add ability to select QEMU from current build
  tests/vm: Pass --debug through for vm-boot-ssh
  python/qemu/qtest: Check before accessing _qtest
  python/qemu/qmp: assert sockfile is not None
  python/qemu/qmp: use True/False for non/blocking modes
  python/qemu: Adjust traceback typing
  python/qemu: fix socket.makefile() typing
  python/qemu: remove Python2 style super() calls
  python/qemu: delint; add flake8 config
  python/qemu: delint and add pylintrc
  python/qemu/machine: remove logging configuration
  python/qemu/machine: add kill() method
  python: remove more instances of sys.version_info
  scripts/qmp: Fix shebang and imports
  ...

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2020-05-31 21:49:07 +01:00
John Snow
2d110c1149 python: remove more instances of sys.version_info
We guarantee 3.5+ everywhere; remove more dead checks. In general, try
to avoid using version checks and instead prefer to attempt behavior
when possible.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200514035230.25756-1-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2020-05-31 18:25:07 +02:00
Eric Blake
cf2d1203dc iotests: Add test 291 to for qemu-img bitmap coverage
Add a new test covering the 'qemu-img bitmap' subcommand, as well as
'qemu-img convert --bitmaps', both added in recent patches.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200521192137.1120211-6-eblake@redhat.com>
2020-05-28 13:16:30 -05:00
Eric Blake
5d72c68b49 qcow2: Expose bitmaps' size during measure
It's useful to know how much space can be occupied by qcow2 persistent
bitmaps, even though such metadata is unrelated to the guest-visible
data.  Report this value as an additional QMP field, present when
measuring an existing image and output format that both support
bitmaps.  Update iotest 178 and 190 to updated output, as well as new
coverage in 190 demonstrating non-zero values made possible with the
recently-added qemu-img bitmap command (see 3b51ab4b).

The new 'bitmaps size:' field is displayed automatically as part of
'qemu-img measure' any time it is present in QMP (that is, any time
both the source image being measured and destination format support
bitmaps, even if the measurement is 0 because there are no bitmaps
present).  If the field is absent, it means that no bitmaps can be
copied (source, destination, or both lack bitmaps, including when
measuring based on size rather than on a source image).  This behavior
is compatible with an upcoming patch adding 'qemu-img convert
--bitmaps': that command will fail in the same situations where this
patch omits the field.

The addition of a new field demonstrates why we should always
zero-initialize qapi C structs; while the qcow2 driver still fully
populates all fields, the raw and crypto drivers had to be tweaked to
avoid uninitialized data.

Consideration was also given towards having a 'qemu-img measure
--bitmaps' which errors out when bitmaps are not possible, and
otherwise sums the bitmaps into the existing allocation totals rather
than displaying as a separate field, as a potential convenience
factor.  But this was ultimately decided to be more complexity than
necessary when the QMP interface was sufficient enough with bitmaps
remaining a separate field.

See also: https://bugzilla.redhat.com/1779904

Reported-by: Nir Soffer <nsoffer@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200521192137.1120211-3-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
2020-05-28 13:16:16 -05:00
Eric Blake
ca01b7a641 iotests: Fix test 178
A recent change to qemu-img changed expected error message output, but
178 takes long enough to execute that it does not get run by 'make
check' or './check -g quick'.

Fixes: 43d589b074
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200521192137.1120211-2-eblake@redhat.com>
2020-05-28 13:15:23 -05:00
Vladimir Sementsov-Ogievskiy
ae00aa2398 iotests: 194: test also migration of dirty bitmap
Test that dirty bitmap migration works when we deal with mirror.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200521220648.3255-5-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
2020-05-28 13:15:22 -05:00