If the .data field of a QAPI Union is NULL, we don't need to free
any of the union fields.
Make use of the new visit_start_union interface to access this
information and instruct the generated code to not visit these
fields when this occurs.
Cc: qemu-stable@nongnu.org
Reported-by: Fam Zheng <famz@redhat.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
In some cases an input visitor might bail out on filling out a
struct for various reasons, such as missing fields when running
in strict mode. In the case of a QAPI Union type, this may lead
to cases where the .kind field which encodes the union type
is uninitialized. Subsequently, other visitors, such as the
dealloc visitor, may use this .kind value as if it were
initialized, leading to assumptions about the union type which
in this case may lead to segfaults. For example, freeing an
integer value.
However, we can generally rely on the fact that the always-present
.data void * field that we generate for these union types will
always be NULL in cases where .kind is uninitialized (at least,
there shouldn't be a reason where we'd do this purposefully).
So pass this information on to Visitor implementation via these
optional start_union/end_union interfaces so this information
can be used to guard against the situation above. We will make
use of this information in a subsequent patch for the dealloc
visitor.
Cc: qemu-stable@nongnu.org
Reported-by: Fam Zheng <famz@redhat.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
This patch removes support for the cow file format.
Normally we do not break backwards compatibility but in this case there
is no impact and it is the most logical option. Extraordinary claims
require extraordinary evidence so I will show why removing the cow block
driver is the right thing to do.
The cow file format is the disk image format for Usermode Linux, a way
of running a Linux system in userspace. The performance of UML was
never great and it was hacky, but it enjoyed some popularity before
hardware virtualization support became mainstream.
QEMU's block/cow.c is supposed to read this image file format.
Unfortunately the file format was underspecified:
1. Earlier Linux versions used the MAXPATHLEN constant for the backing
filename field. The value of MAXPATHLEN can change, so Linux
switched to a 4096 literal but QEMU has a 1024 literal.
2. Padding was not used on the header struct (both in the Linux kernel
and in QEMU) so the struct layout varied across architectures. In
particular, i386 and x86_64 were different due to int64_t alignment
differences. Linux now uses __attribute__((packed)), QEMU does not.
Therefore:
1. QEMU cow images do not conform to the Linux cow image file format.
2. cow images cannot be shared between different host architectures.
This means QEMU cow images are useless and QEMU has not had bug reports
from users actually hitting these issues.
Let's get rid of this thing, it serves no purpose and no one will be
affected.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-id: 1410877464-20481-1-git-send-email-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
qcow2 supports more than four options by now, add the new options
(overlap check mode and metadata cache size)
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1408557576-14574-5-git-send-email-mreitz@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Not every BLOCK_IMAGE_CORRUPTED event must be fatal; for example, when
reading from an image, they should generally not be. Nonetheless, even
an image only read from may of course be corrupted and this can be
detected during normal operation. In this case, a non-fatal event should
be emitted, but the image should not be marked corrupt (in accordance to
"fatal" set to false).
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1409926039-29044-2-git-send-email-mreitz@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
This is an analogue to Linux null_blk. It can be used for testing or
benchmarking block device emulation and general block layer
functionalities such as coroutines and throttling, where disk IO is not
necessary or wanted.
Use null-aio:// for AIO version, and null-co:// for coroutine version.
[Resolved conflict with Fam's async bdrv_aio_cancel() series:
1. Drop .bdrv_aio_cancel() since it is now done by block.c
2. Rename qemu_aio_release() to qemu_aio_unref()
--Stefan]
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1410415798-20673-2-git-send-email-famz@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
This patch prepares for the subsequent patches.
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
BLOCK_IO_ERROR events are logged by libvirt, which helps with
post mortem analysis of guests. However, one information that
we miss today is a human readable string describing the cause
of the I/O error.
This commit adds that string it to BLOCK_IO_ERROR. Note that
this string is a debugging aid for humans, meaning that it
should not parsed by applications.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Management software, such as RHEV's vdsm, want to be able to allocate
disk space on demand. The basic use case is to start a VM with a small
disk and then the disk is enlarged when QEMU hits a ENOSPC condition.
To this end, the management software has to be notified when QEMU
encounters ENOSPC. The solution implemented by this commit is simple:
it extends the BLOCK_IO_ERROR with a 'nospace' key, which is true
when QEMU is stopped due to ENOSPC.
Note that support for querying this event is already present in
query-block by means of the 'io-status' key. Also, the new 'nospace'
BLOCK_IO_ERROR field shares the same semantics with 'io-status',
which basically means that werror= has to be set to either
'stop' or 'enospc' to enable 'nospace'.
Finally, this commit also updates the 'io-status' key doc in the
schema with a list of supported device models.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
relaxing the license to LGPLv2+ is intentional.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Signed-off-by: Peter Lieven <pl@kamp.de>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Benoit Canet <benoit.canet@nodalink.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Benoît Canet <benoit.canet@nodalink.com>
Signed-off-by: Liu Yuan <namei.unix@gmail.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Introduce new enum BlockdevOptionsArchipelago.
@volume: #Name of the Archipelago volume image
@mport: #'mport' is the port number on which mapperd is
listening. This is optional and if not specified,
QEMU will make Archipelago to use the default port.
@vport: #'vport' is the port number on which vlmcd is
listening. This is optional and if not specified,
QEMU will make Archipelago to use the default port.
@segment: #optional The name of the shared memory segment
Archipelago stack is using. This is optional
and if not specified, QEMU will make Archipelago
use the default value, 'archipelago'.
Signed-off-by: Chrysostomos Nanakos <cnanakos@grnet.gr>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Add nocow info in 'qemu-img info' output to show whether the file
currently has NOCOW flag set or not.
Signed-off-by: Chunyan Liu <cyliu@suse.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
On some image chains, QEMU may not always be able to resolve the
filenames properly, when updating the backing file of an image
after a block job.
For instance, certain relative pathnames may fail, or drives may
have been specified originally by file descriptor (e.g. /dev/fd/???),
or a relative protocol pathname may have been used.
In these instances, QEMU may lack the information to be able to make
the correct choice, but the user or management layer most likely does
have that knowledge.
With this extension to the block-stream api, the user is able to change
the backing file of the active layer as part of the block-stream
operation.
This allows the change to be 'safe', in the sense that if the attempt
to write the active image metadata fails, then the block-stream
operation returns failure, without disrupting the guest.
If a backing file string is not specified in the command, the backing
file string to use is determined in the same manner as it was
previously.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
On some image chains, QEMU may not always be able to resolve the
filenames properly, when updating the backing file of an image
after a block commit.
For instance, certain relative pathnames may fail, or drives may
have been specified originally by file descriptor (e.g. /dev/fd/???),
or a relative protocol pathname may have been used.
In these instances, QEMU may lack the information to be able to make
the correct choice, but the user or management layer most likely does
have that knowledge.
With this extension to the block-commit api, the user is able to change
the backing file of the overlay image as part of the block-commit
operation.
This allows the change to be 'safe', in the sense that if the attempt
to write the overlay image metadata fails, then the block-commit
operation returns failure, without disrupting the guest.
If the commit top is the active layer, then specifying the backing
file string will be treated as an error (there is no overlay image
to modify in that case).
If a backing file string is not specified in the command, the backing
file string to use is determined in the same manner as it was
previously.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
This allows a user to make a live change to the backing file recorded in
an open image.
The image file to modify can be specified 2 ways:
1) image filename
2) image node-name
Note: this does not cause the backing file itself to be reopened; it
merely changes the backing filename in the image file structure, and
in internal BDS structures.
It is the responsibility of the user to pass a filename string that
can be resolved when the image chain is reopened, and the filename
string is not validated.
A good analogy for this command is that it is a live version of
'qemu-img rebase -u', with respect to changing the backing file string.
[Jeff is offline so I respun this patch in his absence. Dropped image
filename since using node-name is preferred and this is a new command.
No need to introduce the limitations of finding images by filename.
--Stefan]
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
fe069d9d had aligned code and documentation while dropping the s from the
actual JSON output. Fix that.
This also fix test/qemu-iotest/081 since the missing s was causing a permutation.
Signed-off-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Now that active layer block-commit is supported, the 'top' argument
no longer needs to be mandatory.
Change it to optional, with the default being the active layer in the
device chain.
[kwolf: Rebased and resolved conflict in tests/qemu-iotests/040]
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (GNU/Linux)
iQIcBAABAgAGBQJTrbz4AAoJEH8JsnLIjy/WlIkP/RepIwS29f19i3B/idGzUdYW
9XJnVowRvpkUzDqUprrr7lPHMW/CwAswLNis9B1hZ59rx+tx4Hm/rZGARlqhSOSO
ZMdW32GFW0SyC5PglFBwGQAk4U0FxwW5cJD6US7h3L4pACIdCkzFSNxehyfCMyU/
oJkjuAH4a2IQoQf/M7WMm5kPkrdpRp6ZgbQvJGHaR63cuulZDb7rbHMyG66MWH8P
wahhFFPY1wOeMBiISxPbmcTus+AlfCffG5qPqq83OtaIuWzINTmWlpiFmtx+Aqwy
HSvGnFJ4Rf7J6Fw8sdTsABdqUTc/gxDYmhAuftm/hsjD9MvPeuFSLPMPLfGg6aPR
umKaeBOw8NoMTPgbxg403gxFTrHar+TidBu8KgZw5T189/oJSSpT2J53uHWazmd9
8USkcYQ7VHdFUQVXluLEzHMIWc7kf87ylQ8c9S1yCkNeWYxRZDZGgHEU49ov7FFU
FnA0w+ZFyDkU8d5gryG+vxOeBDlmXD4UHa676gGlaYhs7YC/BY/JaMgqY4Fd6MMW
dS5ibPjdtbxEZTh29eWEByMWpzuitr+iPPzsJEdC29LeIIj3XRQq/4FyiQ6EMAAO
iOlcqE3tws0Ty8GEp78xsAYjaLuH3zmvOTa4aHUQ+K9kwpMPFSJKEcLkwPWWYRbs
qR2ZL6M+95oQTYkYzv8i
=Wwqx
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging
Block patches for 2.1.0-rc0
# gpg: Signature made Fri 27 Jun 2014 19:50:32 BST using RSA key ID C88F2FD6
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>"
* remotes/kevin/tags/for-upstream: (47 commits)
iotests: Fix 083 for out-of-tree builds
iotests: Drop Python version from 065's Shebang
iotests: Use $PYTHON for Python scripts
iotests: Source common.env
configure: Enable out-of-tree iotests
iotests: Allow out-of-tree run
block.c: Don't return success for bdrv_append_temp_snapshot() failure
qemu-iotests: Add TestRepairQuorum to 041 to test drive-mirror node-name mode.
block: Add replaces argument to drive-mirror
blockjob: Fix recent BLOCK_JOB_ERROR regression
blockjob: Fix recent BLOCK_JOB_READY regression
virtio-blk: Rename complete_request_early to complete_request_vring
virtio-blk: Unify {non-,}dataplane's request handlings
virtio-blk: Schedule BH in the right context
virtio-blk: Export request handling functions to dataplane
virtio-blk: Make request completion function virtual
block: acquire AioContext in qmp_query_blockstats()
block: make bdrv_query_stats() static
virtio-blk: Fix and clean up the in_sg and out_sg check
virtio-blk: Fill in VirtIOBlockReq.out in dataplane code
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
drive-mirror will bdrv_swap the new BDS named node-name with the one
pointed by replaces when the mirroring is finished.
Signed-off-by: Benoit Canet <benoit@irqsave.net>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit 5a2d2cb screwed up the the value of members device and action,
breaking tests/qemu-iotests/041.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Tested-By: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit bcada37 dropped the (up to now undocumented) members type, len,
offset, speed, breaking tests/qemu-iotests/040 and 041.
Restore and document them. This fixes 040, and partially fixes 041.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Tested-By: Benoit Canet <benoit@irqsave.net>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Libvirt wants to know about the guest-side connection state of some
virtio-serial ports (in particular the one(s) assigned to guest agent(s)).
Report such states with a new monitor event.
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1080376
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
This patch improves docs and address small issues in event
callers.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
This new argument can be used to specify the node-name of the new mirrored BDS.
Signed-off-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
On read operations when this parameter is set and some replicas are corrupted
while quorum can be reached quorum will proceed to rewrite the correct version
of the data to fix the corrupted replicas.
This will shine with SSD where the FTL will remap the same block at another
place on rewrite.
Signed-off-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Since BLOCK_JOB_COMPLETED, BLOCK_JOB_CANCELLED, BLOCK_JOB_READY are
related, convert them in one patch. The block_job_event_* functions
are used to keep encapsulation of BlockJob structure.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
In order to let event defines use existing types later, instead of
redefine new ones, some old type defines for spice and vnc are changed,
and BlockErrorAction is moved from block.h to qapi schema. Note that
BlockErrorAction is not merged with BlockdevOnError.
At this point, VncInfo is not made a child of VncBasicInfo, because
VncBasicInfo has mandatory fields where VncInfo makes them optional.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
This file holds some functions that do not need to be generated.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
"0x1-0x10" looks better than "0x1-10"
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
in human mode, we are creating the string:
16-31 (16-31)
instead of
16-17 (10-1f)
because we forgot to pass 'true' as the human parameter on one of the
two calls to format_string.
Also, this is a worsening of quality; previously we would produce
16 (0x10)
to make it obvious which number was hex.
Fix these issues.
Reported-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Remove dead code. Reset errno to 0 before each strtoull call, as the
man page requires.
Reported-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
The following commits:
qapi: make string output visitor parse int list
qapi: make string input visitor parse int list
break with glib < 2.28 since they use the
new g_list_free_full function.
Open-code that to fix build on old systems.
Cc: Hu Tao <hutao@cn.fujitsu.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Tested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
MST: split up patch
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Tested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
MST: split up patch
qemu_opt_del() already assumes that all QemuOpt instances contain
malloc'd name and value; but it had to cast away const because
opts_start_struct() was doing its own thing and using static storage
instead. By using the correct type and malloced strings everywhere, the
usage of this struct becomes clearer.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Leandro Dorileo <l@dorileo.org>
Signed-off-by: Chunyan Liu <cyliu@suse.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
qapi/block-core.json contains block definitions unrelated to emulation.
qapi/block.json is a superset of the previous and contains definitions related
to emulation.
The purpose of these extractions is to be able to hook qapi/block-core.json
generated code on qemu-nbd.
Signed-off-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
A NULL value is not added to visitor's stack, but there
is no check for that when the visitor tries to return
that value, leading to QEMU crash.
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
We commonly use the error API like this:
err = NULL;
foo(..., &err);
if (err) {
goto out;
}
bar(..., &err);
Every error source is checked separately. The second function is only
called when the first one succeeds. Both functions are free to pass
their argument to error_set(). Because error_set() asserts no error
has been set, this effectively means they must not be called with an
error set.
The qapi-generated code uses the error API differently:
// *errp was initialized to NULL somewhere up the call chain
frob(..., errp);
gnat(..., errp);
Errors accumulate in *errp: first error wins, subsequent errors get
dropped. To make this work, the second function does nothing when
called with an error set. Requires non-null errp, or else the second
function can't see the first one fail.
This usage has also bled into visitor tests, and two device model
object property getters rtc_get_date() and balloon_stats_get_all().
With the "accumulate" technique, you need fewer error checks in
callers, and buy that with an error check in every callee. Can be
nice.
However, mixing the two techniques is confusing. You can't use the
"accumulate" technique with functions designed for the "check
separately" technique. You can use the "check separately" technique
with functions designed for the "accumulate" technique, but then
error_set() can't catch you setting an error more than once.
Standardize on the "check separately" technique for now, because it's
overwhelmingly prevalent.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Semantics of end_optional() differ subtly from the other end_FOO()
callbacks: when start_FOO() succeeds, the matching end_FOO() gets
called regardless of what happens in between. end_optional() gets
called only when everything in between succeeds as well. Entirely
undocumented, like all of the visitor API.
The only user of Visitor Callback end_optional() never did anything,
and was removed in commit 9f9ab46.
I'm about to clean up error handling in the generated visitor code,
and end_optional() is in my way. No users mean no test cases, and
making non-trivial cleanup transformations without test cases doesn't
strike me as a good idea.
Drop end_optional(), and rename start_optional() to optional(). We
can always go back to a pair of callbacks when we have an actual need.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
These have never been called or implemented by anything, and their
intended use is undocumented, like all of the visitor API.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
This reverts commit 25a7017555.
Turns out the argument *can* be null: QEMU now segfaults if it
receives an invalid parameter via a qmp command instead of throwing an
error.
For example:
{ "execute": "blockdev-add",
"arguments": { "options" : { "driver": "invalid-driver" } } }
CC: qemu-stable@nongnu.org
Signed-off-by: Peter Lieven <pl@kamp.de>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Using error_is_set(ERRP) to find out whether a function failed is
either wrong, fragile, or unnecessarily opaque. It's wrong when ERRP
may be null, because errors go undetected when it is. It's fragile
when proving ERRP non-null involves a non-local argument. Else, it's
unnecessarily opaque (see commit 84d18f0).
The error_is_set(errp) in do_qmp_dispatch() is merely fragile, because
the caller never passes a null errp argument.
Make the code more robust and more obviously correct: receive the
error in a local variable, then propagate it through the parameter.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
do_qmp_dispatch()'s test for qmp_dispatch_check_obj() failure examines
both the return value and the error object. The latter part is
unclean; it works only when do_qmp_dispatch()'s caller passes a
non-null errp argument. That's the case, but it's not locally
obvious. Unclean.
Cleanup would be easy enough, but since the unclean code is also
redundant, let's just drop it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
strtosz_suffix() might return negative error, this patch fixes
the error handling.
This patch also changes to handle error in the if statement
rather than handle success specially, this will make this use
of strtosz_suffix consistent with all other uses.
Signed-off-by: Amos Kong <akong@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Just hardcode them in the callers
Cc: Luiz Capitulino <lcapitulino@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Argument is null when visiting an unboxed struct. I can't see such a
visit in the current code. Fix it anyway.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Argument can't be null. No other Visitor method type_str() checks for
null.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
This fixes several bugs or shortcomings of the previous pretty-printer.
In particular:
* use PRIu64 instead of casting to long long
* the exact value is included too
* the correct unit of measure (MiB, GiB, etc.) is used. PiB and EiB
are added too.
* due to an off-by-one error, 512*2^30 was printed as 0.500MiB rather than
512MiB. floor(log2(val)) is equal to 63 - clz(val), while the code used 64.
* The desired specification is %g rather than %f, which always uses three
decimals in the current code. However %g would switch to scientific
notation when the integer part is >= 1000 (e.g. 1000*2^30). To keep the
code simple, switch to the higher power when the integer part is >= 1000;
overflow is avoided by using frexp instead of clz.
Suggested-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
This will be used by "info qtree". For numbers it prints both the
decimal and hex values. For sizes it rounds to the nearest power
of 2^10. For strings, it puts quotes around the string and separates
NULL and empty string.
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
Now we have several qemu-ga commands not returning response on success.
It has been documented in qga/qapi-schema.json already. This patch exposes
the 'success-response' flag by extending 'guest-info' command. With this
change, the clients can handle the command response more flexibly.
Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
*fixed up commit subject
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
In the original code, qmp_get_command_list is used to construct
a list of all commands' name. To get the information of all qga
commands, it traverses the name list and search the command info
with its name. So it can cause O(n^2) in the number of commands.
This patch adds an interface to traverse the qmp command list by
QmpCommand to replace qmp_get_command_list. It can decrease the
complexity from O(n^2) to O(n).
Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
*fix up commit subject
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Prevent mistyped command line options from incurring high memory and CPU
usage at startup. 64K elements in a range should be enough for everyone
(TM).
The OPTS_VISITOR_RANGE_MAX macro is public so that unit tests can
construct corner cases with it.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
When a well-formed range value, bounded by unsigned integers, is
encountered while processing a repeated option, enter LM_UNSIGNED_INTERVAL
and return the low bound.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Simplify the code in preparation for the next patch.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
When a well-formed range value, bounded by signed integers, is encountered
while processing a repeated option, enter LM_SIGNED_INTERVAL and return
the low bound.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
The new modes are equal-rank, exclusive alternatives of LM_IN_PROGRESS.
Teach opts_next_list(), opts_type_int() and opts_type_uint64() to handle
them.
Also enumerate explicitly what functions are valid to call in what modes:
- opts_next_list() is valid to call while flattening a range,
- opts_end_list(): ditto,
- lookup_scalar() is invalid to call during flattening; generated qapi
traversal code must continue asking for the same kind of signed/unsigned
list element until the interval is fully flattened,
- processed(): ditto.
List mode restrictions are always formulated in positive / inclusive
sense. The restrictions for lookup_scalar() and processed() are
automatically satisfied by current qapi traversals if the schema to build
is compatible with OptsVisitor.
The new list modes are not entered yet.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
We're going to need more state while processing a list of repeated
options. This change eliminates "repeated_opts_first" and adds a new state
variable:
list_mode repeated_opts repeated_opts_first
-------------- ------------- -------------------
LM_NONE NULL false
LM_STARTED non-NULL true
LM_IN_PROGRESS non-NULL false
Additionally, it is documented that lookup_scalar() and processed(), both
called by opts_type_XXX(), are invalid in LM_STARTED -- generated qapi
code calls opts_next_list() to allocate the very first link before trying
to parse a scalar into it. List mode restrictions are expressed in
positive / inclusive form.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Currently visit_type_size checks if the visitor's type_size function pointer is
NULL. If not, it calls it, otherwise it calls v->type_uint64(). But neither of
these pointers are ever set. Fallback to calling v->type_int() in this third
(default) case.
Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Andreas Färber <afaerber@suse.de>
Message-id: 1375109277-25561-6-git-send-email-imammedo@redhat.com
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
The discriminator for anonymous unions is the data type. This allows to
have a union type that allows both of these:
{ 'file': 'my_existing_block_device_id' }
{ 'file': { 'filename': '/tmp/mydisk.qcow2', 'read-only': true } }
Unions like this are specified in the schema with an empty dict as
discriminator. For this example you could take:
{ 'union': 'BlockRef',
'discriminator': {},
'data': { 'definition': 'BlockOptions',
'reference': 'str' } }
{ 'type': 'ExampleObject',
'data: { 'file': 'BlockRef' } }
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
This allows to just look at the next element without actually consuming
it.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
These can be used when an embedded struct is parsed and members not
belonging to the struct may be present in the input (e.g. parsing a
flat namespace QMP union, where fields from both the base and one
of the alternative types are mixed in the JSON object)
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Note that we already free with g_free().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
The file is only including error.h and qerror.h. Prefer explicit
inclusion of whatever files are needed.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Include:
- <glib.h> for g_malloc0()
- <string.h> for strcmp()
Some of those headers were probably being included by accident because
some other headers were including qemu-common.h, but those headers
should eventually stop including qemu-common.h.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
The second parameter to qapi_dealloc_type_size should be a uint64_t *,
not a size_t *. This was causing our 32 bit x86 build to fail, since
warnings are treated as errors.
Signed-off-by: Bruce Rogers <brogers@suse.com>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
visit_type_size() requires either visitor->type_size() or
visitor_uint64() to be implemented, otherwise a NULL function pointer is
invoked.
It is possible to trigger this crash as follows:
$ qemu-system-x86_64 -netdev tap,sndbuf=0,id=netdev0 \
-device virtio-blk-pci,netdev=netdev0
The 'sndbuf' option has type "size".
Reviewed-by: Andreas Färber <afaerber@suse.de>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
valgrind report:
==24534== 232 bytes in 2 blocks are definitely lost in loss record 1,245 of 1,601
==24534== at 0x4824F20: malloc (vg_replace_malloc.c:236)
==24534== by 0x293C88: malloc_and_trace (vl.c:2281)
==24534== by 0x489AD99: ??? (in /lib/libglib-2.0.so.0.2400.1)
==24534== by 0x489B23B: g_malloc0 (in /lib/libglib-2.0.so.0.2400.1)
==24534== by 0x2B4EFC: opts_visitor_new (opts-visitor.c:376)
==24534== by 0x29DEA5: net_client_init (net.c:708)
==24534== by 0x29E6C7: net_init_client (net.c:966)
==24534== by 0x2C2179: qemu_opts_foreach (qemu-option.c:1114)
==24534== by 0x29E85B: net_init_clients (net.c:1008)
==24534== by 0x296F40: main (vl.c:3463)
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
error_get_qobject() is unused since last commit, error_set_qobject()
has never been used. Also drops error_int.h.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
IMPORTANT: this BREAKS qemu-ga compatibility for the error response.
Instead of returning something like:
{ "error": { "class": "InvalidParameterValue",
"data": {"name": "mode", "expected": "halt|powerdown|reboot" } } }
qemu-ga now returns:
{ "error": { "class": "GenericError",
"desc": "Parameter 'mode' expects halt|powerdown|reboot" } }
Notice that this is also a bug fix, as qemu-ga wasn't returning the
human message.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
This visitor supports parsing
-option [type=]discriminator[,optarg1=val1][,optarg2=val2][,...]
style QemuOpts objects into "native" C structures. After defining the type
tree in the qapi schema (see below), a root type traversal with this
visitor linked to the underlying QemuOpts object will build the "native" C
representation of the option.
The type tree in the schema, corresponding to an option with a
discriminator, must have the following structure:
struct
scalar member for non-discriminated optarg 1 [*]
list for repeating non-discriminated optarg 2 [*]
wrapper struct
single scalar member
union
struct for discriminator case 1
scalar member for optarg 3 [*]
list for repeating optarg 4 [*]
wrapper struct
single scalar member
scalar member for optarg 5 [*]
struct for discriminator case 2
...
The "type" optarg name is fixed for the discriminator role. Its schema
representation is "union of structures", and each discriminator value must
correspond to a member name in the union.
If the option takes no "type" descriminator, then the type subtree rooted
at the union must be absent from the schema (including the union itself).
Optarg values can be of scalar types str / bool / integers / size.
Members marked with [*] may be defined as optional in the schema,
describing an optional optarg.
Repeating an optarg is supported; its schema representation must be "list
of structure with single mandatory scalar member". If an optarg is not
described as repeating in the schema (ie. it is defined as a scalar field
instead of a list), its last occurrence will take effect. Ordering between
differently named optargs is not preserved.
A mandatory list (or an optional one which is reported to be available),
corresponding to a repeating optarg, has at least one element after
successful parsing.
v1->v2:
- Update opts_type_size() prototype to uint64_t.
- Add opts_type_uint64() for options needing the full uint64_t range.
(Internals could be extracted to "cutils.c".)
- Allow negative values in opts_type_int().
- Rebase to nested Makefiles.
v2->v3:
- Factor opts_visitor_insert() out of opts_start_struct() and call it
separately for opts_root->id if there's any.
- Don't require non-negative values in opts_type_int()'s error message.
- g_malloc0() may return NULL for zero-sized requests. Support empty
structures by requesting 1 byte for them instead.
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
v1->v2:
- fall back to uint64 rather than int
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Don't overwrite / leak previously set errors.
Make traversal cope with missing mandatory sub-structs.
Don't try to end a container that could not be started.
v1->v2:
- unchanged
v2->v3:
- instead of examining, assert that we never overwrite errors with
error_set()
- allow visitors to set a NULL struct pointer successfully, so traversal
of incomplete objects can continue
- check for a NULL "obj" before accessing "(*obj)->has_XXX" (this is not a
typo, "obj != NULL" implies "*obj != NULL" here)
- fix start_struct / end_struct balance for unions as well
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
The enum string is pointed to by 'enum_str' not 'name'. This bug
causes the error message to be:
{ "error": { "class": "InvalidParameter",
"desc": "Invalid parameter 'null'",
"data": { "name": "null" } } }
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Amos Kong <akong@redhat.com>
The comment is stale, monitor.h is not needed anymore (only qerror.h
is, because it contains the schema for errors).
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Currently string-output-visitor formats floats as %g, which is nice in
that trailing 0's are automatically truncated, but otherwise this causes
some issues:
- it uses 6 significant figures instead of 6 decimal places, which
means something like 155777.5 (which even has an exact floating point
representation) will be rounded to 155778 when converted to a string.
- output will be presented in scientific notation when the normalized
form requires a 10^x multiplier. Not a huge deal, but arguably less
readable for command-line arguments.
- due to using scientific notation for numbers requiring more than 6
significant figures, instead of hard-defined decimal places, it
fails a lot of the test-visitor-serialization unit tests for floats.
Instead, let's just use %f, which is what the QJSON and the QMP visitors
use.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
This adds visitor interfaces for fixed-width integers types.
Implementing these in visitors is optional, otherwise we fall back to
visit_type_int() (int64_t) with some additional bounds checking to avoid
integer overflows for cases where the value fetched exceeds the bounds
of our target C type.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
[LE: exclude negative values in uint*_t Visitor interfaces]
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
[AF: Merged fix by Laszlo]
Signed-off-by: Andreas Färber <afaerber@suse.de>
Options allow for changes in commands behavior. This commit introduces
the QCO_NO_SUCCESS_RESP option, which causes a command to not emit a
success response.
This is needed by commands such as qemu-ga's guest-shutdown, which
may not be able to complete before the VM vanishes. In this case, it's
useful and simpler not to bother sending a success response.
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
JSON numbers can be interpreted as either integers or floating point
values depending on their representation. As a result, QMP input visitor
might visit a QInt when it was expecting a QFloat, so add handling to
account for this.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Acked-by: Andreas Färber <afaerber@suse.de>
