summary refs log tree commit diff stats
Commit message (Collapse)AuthorAgeFilesLines
...
| * | tests/qtest: add various qtest_qmp_assert_success() variantsDaniel P. Berrangé2023-06-022-7/+205
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add several counterparts of qtest_qmp_assert_success() that can * Use va_list instead of ... * Accept a list of FDs to send * Return the response data Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Message-Id: <20230601161347.1803440-2-berrange@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com>
* | | Merge tag 'pull-nbd-2023-06-01-v2' of https://repo.or.cz/qemu/ericb into stagingRichard Henderson2023-06-0219-730/+1983
|\ \ \ | |/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | cutils: Improve qemu_strtosz handling of fractionsEric Blake2023-06-022-53/+87
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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]
| * | cutils: Improve qemu_strtod* error pathsEric Blake2023-06-022-40/+55
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | cutils: Use parse_uint in qemu_strtosz for negative rejectionEric Blake2023-06-025-19/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | cutils: Set value in all integral qemu_strto* error pathsEric Blake2023-06-022-28/+38
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | cutils: Set value in all qemu_strtosz* error pathsEric Blake2023-06-022-60/+63
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | test-cutils: Add more coverage to qemu_strtoszEric Blake2023-06-021-11/+129
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | numa: Check for qemu_strtosz_MiB errorEric Blake2023-06-021-2/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | cutils: Allow NULL str in qemu_strtoszEric Blake2023-06-022-2/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | test-cutils: Refactor qemu_strtosz tests for less boilerplateEric Blake2023-06-021-404/+101
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | test-cutils: Prepare for upcoming semantic change in qemu_strtoszEric Blake2023-06-021-15/+27
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | test-cutils: Add coverage of qemu_strtodEric Blake2023-06-021-0/+512
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | cutils: Allow NULL endptr in parse_uint()Eric Blake2023-06-022-24/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | cutils: Adjust signature of parse_uint[_full]Eric Blake2023-06-0212-101/+86
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | cutils: Document differences between parse_uint and qemu_strtou64Eric Blake2023-06-021-8/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | cutils: Fix wraparound parsing in qemu_strtouiEric Blake2023-06-022-17/+28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | test-cutils: Test more integer corner casesEric Blake2023-06-021-63/+862
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | test-cutils: Test integral qemu_strto* value on failuresEric Blake2023-06-021-7/+51
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | test-cutils: Use g_assert_cmpuint where appropriateEric Blake2023-06-021-74/+74
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | test-cutils: Avoid g_assert in unit testsEric Blake2023-06-021-162/+162
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | qcow2: Explicit mention of padding bytesEric Blake2023-06-021-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | iotests: Fix test 104 under NBDEric Blake2023-06-022-4/+3
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* | Merge tag 'migration-20230601-pull-request' of ↵Richard Henderson2023-06-017-71/+66
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | https://gitlab.com/juan.quintela/qemu into staging Migration Pull request (20230601) Hi In this series: - improve background migration (fiona) - improve vmstate failure states (vladimir) - dropped all the RDMA cleanups Please, apply. # -----BEGIN PGP SIGNATURE----- # # iQIzBAABCAAdFiEEGJn/jt6/WMzuA0uC9IfvGFhy1yMFAmR5LAoACgkQ9IfvGFhy # 1yPOuxAA5QzUfswCIWehrkY8FApDjsecPDq5R6hS1p5pDZkHTF5y6j49J93I65o2 # E4qr4l0+DJvBvnxTW29JQ1i0RPqJHnJBFC9Ib4o0NaA/7iRP1sTwYxIN4wWZz6H/ # pqG3oQC0WPPqgj9tHSgDW4TNkFjETYaezTN8nddNmyiaO9UxNuR5ZKbeYMroVlfp # KbnAYfXV6CyXKUZFT32BYcajYBDZAqBCO6y3gEn77KPlT1/TqnucoYEVuNudq5SE # jeCamTzoAQ6SIRFM/eY+aASxdsSryqDS/WLqBFsleXs1kkJ6mkDnNels4HqS+xs9 # p2Vhv/59ktoC57XsRgTgzEklAaSHunZivcQkc5szyGVE5TZyKtWg8WhA6rvlqbjK # lb3kKpvtVi73+pAWU0hhKFdnCrB6ieCHI70CJ5mpiIu3MzLUyrNJOK4FoKNoJDOD # Dp45DK+W/EMg51pXyHJZZqHM1p0GGj0fmhv5T05nJ590fIWV4iqDdHFxsMZ9vEPN # iEvAB7/pXz+yECznDFrp2e047rshOGaKKNSW3zl3/7D32Ds2FKur76dL4BoymztW # HHLxmRWn8HmHMoKYLoawWVmCBsDqy8BFct+rHbA6h/0nSCPYIUCmMrSYVqajUbXD # Ulkh4KNQGoBCzp5Toa0dYEXVc891wVOw4k8PTARwf2OYskSkT88= # =Km5X # -----END PGP SIGNATURE----- # gpg: Signature made Thu 01 Jun 2023 04:38:50 PM 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-20230601-pull-request' of https://gitlab.com/juan.quintela/qemu: migration: stop tracking ram writes when cancelling background migration migration: restore vmstate on migration failure migration: switch from .vm_was_running to .vm_old_state runstate: drop unused runstate_store() migration: never fail in global_state_store() runstate: add runstate_get() Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
| * | migration: stop tracking ram writes when cancelling background migrationFiona Ebner2023-06-021-7/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, it is only done when the iteration finishes successfully. Not cleaning up the userfaultfd write protection can lead to symptoms/issues such as the process hanging in memmove or GDB not being able to attach. Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Message-Id: <20230526115908.196171-1-f.ebner@proxmox.com> Signed-off-by: Juan Quintela <quintela@redhat.com>
| * | migration: restore vmstate on migration failureVladimir Sementsov-Ogievskiy2023-06-022-2/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 1. Otherwise failed migration just drops guest-panicked state, which is not good for management software. 2. We do keep different paused states like guest-panicked during migration with help of global_state state. 3. We do restore running state on source when migration is cancelled or failed. 4. "postmigrate" state is documented as "guest is paused following a successful 'migrate'", so originally it's only for successful path and we never documented current behavior. Let's restore paused states like guest-panicked in case of cancel or fail too. Allow same transitions like for inmigrate state. This commit changes the behavior that was introduced by commit 42da5550d6 "migration: set state to post-migrate on failure" and provides a bit different fix on related https://bugzilla.redhat.com/show_bug.cgi?id=1355683 Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Juan Quintela <quintela@redhat.com> Message-Id: <20230517123752.21615-6-vsementsov@yandex-team.ru> Signed-off-by: Juan Quintela <quintela@redhat.com>
| * | migration: switch from .vm_was_running to .vm_old_stateVladimir Sementsov-Ogievskiy2023-06-022-8/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | No logic change here, only refactoring. That's a preparation for next commit where we finally restore the stopped vm state on migration failure or cancellation. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Juan Quintela <quintela@redhat.com> Message-Id: <20230517123752.21615-5-vsementsov@yandex-team.ru> Signed-off-by: Juan Quintela <quintela@redhat.com>
| * | runstate: drop unused runstate_store()Vladimir Sementsov-Ogievskiy2023-06-022-13/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | The function is unused since previous commit. Drop it. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Juan Quintela <quintela@redhat.com> Message-Id: <20230517123752.21615-4-vsementsov@yandex-team.ru> Signed-off-by: Juan Quintela <quintela@redhat.com>
| * | migration: never fail in global_state_store()Vladimir Sementsov-Ogievskiy2023-06-024-41/+33
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Actually global_state_store() can never fail. Let's get rid of extra error paths. To make things clear, use new runstate_get() and use same approach for global_state_store() and global_state_store_running(). Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Juan Quintela <quintela@redhat.com> Message-Id: <20230517123752.21615-3-vsementsov@yandex-team.ru> Signed-off-by: Juan Quintela <quintela@redhat.com>
| * | runstate: add runstate_get()Vladimir Sementsov-Ogievskiy2023-06-022-0/+6
|/ / | | | | | | | | | | | | | | | | | | It's necessary to restore the state after failed/cancelled migration in further commit. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Juan Quintela <quintela@redhat.com> Message-Id: <20230517123752.21615-2-vsementsov@yandex-team.ru> Signed-off-by: Juan Quintela <quintela@redhat.com>
* | Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into stagingRichard Henderson2023-06-0120-265/+293
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Pull request - Stefano Garzarella's blkio block driver 'fd' parameter - My thread-local blk_io_plug() series # -----BEGIN PGP SIGNATURE----- # # iQEzBAABCAAdFiEEhpWov9P5fNqsNXdanKSrs4Grc8gFAmR4uHoACgkQnKSrs4Gr # c8hFBAgAo+SFrOteYgdELM9s0EWb0AU39MTOyNXW7i5mPZNXrn5J7pfRD/5wvI6l # wl5GNMQ+M5HVYO7CumKWr4M1IpKV5Jin6FN/2h15fWkeg17lBOmNHUF+LctLYQbq # HwtNA4hdw1+SEv8kQLBgiqSJMqWcn80X09emgPMCIwET9zxokRYwVjQJx2alM5bd # SqgitDp5qlHyj5HQPX2orT9KrXYWQdGr8i50bn0S67r1wdqTRMu93wrWdEUUncId # 7otlUaq8cARbRMJzIwDmy/cF24Ynr0wCJb4aHW+trRtf+PNgx1Ki+YOiz+LFyjq7 # t6KOMeignzhz9Uzq8EVG4XW8SHpGkw== # =Ms48 # -----END PGP SIGNATURE----- # gpg: Signature made Thu 01 Jun 2023 08:25:46 AM PDT # gpg: using RSA key 8695A8BFD3F97CDAAC35775A9CA4ABB381AB73C8 # gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>" [full] # gpg: aka "Stefan Hajnoczi <stefanha@gmail.com>" [full] * tag 'block-pull-request' of https://gitlab.com/stefanha/qemu: qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpa block/blkio: use qemu_open() to support fd passing for virtio-blk block: remove bdrv_co_io_plug() API block/linux-aio: convert to blk_io_plug_call() API block/io_uring: convert to blk_io_plug_call() API block/blkio: convert to blk_io_plug_call() API block/nvme: convert to blk_io_plug_call() API block: add blk_io_plug_call() API Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
| * | qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpaStefano Garzarella2023-06-012-0/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The virtio-blk-vhost-vdpa driver in libblkio 1.3.0 supports the fd passing through the new 'fd' property. Since now we are using qemu_open() on '@path' if the virtio-blk driver supports the fd passing, let's announce it. In this way, the management layer can pass the file descriptor of an already opened vhost-vdpa character device. This is useful especially when the device can only be accessed with certain privileges. Add the '@fdset' feature only when the virtio-blk-vhost-vdpa driver in libblkio supports it. Suggested-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-id: 20230530071941.8954-3-sgarzare@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * | block/blkio: use qemu_open() to support fd passing for virtio-blkStefano Garzarella2023-06-011-9/+44
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some virtio-blk drivers (e.g. virtio-blk-vhost-vdpa) supports the fd passing. Let's expose this to the user, so the management layer can pass the file descriptor of an already opened path. If the libblkio virtio-blk driver supports fd passing, let's always use qemu_open() to open the `path`, so we can handle fd passing from the management layer through the "/dev/fdset/N" special path. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-id: 20230530071941.8954-2-sgarzare@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * | block: remove bdrv_co_io_plug() APIStefan Hajnoczi2023-06-013-51/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | No block driver implements .bdrv_co_io_plug() anymore. Get rid of the function pointers. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Acked-by: Kevin Wolf <kwolf@redhat.com> Message-id: 20230530180959.1108766-7-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * | block/linux-aio: convert to blk_io_plug_call() APIStefan Hajnoczi2023-06-013-65/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Stop using the .bdrv_co_io_plug() API because it is not multi-queue block layer friendly. Use the new blk_io_plug_call() API to batch I/O submission instead. Note that a dev_max_batch check is dropped in laio_io_unplug() because the semantics of unplug_fn() are different from .bdrv_co_unplug(): 1. unplug_fn() is only called when the last blk_io_unplug() call occurs, not every time blk_io_unplug() is called. 2. unplug_fn() is per-thread, not per-BlockDriverState, so there is no way to get per-BlockDriverState fields like dev_max_batch. Therefore this condition cannot be moved to laio_unplug_fn(). It is not obvious that this condition affects performance in practice, so I am removing it instead of trying to come up with a more complex mechanism to preserve the condition. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Acked-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Message-id: 20230530180959.1108766-6-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * | block/io_uring: convert to blk_io_plug_call() APIStefan Hajnoczi2023-06-014-47/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Stop using the .bdrv_co_io_plug() API because it is not multi-queue block layer friendly. Use the new blk_io_plug_call() API to batch I/O submission instead. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Acked-by: Kevin Wolf <kwolf@redhat.com> Message-id: 20230530180959.1108766-5-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * | block/blkio: convert to blk_io_plug_call() APIStefan Hajnoczi2023-06-011-19/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Stop using the .bdrv_co_io_plug() API because it is not multi-queue block layer friendly. Use the new blk_io_plug_call() API to batch I/O submission instead. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Acked-by: Kevin Wolf <kwolf@redhat.com> Message-id: 20230530180959.1108766-4-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * | block/nvme: convert to blk_io_plug_call() APIStefan Hajnoczi2023-06-012-33/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Stop using the .bdrv_co_io_plug() API because it is not multi-queue block layer friendly. Use the new blk_io_plug_call() API to batch I/O submission instead. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Acked-by: Kevin Wolf <kwolf@redhat.com> Message-id: 20230530180959.1108766-3-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * | block: add blk_io_plug_call() APIStefan Hajnoczi2023-06-018-41/+173
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Introduce a new API for thread-local blk_io_plug() that does not traverse the block graph. The goal is to make blk_io_plug() multi-queue friendly. Instead of having block drivers track whether or not we're in a plugged section, provide an API that allows them to defer a function call until we're unplugged: blk_io_plug_call(fn, opaque). If blk_io_plug_call() is called multiple times with the same fn/opaque pair, then fn() is only called once at the end of the function - resulting in batching. This patch introduces the API and changes blk_io_plug()/blk_io_unplug(). blk_io_plug()/blk_io_unplug() no longer require a BlockBackend argument because the plug state is now thread-local. Later patches convert block drivers to blk_io_plug_call() and then we can finally remove .bdrv_co_io_plug() once all block drivers have been converted. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Acked-by: Kevin Wolf <kwolf@redhat.com> Message-id: 20230530180959.1108766-2-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
* | | Merge tag 'tracing-pull-request' of https://gitlab.com/stefanha/qemu into ↵Richard Henderson2023-06-0128-497/+94
|\ \ \ | |/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | staging Pull request This pull request contains Alex Bennée's vcpu trace events removal patches. # -----BEGIN PGP SIGNATURE----- # # iQEzBAABCAAdFiEEhpWov9P5fNqsNXdanKSrs4Grc8gFAmR4tAMACgkQnKSrs4Gr # c8ht/AgAiVslnH4vmD5IZloBHVRNEZKifODZbHW75yDgIirj/MhqlXPZ7bWoGwTN # MLsTVuihhYnJBQKknN7lKyhkoQjgiJSkYhQbXSlcN7T3UE0+iG47FSudYTLDZSov # M5wu1Edzi4q1uWr7ZIn/NS39iHVvQ7fdDMosHQmI0HKl25yx5936c0T2A4yyj96e # LEtg4wLKo1uRgEMvCWrpiDz8ohNVwexAxCggwHE17tCebBmik+2cBEWAS+fcTbSr # Nx3yWRat5VbqHOe3ghudLMNXHySQjNYrexULOVzyUUoaqUDt2eWCr9A4312BflEl # 8U9FFl99BZX5rWkyUzsHxEmPlRsazQ== # =oMRe # -----END PGP SIGNATURE----- # gpg: Signature made Thu 01 Jun 2023 08:06:43 AM PDT # gpg: using RSA key 8695A8BFD3F97CDAAC35775A9CA4ABB381AB73C8 # gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>" [full] # gpg: aka "Stefan Hajnoczi <stefanha@gmail.com>" [full] * tag 'tracing-pull-request' of https://gitlab.com/stefanha/qemu: accel/tcg: include cs_base in our hash calculations hw/9pfs: use qemu_xxhash4 tcg: remove the final vestiges of dstate trace: remove control-vcpu.h trace: remove code that depends on setting vcpu qapi: make the vcpu parameters deprecated for 8.1 docs/deprecated: move QMP events bellow QMP command section scripts/qapi: document the tool that generated the file trace: remove vcpu_id from the TraceEvent structure trace-events: remove the remaining vcpu trace events *-user: remove the guest_user_syscall tracepoints Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
| * | accel/tcg: include cs_base in our hash calculationsAlex Bennée2023-06-015-12/+23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We weren't using cs_base in the hash calculations before. Since the arm front end moved a chunk of flags in a378206a20 (target/arm: Move mode specific TB flags to tb->cs_base) they comprise of an important part of the execution state. Widen the tb_hash_func to include cs_base and expand to qemu_xxhash8() to accommodate it. My initial benchmark shows very little difference in the runtime. Before: armhf ➜ hyperfine -w 2 -m 20 "./arm-softmmu/qemu-system-arm -cpu cortex-a15 -machine type=virt,highmem=off -display none -m 2048 -serial mon:stdio -netdev user,id=unet,hostfwd=tcp::2222-:22 -device virtio-net-pci,netdev=unet -device virtio-scsi-pci -blockdev driver=raw,node-name=hd,discard=unmap,file.driver=host_device,file.filename=/dev/zen-disk/debian-bullseye-armhf -device scsi-hd,drive=hd -smp 4 -kernel /home/alex/lsrc/linux.git/builds/arm/arch/arm/boot/zImage -append 'console=ttyAMA0 root=/dev/sda2 systemd.unit=benchmark.service' -snapshot" Benchmark 1: ./arm-softmmu/qemu-system-arm -cpu cortex-a15 -machine type=virt,highmem=off -display none -m 2048 -serial mon:stdio -netdev user,id=unet,hostfwd=tcp::2222-:22 -device virtio-net-pci,netdev=unet -device virtio-scsi-pci -blockdev driver=raw,node-name=hd,discard=unmap,file.driver=host_device,file.filename=/dev/zen-disk/debian-bullseye-armhf -device scsi-hd,drive=hd -smp 4 -kernel /home/alex/lsrc/linux.git/builds/arm/arch/arm/boot/zImage -append 'console=ttyAMA0 root=/dev/sda2 systemd.unit=benchmark.service' -snapshot Time (mean ± σ): 24.627 s ± 2.708 s [User: 34.309 s, System: 1.797 s] Range (min … max): 22.345 s … 29.864 s 20 runs arm64 ➜ hyperfine -w 2 -n 20 "./qemu-system-aarch64 -cpu max,pauth-impdef=on -machine type=virt,virtualization=on,gic-version=3 -display none -serial mon:stdio -netdev user,id=unet,hostfwd=tcp::2222-:22,hostfwd=tcp::1234-:1234 -device virtio-net-pci,netdev=unet -device virtio-scsi-pci -blockdev driver=raw,node-name=hd,discard=unmap,file.driver=host_device,file.filename=/dev/zen-disk/debian-bullseye-arm64 -device scsi-hd,drive=hd -smp 4 -kernel ~/lsrc/linux.git/builds/arm64/arch/arm64/boot/Image.gz -append 'console=ttyAMA0 root=/dev/sda2 systemd.unit=benchmark-pigz.service' -snapshot" Benchmark 1: 20 Time (mean ± σ): 62.559 s ± 2.917 s [User: 189.115 s, System: 4.089 s] Range (min … max): 59.997 s … 70.153 s 10 runs After: armhf Benchmark 1: ./arm-softmmu/qemu-system-arm -cpu cortex-a15 -machine type=virt,highmem=off -display none -m 2048 -serial mon:stdio -netdev user,id=unet,hostfwd=tcp::2222-:22 -device virtio-net-pci,netdev=unet -device virtio-scsi-pci -blockdev driver=raw,node-name=hd,discard=unmap,file.driver=host_device,file.filename=/dev/zen-disk/debian-bullseye-armhf -device scsi-hd,drive=hd -smp 4 -kernel /home/alex/lsrc/linux.git/builds/arm/arch/arm/boot/zImage -append 'console=ttyAMA0 root=/dev/sda2 systemd.unit=benchmark.service' -snapshot Time (mean ± σ): 24.223 s ± 2.151 s [User: 34.284 s, System: 1.906 s] Range (min … max): 22.000 s … 28.476 s 20 runs arm64 hyperfine -w 2 -n 20 "./qemu-system-aarch64 -cpu max,pauth-impdef=on -machine type=virt,virtualization=on,gic-version=3 -display none -serial mon:stdio -netdev user,id=unet,hostfwd=tcp::2222-:22,hostfwd=tcp::1234-:1234 -device virtio-net-pci,netdev=unet -device virtio-scsi-pci -blockdev driver=raw,node-name=hd,discard=unmap,file.driver=host_device,file.filename=/dev/zen-disk/debian-bullseye-arm64 -device scsi-hd,drive=hd -smp 4 -kernel ~/lsrc/linux.git/builds/arm64/arch/arm64/boot/Image.gz -append 'console=ttyAMA0 root=/dev/sda2 systemd.unit=benchmark-pigz.service' -snapshot" Benchmark 1: 20 Time (mean ± σ): 62.769 s ± 1.978 s [User: 188.431 s, System: 5.269 s] Range (min … max): 60.285 s … 66.868 s 10 runs Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20230526165401.574474-12-alex.bennee@linaro.org Message-Id: <20230524133952.3971948-11-alex.bennee@linaro.org> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * | hw/9pfs: use qemu_xxhash4Alex Bennée2023-06-011-3/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | No need to pass zeros as we have helpers that do that for us. Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20230526165401.574474-11-alex.bennee@linaro.org Message-Id: <20230524133952.3971948-10-alex.bennee@linaro.org> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * | tcg: remove the final vestiges of dstateAlex Bennée2023-06-016-26/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now we no longer have dynamic state affecting things we can remove the additional fields in cpu.h and simplify the TB hash calculation. For the benchmark: hyperfine -w 2 -m 20 \ "./arm-softmmu/qemu-system-arm -cpu cortex-a15 \ -machine type=virt,highmem=off \ -display none -m 2048 \ -serial mon:stdio \ -netdev user,id=unet,hostfwd=tcp::2222-:22 \ -device virtio-net-pci,netdev=unet \ -device virtio-scsi-pci \ -blockdev driver=raw,node-name=hd,discard=unmap,file.driver=host_device,file.filename=/dev/zen-disk/debian-bullseye-armhf \ -device scsi-hd,drive=hd -smp 4 \ -kernel /home/alex/lsrc/linux.git/builds/arm/arch/arm/boot/zImage \ -append 'console=ttyAMA0 root=/dev/sda2 systemd.unit=benchmark.service' \ -snapshot" It has a marginal effect on runtime, before: Time (mean ± σ): 26.279 s ± 2.438 s [User: 41.113 s, System: 1.843 s] Range (min … max): 24.420 s … 32.565 s 20 runs after: Time (mean ± σ): 24.440 s ± 2.885 s [User: 34.474 s, System: 2.028 s] Range (min … max): 21.663 s … 29.937 s 20 runs Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1358 Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20230526165401.574474-10-alex.bennee@linaro.org Message-Id: <20230524133952.3971948-9-alex.bennee@linaro.org> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * | trace: remove control-vcpu.hAlex Bennée2023-06-013-52/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now we no longer have vcpu controlled trace events we can excise the code that allows us to query its status. Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20230526165401.574474-9-alex.bennee@linaro.org Message-Id: <20230524133952.3971948-8-alex.bennee@linaro.org> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * | trace: remove code that depends on setting vcpuAlex Bennée2023-06-019-285/+20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now we no longer have any events that are for vcpus we can start excising the code from the trace control. As the vcpu parameter is encoded as part of QMP we just stub out the has_vcpu/vcpu parameters rather than alter the API. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20230526165401.574474-8-alex.bennee@linaro.org Message-Id: <20230524133952.3971948-7-alex.bennee@linaro.org> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * | qapi: make the vcpu parameters deprecated for 8.1Alex Bennée2023-06-012-23/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I don't think I can remove the parameters directly but certainly mark them as deprecated. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20230526165401.574474-7-alex.bennee@linaro.org Message-Id: <20230524133952.3971948-6-alex.bennee@linaro.org> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * | docs/deprecated: move QMP events bellow QMP command sectionAlex Bennée2023-06-011-9/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Also rename the section to make the fact this is part of the management protocol even clearer. Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20230526165401.574474-6-alex.bennee@linaro.org Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * | scripts/qapi: document the tool that generated the fileAlex Bennée2023-06-011-2/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This makes it a little easier for developers to find where things where being generated. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20230526165401.574474-5-alex.bennee@linaro.org Message-Id: <20230524133952.3971948-5-alex.bennee@linaro.org> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * | trace: remove vcpu_id from the TraceEvent structureAlex Bennée2023-06-015-30/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This does involve temporarily stubbing out some helper functions before we excise the rest of the code. Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20230526165401.574474-4-alex.bennee@linaro.org Message-Id: <20230524133952.3971948-4-alex.bennee@linaro.org> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * | trace-events: remove the remaining vcpu trace eventsAlex Bennée2023-06-015-36/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While these are all in helper functions being designated vcpu events complicates the removal of the dynamic vcpu state code. TCG plugins allow you to instrument vcpu_[init|exit|idle]. We rename cpu_reset and make it a normal trace point. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-id: 20230526165401.574474-3-alex.bennee@linaro.org Message-Id: <20230524133952.3971948-3-alex.bennee@linaro.org> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>