aboutsummaryrefslogtreecommitdiff
path: root/qga
AgeCommit message (Collapse)Author
2020-11-18meson: Fix build with --disable-guest-agent-msiStefan Weil
The QGA MSI target requires several macros which are only available without --disable-guest-agent-msi. Don't define that target if configure was called with --disable-guest-agent-msi. Signed-off-by: Stefan Weil <sw@weilnetz.de> Message-Id: <20201117201834.408892-1-sw@weilnetz.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-11-16qga: update schema for guest-get-disks 'dependents' fieldMichael Roth
The recently-added 'guest-get-disk' command returns a list of GuestDiskInfo entries, which in turn have a 'dependents' field which lists devices these entries are dependent upon. Thus, 'dependencies' is a better name for this field. Address this by renaming the field accordingly. Additionally, 'dependents' is specified as non-optional, even though it's not implemented for w32. This is misleading, since it gives users the impression that a particular disk might not have dependencies, when in reality that information is simply not known to the guest agent. Address this by making 'dependents' an optional field, and only marking it as in-use when the facilities to obtain this information are available to the guest agent. Cc: Eric Blake <eblake@redhat.com> Cc: Tomáš Golembiovský <tgolembi@redhat.com> Cc: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Michael Roth <michael.roth@amd.com>
2020-11-09qga: fix missing closedir() in qmp_guest_get_disks()Michael Roth
We opendir("/sys/block") at the beginning of the function, but we never close it prior to returning. Fixes: Coverity CID 1436130 Fixes: fed3956429d5 ("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>
2020-11-02qga: add ssh-get-authorized-keysMarc-André Lureau
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>
2020-11-02meson: minor simplificationMarc-André Lureau
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Michael Roth <michael.roth@amd.com>
2020-11-02qga: add *reset argument to ssh-add-authorized-keysMichael Roth
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>
2020-11-02qga: add ssh-{add,remove}-authorized-keysMarc-André Lureau
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>
2020-11-02qga: add implementation of guest-get-disks for WindowsTomáš Golembiovský
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>
2020-11-02qga: add implementation of guest-get-disks for LinuxTomáš Golembiovský
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>
2020-11-02qga: add command guest-get-disksTomáš Golembiovský
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>
2020-11-02qga: Flatten simple union GuestDeviceIdMarkus Armbruster
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>
2020-11-02qga-win: Fix guest-get-devices error API violationsMarkus Armbruster
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>
2020-11-02qga: Use common time encoding for guest-get-devices 'driver-date'Markus Armbruster
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>
2020-11-02qga: Rename guest-get-devices return member 'address' to 'id'Markus Armbruster
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>
2020-10-09qmp: Call monitor_set_cur() only in qmp_dispatch()Kevin Wolf
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>
2020-09-30qga: relocate path to default configuration and hookPaolo Bonzini
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-09-30meson: fix MSI ruleMarc-André Lureau
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>
2020-09-29qga/qapi-schema.json: Add some headingsPeter Maydell
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>
2020-09-29docs/interop: Convert qemu-ga-ref to rSTPeter Maydell
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>
2020-09-23qemu/atomic.h: rename atomic_ to qatomic_Stefan Hajnoczi
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>
2020-09-17Merge remote-tracking branch ↵Peter Maydell
'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>
2020-09-16util: rename qemu_open() to qemu_open_old()Daniel P. Berrangé
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>
2020-09-16qga/commands-win32: Fix problem with redundant protype declarationThomas Huth
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>
2020-09-12qga: add command guest-get-devices for reporting VirtIO devicesTomáš Golembiovský
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>
2020-09-12qga/commands-posix: Support fsinfo for non-PCI virtio devices, tooThomas Huth
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>
2020-09-12qga/commands-posix: Move the udev code from the pci to the generic functionThomas Huth
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>
2020-09-12qga/commands-posix: Rework build_guest_fsinfo_for_real_device() functionThomas Huth
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>
2020-09-01meson: install $localstatedir/run for qgaMarc-André Lureau
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <20200826130622.553318-6-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-08-21meson: build texi docPaolo Bonzini
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-08-21meson: convert dummy Windows qga/qemu-ga targetPaolo Bonzini
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-08-21meson: add msi generationMarc-André Lureau
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-08-21meson: convert vss-win32Marc-André Lureau
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-08-21meson: convert qemu-gaPaolo Bonzini
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-08-03schemas: Add vim modelineAndrea Bolognani
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>
2020-07-27qga/qapi-schema: Document -1 for invalid PCI address fieldsThomas Huth
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>
2020-07-27qga-win: fix "guest-get-fsinfo" wrong filesystem typeBasil Salman
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>
2020-07-13qga: Use qemu_get_host_name() instead of g_get_host_name()Michal Privoznik
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>
2020-07-13qga: fix assert regression on guest-shutdownMarc-André Lureau
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: 781f2b3d1e5ef389b44016a897fd55e7a780bf35 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>
2020-07-13qga-win: Fix QGA VSS Provider service stop failureBasil Salman
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>
2020-07-10error: Eliminate error_propagate() manuallyMarkus Armbruster
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>
2020-07-10error: Eliminate error_propagate() with Coccinelle, part 1Markus Armbruster
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>
2020-07-10error: Avoid unnecessary error_propagate() after error_setg()Markus Armbruster
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>
2020-07-02qga: Plug unlikely memory leak in guest-set-memory-blocksMarkus Armbruster
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: bd240fca42d5f072fb758a71720d9de9990ac553 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>
2020-04-29qga: Fix qmp_guest_suspend_{disk, ram}() error handlingMarkus Armbruster
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: aa59637ea1c6a4c83430933f9c44c43e6c3f1b69 Fixes: f54603b6aa765514b2519e74114a2f417759d727 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>
2020-04-29qga: Fix qmp_guest_get_memory_blocks() error handlingMarkus Armbruster
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>
2020-04-15qga: Restrict guest-file-read count to 48 MB to avoid crashesPhilippe Mathieu-Daudé
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>
2020-04-15qga: Extract qmp_guest_file_read() to common commands.cPhilippe Mathieu-Daudé
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>
2020-04-15qga: Extract guest_file_handle_find() to commands-common.hPhilippe Mathieu-Daudé
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>
2020-04-15Revert "prevent crash when executing guest-file-read with large count"Philippe Mathieu-Daudé
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 807e2b6fce022707418bc8f61c069d91c613b3d2. [*] 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>
2020-04-04qga/commands-posix: fix use after free of local_errVladimir Sementsov-Ogievskiy
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>