We opendir("/sys/block") at the beginning of the function, but we never
close it prior to returning.
Fixes: Coverity CID 1436130
Fixes: fed3956429 ("qga: add implementation of guest-get-disks for Linux")
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Tomáš Golembiovský <tgolembi@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
*fix-up merge conflicts due to qga-ssh-test being disabled in earlier
patch due to G_TEST_OPTION_ISOLATE_DIRS triggering build-oss-fuzz
leak detector.
*fix up style and disallowed g_assert* usage reported by checkpatch
Signed-off-by: Michael Roth <michael.roth@amd.com>
I prefer 'reset' over 'clear', since 'clear' and keys may have some
other relations or meaning.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
*fix disallowed g_assert* usage reported by checkpatch
Signed-off-by: Michael Roth <michael.roth@amd.com>
Add new commands to add and remove SSH public keys from
~/.ssh/authorized_keys.
I took a different approach for testing, including the unit tests right
with the code. I wanted to overwrite the function to get the user
details, I couldn't easily do that over QMP. Furthermore, I prefer
having unit tests very close to the code, and unit files that are domain
specific (commands-posix is too crowded already). FWIW, that
coding/testing style is Rust-style (where tests can or should even be
part of the documentation!).
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1885332
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
*squashed in fix-ups for setting file ownership and use of QAPI
conditionals for CONFIG_POSIX instead of stub definitions
*disable qga-ssh-test for now due to G_TEST_OPTION_ISOLATE_DIRS
triggering leak detector in build-oss-fuzz
*fix disallowed g_assert* usage reported by checkpatch
Signed-off-by: Michael Roth <michael.roth@amd.com>
The command lists all the physical disk drives. Unlike for Linux
partitions and virtual volumes are not listed.
Example output:
{
"return": [
{
"name": "\\\\.\\PhysicalDrive0",
"partition": false,
"address": {
"serial": "QM00001",
"bus-type": "sata",
...
},
"dependents": []
}
]
}
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
The command lists all disks (real and virtual) as well as disk
partitions. For each disk the list of dependent disks is also listed and
/dev path is used as a handle so it can be matched with "name" field of
other returned disk entries. For disk partitions the "dependents" list
is populated with the the parent device for easier tracking of
hierarchy.
Example output:
{
"return": [
...
{
"name": "/dev/dm-0",
"partition": false,
"dependents": [
"/dev/sda2"
],
"alias": "luks-7062202e-5b9b-433e-81e8-6628c40da9f7"
},
{
"name": "/dev/sda2",
"partition": true,
"dependents": [
"/dev/sda"
]
},
{
"name": "/dev/sda",
"partition": false,
"address": {
"serial": "SAMSUNG_MZ7LN512HCHP-000L1_S1ZKNXAG822493",
"bus-type": "sata",
...
"dev": "/dev/sda",
"target": 0
},
"dependents": []
},
...
]
}
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
*add missing stub for !defined(CONFIG_FSFREEZE)
*remove unused deps_dir variable
Signed-off-by: Michael Roth <michael.roth@amd.com>
Add API and stubs for new guest-get-disks command.
The command guest-get-fsinfo can be used to list information about disks
and partitions but it is limited only to mounted disks with filesystem.
This new command should allow listing information about disks of the VM
regardles whether they are mounted or not. This can be usefull for
management applications for mapping virtualized devices or pass-through
devices to device names in the guest OS.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
Simple unions are simpler than flat unions in the schema, but more
complicated in C and on the QMP wire: there's extra indirection in C
and extra nesting on the wire, both pointless. They should be avoided
in new code.
GuestDeviceId was recently added for guest-get-devices. Convert it to
a flat union.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
qmp_guest_get_devices() is wrong that way: it calls error_setg() in a
loop.
If no iteration fails, the function returns a value and sets no error.
Okay.
If exactly one iteration fails, the function returns a value and sets
an error. Wrong.
If multiple iterations fail, the function trips error_setv()'s
assertion.
Fix it to return immediately on error.
Perhaps the failure to convert the driver version to UTF-8 should not
be an error. We could simply not report the botched version string
instead.
Drop a superfluous continue while there.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
guest-get-devices returns 'driver-date' as string in the format
YYYY-MM-DD. Goes back to recent commit 2e4211cee4 "qga: add command
guest-get-devices for reporting VirtIO devices".
We should avoid use of multiple encodings for the same kind of data.
Especially string encodings. Change it to return nanoseconds since
the epoch, like guest-get-time does.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
Member 'address' is union GuestDeviceAddress with a single branch
GuestDeviceAddressPCI, containing PCI vendor ID and device ID. This
is not a PCI address. Type GuestPCIAddress is. Messed up in recent
commit 2e4211cee4 "qga: add command guest-get-devices for reporting
VirtIO devices".
Rename type GuestDeviceAddressPCI to GuestDeviceIdPCI, type
GuestDeviceAddress to GuestDeviceId, and member 'address' to 'id'.
Document the member properly while there.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
The correct way to set the current monitor for a coroutine handler will
be different than for a blocking handler, so monitor_set_cur() needs to
be called in qmp_dispatch().
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20201005155855.256490-7-kwolf@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The environment variables can't be passed through an env: argument
yet (meson#2723), use 'env' as suggested in:
https://github.com/mesonbuild/meson/issues/2723#issuecomment-348630957
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Add some section headings to the QGA json; this is purely so that we
have some H1 headings, as otherwise each command ends up being
visible in the interop/ manual's table of contents. In an ideal
world there might be a proper 'Introduction' section the way there is
in qapi/qapi-schema.json.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20200925162316.21205-12-peter.maydell@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Convert qemu-ga-ref to rST format. This includes dropping
the plain-text, pdf and info format outputs for this document;
as with all our other Sphinx-based documentation, we provide
HTML and manpage only.
The qemu-ga-ref.rst is somewhat more stripped down than
the .texi was, because we do not (currently) attempt to
generate indexes for the commands, events and data types
being documented.
As the GA ref is now part of the Sphinx 'interop' manual,
we can delete the direct link from index.html.in.
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20200925162316.21205-9-peter.maydell@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Unicode legacy literal dumbed down to plain string literal, TODO
comment on displaying QEMU version added]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
clang's C11 atomic_fetch_*() functions only take a C11 atomic type
pointer argument. QEMU uses direct types (int, etc) and this causes a
compiler error when a QEMU code calls these functions in a source file
that also included <stdatomic.h> via a system header file:
$ CC=clang CXX=clang++ ./configure ... && make
../util/async.c:79:17: error: address argument to atomic operation must be a pointer to _Atomic type ('unsigned int *' invalid)
Avoid using atomic_*() names in QEMU's atomic.h since that namespace is
used by <stdatomic.h>. Prefix QEMU's APIs with 'q' so that atomic.h
and <stdatomic.h> can co-exist. I checked /usr/include on my machine and
searched GitHub for existing "qatomic_" users but there seem to be none.
This patch was generated using:
$ git grep -h -o '\<atomic\(64\)\?_[a-z0-9_]\+' include/qemu/atomic.h | \
sort -u >/tmp/changed_identifiers
$ for identifier in $(</tmp/changed_identifiers); do
sed -i "s%\<$identifier\>%q$identifier%g" \
$(git grep -I -l "\<$identifier\>")
done
I manually fixed line-wrap issues and misaligned rST tables.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20200923105646.47864-1-stefanha@redhat.com>
* Some minor qtest improvements
* Fix the unit tests to work on MSYS2, too
* Enable building and testing on MSYS2 in the Cirrus-CI
* Build FreeBSD with one task again in the Cirrus-CI
-----BEGIN PGP SIGNATURE-----
iQJFBAABCAAvFiEEJ7iIR+7gJQEY8+q5LtnXdP5wLbUFAl9h9e0RHHRodXRoQHJl
ZGhhdC5jb20ACgkQLtnXdP5wLbXXMA/6AvNOOEgYeW+YrkIjMgh+jjgrmBK5FH0J
REJiJ5CxQBh9v3gPV5ehWv4/R9pmaEPtbsZ4Bc1jmRwLHcAWIJ/JTYo11M4vTYa3
IjS9+dlqgznzxZHFavwJ8USjcyeVjkqyaUTE7CNPgzE2b0237oQ8MHzFGlsHwGZV
AiRhDHI0StCE3QeKICnpB91Us+KF/+UjZnCwSaC/SM8Sq+6LnTF0bEYYUH44SfZe
AX3ax9kxzWFtzpXXh/3qL0gdGwiVqwv35V7MYpQWZJAPA3TdxVnUDE7/XP1RTOjL
hhJLf6IqgPwbRWLszmYmTiUCDGE8kqO8wj5MkKlJcjLY9n4zv0ErOjy6Nhnr8b5Q
TA9hjRfkRkUoquVRm7ZBOE9l2jIkWV9olxYFqBipqBMujSlt9T0seUi+eaY6NuAA
Z8NOQslqi8xP7wN4Lw3DpGOfbeTvtOlDtA7O7HwwTChTlhCJX7FCoNmpqhCiFRpH
s7VkNCXoc6l8NDI+Py5sjpRRHMQIsFWUCnZLWJQ+UJWZvfnNoLTM3ErdqzIasVLt
vW/behHRd7L/hGMa7zNtQa+wv2bgXY/hbFFpNK6RUEaPBzUq3ZixFrMW2Fw6X7mg
eIVPNrh/LloiJGQfpUuNkqiZ4vdgUeBq7Z89TCU49xskQAgHb0KglnveU42nP8Yf
pO8OCBOjfJg=
=ErBp
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2020-09-16' into staging
* Fix "readlink -f" problem in iotests on macOS (to fix the Cirrus-CI tests)
* Some minor qtest improvements
* Fix the unit tests to work on MSYS2, too
* Enable building and testing on MSYS2 in the Cirrus-CI
* Build FreeBSD with one task again in the Cirrus-CI
# gpg: Signature made Wed 16 Sep 2020 12:24:29 BST
# gpg: using RSA key 27B88847EEE0250118F3EAB92ED9D774FE702DB5
# gpg: issuer "thuth@redhat.com"
# gpg: Good signature from "Thomas Huth <th.huth@gmx.de>" [full]
# gpg: aka "Thomas Huth <thuth@redhat.com>" [full]
# gpg: aka "Thomas Huth <huth@tuxfamily.org>" [full]
# gpg: aka "Thomas Huth <th.huth@posteo.de>" [unknown]
# Primary key fingerprint: 27B8 8847 EEE0 2501 18F3 EAB9 2ED9 D774 FE70 2DB5
* remotes/huth-gitlab/tags/pull-request-2020-09-16: (24 commits)
cirrus: Building freebsd in a single shot
ci: Enable msys2 ci in cirrus
tests: Fixes test-qdev-global-props.c
tests: fix test-util-sockets.c
tests: Fixes test-io-channel-file by mask only owner file state mask bits
tests: fixes aio-win32 about aio_remove_fd_handler, get it consistence with aio-posix.c
tests: Fixes test-io-channel-socket.c tests under msys2/mingw
vmstate: Fixes test-vmstate.c on msys2/mingw
meson: remove empty else and duplicated gio deps
meson: Use -b to ignore CR vs. CR-LF issues on Windows
osdep: file locking functions are not available on Win32
tests: test-replication disable /replication/secondary/* on msys2/mingw.
tests: Fixes test-replication.c on msys2/mingw.
meson: disable crypto tests are empty under win32
meson: Disable test-char on msys2/mingw for fixing tests stuck
rcu: fixes test-logging.c by call drain_call_rcu before rmdir_full
tests: Convert g_free to g_autofree macro in test-logging.c
rcu: Implement drain_call_rcu
qga/commands-win32: Fix problem with redundant protype declaration
Simplify the .gitignore file
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
We want to introduce a new version of qemu_open() that uses an Error
object for reporting problems and make this it the preferred interface.
Rename the existing method to release the namespace for the new impl.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When compiling QEMU with MSYS2 on Windows, there is currently the
following error:
../qga/commands-win32.c:62:24: error: redundant redeclaration of
'CM_Get_DevNode_PropertyW' [-Werror=redundant-decls]
62 | CMAPI CONFIGRET WINAPI CM_Get_DevNode_PropertyW(
| ^~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../qga/commands-win32.c:26:
C:/tools/msys64/mingw64/x86_64-w64-mingw32/include/cfgmgr32.h:840:26: note:
previous declaration of 'CM_Get_DevNode_PropertyW' was here
840 | CMAPI CONFIGRET WINAPI CM_Get_DevNode_PropertyW(DEVINST dnDevInst,
const DEVPROPKEY *PropertyKey, DEVPROPTYPE *PropertyType, PBYTE PropertyBuffer,
PULONG PropertyBufferSize, ULONG ulFlags);
Seems like this protype is sometimes available in the cfgmgr32.h
header, and sometimes not. Let's silence the compiler warning here
to let the build pass with -Werror, too.
Message-Id: <20200915114757.55635-1-thuth@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Add command for reporting devices on Windows guest. The intent is not so
much to report the devices but more importantly the driver (and its
version) that is assigned to the device. This gives caller the
information whether VirtIO drivers are installed and/or whether
inadequate driver is used on a device (e.g. QXL device with base VGA
driver).
Example:
[
{
"driver-date": "2019-08-12",
"driver-name": "Red Hat VirtIO SCSI controller",
"driver-version": "100.80.104.17300",
"address": {
"type": "pci",
"data": {
"device-id": 4162,
"vendor-id": 6900
}
}
},
...
]
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
*remove redundant glib autoptr declaration for GuestDeviceInfo
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
QEMU on s390x uses virtio via channel I/O instead of PCI by default.
Add a function to detect and provide information for virtio-scsi and
virtio-block devices here, too.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
The libudev-related code is independent from the other pci-related code
and can be re-used for non-pci devices (like ccw devices on s390x). Thus
move this part to the generic function.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1755075
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
We are going to support non-PCI devices soon. For this we need to split
the generic GuestDiskAddress and GuestDiskAddressList memory allocation
and list chaining into a separate function first.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
The various schemas included in QEMU use a JSON-based format which
is, however, strictly speaking not valid JSON.
As a consequence, when vim tries to apply syntax highlight rules
for JSON (as guessed from the file name), the result is an unreadable
mess which mostly consist of red markers pointing out supposed errors
in, well, pretty much everything.
Using Python syntax highlighting produces much better results, and
in fact these files already start with specially-formatted comments
that instruct Emacs to process them as if they were Python files.
This commit adds the equivalent special comments for vim.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Message-Id: <20200729185024.121766-1-abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The "guest-get-fsinfo" could also be used for non-PCI devices in the
future. And the code in GuestPCIAddress() in qga/commands-win32.c seems
to be using "-1" for fields that it can not determine already. Thus
let's properly document "-1" as value for invalid PCI address fields.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
This patch handles the case where unmounted volumes exist,
where in that case GetVolumePathNamesForVolumeName returns
empty path, GetVolumeInformation will use the current working
directory instead.
This patch fixes the issue by opening a handle to the volumes,
and using GetVolumeInformationByHandleW instead.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1746667
Signed-off-by: Basil Salman <bsalman@redhat.com>
Signed-off-by: Basil Salman <basil@daynix.com>
*fix crash when guest_build_fsinfo() sets errp multiple times
*make new error message more distinct from existing ones
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Problem with g_get_host_name() is that on the first call it saves
the hostname into a global variable and from then on, every
subsequent call returns the saved hostname. Even if the hostname
changes. This doesn't play nicely with guest agent, because if
the hostname is acquired before the guest is set up (e.g. on the
first boot, or before DHCP) we will report old, invalid hostname.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1845127
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Since commit 781f2b3d1e ("qga: process_event() simplification"),
send_response() is called unconditionally, but will assert when "rsp" is
NULL. This may happen with QCO_NO_SUCCESS_RESP commands, such as
"guest-shutdown".
Fixes: 781f2b3d1e
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Reported-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Tested-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
On one hand "guest-fsfreeze-freeze" command, "COM+ System Application service" is
stopped, on the other hand "guest-fsfreeze-thaw" stops QGA VSS Provider service from
"COM+ Application Admin Catalog".
Invoking a series of freeze and thaw commands may result in QGA failing to stop
VSS Provider service as "COM+ System Application service" is stopped, which can
cause some delay in qga response.
In this commit StopService function was changed and VSS Provider service is now
stopped using Winsvc library API.
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1549425
Signed-off-by: Basil Salman <bsalman@redhat.com>
Signed-off-by: Basil Salman <basil@daynix.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
When all we do with an Error we receive into a local variable is
propagating to somewhere else, we can just as well receive it there
right away. The previous two commits did that for sufficiently simple
cases with Coccinelle. Do it for several more manually.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200707160613.848843-37-armbru@redhat.com>
When all we do with an Error we receive into a local variable is
propagating to somewhere else, we can just as well receive it there
right away. Convert
if (!foo(..., &err)) {
...
error_propagate(errp, err);
...
return ...
}
to
if (!foo(..., errp)) {
...
...
return ...
}
where nothing else needs @err. Coccinelle script:
@rule1 forall@
identifier fun, err, errp, lbl;
expression list args, args2;
binary operator op;
constant c1, c2;
symbol false;
@@
if (
(
- fun(args, &err, args2)
+ fun(args, errp, args2)
|
- !fun(args, &err, args2)
+ !fun(args, errp, args2)
|
- fun(args, &err, args2) op c1
+ fun(args, errp, args2) op c1
)
)
{
... when != err
when != lbl:
when strict
- error_propagate(errp, err);
... when != err
(
return;
|
return c2;
|
return false;
)
}
@rule2 forall@
identifier fun, err, errp, lbl;
expression list args, args2;
expression var;
binary operator op;
constant c1, c2;
symbol false;
@@
- var = fun(args, &err, args2);
+ var = fun(args, errp, args2);
... when != err
if (
(
var
|
!var
|
var op c1
)
)
{
... when != err
when != lbl:
when strict
- error_propagate(errp, err);
... when != err
(
return;
|
return c2;
|
return false;
|
return var;
)
}
@depends on rule1 || rule2@
identifier err;
@@
- Error *err = NULL;
... when != err
Not exactly elegant, I'm afraid.
The "when != lbl:" is necessary to avoid transforming
if (fun(args, &err)) {
goto out
}
...
out:
error_propagate(errp, err);
even though other paths to label out still need the error_propagate().
For an actual example, see sclp_realize().
Without the "when strict", Coccinelle transforms vfio_msix_setup(),
incorrectly. I don't know what exactly "when strict" does, only that
it helps here.
The match of return is narrower than what I want, but I can't figure
out how to express "return where the operand doesn't use @err". For
an example where it's too narrow, see vfio_intx_enable().
Silently fails to convert hw/arm/armsse.c, because Coccinelle gets
confused by ARMSSE being used both as typedef and function-like macro
there. Converted manually.
Line breaks tidied up manually. One nested declaration of @local_err
deleted manually. Preexisting unwanted blank line dropped in
hw/riscv/sifive_e.c.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200707160613.848843-35-armbru@redhat.com>
Replace
error_setg(&err, ...);
error_propagate(errp, err);
by
error_setg(errp, ...);
Related pattern:
if (...) {
error_setg(&err, ...);
goto out;
}
...
out:
error_propagate(errp, err);
return;
When all paths to label out are that way, replace by
if (...) {
error_setg(errp, ...);
return;
}
and delete the label along with the error_propagate().
When we have at most one other path that actually needs to propagate,
and maybe one at the end that where propagation is unnecessary, e.g.
foo(..., &err);
if (err) {
goto out;
}
...
bar(..., &err);
out:
error_propagate(errp, err);
return;
move the error_propagate() to where it's needed, like
if (...) {
foo(..., &err);
error_propagate(errp, err);
return;
}
...
bar(..., errp);
return;
and transform the error_setg() as above.
In some places, the transformation results in obviously unnecessary
error_propagate(). The next few commits will eliminate them.
Bonus: the elimination of gotos will make later patches in this series
easier to review.
Candidates for conversion tracked down with this Coccinelle script:
@@
identifier err, errp;
expression list args;
@@
- error_setg(&err, args);
+ error_setg(errp, args);
... when != err
error_propagate(errp, err);
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200707160613.848843-34-armbru@redhat.com>
transfer_memory_block() leaks an Error object when reading file
/sys/devices/system/memory/memory<INDEX>/state fails with errno other
than ENOENT, and @sys2memblk is false, i.e. when the state file exists
but cannot be read (seems quite unlikely), and this is
guest-set-memory-blocks, not guest-get-memory-blocks.
Plug the leak.
Fixes: bd240fca42
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: Hailiang Zhang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Message-Id: <20200630090351.1247703-9-armbru@redhat.com>
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
qmp_guest_suspend_disk() and qmp_guest_suspend_ram() pass @local_err
first to check_suspend_mode(), then to acquire_privilege(), then to
execute_async(). Continuing after errors here can only end in tears.
For instance, we risk tripping error_setv()'s assertion.
Fixes: aa59637ea1
Fixes: f54603b6aa
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200422130719.28225-15-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.
qmp_guest_get_memory_blocks() passes &local_err to
transfer_memory_block() in a loop. If this fails in more than one
iteration, it can trip error_setv()'s assertion.
Fix it to break the loop.
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200422130719.28225-14-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
On [*] Daniel Berrangé commented:
The QEMU guest agent protocol is not sensible way to access huge
files inside the guest. It requires the inefficient process of
reading the entire data into memory than duplicating it again in
base64 format, and then copying it again in the JSON serializer /
monitor code.
For arbitrary general purpose file access, especially for large
files, use a real file transfer program or use a network block
device, not the QEMU guest agent.
To avoid bug reports as BZ#1594054 (CVE-2018-12617), follow his
suggestion to put a low, hard limit on "count" in the guest agent
QAPI schema, and don't allow count to be larger than 48 MB.
[*] https://www.mail-archive.com/qemu-devel@nongnu.org/msg693176.html
Fixes: CVE-2018-12617
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1594054
Reported-by: Fakhri Zulkifli <mohdfakhrizulkifli@gmail.com>
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
*update schema documentation to indicate 48MB limit instead of 10MB
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Extract the common code shared by both POSIX/Win32 implementations.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
As we are going to reuse this method, declare it in common
header.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
As noted by Daniel Berrangé in [*], the fix from commit 807e2b6fce
which replaced malloc() by try_malloc() is not enough, the process
can still run out of memory a few line later:
346 buf = g_try_malloc0(count + 1);
347 if (!buf) {
348 error_setg(errp,
349 "failed to allocate sufficient memory "
350 "to complete the requested service");
351 return NULL;
352 }
353 is_ok = ReadFile(fh, buf, count, &read_count, NULL);
354 if (!is_ok) {
355 error_setg_win32(errp, GetLastError(), "failed to read file");
356 slog("guest-file-read failed, handle %" PRId64, handle);
357 } else {
358 buf[read_count] = 0;
359 read_data = g_new0(GuestFileRead, 1);
^^^^^^
Instead we are going to put a low hard limit on 'count' in the next
commits. This reverts commit 807e2b6fce.
[*] https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg03471.html
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
local_err is used several times in guest_suspend(). Setting non-NULL
local_err will crash, so let's zero it after freeing. Also fix possible
leak of local_err in final if().
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20200324153630.11882-7-vsementsov@virtuozzo.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Although qemu-ga has supported vsock since 2016 it was not documented on
the man page.
Also add the socket address representation to the qga --help output.
Fixes: 586ef5dee7
("qga: add vsock-listen method")
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
The QAPI struct GuestFileWhence has a comment about how we are
exploiting equivalent values between two different integer types
shared in a union. But C says behavior is undefined on assignments to
overlapping storage when the two types are not the same width, and
indeed, 'int64_t value' and 'enum QGASeek name' are very likely to be
different in width. Utilize a temporary variable to fix things.
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Fixes: 0b4b49387
Fixes: Coverity CID 1421990
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
