Skip to content
Snippets Groups Projects
  1. Jun 05, 2023
  2. Jun 03, 2023
    • Richard Henderson's avatar
      Merge tag 'migration-20230602-pull-request' of https://gitlab.com/juan.quintela/qemu into staging · 848a6caa
      Richard Henderson authored
      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: default avatarRichard Henderson <richard.henderson@linaro.org>
      848a6caa
  3. Jun 02, 2023
    • Richard Henderson's avatar
      Merge tag 'pull-nbd-2023-06-01-v2' of https://repo.or.cz/qemu/ericb into staging · 24bc242c
      Richard Henderson authored
      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: default avatarRichard Henderson <richard.henderson@linaro.org>
      24bc242c
    • Eric Blake's avatar
      cutils: Improve qemu_strtosz handling of fractions · 42cc08d1
      Eric Blake authored
      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: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarHanna Czenczek <hreitz@redhat.com>
      Message-Id: <20230522190441.64278-20-eblake@redhat.com>
      [eblake: tweak function comment for accuracy]
      42cc08d1
    • Eric Blake's avatar
      cutils: Improve qemu_strtod* error paths · c25b1683
      Eric Blake authored
      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: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarHanna Czenczek <hreitz@redhat.com>
      CC: qemu-stable@nongnu.org
      Message-Id: <20230522190441.64278-19-eblake@redhat.com>
      c25b1683
    • Eric Blake's avatar
      cutils: Use parse_uint in qemu_strtosz for negative rejection · b87ac966
      Eric Blake authored
      
      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: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarHanna Czenczek <hreitz@redhat.com>
      Message-Id: <20230522190441.64278-18-eblake@redhat.com>
      b87ac966
    • Eric Blake's avatar
      cutils: Set value in all integral qemu_strto* error paths · 3c5f2467
      Eric Blake authored
      
      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: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarHanna Czenczek <hreitz@redhat.com>
      Message-Id: <20230522190441.64278-17-eblake@redhat.com>
      3c5f2467
    • Eric Blake's avatar
      cutils: Set value in all qemu_strtosz* error paths · 896fbd90
      Eric Blake authored
      
      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: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarHanna Czenczek <hreitz@redhat.com>
      Message-Id: <20230522190441.64278-16-eblake@redhat.com>
      896fbd90
    • Eric Blake's avatar
      test-cutils: Add more coverage to qemu_strtosz · e1cf34b6
      Eric Blake authored
      
      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: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarHanna Czenczek <hreitz@redhat.com>
      Message-Id: <20230522190441.64278-15-eblake@redhat.com>
      e1cf34b6
    • Eric Blake's avatar
      numa: Check for qemu_strtosz_MiB error · a73049b2
      Eric Blake authored
      
      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: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarHanna Czenczek <hreitz@redhat.com>
      Message-Id: <20230522190441.64278-14-eblake@redhat.com>
      a73049b2
    • Eric Blake's avatar
      cutils: Allow NULL str in qemu_strtosz · f49371ec
      Eric Blake authored
      
      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: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarPhilippe Mathieu-Daudé <philmd@linaro.org>
      Reviewed-by: default avatarHanna Czenczek <hreitz@redhat.com>
      Message-Id: <20230522190441.64278-13-eblake@redhat.com>
      f49371ec
    • Eric Blake's avatar
      test-cutils: Refactor qemu_strtosz tests for less boilerplate · 157367cf
      Eric Blake authored
      
      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: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarHanna Czenczek <hreitz@redhat.com>
      Message-Id: <20230522190441.64278-12-eblake@redhat.com>
      157367cf
    • Eric Blake's avatar
      test-cutils: Prepare for upcoming semantic change in qemu_strtosz · edafce69
      Eric Blake authored
      
      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: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarHanna Czenczek <hreitz@redhat.com>
      Message-Id: <20230522190441.64278-11-eblake@redhat.com>
      edafce69
Loading