We have multiple clients of qemu_strtosz (qemu-io, the opts visitor,
the keyval visitor), and it gets annoying that edge-case testing is
impacted by implicit rounding to 53 bits of precision due to parsing
with strtod(). As an example posted by Rich Jones:
$ nbdkit memory $(( 2**63 - 2**30 )) --run \
'build/qemu-io -f raw "$uri" -c "w -P 3 $(( 2**63 - 2**30 - 512 )) 512" '
write failed: Input/output error
because 9223372035781033472 got rounded to 0x7fffffffc0000000 which is
out of bounds.
It is also worth noting that our existing parser, by virtue of using
strtod(), accepts decimal AND hex numbers, even though test-cutils
previously lacked any coverage of the latter until the previous patch.
We do have existing clients that expect a hex parse to work (for
example, iotest 33 using qemu-io -c "write -P 0xa 0x200 0x400"), but
strtod() parses "08" as 8 rather than as an invalid octal number, so
we know there are no clients that depend on octal. Our use of
strtod() also means that "0x1.8k" would actually parse as 1536 (the
fraction is 8/16), rather than 1843 (if the fraction were 8/10); but
as this was not covered in the testsuite, I have no qualms forbidding
hex fractions as invalid, so this patch declares that the use of
fractions is only supported with decimal input, and enhances the
testsuite to document that.
Our previous use of strtod() meant that -1 parsed as a negative; now
that we parse with strtoull(), negative values can wrap around modulo
2^64, so we have to explicitly check whether the user passed in a '-';
and make it consistent to also reject '-0'. This has the minor effect
of treating negative values as EINVAL (with no change to endptr)
rather than ERANGE (with endptr advanced to what was parsed), visible
in the updated iotest output.
We also had no testsuite coverage of "1.1e0k", which happened to parse
under strtod() but is unlikely to occur in practice; as long as we are
making things more robust, it is easy enough to reject the use of
exponents in a strtod parse.
The fix is done by breaking the parse into an integer prefix (no loss
in precision), rejecting negative values (since we can no longer rely
on strtod() to do that), determining if a decimal or hexadecimal parse
was intended (with the new restriction that a fractional hex parse is
not allowed), and where appropriate, using a floating point fractional
parse (where we also scan to reject use of exponents in the fraction).
The bulk of the patch is then updates to the testsuite to match our
new precision, as well as adding new cases we reject (whether they
were rejected or inadvertently accepted before).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20210211204438.1184395-3-eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Options such as "server" or "nowait", that are commonly found in -chardev,
are sugar for "server=on" and "wait=off". This is quite surprising and
also does not have any notion of typing attached. It is even possible to
do "-device e1000,noid" and get a device with "id=off".
Deprecate it and print a warning when it is encountered. In general,
this short form for boolean options only seems to be in wide use for
-chardev and -spice.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
qemu_opts_set is used to create default network backends and to
parse sugar options -kernel, -initrd, -append, -bios and -dtb.
These are very different uses:
I would *expect* a function named qemu_opts_set to set an option in a
merge-lists QemuOptsList, such as -kernel, and possibly to set an option
in a non-merge-lists QemuOptsList with non-NULL id, similar to -set.
However, it wouldn't *work* to use qemu_opts_set for the latter
because qemu_opts_set uses fail_if_exists==1. So, for non-merge-lists
QemuOptsList and non-NULL id, the semantics of qemu_opts_set (fail if the
(QemuOptsList, id) pair already exists) are debatable.
On the other hand, I would not expect qemu_opts_set to create a
non-merge-lists QemuOpts with a single option; which it does, though.
For this case of non-merge-lists QemuOptsList and NULL id, qemu_opts_set
hardly adds value over qemu_opts_parse. It does skip some parsing and
unescaping, but that's not needed when creating default network
backends.
So qemu_opts_set has warty behavior for non-merge-lists QemuOptsList
if id is non-NULL, and it's mostly pointless if id is NULL. My
solution to keeping the API as simple as possible is to limit
qemu_opts_set to merge-lists QemuOptsList. For them, it's useful (we
don't want comma-unescaping for -kernel) *and* has sane semantics.
Network backend creation is switched to qemu_opts_parse.
qemu_opts_set is now only used on merge-lists QemuOptsList... except
in the testcase, which is changed to use a merge-list QemuOptsList.
With this change we can also remove the id parameter. With the
parameter always NULL, we know that qemu_opts_create cannot fail
and can pass &error_abort to it.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Receiving the error in a local variable only to assert there is none
is less clear than passing &error_abort. Clean up.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20200630090351.1247703-5-armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200415074927.19897-6-armbru@redhat.com>
has_help_option() uses its own parser. It's inconsistent with
qemu_opts_parse(), as demonstrated by test-qemu-opts case
/qemu-opts/has_help_option. Fix by reusing the common parser.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200415074927.19897-5-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200415074927.19897-4-armbru@redhat.com>
The two turn out to be inconsistent for "a,b,,help". Test case
marked /* BUG */.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200415074927.19897-2-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Now that we can safely call QOBJECT() on QObject * as well as its
subtypes, we can have macros qobject_ref() / qobject_unref() that work
everywhere instead of having to use QINCREF() / QDECREF() for QObject
and qobject_incref() / qobject_decref() for its subtypes.
The replacement is mechanical, except I broke a long line, and added a
cast in monitor_qmp_cleanup_req_queue_locked(). Unlike
qobject_decref(), qobject_unref() doesn't accept void *.
Note that the new macros evaluate their argument exactly once, thus no
need to shout them.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180419150145.24795-4-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Rebased, semantic conflict resolved, commit message improved]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Basic test for merging two QemuOptsLists.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
qemu-common.h includes qemu/option.h, but most places that include the
former don't actually need the latter. Drop the include, and add it
to the places that actually need it.
While there, drop superfluous includes of both headers, and
separate #include from file comment with a blank line.
This cleanup makes the number of objects depending on qemu/option.h
drop from 4545 (out of 4743) to 284 in my "build everything" tree.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180201111846.21846-20-armbru@redhat.com>
[Semantic conflict with commit bdd6a90a9e in block/nvme.c resolved]
This cleanup makes the number of objects depending on qapi/qmp/qdict.h
drop from 4550 (out of 4743) to 368 in my "build everything" tree.
For qapi/qmp/qobject.h, the number drops from 4552 to 390.
While there, separate #include from file comment with a blank line.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180201111846.21846-13-armbru@redhat.com>
We now have macros in place to make it less verbose to add a scalar
to QDict and QList, so use them.
Patch created mechanically via:
spatch --sp-file scripts/coccinelle/qobject.cocci \
--macro-file scripts/cocci-macro-file.h --dir . --in-place
then touched up manually to fix a couple of '?:' back to original
spacing, as well as avoiding a long line in monitor.c.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20170427215821.19397-7-eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
qemu_opts_parse() interprets "no" as negated empty key. Consistent
with its acceptance of empty keys elsewhere, whatever that's worth.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <1488317230-26248-2-git-send-email-armbru@redhat.com>
parse_option_size()'s checking for overflow and trailing crap is
wrong. Has always been that way. qemu_strtosz() gets it right, so
use that.
This adds support for size suffixes 'P', 'E', and ignores case for all
suffixes, not just 'k'.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1487708048-2131-25-git-send-email-armbru@redhat.com>
parse_option_number() fails to check for these errors after
strtoull(). Has always been broken. Fix that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1487708048-2131-10-git-send-email-armbru@redhat.com>
The new tests demonstrate a few bugs, all clearly marked.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1487708048-2131-2-git-send-email-armbru@redhat.com>
[A few additional test cases squashed in, see
Message-ID: <871supjijq.fsf@dusky.pond.sub.org>]
Remove glib.h includes, as it is provided by osdep.h.
This commit was created with scripts/clean-includes.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Tested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Commit 57cb38b included qapi/error.h into qemu/osdep.h to get the
Error typedef. Since then, we've moved to include qemu/osdep.h
everywhere. Its file comment explains: "To avoid getting into
possible circular include dependencies, this file should not include
any other QEMU headers, with the exceptions of config-host.h,
compiler.h, os-posix.h and os-win32.h, all of which are doing a
similar job to this file and are under similar constraints."
qapi/error.h doesn't do a similar job, and it doesn't adhere to
similar constraints: it includes qapi-types.h. That's in excess of
100KiB of crap most .c files don't actually need.
Add the typedef to qemu/typedefs.h, and include that instead of
qapi/error.h. Include qapi/error.h in .c files that need it and don't
get it now. Include qapi-types.h in qom/object.h for uint16List.
Update scripts/clean-includes accordingly. Update it further to match
reality: replace config.h by config-target.h, add sysemu/os-posix.h,
sysemu/os-win32.h. Update the list of includes in the qemu/osdep.h
comment quoted above similarly.
This reduces the number of objects depending on qapi/error.h from "all
of them" to less than a third. Unfortunately, the number depending on
qapi-types.h shrinks only a little. More work is needed for that one.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
[Fix compilation without the spice devel packages. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.
This commit was created with scripts/clean-includes.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Tested-by: Eric Blake <eblake@redhat.com>
qerror_report_err() is a transitional interface to help with
converting existing monitor commands to QMP. It should not be used
elsewhere.
The only remaining user in qemu-option.c is qemu_opts_parse(). Is it
used in QMP context? If not, we can simply replace
qerror_report_err() by error_report_err().
The uses in qemu-img.c, qemu-io.c, qemu-nbd.c and under tests/ are
clearly not in QMP context.
The uses in vl.c aren't either, because the only QMP command handlers
there are qmp_query_status() and qmp_query_machines(), and they don't
call it.
Remaining uses:
* drive_def(): Command line -drive and such, HMP drive_add and pci_add
* hmp_chardev_add(): HMP chardev-add
* monitor_parse_command(): HMP core
* tmp_config_parse(): Command line -tpmdev
* net_host_device_add(): HMP host_net_add
* net_client_parse(): Command line -net and -netdev
* qemu_global_option(): Command line -global
* vnc_parse_func(): Command line -display, -vnc, default display, HMP
change, QMP change. Bummer.
* qemu_pci_hot_add_nic(): HMP pci_add
* usb_net_init(): Command line -usbdevice, HMP usb_add
Propagate errors through qemu_opts_parse(). Create a convenience
function qemu_opts_parse_noisily() that passes errors to
error_report_err(). Switch all non-QMP users outside tests to it.
That leaves vnc_parse_func(). Propagate errors through it. Since I'm
touching it anyway, rename it to vnc_parse().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
qemu_opt_set() is a wrapper around qemu_opt_set() that reports the
error with qerror_report_err().
Most of its users assume the function can't fail. Make them use
qemu_opt_set_err() with &error_abort, so that should the assumption
ever break, it'll break noisily.
Just two users remain, in util/qemu-config.c. Switch them to
qemu_opt_set_err() as well, then rename qemu_opt_set_err() to
qemu_opt_set().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Return the Error object instead of reporting it with
qerror_report_err().
Change callers that assume the function can't fail to pass
&error_abort, so that should the assumption ever break, it'll break
noisily.
Turns out all callers outside its unit test assume that. We could
drop the Error ** argument, but that would make the interface less
regular, so don't.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Return the Error object instead of reporting it with
qerror_report_err().
Change callers that assume the function can't fail to pass
&error_abort, so that should the assumption ever break, it'll break
noisily.
Turns out all callers outside its unit test assume that. We could
drop the Error ** argument, but that would make the interface less
regular, so don't.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Return the Error object instead of reporting it with
qerror_report_err().
Change callers that assume the function can't fail to pass
&error_abort, so that should the assumption ever break, it'll break
noisily.
Turns out all callers outside its unit test assume that. We could
drop the Error ** argument, but that would make the interface less
regular, so don't.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Smatch also complains about 0 used for pointers, so replace those by
NULL in test-visitor-serialization.c, too.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Cover basic aspects and API usage for QemuOpt. The current implementation
covers the API's planned to be changed by Chunyan Liu in his QEMUOptionParameter
replacement/cleanup job.
Other APIs should be covered in future improvements.
[Squashing in a small fix "QemuOpt: use qemu_find_opts_err() to avoid
output on stderr in tests".
qemu_find_opts() calls error_report() instead of propagating the Error
object. It is undesirable to clutter test case output with error
messages from a passing test.
Use qemu_find_opts_err() to avoid the output on stderr.
--Stefan]
Signed-off-by: Leandro Dorileo <l@dorileo.org>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Eric Blake
Paolo Bonzini
Markus Armbruster
Philippe Mathieu-Daudé
Daniel P. Berrangé
Marc-André Lureau
Kevin Wolf
Peter Maydell
Stefan Weil
Leandro Dorileo