Commit Graph

104563 Commits

Author SHA1 Message Date
Alexander Ivanov
96de69c7d9 parallels: Move check of unclean image to a separate function
We will add more and more checks so we need a better code structure
in parallels_co_check. Let each check performs in a separate loop
in a separate helper.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-Id: <20230424093147.197643-7-alexander.ivanov@virtuozzo.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
2023-06-05 13:13:53 +02:00
Alexander Ivanov
3569cb7b78 parallels: Use generic infrastructure for BAT writing in parallels_co_check()
BAT is written in the context of conventional operations over the image
inside bdrv_co_flush() when it calls parallels_co_flush_to_os() callback.
Thus we should not modify BAT array directly, but call
parallels_set_bat_entry() helper and bdrv_co_flush() further on. After
that there is no need to manually write BAT and track its modification.

This makes code more generic and allows to split parallels_set_bat_entry()
for independent pieces.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Message-Id: <20230424093147.197643-6-alexander.ivanov@virtuozzo.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
2023-06-05 13:13:52 +02:00
Alexander Ivanov
b64b29b96b parallels: create parallels_set_bat_entry_helper() to assign BAT value
This helper will be reused in next patches during parallels_co_check
rework to simplify its code.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-Id: <20230424093147.197643-5-alexander.ivanov@virtuozzo.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
2023-06-05 13:13:51 +02:00
Alexander Ivanov
679749ce41 parallels: Fix image_end_offset and data_end after out-of-image check
Set data_end to the end of the last cluster inside the image. In such a
way we can be sure that corrupted offsets in the BAT can't affect on the
image size. If there are no allocated clusters set image_end_offset by
data_end.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Message-Id: <20230424093147.197643-4-alexander.ivanov@virtuozzo.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
2023-06-05 13:13:50 +02:00
Alexander Ivanov
ab2d739c41 parallels: Fix high_off calculation in parallels_co_check()
Don't let high_off be more than the file size even if we don't fix the
image.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Reviewed-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-Id: <20230424093147.197643-3-alexander.ivanov@virtuozzo.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
2023-06-05 13:13:49 +02:00
Alexander Ivanov
f5e715dbbb parallels: Out of image offset in BAT leads to image inflation
data_end field in BDRVParallelsState is set to the biggest offset present
in BAT. If this offset is outside of the image, any further write will
create the cluster at this offset and/or the image will be truncated to
this offset on close. This is definitely not correct.

Raise an error in parallels_open() if data_end points outside the image
and it is not a check (let the check to repaire the image). Set data_end
to the end of the cluster with the last correct offset.

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Message-Id: <20230424093147.197643-2-alexander.ivanov@virtuozzo.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
2023-06-05 13:13:47 +02:00
Hanna Czenczek
d7e1905e3f iotests/iov-padding: New test
Test that even vectored IO requests with 1024 vector elements that are
not aligned to the device's request alignment will succeed.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230411173418.19549-5-hreitz@redhat.com>
2023-06-05 13:11:26 +02:00
Hanna Czenczek
cc63f6f6fa util/iov: Remove qemu_iovec_init_extended()
bdrv_pad_request() was the main user of qemu_iovec_init_extended().
HEAD^ has removed that use, so we can remove qemu_iovec_init_extended()
now.

The only remaining user is qemu_iovec_init_slice(), which can easily
inline the small part it really needs.

Note that qemu_iovec_init_extended() offered a memcpy() optimization to
initialize the new I/O vector.  qemu_iovec_concat_iov(), which is used
to replace its functionality, does not, but calls qemu_iovec_add() for
every single element.  If we decide this optimization was important, we
will need to re-implement it in qemu_iovec_concat_iov(), which might
also benefit its pre-existing users.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230411173418.19549-4-hreitz@redhat.com>
2023-06-05 13:11:24 +02:00
Hanna Czenczek
18743311b8 block: Collapse padded I/O vecs exceeding IOV_MAX
When processing vectored guest requests that are not aligned to the
storage request alignment, we pad them by adding head and/or tail
buffers for a read-modify-write cycle.

