From 65650f0182c11a000d488a3b8e8e6ad071853f31 Mon Sep 17 00:00:00 2001 From: Chen Hanxiao Date: Thu, 14 Jun 2018 16:10:13 +0800 Subject: [PATCH 01/14] qga: unset frozen state if no mount points are frozen If we set mountpoints to qmp_guest_fsfreeze_freeze_list, we may got nothing to freeze as all mountpoints are not valid. So call ga_unset_frozen in this senario. Also, if we return 0 frozen fs, there is no need to call guest-fsfreeze-thaw. Cc: Michael Roth Signed-off-by: Chen Hanxiao Signed-off-by: Michael Roth --- qga/commands-posix.c | 6 ++++++ qga/qapi-schema.json | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index eae817191b..594d21ef3e 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1274,6 +1274,12 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints, } free_fs_mount_list(&mounts); + /* We may not issue any FIFREEZE here. + * Just unset ga_state here and ready for the next call. + */ + if (i == 0) { + ga_unset_frozen(ga_state); + } return i; error: diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index 17884c7c70..1045cef386 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -435,7 +435,9 @@ # for up to 10 seconds by VSS. # # Returns: Number of file systems currently frozen. On error, all filesystems -# will be thawed. +# will be thawed. If no filesystems are frozen as a result of this call, +# then @guest-fsfreeze-status will remain "thawed" and calling +# @guest-fsfreeze-thaw is not necessary. # # Since: 0.15.0 ## From 141b197408ab398c4f474ac1a728ab316e921f2b Mon Sep 17 00:00:00 2001 From: Prasad J Pandit Date: Wed, 13 Jun 2018 11:46:57 +0530 Subject: [PATCH 02/14] qga: check bytes count read by guest-file-read While reading file content via 'guest-file-read' command, 'qmp_guest_file_read' routine allocates buffer of count+1 bytes. It could overflow for large values of 'count'. Add check to avoid it. Reported-by: Fakhri Zulkifli Signed-off-by: Prasad J Pandit Cc: qemu-stable@nongnu.org Signed-off-by: Michael Roth --- qga/commands-posix.c | 2 +- qga/commands-win32.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 594d21ef3e..9284e71666 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -458,7 +458,7 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count, if (!has_count) { count = QGA_READ_COUNT_DEFAULT; - } else if (count < 0) { + } else if (count < 0 || count >= UINT32_MAX) { error_setg(errp, "value '%" PRId64 "' is invalid for argument count", count); return NULL; diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 70ee5379f6..73f31fa8c2 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -318,7 +318,7 @@ GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count, } if (!has_count) { count = QGA_READ_COUNT_DEFAULT; - } else if (count < 0) { + } else if (count < 0 || count >= UINT32_MAX) { error_setg(errp, "value '%" PRId64 "' is invalid for argument count", count); return NULL; From 25b5ff1a860634c2b1463b2882ed02cd55f8ac62 Mon Sep 17 00:00:00 2001 From: Chen Hanxiao Date: Thu, 14 Jun 2018 16:06:06 +0800 Subject: [PATCH 03/14] qga: add mountpoint usage info to GuestFilesystemInfo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch adds support for getting the usage of mounted filesystem. The usage of fs stored as used_bytes and total_bytes. It's very useful when we try to monitor guest's filesystem. Cc: Michael Roth Cc: Daniel P. Berrangé Reviewed-by: Eric Blake Signed-off-by: Chen Hanxiao Signed-off-by: Michael Roth --- qga/commands-posix.c | 15 +++++++++++++++ qga/qapi-schema.json | 3 +++ 2 files changed, 18 insertions(+) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 9284e71666..ae8535e497 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -46,6 +46,7 @@ extern char **environ; #include #include #include +#include #ifdef FIFREEZE #define CONFIG_FSFREEZE @@ -1072,6 +1073,8 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct FsMount *mount, Error **errp) { GuestFilesystemInfo *fs = g_malloc0(sizeof(*fs)); + struct statvfs buf; + unsigned long used, nonroot_total, fr_size; char *devpath = g_strdup_printf("/sys/dev/block/%u:%u", mount->devmajor, mount->devminor); @@ -1079,7 +1082,19 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct FsMount *mount, fs->type = g_strdup(mount->devtype); build_guest_fsinfo_for_device(devpath, fs, errp); + if (statvfs(fs->mountpoint, &buf) == 0) { + fr_size = buf.f_frsize; + used = buf.f_blocks - buf.f_bfree; + nonroot_total = used + buf.f_bavail; + fs->used_bytes = used * fr_size; + fs->total_bytes = nonroot_total * fr_size; + + fs->has_total_bytes = true; + fs->has_used_bytes = true; + } + g_free(devpath); + return fs; } diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index 1045cef386..2df6356b8c 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -848,6 +848,8 @@ # @name: disk name # @mountpoint: mount point path # @type: file system type string +# @used-bytes: file system used bytes (since 3.0) +# @total-bytes: non-root file system total bytes (since 3.0) # @disk: an array of disk hardware information that the volume lies on, # which may be empty if the disk type is not supported # @@ -855,6 +857,7 @@ ## { 'struct': 'GuestFilesystemInfo', 'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str', + '*used-bytes': 'uint64', '*total-bytes': 'uint64', 'disk': ['GuestDiskAddress']} } ## From c07e5e6ef3b5a2737387f86912ac30d1e9d48909 Mon Sep 17 00:00:00 2001 From: Chen Hanxiao Date: Thu, 14 Jun 2018 16:06:07 +0800 Subject: [PATCH 04/14] qga-win: add driver path usage to GuestFilesystemInfo This patch adds support for getting the usage of windows driver path. The usage of fs stored as used_bytes and total_bytes. Cc: Michael Roth Signed-off-by: Chen Hanxiao Signed-off-by: Michael Roth --- qga/commands-win32.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 73f31fa8c2..318d760a74 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -670,6 +670,7 @@ static GuestFilesystemInfo *build_guest_fsinfo(char *guid, Error **errp) char fs_name[32]; char vol_info[MAX_PATH+1]; size_t len; + uint64_t i64FreeBytesToCaller, i64TotalBytes, i64FreeBytes; GuestFilesystemInfo *fs = NULL; GetVolumePathNamesForVolumeName(guid, (LPCH)&mnt, 0, &info_size); @@ -699,10 +700,21 @@ static GuestFilesystemInfo *build_guest_fsinfo(char *guid, Error **errp) fs_name[sizeof(fs_name) - 1] = 0; fs = g_malloc(sizeof(*fs)); fs->name = g_strdup(guid); + fs->has_total_bytes = false; + fs->has_used_bytes = false; if (len == 0) { fs->mountpoint = g_strdup("System Reserved"); } else { fs->mountpoint = g_strndup(mnt_point, len); + if (GetDiskFreeSpaceEx(fs->mountpoint, + (PULARGE_INTEGER) & i64FreeBytesToCaller, + (PULARGE_INTEGER) & i64TotalBytes, + (PULARGE_INTEGER) & i64FreeBytes)) { + fs->used_bytes = i64TotalBytes - i64FreeBytes; + fs->total_bytes = i64TotalBytes; + fs->has_total_bytes = true; + fs->has_used_bytes = true; + } } fs->type = g_strdup(fs_name); fs->disk = build_guest_disk_info(guid, errp); From 509b97fd079c9666ae7318bae6e1273fd5d6b574 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Golembiovsk=C3=BD?= Date: Fri, 20 Apr 2018 20:04:34 +0200 Subject: [PATCH 05/14] test-qga: add trivial tests for some commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These commands did not get their tests in the original commits: - guest-get-host-name - guest-get-timezone - guest-get-users Trivial tests that mostly only call the commands were added. Signed-off-by: Tomáš Golembiovský Reviewed-by: Marc-André Lureau * replace QDECREF() with qobject_unref() Signed-off-by: Michael Roth --- tests/test-qga.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/tests/test-qga.c b/tests/test-qga.c index 30c9643257..350f27812a 100644 --- a/tests/test-qga.c +++ b/tests/test-qga.c @@ -854,6 +854,54 @@ static void test_qga_guest_exec_invalid(gconstpointer fix) qobject_unref(ret); } +static void test_qga_guest_get_host_name(gconstpointer fix) +{ + const TestFixture *fixture = fix; + QDict *ret, *val; + + ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-host-name'}"); + g_assert_nonnull(ret); + qmp_assert_no_error(ret); + + val = qdict_get_qdict(ret, "return"); + g_assert(qdict_haskey(val, "host-name")); + + qobject_unref(ret); +} + +static void test_qga_guest_get_timezone(gconstpointer fix) +{ + const TestFixture *fixture = fix; + QDict *ret, *val; + + ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-timezone'}"); + g_assert_nonnull(ret); + qmp_assert_no_error(ret); + + /* Make sure there's at least offset */ + val = qdict_get_qdict(ret, "return"); + g_assert(qdict_haskey(val, "offset")); + + qobject_unref(ret); +} + +static void test_qga_guest_get_users(gconstpointer fix) +{ + const TestFixture *fixture = fix; + QDict *ret; + QList *val; + + ret = qmp_fd(fixture->fd, "{'execute': 'guest-get-users'}"); + g_assert_nonnull(ret); + qmp_assert_no_error(ret); + + /* There is not much to test here */ + val = qdict_get_qlist(ret, "return"); + g_assert_nonnull(val); + + qobject_unref(ret); +} + static void test_qga_guest_get_osinfo(gconstpointer data) { TestFixture fixture; @@ -946,6 +994,12 @@ int main(int argc, char **argv) test_qga_guest_exec_invalid); g_test_add_data_func("/qga/guest-get-osinfo", &fix, test_qga_guest_get_osinfo); + g_test_add_data_func("/qga/guest-get-host-name", &fix, + test_qga_guest_get_host_name); + g_test_add_data_func("/qga/guest-get-timezone", &fix, + test_qga_guest_get_timezone); + g_test_add_data_func("/qga/guest-get-users", &fix, + test_qga_guest_get_users); ret = g_test_run(); From 84ece8ef6c70ea1852ec47805531017b75d46d68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Golembiovsk=C3=BD?= Date: Fri, 20 Apr 2018 20:12:45 +0200 Subject: [PATCH 06/14] qga/schema: fix documentation for GuestOSInfo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The documentation for kernel-version and kernel-release on Windows was swapped. Signed-off-by: Tomáš Golembiovský Reviewed-by: Marc-André Lureau Signed-off-by: Michael Roth --- qga/qapi-schema.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index 2df6356b8c..dfbc4a5e32 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -1173,10 +1173,10 @@ # # @kernel-release: # * POSIX: release field returned by uname(2) -# * Windows: version number of the OS +# * Windows: build number of the OS # @kernel-version: # * POSIX: version field returned by uname(2) -# * Windows: build number of the OS +# * Windows: version number of the OS # @machine: # * POSIX: machine field returned by uname(2) # * Windows: one of x86, x86_64, arm, ia64 From a972304d187b104ebec327bc824b23a5a3589ac3 Mon Sep 17 00:00:00 2001 From: Bishara AbuHattoum Date: Thu, 28 Jun 2018 17:58:16 +0300 Subject: [PATCH 07/14] qga-win: Fixing msi upgrade disallow in WiX file Issue: When upgrading qemu-ga using the msi from an old version to a newer one, the upgrade is not allowed by the msi showing this error message "Another version of this product is already installed." BZ# 1536331: https://bugzilla.redhat.com/show_bug.cgi?id=1536331 Fix: For the upgrade to be allowed by the msi the WiX file must provide three things: 1. Changing product's Id. (assigning it to "*") 2. Constant product's UpgradeId. (exists) 3. Changing version. (exists) Reference: http://wixtoolset.org/documentation/manual/v3/howtos/updates/major_upgrade.html Signed-off-by: Bishara AbuHattoum Signed-off-by: Michael Roth --- qga/installer/qemu-ga.wxs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs index 5af11627f8..f751a7e9f7 100644 --- a/qga/installer/qemu-ga.wxs +++ b/qga/installer/qemu-ga.wxs @@ -41,7 +41,7 @@ Date: Mon, 14 May 2018 14:38:55 +0200 Subject: [PATCH 08/14] qemu-ga: make get-fsinfo work over pci bridges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Iterate over the PCI bridges to lookup the PCI device associated with the block device. This allows to lookup the driver under the following syspath: /sys/devices/pci0000:00/0000:00:02.2/0000:03:00.0/virtio2/block/vda/vda3 It also works with an "old-style" Q35 libvirt hierarchy: root complex -> DMI-PCI bridge -> PCI-PCI bridge -> virtio controller, ex: /sys/devices/pci0000:00/0000:00:03.0/0000:01:01.0/0000:02:01.0/virtio1/block/vda/vda3 The setup can be reproduced with the following qemu command line (Thanks Marcel for help): qemu-system-x86_64 -M q35 \ -device i82801b11-bridge,id=dmi2pci_bridge,bus=pcie.0 -device pci-bridge,id=pci_bridge,bus=dmi2pci_bridge,addr=0x1,chassis_nr=1 -device virtio-blk-pci,scsi=off,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,bus=pci_bridge,addr=0x1 For consistency with other syspath-related debug messages, replace a \"%s\" in the message with '%s'. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1567041 Signed-off-by: Marc-André Lureau Reviewed-by: Laszlo Ersek Signed-off-by: Michael Roth --- qga/commands-posix.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index ae8535e497..7b7c78d85a 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -876,13 +876,28 @@ static void build_guest_fsinfo_for_real_device(char const *syspath, p = strstr(syspath, "/devices/pci"); if (!p || sscanf(p + 12, "%*x:%*x/%x:%x:%x.%x%n", pci, pci + 1, pci + 2, pci + 3, &pcilen) < 4) { - g_debug("only pci device is supported: sysfs path \"%s\"", syspath); + g_debug("only pci device is supported: sysfs path '%s'", syspath); return; } - driver = get_pci_driver(syspath, (p + 12 + pcilen) - syspath, errp); - if (!driver) { - goto cleanup; + p += 12 + pcilen; + while (true) { + driver = get_pci_driver(syspath, p - syspath, errp); + if (driver && (g_str_equal(driver, "ata_piix") || + g_str_equal(driver, "sym53c8xx") || + g_str_equal(driver, "virtio-pci") || + g_str_equal(driver, "ahci"))) { + break; + } + + if (sscanf(p, "/%x:%x:%x.%x%n", + pci, pci + 1, pci + 2, pci + 3, &pcilen) == 4) { + p += pcilen; + continue; + } + + g_debug("unsupported driver or sysfs path '%s'", syspath); + return; } p = strstr(syspath, "/target"); From 304a0fcb397efba40dffded985a6539143cca82c Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Thu, 21 Jun 2018 07:21:48 -0300 Subject: [PATCH 09/14] qga: refactoring qmp_guest_suspend_* functions To be able to add new suspend mechanisms we need to detach the existing QMP functions from the current implementation specifics. At this moment we have functions such as qmp_guest_suspend_ram calling bios_suspend_mode and guest_suspend passing the pmutils command and arguments as parameters. This patch removes this logic from the QMP functions, moving them to the respective functions that will have to deal with which binary to use. Signed-off-by: Daniel Henrique Barboza Signed-off-by: Michael Roth --- qga/commands-posix.c | 87 ++++++++++++++++++++++++++++---------------- 1 file changed, 55 insertions(+), 32 deletions(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 7b7c78d85a..bd2c3514d8 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1474,15 +1474,38 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp) #define LINUX_SYS_STATE_FILE "/sys/power/state" #define SUSPEND_SUPPORTED 0 #define SUSPEND_NOT_SUPPORTED 1 +#define SUSPEND_MODE_DISK 1 +#define SUSPEND_MODE_RAM 2 +#define SUSPEND_MODE_HYBRID 3 -static void bios_supports_mode(const char *pmutils_bin, const char *pmutils_arg, - const char *sysfile_str, Error **errp) +static void bios_supports_mode(int suspend_mode, Error **errp) { Error *local_err = NULL; + const char *pmutils_arg, *sysfile_str; + const char *pmutils_bin = "pm-is-supported"; char *pmutils_path; pid_t pid; int status; + switch (suspend_mode) { + + case SUSPEND_MODE_DISK: + pmutils_arg = "--hibernate"; + sysfile_str = "disk"; + break; + case SUSPEND_MODE_RAM: + pmutils_arg = "--suspend"; + sysfile_str = "mem"; + break; + case SUSPEND_MODE_HYBRID: + pmutils_arg = "--suspend-hybrid"; + sysfile_str = NULL; + break; + default: + error_setg(errp, "guest suspend mode not supported"); + return; + } + pmutils_path = g_find_program_in_path(pmutils_bin); pid = fork(); @@ -1559,14 +1582,39 @@ out: g_free(pmutils_path); } -static void guest_suspend(const char *pmutils_bin, const char *sysfile_str, - Error **errp) +static void guest_suspend(int suspend_mode, Error **errp) { Error *local_err = NULL; + const char *pmutils_bin, *sysfile_str; char *pmutils_path; pid_t pid; int status; + bios_supports_mode(suspend_mode, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + + switch (suspend_mode) { + + case SUSPEND_MODE_DISK: + pmutils_bin = "pm-hibernate"; + sysfile_str = "disk"; + break; + case SUSPEND_MODE_RAM: + pmutils_bin = "pm-suspend"; + sysfile_str = "mem"; + break; + case SUSPEND_MODE_HYBRID: + pmutils_bin = "pm-suspend-hybrid"; + sysfile_str = NULL; + break; + default: + error_setg(errp, "unknown guest suspend mode"); + return; + } + pmutils_path = g_find_program_in_path(pmutils_bin); pid = fork(); @@ -1629,42 +1677,17 @@ out: void qmp_guest_suspend_disk(Error **errp) { - Error *local_err = NULL; - - bios_supports_mode("pm-is-supported", "--hibernate", "disk", &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - - guest_suspend("pm-hibernate", "disk", errp); + guest_suspend(SUSPEND_MODE_DISK, errp); } void qmp_guest_suspend_ram(Error **errp) { - Error *local_err = NULL; - - bios_supports_mode("pm-is-supported", "--suspend", "mem", &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - - guest_suspend("pm-suspend", "mem", errp); + guest_suspend(SUSPEND_MODE_RAM, errp); } void qmp_guest_suspend_hybrid(Error **errp) { - Error *local_err = NULL; - - bios_supports_mode("pm-is-supported", "--suspend-hybrid", NULL, - &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - - guest_suspend("pm-suspend-hybrid", NULL, errp); + guest_suspend(SUSPEND_MODE_HYBRID, errp); } static GuestNetworkInterfaceList * From a5fcf0e3e491ede0d03e71f064e8e6d24e889aab Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Thu, 21 Jun 2018 07:21:49 -0300 Subject: [PATCH 10/14] qga: bios_supports_mode: decoupling pm-utils and sys logic In bios_supports_mode there is a verification to assert if the chosen suspend mode is supported by the pmutils tools and, if not, we see if the Linux sys state files supports it. This verification is done in the same function, one after the other, and it works for now. But, when adding a new suspend mechanism that will not necessarily follow the same return 0 or 1 logic of pmutils, this code will be hard to deal with. This patch decouple the two existing logics into their own functions, pmutils_supports_mode and linux_sys_state_supports_mode, which in turn are used inside bios_support_mode. The existing logic is kept but now it's easier to extend it. Signed-off-by: Daniel Henrique Barboza Signed-off-by: Michael Roth --- qga/commands-posix.c | 116 +++++++++++++++++++++++++------------------ 1 file changed, 68 insertions(+), 48 deletions(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index bd2c3514d8..a8299ecc77 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1478,75 +1478,43 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp) #define SUSPEND_MODE_RAM 2 #define SUSPEND_MODE_HYBRID 3 -static void bios_supports_mode(int suspend_mode, Error **errp) +static bool pmutils_supports_mode(int suspend_mode, Error **errp) { Error *local_err = NULL; - const char *pmutils_arg, *sysfile_str; + const char *pmutils_arg; const char *pmutils_bin = "pm-is-supported"; char *pmutils_path; pid_t pid; int status; + bool ret = false; switch (suspend_mode) { case SUSPEND_MODE_DISK: pmutils_arg = "--hibernate"; - sysfile_str = "disk"; break; case SUSPEND_MODE_RAM: pmutils_arg = "--suspend"; - sysfile_str = "mem"; break; case SUSPEND_MODE_HYBRID: pmutils_arg = "--suspend-hybrid"; - sysfile_str = NULL; break; default: - error_setg(errp, "guest suspend mode not supported"); - return; + return ret; } pmutils_path = g_find_program_in_path(pmutils_bin); + if (!pmutils_path) { + return ret; + } pid = fork(); if (!pid) { - char buf[32]; /* hopefully big enough */ - ssize_t ret; - int fd; - setsid(); - reopen_fd_to_null(0); - reopen_fd_to_null(1); - reopen_fd_to_null(2); - - if (pmutils_path) { - execle(pmutils_path, pmutils_bin, pmutils_arg, NULL, environ); - } - + execle(pmutils_path, pmutils_bin, pmutils_arg, NULL, environ); /* - * If we get here either pm-utils is not installed or execle() has - * failed. Let's try the manual method if the caller wants it. + * If we get here execle() has failed. */ - - if (!sysfile_str) { - _exit(SUSPEND_NOT_SUPPORTED); - } - - fd = open(LINUX_SYS_STATE_FILE, O_RDONLY); - if (fd < 0) { - _exit(SUSPEND_NOT_SUPPORTED); - } - - ret = read(fd, buf, sizeof(buf)-1); - if (ret <= 0) { - _exit(SUSPEND_NOT_SUPPORTED); - } - buf[ret] = '\0'; - - if (strstr(buf, sysfile_str)) { - _exit(SUSPEND_SUPPORTED); - } - _exit(SUSPEND_NOT_SUPPORTED); } else if (pid < 0) { error_setg_errno(errp, errno, "failed to create child process"); @@ -1559,17 +1527,11 @@ static void bios_supports_mode(int suspend_mode, Error **errp) goto out; } - if (!WIFEXITED(status)) { - error_setg(errp, "child process has terminated abnormally"); - goto out; - } - switch (WEXITSTATUS(status)) { case SUSPEND_SUPPORTED: + ret = true; goto out; case SUSPEND_NOT_SUPPORTED: - error_setg(errp, - "the requested suspend mode is not supported by the guest"); goto out; default: error_setg(errp, @@ -1580,6 +1542,64 @@ static void bios_supports_mode(int suspend_mode, Error **errp) out: g_free(pmutils_path); + return ret; +} + +static bool linux_sys_state_supports_mode(int suspend_mode, Error **errp) +{ + const char *sysfile_str; + char buf[32]; /* hopefully big enough */ + int fd; + ssize_t ret; + + switch (suspend_mode) { + + case SUSPEND_MODE_DISK: + sysfile_str = "disk"; + break; + case SUSPEND_MODE_RAM: + sysfile_str = "mem"; + break; + default: + return false; + } + + fd = open(LINUX_SYS_STATE_FILE, O_RDONLY); + if (fd < 0) { + return false; + } + + ret = read(fd, buf, sizeof(buf) - 1); + if (ret <= 0) { + return false; + } + buf[ret] = '\0'; + + if (strstr(buf, sysfile_str)) { + return true; + } + return false; +} + +static void bios_supports_mode(int suspend_mode, Error **errp) +{ + Error *local_err = NULL; + bool ret; + + ret = pmutils_supports_mode(suspend_mode, &local_err); + if (ret) { + return; + } + if (local_err) { + error_propagate(errp, local_err); + return; + } + ret = linux_sys_state_supports_mode(suspend_mode, errp); + if (!ret) { + error_setg(errp, + "the requested suspend mode is not supported by the guest"); + return; + } } static void guest_suspend(int suspend_mode, Error **errp) From 246d76eba1944d7e59affb288ec27d7fcfb5d256 Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Thu, 21 Jun 2018 07:21:50 -0300 Subject: [PATCH 11/14] qga: guest_suspend: decoupling pm-utils and sys logic Following the same logic of the previous patch, let's also decouple the suspend logic from guest_suspend into specialized functions, one for each strategy we support at this moment. Signed-off-by: Daniel Henrique Barboza Signed-off-by: Michael Roth --- qga/commands-posix.c | 194 ++++++++++++++++++++++++++----------------- 1 file changed, 120 insertions(+), 74 deletions(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index a8299ecc77..9ebd77b894 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1545,6 +1545,65 @@ out: return ret; } +static void pmutils_suspend(int suspend_mode, Error **errp) +{ + Error *local_err = NULL; + const char *pmutils_bin; + char *pmutils_path; + pid_t pid; + int status; + + switch (suspend_mode) { + + case SUSPEND_MODE_DISK: + pmutils_bin = "pm-hibernate"; + break; + case SUSPEND_MODE_RAM: + pmutils_bin = "pm-suspend"; + break; + case SUSPEND_MODE_HYBRID: + pmutils_bin = "pm-suspend-hybrid"; + break; + default: + error_setg(errp, "unknown guest suspend mode"); + return; + } + + pmutils_path = g_find_program_in_path(pmutils_bin); + if (!pmutils_path) { + error_setg(errp, "the helper program '%s' was not found", pmutils_bin); + return; + } + + pid = fork(); + if (!pid) { + setsid(); + execle(pmutils_path, pmutils_bin, NULL, environ); + /* + * If we get here execle() has failed. + */ + _exit(EXIT_FAILURE); + } else if (pid < 0) { + error_setg_errno(errp, errno, "failed to create child process"); + goto out; + } + + ga_wait_child(pid, &status, &local_err); + if (local_err) { + error_propagate(errp, local_err); + goto out; + } + + if (WEXITSTATUS(status)) { + error_setg(errp, + "the helper program '%s' returned an unexpected exit status" + " code (%d)", pmutils_path, WEXITSTATUS(status)); + } + +out: + g_free(pmutils_path); +} + static bool linux_sys_state_supports_mode(int suspend_mode, Error **errp) { const char *sysfile_str; @@ -1581,6 +1640,63 @@ static bool linux_sys_state_supports_mode(int suspend_mode, Error **errp) return false; } +static void linux_sys_state_suspend(int suspend_mode, Error **errp) +{ + Error *local_err = NULL; + const char *sysfile_str; + pid_t pid; + int status; + + switch (suspend_mode) { + + case SUSPEND_MODE_DISK: + sysfile_str = "disk"; + break; + case SUSPEND_MODE_RAM: + sysfile_str = "mem"; + break; + default: + error_setg(errp, "unknown guest suspend mode"); + return; + } + + pid = fork(); + if (!pid) { + /* child */ + int fd; + + setsid(); + reopen_fd_to_null(0); + reopen_fd_to_null(1); + reopen_fd_to_null(2); + + fd = open(LINUX_SYS_STATE_FILE, O_WRONLY); + if (fd < 0) { + _exit(EXIT_FAILURE); + } + + if (write(fd, sysfile_str, strlen(sysfile_str)) < 0) { + _exit(EXIT_FAILURE); + } + + _exit(EXIT_SUCCESS); + } else if (pid < 0) { + error_setg_errno(errp, errno, "failed to create child process"); + return; + } + + ga_wait_child(pid, &status, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + + if (WEXITSTATUS(status)) { + error_setg(errp, "child process has failed to suspend"); + } + +} + static void bios_supports_mode(int suspend_mode, Error **errp) { Error *local_err = NULL; @@ -1605,10 +1721,6 @@ static void bios_supports_mode(int suspend_mode, Error **errp) static void guest_suspend(int suspend_mode, Error **errp) { Error *local_err = NULL; - const char *pmutils_bin, *sysfile_str; - char *pmutils_path; - pid_t pid; - int status; bios_supports_mode(suspend_mode, &local_err); if (local_err) { @@ -1616,83 +1728,17 @@ static void guest_suspend(int suspend_mode, Error **errp) return; } - switch (suspend_mode) { - - case SUSPEND_MODE_DISK: - pmutils_bin = "pm-hibernate"; - sysfile_str = "disk"; - break; - case SUSPEND_MODE_RAM: - pmutils_bin = "pm-suspend"; - sysfile_str = "mem"; - break; - case SUSPEND_MODE_HYBRID: - pmutils_bin = "pm-suspend-hybrid"; - sysfile_str = NULL; - break; - default: - error_setg(errp, "unknown guest suspend mode"); + pmutils_suspend(suspend_mode, &local_err); + if (!local_err) { return; } - pmutils_path = g_find_program_in_path(pmutils_bin); + error_free(local_err); - pid = fork(); - if (pid == 0) { - /* child */ - int fd; - - setsid(); - reopen_fd_to_null(0); - reopen_fd_to_null(1); - reopen_fd_to_null(2); - - if (pmutils_path) { - execle(pmutils_path, pmutils_bin, NULL, environ); - } - - /* - * If we get here either pm-utils is not installed or execle() has - * failed. Let's try the manual method if the caller wants it. - */ - - if (!sysfile_str) { - _exit(EXIT_FAILURE); - } - - fd = open(LINUX_SYS_STATE_FILE, O_WRONLY); - if (fd < 0) { - _exit(EXIT_FAILURE); - } - - if (write(fd, sysfile_str, strlen(sysfile_str)) < 0) { - _exit(EXIT_FAILURE); - } - - _exit(EXIT_SUCCESS); - } else if (pid < 0) { - error_setg_errno(errp, errno, "failed to create child process"); - goto out; - } - - ga_wait_child(pid, &status, &local_err); + linux_sys_state_suspend(suspend_mode, &local_err); if (local_err) { error_propagate(errp, local_err); - goto out; } - - if (!WIFEXITED(status)) { - error_setg(errp, "child process has terminated abnormally"); - goto out; - } - - if (WEXITSTATUS(status)) { - error_setg(errp, "child process has failed to suspend"); - goto out; - } - -out: - g_free(pmutils_path); } void qmp_guest_suspend_disk(Error **errp) From 8b020b5eb708091e723518907184e609350f6d41 Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Thu, 21 Jun 2018 07:21:51 -0300 Subject: [PATCH 12/14] qga: removing switch statements, adding run_process_child This is a cleanup of the resulting code after detaching pmutils and Linux sys state file logic: - remove the SUSPEND_MODE_* macros and use an enumeration instead. At the same time, drop the switch statements at the start of each function and use the enumeration index to get the right binary/argument; - create a new function called run_process_child(). This function uses g_spawn_sync() to execute a shell command, returning the exit code. This is a common operation in the pmutils functions and will be used in the systemd implementation as well, so this function will avoid code repetition. There are more places inside commands-posix.c where this new run_process_child function can also be used, but one step at a time. Signed-off-by: Daniel Henrique Barboza *check/propagate local_err before setting errp directly Signed-off-by: Michael Roth --- qga/commands-posix.c | 227 ++++++++++++++++++------------------------- 1 file changed, 94 insertions(+), 133 deletions(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 9ebd77b894..1559753d85 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1474,152 +1474,117 @@ qmp_guest_fstrim(bool has_minimum, int64_t minimum, Error **errp) #define LINUX_SYS_STATE_FILE "/sys/power/state" #define SUSPEND_SUPPORTED 0 #define SUSPEND_NOT_SUPPORTED 1 -#define SUSPEND_MODE_DISK 1 -#define SUSPEND_MODE_RAM 2 -#define SUSPEND_MODE_HYBRID 3 -static bool pmutils_supports_mode(int suspend_mode, Error **errp) +typedef enum { + SUSPEND_MODE_DISK = 0, + SUSPEND_MODE_RAM = 1, + SUSPEND_MODE_HYBRID = 2, +} SuspendMode; + +/* + * Executes a command in a child process using g_spawn_sync, + * returning an int >= 0 representing the exit status of the + * process. + * + * If the program wasn't found in path, returns -1. + * + * If a problem happened when creating the child process, + * returns -1 and errp is set. + */ +static int run_process_child(const char *command[], Error **errp) { - Error *local_err = NULL; - const char *pmutils_arg; - const char *pmutils_bin = "pm-is-supported"; - char *pmutils_path; - pid_t pid; - int status; - bool ret = false; + int exit_status, spawn_flag; + GError *g_err = NULL; + bool success; - switch (suspend_mode) { + spawn_flag = G_SPAWN_SEARCH_PATH | G_SPAWN_STDOUT_TO_DEV_NULL | + G_SPAWN_STDERR_TO_DEV_NULL; - case SUSPEND_MODE_DISK: - pmutils_arg = "--hibernate"; - break; - case SUSPEND_MODE_RAM: - pmutils_arg = "--suspend"; - break; - case SUSPEND_MODE_HYBRID: - pmutils_arg = "--suspend-hybrid"; - break; - default: - return ret; + success = g_spawn_sync(NULL, (char **)command, environ, spawn_flag, + NULL, NULL, NULL, NULL, + &exit_status, &g_err); + + if (success) { + return WEXITSTATUS(exit_status); } - pmutils_path = g_find_program_in_path(pmutils_bin); - if (!pmutils_path) { - return ret; + if (g_err && (g_err->code != G_SPAWN_ERROR_NOENT)) { + error_setg(errp, "failed to create child process, error '%s'", + g_err->message); } - pid = fork(); - if (!pid) { - setsid(); - execle(pmutils_path, pmutils_bin, pmutils_arg, NULL, environ); - /* - * If we get here execle() has failed. - */ - _exit(SUSPEND_NOT_SUPPORTED); - } else if (pid < 0) { - error_setg_errno(errp, errno, "failed to create child process"); - goto out; - } - - ga_wait_child(pid, &status, &local_err); - if (local_err) { - error_propagate(errp, local_err); - goto out; - } - - switch (WEXITSTATUS(status)) { - case SUSPEND_SUPPORTED: - ret = true; - goto out; - case SUSPEND_NOT_SUPPORTED: - goto out; - default: - error_setg(errp, - "the helper program '%s' returned an unexpected exit status" - " code (%d)", pmutils_path, WEXITSTATUS(status)); - goto out; - } - -out: - g_free(pmutils_path); - return ret; + g_error_free(g_err); + return -1; } -static void pmutils_suspend(int suspend_mode, Error **errp) +static bool pmutils_supports_mode(SuspendMode mode, Error **errp) { Error *local_err = NULL; - const char *pmutils_bin; - char *pmutils_path; - pid_t pid; + const char *pmutils_args[3] = {"--hibernate", "--suspend", + "--suspend-hybrid"}; + const char *cmd[3] = {"pm-is-supported", pmutils_args[mode], NULL}; int status; - switch (suspend_mode) { + status = run_process_child(cmd, &local_err); - case SUSPEND_MODE_DISK: - pmutils_bin = "pm-hibernate"; - break; - case SUSPEND_MODE_RAM: - pmutils_bin = "pm-suspend"; - break; - case SUSPEND_MODE_HYBRID: - pmutils_bin = "pm-suspend-hybrid"; - break; - default: - error_setg(errp, "unknown guest suspend mode"); + if (status == SUSPEND_SUPPORTED) { + return true; + } + + if ((status == -1) && !local_err) { + return false; + } + + if (local_err) { + error_propagate(errp, local_err); + } else { + error_setg(errp, + "the helper program '%s' returned an unexpected exit" + " status code (%d)", "pm-is-supported", status); + } + + return false; +} + +static void pmutils_suspend(SuspendMode mode, Error **errp) +{ + Error *local_err = NULL; + const char *pmutils_binaries[3] = {"pm-hibernate", "pm-suspend", + "pm-suspend-hybrid"}; + const char *cmd[2] = {pmutils_binaries[mode], NULL}; + int status; + + status = run_process_child(cmd, &local_err); + + if (status == 0) { return; } - pmutils_path = g_find_program_in_path(pmutils_bin); - if (!pmutils_path) { - error_setg(errp, "the helper program '%s' was not found", pmutils_bin); + if ((status == -1) && !local_err) { + error_setg(errp, "the helper program '%s' was not found", + pmutils_binaries[mode]); return; } - pid = fork(); - if (!pid) { - setsid(); - execle(pmutils_path, pmutils_bin, NULL, environ); - /* - * If we get here execle() has failed. - */ - _exit(EXIT_FAILURE); - } else if (pid < 0) { - error_setg_errno(errp, errno, "failed to create child process"); - goto out; - } - - ga_wait_child(pid, &status, &local_err); if (local_err) { error_propagate(errp, local_err); - goto out; - } - - if (WEXITSTATUS(status)) { + } else { error_setg(errp, - "the helper program '%s' returned an unexpected exit status" - " code (%d)", pmutils_path, WEXITSTATUS(status)); + "the helper program '%s' returned an unexpected exit" + " status code (%d)", pmutils_binaries[mode], status); } - -out: - g_free(pmutils_path); } -static bool linux_sys_state_supports_mode(int suspend_mode, Error **errp) +static bool linux_sys_state_supports_mode(SuspendMode mode, Error **errp) { - const char *sysfile_str; + const char *sysfile_strs[3] = {"disk", "mem", NULL}; + const char *sysfile_str = sysfile_strs[mode]; char buf[32]; /* hopefully big enough */ int fd; ssize_t ret; - switch (suspend_mode) { - - case SUSPEND_MODE_DISK: - sysfile_str = "disk"; - break; - case SUSPEND_MODE_RAM: - sysfile_str = "mem"; - break; - default: + if (!sysfile_str) { + error_setg(errp, "unknown guest suspend mode"); return false; } @@ -1640,22 +1605,15 @@ static bool linux_sys_state_supports_mode(int suspend_mode, Error **errp) return false; } -static void linux_sys_state_suspend(int suspend_mode, Error **errp) +static void linux_sys_state_suspend(SuspendMode mode, Error **errp) { Error *local_err = NULL; - const char *sysfile_str; + const char *sysfile_strs[3] = {"disk", "mem", NULL}; + const char *sysfile_str = sysfile_strs[mode]; pid_t pid; int status; - switch (suspend_mode) { - - case SUSPEND_MODE_DISK: - sysfile_str = "disk"; - break; - case SUSPEND_MODE_RAM: - sysfile_str = "mem"; - break; - default: + if (!sysfile_str) { error_setg(errp, "unknown guest suspend mode"); return; } @@ -1697,12 +1655,12 @@ static void linux_sys_state_suspend(int suspend_mode, Error **errp) } -static void bios_supports_mode(int suspend_mode, Error **errp) +static void bios_supports_mode(SuspendMode mode, Error **errp) { Error *local_err = NULL; bool ret; - ret = pmutils_supports_mode(suspend_mode, &local_err); + ret = pmutils_supports_mode(mode, &local_err); if (ret) { return; } @@ -1710,32 +1668,35 @@ static void bios_supports_mode(int suspend_mode, Error **errp) error_propagate(errp, local_err); return; } - ret = linux_sys_state_supports_mode(suspend_mode, errp); + ret = linux_sys_state_supports_mode(mode, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } if (!ret) { error_setg(errp, "the requested suspend mode is not supported by the guest"); - return; } } -static void guest_suspend(int suspend_mode, Error **errp) +static void guest_suspend(SuspendMode mode, Error **errp) { Error *local_err = NULL; - bios_supports_mode(suspend_mode, &local_err); + bios_supports_mode(mode, &local_err); if (local_err) { error_propagate(errp, local_err); return; } - pmutils_suspend(suspend_mode, &local_err); + pmutils_suspend(mode, &local_err); if (!local_err) { return; } error_free(local_err); - linux_sys_state_suspend(suspend_mode, &local_err); + linux_sys_state_suspend(mode, &local_err); if (local_err) { error_propagate(errp, local_err); } From 067927d62e097a8a3624a926e792756ce7a353ed Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Thu, 21 Jun 2018 07:21:52 -0300 Subject: [PATCH 13/14] qga: systemd hibernate/suspend/hybrid-sleep support pmutils isn't being supported by newer OSes like Fedora 27 or Mint. This means that the only suspend option QGA offers for these guests are writing directly into the Linux sys state file. This also means that QGA also loses the ability to do hybrid suspend in those guests - this suspend mode is only available when using pmutils. Newer guests can use systemd facilities to do all the suspend types QGA supports. The mapping in comparison with pmutils is: - pm-hibernate -> systemctl hibernate - pm-suspend -> systemctl suspend - pm-suspend-hybrid -> systemctl hybrid-sleep To discover whether systemd supports these functions, we inspect the status of the services that implements them. With this patch, we can offer hybrid suspend again for newer guests that do not have pmutils support anymore. Signed-off-by: Daniel Henrique Barboza Signed-off-by: Michael Roth --- qga/commands-posix.c | 72 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 1559753d85..cdb825993f 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1517,6 +1517,63 @@ static int run_process_child(const char *command[], Error **errp) return -1; } +static bool systemd_supports_mode(SuspendMode mode, Error **errp) +{ + Error *local_err = NULL; + const char *systemctl_args[3] = {"systemd-hibernate", "systemd-suspend", + "systemd-hybrid-sleep"}; + const char *cmd[4] = {"systemctl", "status", systemctl_args[mode], NULL}; + int status; + + status = run_process_child(cmd, &local_err); + + /* + * systemctl status uses LSB return codes so we can expect + * status > 0 and be ok. To assert if the guest has support + * for the selected suspend mode, status should be < 4. 4 is + * the code for unknown service status, the return value when + * the service does not exist. A common value is status = 3 + * (program is not running). + */ + if (status > 0 && status < 4) { + return true; + } + + if (local_err) { + error_propagate(errp, local_err); + } + + return false; +} + +static void systemd_suspend(SuspendMode mode, Error **errp) +{ + Error *local_err = NULL; + const char *systemctl_args[3] = {"hibernate", "suspend", "hybrid-sleep"}; + const char *cmd[3] = {"systemctl", systemctl_args[mode], NULL}; + int status; + + status = run_process_child(cmd, &local_err); + + if (status == 0) { + return; + } + + if ((status == -1) && !local_err) { + error_setg(errp, "the helper program 'systemctl %s' was not found", + systemctl_args[mode]); + return; + } + + if (local_err) { + error_propagate(errp, local_err); + } else { + error_setg(errp, "the helper program 'systemctl %s' returned an " + "unexpected exit status code (%d)", + systemctl_args[mode], status); + } +} + static bool pmutils_supports_mode(SuspendMode mode, Error **errp) { Error *local_err = NULL; @@ -1660,6 +1717,14 @@ static void bios_supports_mode(SuspendMode mode, Error **errp) Error *local_err = NULL; bool ret; + ret = systemd_supports_mode(mode, &local_err); + if (ret) { + return; + } + if (local_err) { + error_propagate(errp, local_err); + return; + } ret = pmutils_supports_mode(mode, &local_err); if (ret) { return; @@ -1689,6 +1754,13 @@ static void guest_suspend(SuspendMode mode, Error **errp) return; } + systemd_suspend(mode, &local_err); + if (!local_err) { + return; + } + + error_free(local_err); + pmutils_suspend(mode, &local_err); if (!local_err) { return; From 73e1d8eb9b738cef3dee2da26bb669b1092a4c12 Mon Sep 17 00:00:00 2001 From: Daniel Henrique Barboza Date: Thu, 21 Jun 2018 07:21:53 -0300 Subject: [PATCH 14/14] qga: removing bios_supports_mode bios_support_mode verifies if the guest has support for a certain suspend mode but it doesn't inform back which suspend tool provides it. The caller, guest_suspend, executes all suspend strategies in order again. After adding systemd suspend support, bios_support_mode now will verify for support for systemd, then pmutils, then Linux sys state file. In a worst case scenario where both systemd and pmutils isn't supported but Linux sys state is: - bios_supports_mode will check for systemd, then pmutils, then Linux sys state. It will tell guest_suspend that there is support, but it will not tell who provides it; - guest_suspend will try to execute (and fail) systemd suspend, then pmutils suspend, to only then use the Linux sys suspend. The time spent executing systemd and pmutils suspend was wasted and could be avoided, but only bios_support_mode knew it but didn't inform it back. A quicker approach is to nuke bios_supports_mode and control whether we found support at all with a bool flag inside guest_suspend. guest_suspend will search for suspend support and execute it as soon as possible. If the a given suspend mechanism fails, continue to the next. If no suspend support is found, the "not supported" message is still being sent back to the user. Signed-off-by: Daniel Henrique Barboza Signed-off-by: Michael Roth --- qga/commands-posix.c | 58 ++++++++++++++------------------------------ 1 file changed, 18 insertions(+), 40 deletions(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index cdb825993f..233f78a406 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1712,64 +1712,42 @@ static void linux_sys_state_suspend(SuspendMode mode, Error **errp) } -static void bios_supports_mode(SuspendMode mode, Error **errp) -{ - Error *local_err = NULL; - bool ret; - - ret = systemd_supports_mode(mode, &local_err); - if (ret) { - return; - } - if (local_err) { - error_propagate(errp, local_err); - return; - } - ret = pmutils_supports_mode(mode, &local_err); - if (ret) { - return; - } - if (local_err) { - error_propagate(errp, local_err); - return; - } - ret = linux_sys_state_supports_mode(mode, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - if (!ret) { - error_setg(errp, - "the requested suspend mode is not supported by the guest"); - } -} - static void guest_suspend(SuspendMode mode, Error **errp) { Error *local_err = NULL; + bool mode_supported = false; - bios_supports_mode(mode, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; + if (systemd_supports_mode(mode, &local_err)) { + mode_supported = true; + systemd_suspend(mode, &local_err); } - systemd_suspend(mode, &local_err); if (!local_err) { return; } error_free(local_err); - pmutils_suspend(mode, &local_err); + if (pmutils_supports_mode(mode, &local_err)) { + mode_supported = true; + pmutils_suspend(mode, &local_err); + } + if (!local_err) { return; } error_free(local_err); - linux_sys_state_suspend(mode, &local_err); - if (local_err) { + if (linux_sys_state_supports_mode(mode, &local_err)) { + mode_supported = true; + linux_sys_state_suspend(mode, &local_err); + } + + if (!mode_supported) { + error_setg(errp, + "the requested suspend mode is not supported by the guest"); + } else if (local_err) { error_propagate(errp, local_err); } }