The guest can submit I/O vectors up to IOV_MAX (1024) in length, but
with this padding, the vector can exceed that limit.  As of
4c002cef0e ("util/iov: make
qemu_iovec_init_extended() honest"), we refuse to pad vectors beyond the
limit, instead returning an error to the guest.

To the guest, this appears as a random I/O error.  We should not return
an I/O error to the guest when it issued a perfectly valid request.

Before 4c002cef0e, we just made the vector
longer than IOV_MAX, which generally seems to work (because the guest
assumes a smaller alignment than we really have, file-posix's
raw_co_prw() will generally see bdrv_qiov_is_aligned() return false, and
so emulate the request, so that the IOV_MAX does not matter).  However,
that does not seem exactly great.

I see two ways to fix this problem:
1. We split such long requests into two requests.
2. We join some elements of the vector into new buffers to make it
   shorter.

I am wary of (1), because it seems like it may have unintended side
effects.

(2) on the other hand seems relatively simple to implement, with
hopefully few side effects, so this patch does that.

To do this, the use of qemu_iovec_init_extended() in bdrv_pad_request()
is effectively replaced by the new function bdrv_create_padded_qiov(),
which not only wraps the request IOV with padding head/tail, but also
ensures that the resulting vector will not have more than IOV_MAX
elements.  Putting that functionality into qemu_iovec_init_extended() is
infeasible because it requires allocating a bounce buffer; doing so
would require many more parameters (buffer alignment, how to initialize
the buffer, and out parameters like the buffer, its length, and the
original elements), which is not reasonable.

Conversely, it is not difficult to move qemu_iovec_init_extended()'s
functionality into bdrv_create_padded_qiov() by using public
qemu_iovec_* functions, so that is what this patch does.

Because bdrv_pad_request() was the only "serious" user of
qemu_iovec_init_extended(), the next patch will remove the latter
function, so the functionality is not implemented twice.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2141964
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230411173418.19549-3-hreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
2023-06-05 13:11:06 +02:00
Hanna Czenczek
3d06cea825 util/iov: Make qiov_slice() public
We want to inline qemu_iovec_init_extended() in block/io.c for padding
requests, and having access to qiov_slice() is useful for this.  As a
public function, it is renamed to qemu_iovec_slice().

(We will need to count the number of I/O vector elements of a slice
there, and then later process this slice.  Without qiov_slice(), we
would need to call qemu_iovec_subvec_niov(), and all further
IOV-processing functions may need to skip prefixing elements to
accomodate for a qiov_offset.  Because qemu_iovec_subvec_niov()
internally calls qiov_slice(), we can just have the block/io.c code call
qiov_slice() itself, thus get the number of elements, and also create an
iovec array with the superfluous prefixing elements stripped, so the
following processing functions no longer need to skip them.)

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230411173418.19549-2-hreitz@redhat.com>
2023-06-05 13:11:02 +02:00
Thomas Huth
faae3437b0 tests/qtest: Run ipmi-bt-test only if CONFIG_IPMI_EXTERN is set
The ipmi-bt-test uses "-device ipmi-bmc-extern", thus it should
only be run if this device has been enabled in the configuration.

Message-Id: <20230524081024.1619273-1-thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
2023-06-05 09:48:29 +02:00
Ilya Leoshkevich
2b956244a9 tests/tcg/s390x: Test MXDB and MXDBR
Add a small test to prevent regressions.

Cc: qemu-stable@nongnu.org
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20230601223027.795501-3-iii@linux.ibm.com>
Acked-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
2023-06-05 09:48:29 +02:00
Ilya Leoshkevich
a7f4add793 target/s390x: Fix MXDB and MXDBR
These instructions multiply 64 bits by 64 bits, not 128 bits by 64 bits.

Reported-by: Tulio Magno Quites Machado Filho <tuliom@redhat.com>
Fixes: 2b91240f95 ("target/s390x: Use Int128 for passing float128")
Cc: qemu-stable@nongnu.org
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2211472
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20230601223027.795501-2-iii@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
2023-06-05 09:48:29 +02:00
Bernhard Beschow
36c9189890 hw/isa/i82378: Remove unused "io" attribute
The attribute isn't used since commit 5c9736789b
"i82378: Cleanup implementation".

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-Id: <20230523195608.125820-4-shentey@gmail.com>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
2023-06-05 07:43:23 +01:00
Bernhard Beschow
14e066a7c4 hw/arm/omap: Remove unused omap_uart_attach()
The function is unused since commit
bdad3654d3 ('hw/arm/nseries: Remove
invalid/unnecessary n8x0_uart_setup()').

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20230523195608.125820-3-shentey@gmail.com>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
2023-06-05 07:43:23 +01:00
Bernhard Beschow
02520772ae hw/timer/i8254_common: Share "iobase" property via base class
Both TYPE_KVM_I8254 and TYPE_I8254 have their own but same implementation of
the "iobase" property. The storage for the property already resides in
PITCommonState, so also move the property definition there.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20230523195608.125820-2-shentey@gmail.com>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
2023-06-05 07:43:23 +01:00
Max Fritz
0db0fbb5cf Add conditional dependency for libkeyutils
This modification enables better control over the inclusion of libkeyutils
based on the configuration, enhancing the flexibility of the build system.

Signed-off-by: Max Fritz <antischmock@googlemail.com>
Message-Id: <168471463402.18155.3575359027429939965-1@git.sr.ht>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
[thuth: Remove the "kwargs: static_kwargs" part - it's not necessary anymore]
Signed-off-by: Thomas Huth <thuth@redhat.com>
2023-06-05 07:27:23 +02:00
Ilya Leoshkevich
be4a4cb429 tests/tcg/s390x: Test single-stepping SVC
Add a small test to prevent regressions.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Acked-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20230510230213.330134-3-iii@linux.ibm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
2023-06-05 07:27:23 +02:00
Ilya Leoshkevich
01b9990a3f linux-user/s390x: Fix single-stepping SVC
Currently single-stepping SVC executes two instructions. The reason is
that EXCP_DEBUG for the SVC instruction itself is masked by EXCP_SVC.
Fix by re-raising EXCP_DEBUG.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20230510230213.330134-2-iii@linux.ibm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
2023-06-05 07:27:23 +02:00
Ilya Leoshkevich
230976232f tests/tcg/s390x: Test LOCFHR
Add a small test to prevent regressions.

Cc: qemu-stable@nongnu.org
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20230526181240.1425579-5-iii@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
2023-06-05 07:27:23 +02:00
Ilya Leoshkevich
3180b17362 target/s390x: Fix LOCFHR taking the wrong half of R2
LOCFHR should write top-to-top, but QEMU erroneously writes
bottom-to-top.

Fixes: 45aa9aa3b7 ("target/s390x: Implement load-on-condition-2 insns")
Cc: qemu-stable@nongnu.org
Reported-by: Mikhail Mitskevich <mitskevichmn@gmail.com>
Closes: https://gitlab.com/qemu-project/qemu/-/issues/1668
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20230526181240.1425579-4-iii@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
2023-06-05 07:27:23 +02:00
Ilya Leoshkevich
05d000fb4d tests/tcg/s390x: Test LCBB
Add a test to prevent regressions.

Cc: qemu-stable@nongnu.org
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20230526181240.1425579-3-iii@linux.ibm.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Acked-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
2023-06-05 07:27:23 +02:00
Ilya Leoshkevich
079181b9bc target/s390x: Fix LCBB overwriting the top 32 bits
LCBB is supposed to overwrite only the bottom 32 bits, but QEMU
erroneously overwrites the entire register.

Fixes: 6d9303322e ("s390x/tcg: Implement LOAD COUNT TO BLOCK BOUNDARY")
Cc: qemu-stable@nongnu.org
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20230526181240.1425579-2-iii@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
2023-06-05 07:27:23 +02:00
Richard Henderson
c0a7830292 Fixes Coverity CID: 1512452, 1512453
Fixes: 78464f023b ("hw/loongarch/virt: Modify ipi as percpu device")
 -----BEGIN PGP SIGNATURE-----
 
 iLMEAAEIAB0WIQS4/x2g0v3LLaCcbCxAov/yOSY+3wUCZH1THwAKCRBAov/yOSY+
 37u0BADoodYhyV+jhfBCwKvZbE7yI3vvqcC1hdw8/jclLcIfYVsgUJ+qETmk9s/X
 h/WGevhwADb+fBcBIYEBWeFFfFdBlEDxua/ew0eFlRcB3HoIHRnJJT9L+rZ4ifNW
 b8HDhv/LjDj9dpQ449wpGlSOiSST3+AiCpKkH36KUOEfLP029Q==
 =3GIb
 -----END PGP SIGNATURE-----

Merge tag 'pull-loongarch-20230605' of https://gitlab.com/gaosong/qemu into staging

Fixes Coverity CID: 1512452, 1512453
Fixes: 78464f023b ("hw/loongarch/virt: Modify ipi as percpu device")

# -----BEGIN PGP SIGNATURE-----
#
# iLMEAAEIAB0WIQS4/x2g0v3LLaCcbCxAov/yOSY+3wUCZH1THwAKCRBAov/yOSY+
# 37u0BADoodYhyV+jhfBCwKvZbE7yI3vvqcC1hdw8/jclLcIfYVsgUJ+qETmk9s/X
# h/WGevhwADb+fBcBIYEBWeFFfFdBlEDxua/ew0eFlRcB3HoIHRnJJT9L+rZ4ifNW
# b8HDhv/LjDj9dpQ449wpGlSOiSST3+AiCpKkH36KUOEfLP029Q==
# =3GIb
# -----END PGP SIGNATURE-----
# gpg: Signature made Sun 04 Jun 2023 08:14:39 PM PDT
# gpg:                using RSA key B8FF1DA0D2FDCB2DA09C6C2C40A2FFF239263EDF
# gpg: Good signature from "Song Gao <m17746591750@163.com>" [unknown]
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: B8FF 1DA0 D2FD CB2D A09C  6C2C 40A2 FFF2 3926 3EDF

* tag 'pull-loongarch-20230605' of https://gitlab.com/gaosong/qemu:
  hw/intc/loongarch_ipi: Bring back all 4 IPI mailboxes

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
2023-06-04 20:48:46 -07:00
Jiaxun Yang
8555ddc671
hw/intc/loongarch_ipi: Bring back all 4 IPI mailboxes
As per "Loongson 3A5000/3B5000 Processor Reference Manual",
Loongson 3A5000's IPI implementation have 4 mailboxes per
core.

However, in 78464f023b ("hw/loongarch/virt: Modify ipi as
percpu device"), the number of IPI mailboxes was reduced to
one, which mismatches actual hardware.

It won't affect LoongArch based system as LoongArch boot code
only uses the first mailbox, however MIPS based Loongson boot
code uses all 4 mailboxes.

Fixes Coverity CID: 1512452, 1512453
Fixes: 78464f023b ("hw/loongarch/virt: Modify ipi as percpu device")
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Reviewed-by: Song Gao <gaosong@loongson.cn>
Message-Id: <20230521102307.87081-2-jiaxun.yang@flygoat.com>
Signed-off-by: Song Gao <gaosong@loongson.cn>
2023-06-05 11:08:55 +08:00
Richard Henderson
848a6caa88 Migration Pull request (20230602 vintage)
This PULL request get:
 - All migration-test patches except last one (daniel)
 - Documentation about live test cases (peter)
 
 Please apply.
 -----BEGIN PGP SIGNATURE-----
 
 iQIzBAABCAAdFiEEGJn/jt6/WMzuA0uC9IfvGFhy1yMFAmR5yRwACgkQ9IfvGFhy
 1yOLQQ/+NsrXEj7Bwp2PdGo+wBRkq4Gah/mc9F00pqtJc2CGNWgfgDohhZjBrhRv
 cTABsfEcIKgCYqGYwVCklJGlUMzxlJPPcMfvou5SWN59E4FBFSg4DWaBfDPCS8LW
 yjnz0JcpxJ+Ge0eqP6xpTPKQ0YGisdav/PjF8GZewBCjyrhZop062a92B2t59D8Y
 shJYKaZZU/5/4zx6KqOm9OClD/yJ+w5q6cGn89/rFE0RMSVywZ3Y1O8/LwIAEP6U
 oj88rczh3geGlsmtPIeyhA3BdnYuPonmyLz8CINFH9+y2tR9l1dN59q1uwEOIvff
 BhJvxTNmkTvsi5zeAmbp2CYmRTwhBmlanh8v2OLNj8zlt0cHYNpiYUZO9qxCHIyT
 LnNTTYhrpqAqINdm+Z8c3ymDKkTz0KECBa45hdFtNB4ZOXPDQTHVqkQRfe3CxDKz
 f/WM4TxHEzVMw/Ow1K9Kbk7/AEwIV6Ol2BSf9D+ZcU4ydmu6ENhV9G4cQ9Orlv8I
 opychxf+O/b6yhVFq7J1ufDhfn3aWQmUQC06npEgfrIV/fLrXhYfs2CXkNZs78v6
 MTMNPNBN/UasM8hx+ldsjZEHf625lO3eNWoNY1Xxog5YICnNLA+tG6n69uybew2+
 UOVyoHwX7iqaToK6bQNCS4H/PjCp3v7fzw1Nsz48Pfaklpivz/k=
 =4exy
 -----END PGP SIGNATURE-----

Merge tag 'migration-20230602-pull-request' of https://gitlab.com/juan.quintela/qemu into staging

Migration Pull request (20230602 vintage)

This PULL request get:
- All migration-test patches except last one (daniel)
- Documentation about live test cases (peter)

Please apply.

# -----BEGIN PGP SIGNATURE-----
#
# iQIzBAABCAAdFiEEGJn/jt6/WMzuA0uC9IfvGFhy1yMFAmR5yRwACgkQ9IfvGFhy
# 1yOLQQ/+NsrXEj7Bwp2PdGo+wBRkq4Gah/mc9F00pqtJc2CGNWgfgDohhZjBrhRv
# cTABsfEcIKgCYqGYwVCklJGlUMzxlJPPcMfvou5SWN59E4FBFSg4DWaBfDPCS8LW
# yjnz0JcpxJ+Ge0eqP6xpTPKQ0YGisdav/PjF8GZewBCjyrhZop062a92B2t59D8Y
# shJYKaZZU/5/4zx6KqOm9OClD/yJ+w5q6cGn89/rFE0RMSVywZ3Y1O8/LwIAEP6U
# oj88rczh3geGlsmtPIeyhA3BdnYuPonmyLz8CINFH9+y2tR9l1dN59q1uwEOIvff
# BhJvxTNmkTvsi5zeAmbp2CYmRTwhBmlanh8v2OLNj8zlt0cHYNpiYUZO9qxCHIyT
# LnNTTYhrpqAqINdm+Z8c3ymDKkTz0KECBa45hdFtNB4ZOXPDQTHVqkQRfe3CxDKz
# f/WM4TxHEzVMw/Ow1K9Kbk7/AEwIV6Ol2BSf9D+ZcU4ydmu6ENhV9G4cQ9Orlv8I
# opychxf+O/b6yhVFq7J1ufDhfn3aWQmUQC06npEgfrIV/fLrXhYfs2CXkNZs78v6
# MTMNPNBN/UasM8hx+ldsjZEHf625lO3eNWoNY1Xxog5YICnNLA+tG6n69uybew2+
# UOVyoHwX7iqaToK6bQNCS4H/PjCp3v7fzw1Nsz48Pfaklpivz/k=
# =4exy
# -----END PGP SIGNATURE-----
# gpg: Signature made Fri 02 Jun 2023 03:49:00 AM PDT
# gpg:                using RSA key 1899FF8EDEBF58CCEE034B82F487EF185872D723
# gpg: Good signature from "Juan Quintela <quintela@redhat.com>" [unknown]
# gpg:                 aka "Juan Quintela <quintela@trasno.org>" [unknown]
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 1899 FF8E DEBF 58CC EE03  4B82 F487 EF18 5872 D723

* tag 'migration-20230602-pull-request' of https://gitlab.com/juan.quintela/qemu:
  qtest/migration: Document live=true cases
  tests/qtest: make more migration pre-copy scenarios run non-live
  tests/qtest: distinguish src/dst migration VM stop/resume events
  tests/qtest: capture RESUME events during migration
  tests/qtest: replace wait_command() with qtest_qmp_assert_success
  tests/qtest: switch to using event callbacks for STOP event
  tests/qtest: get rid of some 'qtest_qmp' usage in migration test
  tests/qtest: get rid of 'qmp_command' helper in migration test
  tests/qtest: add support for callback to receive QMP events
  tests/qtest: add various qtest_qmp_assert_success() variants

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
2023-06-02 17:33:29 -07:00
Richard Henderson
24bc242c91 nbd and misc patches for 2023-06-01
- Eric Blake: Fix iotest 104 for NBD
 - Eric Blake: Improve qcow2 spec on padding bytes
 - Eric Blake: Fix read-beyond-bounds bug in qemu_strtosz
 -----BEGIN PGP SIGNATURE-----
 
 iQEzBAABCAAdFiEEccLMIrHEYCkn0vOqp6FrSiUnQ2oFAmR6JzEACgkQp6FrSiUn
 Q2oGwgf+PIaN8iedQo5KR08OEf9YxJXab7nL5Oh12+ZvrPOt8XoJcd585KblQ1YI
 3bGC4CO1l4QO3xmKltVHi7hnlX+3/8WMEvh0jBQBG1AjjPCi5Y1A/gGTEJFX60Ux
 /ffEpo8+1vaHQ8srkxBMWIvpF/dYRaMXSm/CP5SNqTllTalTR46YHKL9odXTzIeN
 0Zu9UQw/Jwp5A9/8KB+0M9SYXA6zOEmEqEyOwVESEAU2Lm7titwqdBny6GZc6DH6
 Sa2lKO0qQA/e9ya6jHm2c9ycoNCtQ/2VR8QuCd6WCf9DX8q/9RhdiJir+EK5gqp6
 3JRUFtx783d0BwPnUDUqPawi4txFtw==
 =Cg4e
 -----END PGP SIGNATURE-----

Merge tag 'pull-nbd-2023-06-01-v2' of https://repo.or.cz/qemu/ericb into staging

nbd and misc patches for 2023-06-01

- Eric Blake: Fix iotest 104 for NBD
- Eric Blake: Improve qcow2 spec on padding bytes
- Eric Blake: Fix read-beyond-bounds bug in qemu_strtosz

# -----BEGIN PGP SIGNATURE-----
#
# iQEzBAABCAAdFiEEccLMIrHEYCkn0vOqp6FrSiUnQ2oFAmR6JzEACgkQp6FrSiUn
# Q2oGwgf+PIaN8iedQo5KR08OEf9YxJXab7nL5Oh12+ZvrPOt8XoJcd585KblQ1YI
# 3bGC4CO1l4QO3xmKltVHi7hnlX+3/8WMEvh0jBQBG1AjjPCi5Y1A/gGTEJFX60Ux
# /ffEpo8+1vaHQ8srkxBMWIvpF/dYRaMXSm/CP5SNqTllTalTR46YHKL9odXTzIeN
# 0Zu9UQw/Jwp5A9/8KB+0M9SYXA6zOEmEqEyOwVESEAU2Lm7titwqdBny6GZc6DH6
# Sa2lKO0qQA/e9ya6jHm2c9ycoNCtQ/2VR8QuCd6WCf9DX8q/9RhdiJir+EK5gqp6
# 3JRUFtx783d0BwPnUDUqPawi4txFtw==
# =Cg4e
# -----END PGP SIGNATURE-----
# gpg: Signature made Fri 02 Jun 2023 10:30:25 AM PDT
# 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]

* tag 'pull-nbd-2023-06-01-v2' of https://repo.or.cz/qemu/ericb: (21 commits)
  cutils: Improve qemu_strtosz handling of fractions
  cutils: Improve qemu_strtod* error paths
  cutils: Use parse_uint in qemu_strtosz for negative rejection
  cutils: Set value in all integral qemu_strto* error paths
  cutils: Set value in all qemu_strtosz* error paths
  test-cutils: Add more coverage to qemu_strtosz
  numa: Check for qemu_strtosz_MiB error
  cutils: Allow NULL str in qemu_strtosz
  test-cutils: Refactor qemu_strtosz tests for less boilerplate
  test-cutils: Prepare for upcoming semantic change in qemu_strtosz
  test-cutils: Add coverage of qemu_strtod
  cutils: Allow NULL endptr in parse_uint()
  cutils: Adjust signature of parse_uint[_full]
  cutils: Document differences between parse_uint and qemu_strtou64
  cutils: Fix wraparound parsing in qemu_strtoui
  test-cutils: Test more integer corner cases
  test-cutils: Test integral qemu_strto* value on failures
  test-cutils: Use g_assert_cmpuint where appropriate
  test-cutils: Avoid g_assert in unit tests
  qcow2: Explicit mention of padding bytes
  ...

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
2023-06-02 14:57:22 -07:00
Eric Blake
42cc08d13a cutils: Improve qemu_strtosz handling of fractions
We have several limitations and bugs worth fixing; they are
inter-related enough that it is not worth splitting this patch into
smaller pieces:

* ".5k" should work to specify 512, just as "0.5k" does
* "1.9999k" and "1." + "9"*50 + "k" should both produce the same
  result of 2048 after rounding
* "1." + "0"*350 + "1B" should not be treated the same as "1.0B";
  underflow in the fraction should not be lost
* "7.99e99" and "7.99e999" look similar, but our code was doing a
  read-out-of-bounds on the latter because it was not expecting ERANGE
  due to overflow. While we document that scientific notation is not
  supported, and the previous patch actually fixed
  qemu_strtod_finite() to no longer return ERANGE overflows, it is
  easier to pre-filter than to try and determine after the fact if
  strtod() consumed more than we wanted.  Note that this is a
  low-level semantic change (when endptr is not NULL, we can now
  successfully parse with a scale of 'E' and then report trailing
  junk, instead of failing outright with EINVAL); but an earlier
  commit already argued that this is not a high-level semantic change
  since the only caller passing in a non-NULL endptr also checks that
  the tail is whitespace-only.

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1629
Fixes: cf923b78 ("utils: Improve qemu_strtosz() to have 64 bits of precision", 6.0.0)
Fixes: 7625a1ed ("utils: Use fixed-point arithmetic in qemu_strtosz", 6.0.0)
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-20-eblake@redhat.com>
[eblake: tweak function comment for accuracy]
2023-06-02 12:29:27 -05:00
Eric Blake
c25b168344 cutils: Improve qemu_strtod* error paths
Previous patches changed all integral qemu_strto*() error paths to
guarantee that *value is never left uninitialized.  Do likewise for
qemu_strtod.  Also, tighten qemu_strtod_finite() to never return a
non-finite value (prior to this patch, we were rejecting "inf" with
-EINVAL and unspecified result 0.0, but failing "9e999" with -ERANGE
and HUGE_VAL - which is infinite on IEEE machines - despite our
function claiming to recognize only finite values).

Auditing callers, we have no external callers of qemu_strtod, and
among the callers of qemu_strtod_finite:

- qapi/qobject-input-visitor.c:qobject_input_type_number_keyval() and
  qapi/string-input-visitor.c:parse_type_number() which reject all
  errors (does not matter what we store)

- utils/cutils.c:do_strtosz() incorrectly assumes that *endptr points
  to '.' on all failures (that is, it is not distinguishing between
  EINVAL and ERANGE; and therefore still does the WRONG THING for
  "9.9e999".  The change here does not entirely fix that (a later
  patch will tackle this more systematically), but at least it fixes
  the read-out-of-bounds first diagnosed in
  https://gitlab.com/qemu-project/qemu/-/issues/1629

- our testsuite, which we can update to match what we document

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
CC: qemu-stable@nongnu.org
Message-Id: <20230522190441.64278-19-eblake@redhat.com>
2023-06-02 12:29:27 -05:00
Eric Blake
b87ac96651 cutils: Use parse_uint in qemu_strtosz for negative rejection
Rather than open-coding two different ways to check for an unwanted
negative sign, reuse the same code in both functions.  That way, if we
decide down the road to accept "-0" instead of rejecting it, we have
fewer places to change.  Also, it means we now get ERANGE instead of
EINVAL for negative values in qemu_strtosz, which is reasonable for
what it represents.  This in turn changes the expected output of a
couple of iotests.

The change is not quite complete: negative fractional scaled values
can trip us up.  This will be fixed in a later patch addressing other
issues with fractional scaled values.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-18-eblake@redhat.com>
2023-06-02 12:29:27 -05:00
Eric Blake
3c5f246798 cutils: Set value in all integral qemu_strto* error paths
Our goal in writing qemu_strtoi() and friends is to have an interface
harder to abuse than libc's strtol().  Leaving the return value
uninitialized on some but not all error paths does not lend itself
well to this goal; and our documentation wasn't helpful on what to
expect.

Note that the previous patch changed all qemu_strtosz() EINVAL error
paths to slam value to 0 rather than stay uninitialized, even when the
EINVAL eror occurs because of trailing junk.  But for the remaining
integral qemu_strto*, it's easier to return the parsed value than to
force things back to zero, in part because of how check_strtox_error
works; in part because people expect that from libc strto* (while
there is no libc strtosz to compare to), and in part because doing so
creates less churn in the testsuite.

Here, the list of affected callers is much longer ('git grep
"qemu_strto[ui]" "*.c" "**/*.c" | grep -v tests/ |wc -l' outputs 107,
although a few of those are the implementation in in cutils.c), so
touching as little as possible is the wisest course of action.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-17-eblake@redhat.com>
2023-06-02 12:29:27 -05:00
Eric Blake
896fbd90aa cutils: Set value in all qemu_strtosz* error paths
Making callers determine whether or not *value was populated on error
is not nice for usability.  Pre-patch, we have unit tests that check
that *result is left unchanged on most EINVAL errors and set to 0 on
many ERANGE errors.  This is subtly different from libc strtoumax()
behavior which returns UINT64_MAX on ERANGE errors, as well as
different from our parse_uint() which slams to 0 on EINVAL on the
grounds that we want our functions to be harder to mis-use than
strtoumax().

Let's audit callers:

- hw/core/numa.c:parse_numa() fixed in the previous patch to check for
  errors

- migration/migration-hmp-cmds.c:hmp_migrate_set_parameter(),
  monitor/hmp.c:monitor_parse_arguments(),
  qapi/opts-visitor.c:opts_type_size(),
  qapi/qobject-input-visitor.c:qobject_input_type_size_keyval(),
  qemu-img.c:cvtnum_full(), qemu-io-cmds.c:cvtnum(),
  target/i386/cpu.c:x86_cpu_parse_featurestr(), and
  util/qemu-option.c:parse_option_size() appear to reject all failures
  (although some with distinct messages for ERANGE as opposed to
  EINVAL), so it doesn't matter what is in the value parameter on
  error.

- All remaining callers are in the testsuite, where we can tweak our
  expectations to match our new desired behavior.

Advancing to the end of the string parsed on overflow (ERANGE), while
still returning 0, makes sense (UINT64_MAX as a size is unlikely to be
useful); likewise, our size parsing code is complex enough that it's
easier to always return 0 when endptr is NULL but trailing garbage was
found, rather than trying to return the value of the prefix actually
parsed (no current caller cared about the value of the prefix).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-16-eblake@redhat.com>
2023-06-02 12:29:27 -05:00
Eric Blake
e1cf34b6b3 test-cutils: Add more coverage to qemu_strtosz
Add some more strings that the user might send our way.  In
particular, some of these additions include FIXME comments showing
where our parser doesn't quite behave the way we want.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-15-eblake@redhat.com>
2023-06-02 12:29:27 -05:00
Eric Blake
a73049b2a1 numa: Check for qemu_strtosz_MiB error
As shown in the previous commit, qemu_strtosz_MiB sometimes leaves the
result value untouched (we have to audit further to learn that in that
case, the QAPI generator says that visit_type_NumaOptions() will have
zero-initialized it), and sometimes leaves it with the value of a
partial parse before -EINVAL occurs because of trailing garbage.
Rather than blindly treating any string the user may throw at us as
valid, we should check for parse failures.

Fixes: cc001888 ("numa: fixup parsed NumaNodeOptions earlier", v2.11.0)
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-14-eblake@redhat.com>
2023-06-02 12:29:27 -05:00
Eric Blake
f49371ecae cutils: Allow NULL str in qemu_strtosz
All the other qemu_strto* and parse_uint allow a NULL str.  Having
qemu_strtosz not crash on qemu_strtosz(NULL, NULL, &value) is an easy
fix that adds some consistency between our string parsers.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-13-eblake@redhat.com>
2023-06-02 12:29:27 -05:00
Eric Blake
157367cf21 test-cutils: Refactor qemu_strtosz tests for less boilerplate
No need to copy-and-paste lots of boilerplate per string tested, when
we can consolidate that behind helper functions.  Plus, this adds a
bit more coverage (we now test all strings both with and without
endptr, whereas before some tests skipped the NULL endptr case), which
exposed a SEGFAULT on qemu_strtosz(NULL, NULL, &val) that will be
fixed in an upcoming patch.

Note that duplicating boilerplate has one advantage lost here - a
failed test tells you which line number failed; but a helper function
does not show the call stack that reached the failure.  Since we call
the helper more than once within many of the "unit tests", even the
unit test name doesn't point out which call is failing.  But that only
matters when tests fail (they normally pass); at which point I'm
debugging the failures under gdb anyways, so I'm not too worried about
it.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-12-eblake@redhat.com>
2023-06-02 12:29:27 -05:00
Eric Blake
edafce694a test-cutils: Prepare for upcoming semantic change in qemu_strtosz
A quick search for 'qemu_strtosz' in the code base shows that outside
of the testsuite, the ONLY place that passes a non-NULL pointer to
@endptr of any variant of a size parser is in hmp.c (the 'o' parser of
monitor_parse_arguments), and that particular caller warns of
"extraneous characters at the end of line" unless the trailing bytes
are purely whitespace.  Thus, it makes no semantic difference at the
high level whether we parse "1.5e1k" as "1" + ".5e1" + "k" (an attempt
to use scientific notation in strtod with a scaling suffix of 'k' with
no trailing junk, but which qemu_strtosz says should fail with
EINVAL), or as "1.5e" + "1k" (a valid size with scaling suffix of 'e'
for exabytes, followed by two junk bytes) - either way, any user
passing such a string will get an error message about a parse failure.

However, an upcoming patch to qemu_strtosz will fix other corner case
bugs in handling the fractional portion of a size, and in doing so, it
is easier to declare that qemu_strtosz() itself stops parsing at the
first 'e' rather than blindly consuming whatever strtod() will
recognize.  Once that is fixed, the difference will be visible at the
low level (getting a valid parse with trailing garbage when @endptr is
non-NULL, while continuing to get -EINVAL when @endptr is NULL); this
is easier to demonstrate by moving the affected strings from
test_qemu_strtosz_invalid() (which declares them as always -EINVAL) to
test_qemu_strtosz_trailing() (where @endptr affects behavior, for now
with FIXME comments).

Note that a similar argument could be made for having "0x1.5" or
"0x1M" parse as 0x1 with ".5" or "M" as trailing junk, instead of
blindly treating it as -EINVAL; however, as these cases do not suffer
from the same problems as floating point, they are not worth changing
at this time.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-11-eblake@redhat.com>
2023-06-02 12:29:27 -05:00
Eric Blake
759573d05b test-cutils: Add coverage of qemu_strtod
It's hard to tweak code for consistency if I can't prove what will or
won't break from those tweaks.  Time to add unit tests for
qemu_strtod() and qemu_strtod_finite().

Among other things, I wrote a check whether we have C99 semantics for
strtod("0x1") (which MUST parse hex numbers) rather than C89 (which
must stop parsing at 'x').  These days, I suspect that is okay; but if
it fails CI checks, knowing the difference will help us decide what we
want to do about it.  Note that C2x, while not final at the time of
this patch, has been considering whether to make strtol("0b1") parse
as 1 with no slop instead of the C17 parse of 0 with slop "b1"; that
decision may also bleed over to strtod().  But for now, I didn't think
it worth adding unit tests on that front (to strtol or strtod) as
things may still change.

Likewise, there are plenty more corner cases of strtod proper that I
don't explicitly test here, but there are enough unit tests added here
that it covers all the branches reached in our wrappers.  In
particular, it demonstrates the difference on when *value is left
uninitialized, which an upcoming patch will normalize.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-10-eblake@redhat.com>
2023-06-02 12:29:27 -05:00
Eric Blake
52d606aa5b cutils: Allow NULL endptr in parse_uint()
All the qemu_strto*() functions permit a NULL endptr, just like their
libc counterparts, leaving parse_uint() as the oddball that caused
SEGFAULT on NULL and required the user to call parse_uint_full()
instead.  Relax things for consistency, even though the testsuite is
the only impacted caller.  Add one more unit test to ensure even
parse_uint_full(NULL, 0, &value) works.  This also fixes our code to
uniformly favor EINVAL over ERANGE when both apply.

Also fixes a doc mismatch @v vs. a parameter named value.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-9-eblake@redhat.com>
2023-06-02 12:29:27 -05:00
Eric Blake
bd1386cce1 cutils: Adjust signature of parse_uint[_full]
It's already confusing that we have two very similar functions for
wrapping the parse of a 64-bit unsigned value, differing mainly on
whether they permit leading '-'.  Adjust the signature of parse_uint()
and parse_uint_full() to be like all of qemu_strto*(): put the result
parameter last, use the same types (uint64_t and unsigned long long
have the same width, but are not always the same type), and mark
endptr const (this latter change only affects the rare caller of
parse_uint).  Adjust all callers in the tree.

While at it, note that since cutils.c already includes:

    QEMU_BUILD_BUG_ON(sizeof(int64_t) != sizeof(long long));

we are guaranteed that the result of parse_uint* cannot exceed
UINT64_MAX (or the build would have failed), so we can drop
pre-existing dead comparisons in opts-visitor.c that were never false.

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-8-eblake@redhat.com>
[eblake: Drop dead code spotted by Markus]
Signed-off-by: Eric Blake <eblake@redhat.com>
2023-06-02 12:27:19 -05:00
Eric Blake
84760bbca9 cutils: Document differences between parse_uint and qemu_strtou64
These two functions are subtly different, and not just because of
swapped parameter order.  It took me adding better unit tests to
figure out why.  Document the differences to make it more obvious to
developers trying to pick which one to use, as well as to aid in
upcoming semantic changes.

While touching the documentation, adjust a mis-statement: parse_uint
does not return -EINVAL on invalid base, but assert()s, like all the
other qemu_strto* functions that take a base argument.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-7-eblake@redhat.com>
2023-06-02 12:23:33 -05:00
Eric Blake
56ddafde3f cutils: Fix wraparound parsing in qemu_strtoui
While we were matching 32-bit strtol in qemu_strtoi, our use of a
64-bit parse was leaking through for some inaccurate answers in
qemu_strtoui in comparison to a 32-bit strtoul (see the unit test for
examples).  The comment for that function even described what we have
to do for a correct parse, but didn't implement it correctly: since
strtoull checks for overflow against the wrong values and then
negates, we have to temporarily undo negation before checking for
overflow against our desired value.

Our int wrappers would be a lot easier to write if libc had a
guaranteed 32-bit parser even on platforms with 64-bit long.

Whether we parse C2x binary strings like "0b1000" is currently up to
what libc does; our unit tests intentionally don't cover that at the
moment, though.

Fixes: 473a2a331e ("cutils: add qemu_strtoi & qemu_strtoui parsers for int/unsigned int types", v2.12.0)
Signed-off-by: Eric Blake <eblake@redhat.com>
CC: qemu-stable@nongnu.org
Message-Id: <20230522190441.64278-6-eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
2023-06-02 12:23:33 -05:00
Eric Blake
3069522bb9 test-cutils: Test more integer corner cases
We have quite a few undertested and underdocumented integer parsing
corner cases.  To ensure that any changes we make in the code are
intentional rather than accidental semantic changes, it is time to add
more unit tests of existing behavior.

In particular, this demonstrates that parse_uint() and qemu_strtou64()
behave differently.  For "-0", it's hard to argue why parse_uint needs
to reject it (it's not a negative integer), but the documentation sort
of mentions it; but it is intentional that all other negative values
are treated as ERANGE with value 0 (compared to qemu_strtou64()
treating "-2" as success and UINT64_MAX-1, for example).

Also, when mixing overflow/underflow with a check for no trailing
junk, parse_uint_full favors ERANGE over EINVAL, while qemu_strto[iu]*
favor EINVAL.  This behavior is outside the C standard, so we can pick
whatever we want, but it would be nice to be consistent.

Note that C requires that "9223372036854775808" fail strtoll() with
ERANGE/INT64_MAX, but "-9223372036854775808" pass with INT64_MIN; we
weren't testing this.  For strtol(), the behavior depends on whether
long is 32- or 64-bits (the cutoff point either being the same as
strtoll() or at "-2147483648").  Meanwhile, C is clear that
"-18446744073709551615" pass stroull() (but not strtoll) with value 1,
even though we want it to fail parse_uint().  And although
qemu_strtoui() has no C counterpart, it makes more sense if we design
it like 32-bit strtoul() (that is, where "-4294967296" be an alternate
acceptable spelling for "1", but "-0xffffffff00000001" should be
treated as overflow and return 0xffffffff rather than 1).  We aren't
there yet, so some of the tests added in this patch have FIXME
comments.

However, note that C2x will (likely) be adding a SILENT semantic
change, where C17 strtol("0b1", &ep, 2) returns 0 with ep="b1", but
C2x will have it return 1 with ep="".  I did not feel like adding
testing for those corner cases, in part because the next version of C
is not standard and libc support for binary parsing is not yet
wide-spread (as of this patch, glibc.git still misparses bare "0b":
https://sourceware.org/bugzilla/show_bug.cgi?id=30371).

Message-Id: <20230522190441.64278-5-eblake@redhat.com>
[eblake: fix a few typos spotted by Hanna]
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
[eblake: fix typo on platforms with 32-bit long]
Signed-off-by: Eric Blake <eblake@redhat.com>
2023-06-02 11:24:53 -05:00
Eric Blake
d326d03bcd test-cutils: Test integral qemu_strto* value on failures
We are inconsistent on the contents of *value after a strto* parse
failure.  I found the following behaviors:

- parse_uint() and parse_uint_full(), which document that *value is
  slammed to 0 on all EINVAL failures and 0 or UINT_MAX on ERANGE
  failures, and has unit tests for that (note that parse_uint requires
  non-NULL endptr, and does not fail with EINVAL for trailing junk)

- qemu_strtosz(), which leaves *value untouched on all failures (both
  EINVAL and ERANGE), and has unit tests but not documentation for
  that

- qemu_strtoi() and other integral friends, which document *value on
  ERANGE failures but is unspecified on EINVAL (other than implicitly
  by comparison to libc strto*); there, *value is untouched for NULL
  string, slammed to 0 on no conversion, and left at the prefix value
  on NULL endptr; unit tests do not consistently check the value

- qemu_strtod(), which documents *value on ERANGE failures but is
  unspecified on EINVAL; there, *value is untouched for NULL string,
  slammed to 0.0 for no conversion, and left at the prefix value on
  NULL endptr; there are no unit tests (other than indirectly through
  qemu_strtosz)

- qemu_strtod_finite(), which documents *value on ERANGE failures but
  is unspecified on EINVAL; there, *value is left at the prefix for
  'inf' or 'nan' and untouched in all other cases; there are no unit
  tests (other than indirectly through qemu_strtosz)

Upcoming patches will change behaviors for consistency, but it's best
to first have more unit test coverage to see the impact of those
changes.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-4-eblake@redhat.com>
2023-06-02 11:24:18 -05:00
Eric Blake
3b4790d4ec test-cutils: Use g_assert_cmpuint where appropriate
When debugging test failures, seeing unsigned values as large positive
values rather than negative values matters (assuming glib 2.78+; given
that I just fixed a bug in glib 2.76 [1] where g_assert_cmpuint
displays signed instead of unsigned values).  No impact when the test
is passing, but using a consistent style will matter more in upcoming
test additions.  Also, some tests are better with cmphex.

While at it, fix some spacing and minor typing issues spotted nearby.

[1] https://gitlab.gnome.org/GNOME/glib/-/issues/2997

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230522190441.64278-3-eblake@redhat.com>
2023-06-02 11:24:18 -05:00
Eric Blake
3a59259225 test-cutils: Avoid g_assert in unit tests
glib documentation[1] is clear: g_assert() should be avoided in unit
tests because it is ineffective if G_DISABLE_ASSERT is defined; unit
tests should stick to constructs based on g_assert_true() instead.
Note that since commit 262a69f428, we intentionally state that you
cannot define G_DISABLE_ASSERT while building qemu; but our code can
be copied to other projects without that restriction, so we should be
consistent.

For most of the replacements in this patch, using g_assert_cmpstr()
would be a regression in quality - although it would helpfully display
the string contents of both pointers on test failure, here, we really
do care about pointer equality, not just string content equality.  But
when a NULL pointer is expected, g_assert_null works fine.

[1] https://libsoup.org/glib/glib-Testing.html#g-assert

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Hanna Czenczek <hreitz@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20230522190441.64278-2-eblake@redhat.com>
2023-06-02 11:24:18 -05:00
Eric Blake
5cf899e215 qcow2: Explicit mention of padding bytes
Although we already covered the need for padding bytes with our
changes in commit 3ae3fcfa, commit 66fcbca5 (both v5.0.0) added one
byte and relied on the rest of the text for implicitly covering 7
padding bytes.  For consistency with other parts of the header (such
as the header extension format listing padding from n - m, or the
snapshot table entry listing variable padding), we might as well call
out the remaining 7 bytes as padding until such time (as any) as they
gain another meaning.

Signed-off-by: Eric Blake <eblake@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-Id: <20230522184631.47211-1-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
2023-06-02 11:24:18 -05:00
Eric Blake
430746359f iotests: Fix test 104 under NBD
In the past, commit a231cb27 ("iotests: Fix 104 for NBD", v2.3.0)
added an additional filter to _filter_img_info to rewrite NBD URIs
into the expected output form.  This recently broke when we tweaked
tests to run in a per-format directory, which did not match the regex,
because _img_info itself is now already changing
SOCK_DIR=/tmp/tmpphjfbphd/raw-nbd-104 into
/tmp/tmpphjfbphd/IMGFMT-nbd-104 prior to _img_info_filter getting a
chance to further filter things.

While diagnosing the problem, I also noticed some filter lines
rendered completely useless by a typo when we switched from TCP to
Unix sockets for NBD (in shell, '\\+' is different from "\\+" (one
gives two backslash to the regex, matching the literal 2-byte sequence
<\+> after a single digit; the other gives one backslash to the regex,
as the metacharacter \+ to match one or more of <[0-9]>); since the
literal string <nbd://127.0.0.1:0\+> is not a valid URI, that regex
hasn't been matching anything for years so it is fine to just drop it
rather than fix the typo.

Fixes: f3923a72 ("iotests: Switch nbd tests to use Unix rather than TCP", v4.2.0)
Fixes: 5ba7db09 ("iotests: always use a unique sub-directory per test", v8.0.0)
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20230519150216.2599189-1-eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2023-06-02 11:24:18 -05:00
Peter Xu
b861383c26 qtest/migration: Document live=true cases
Document every single live=true use cases on why it should be done in the
live manner.  Also document on the parameter so new precopy cases should
always use live=off unless with explicit reasonings.

Cc: Thomas Huth <thuth@redhat.com>
Cc: Juan Quintela <quintela@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20230601172935.175726-1-peterx@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
2023-06-02 11:46:20 +02:00
Daniel P. Berrangé
3c4fb17723 tests/qtest: make more migration pre-copy scenarios run non-live
There are 27 pre-copy live migration scenarios being tested. In all of
these we force non-convergence and run for one iteration, then let it
converge and wait for completion during the second (or following)
iterations. At 3 mbps bandwidth limit the first iteration takes a very
long time (~30 seconds).

While it is important to test the migration passes and convergence
logic, it is overkill to do this for all 27 pre-copy scenarios. The
TLS migration scenarios in particular are merely exercising different
code paths during connection establishment.

To optimize time taken, switch most of the test scenarios to run
non-live (ie guest CPUs paused) with no bandwidth limits. This gives
a massive speed up for most of the test scenarios.

For test coverage the following scenarios are unchanged

 * Precopy with UNIX sockets
 * Precopy with UNIX sockets and dirty ring tracking
 * Precopy with XBZRLE
 * Precopy with UNIX compress
 * Precopy with UNIX compress (nowait)
 * Precopy with multifd

On a test machine this reduces execution time from 13 minutes to
8 minutes.

Tested-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20230601161347.1803440-10-berrange@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
2023-06-02 11:46:19 +02:00