[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target |
Date: |
Fri, 18 May 2018 10:48:31 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) |
Cc'ing a few more people.
Daniel Henrique Barboza <address@hidden> writes:
> When issuing the qmp/hmp 'system_wakeup' command, what happens in a
> nutshell is:
>
> - qmp_system_wakeup_request set runstate to RUNNING, sets a wakeup_reason
> and notify the event
> - in the main_loop, all vcpus are paused, a system reset is issued, all
> subscribers of wakeup_notifiers receives a notification, vcpus are then
> resumed and the wake up QAPI event is fired
>
> Note that this procedure alone doesn't ensure that the guest will awake
> from SUSPENDED state - the subscribers of the wake up event must take
> action to resume the guest, otherwise the guest will simply reboot.
>
> At this moment there are only two subscribers of the wake up event: one
> in hw/acpi/core.c and another one in hw/i386/xen/xen-hvm.c. This means
> that system_wakeup does not work as intended with other architectures.
>
> However, only the presence of 'system_wakeup' is required for QGA to
> support 'guest-suspend-ram' and 'guest-suspend-hybrid' at this moment.
> This means that the user/management will expect to suspend the guest using
> one of those suspend commands and then resume execution using system_wakeup,
> regardless of the support offered in system_wakeup in the first place.
>
> This patch adds a new flag called 'wakeup-suspend-support' in TargetInfo
> that allows the caller to query if the guest supports wake up from
> suspend via system_wakeup. It goes over the subscribers of the wake up
> event and, if it's empty, it assumes that the guest does not support
> wake up from suspend (and thus, pm-suspend itself).
>
> This is the expected output of query-target when running a x86 guest:
>
> {"execute" : "query-target"}
> {"return": {"arch": "x86_64", "wakeup-suspend-support": true}}
>
> This is the output when running a pseries guest:
>
> {"execute" : "query-target"}
> {"return": {"arch": "ppc64", "wakeup-suspend-support": false}}
>
> Given that the TargetInfo structure is read-only, adding a new flag to
> it is backwards compatible. There is no need to deprecate the old
> TargetInfo format.
>
> With this extra tool, management can avoid situations where a guest
> that does not have proper suspend/wake capabilities ends up in
> inconsistent state (e.g.
> https://github.com/open-power-host-os/qemu/issues/31).
>
> Reported-by: Balamuruhan S <address@hidden>
> Signed-off-by: Daniel Henrique Barboza <address@hidden>
Is query-target is the right place to carry this flag? v7 is rather
late for this kind of question; my sincere apologies.
The flag is true after qemu_register_wakeup_notifier(). Callers so far:
* piix4_pm_realize() via acpi_pm1_cnt_init()
This is the realize method of device PIIX4_PM. It's an optional
onboard device (suppressed by -no-acpi) of machine types
pc-i440fx-VERSION, pc-VERSION, malta.
* pc_q35_init() via ich9_lpc_pm_init(), ich9_pm_init(),
acpi_pm1_cnt_init()
This is the initialization method of machine types pc-q35-VERSION.
Note that -no-acpi is not honored.
* vt82c686b_pm_realize() via acpi_pm1_cnt_init()
This is the realize method of device VT82C686B_PM. It's an onboard
device of machine type fulong2e. Again, -no-acpi is not honored.
* xen_hvm_init()
This one gets called with -accel xen. I suspect the actual callback
xen_wakeup_notifier() doesn't actually make wakeup work, unlike the
acpi_notify_wakeup() callback registered by the other callers.
Issue#1: this calls into question your assumption that the existence
of a wake-up notifier implies wake-up works. It still holds if
--accel xen is only accepted together with other configuration that
triggers registration of acpi_notify_wakeup(). Is it? Stefano,
Anthony?
Issue#2: the flag isn't a property of the target. Due to -no-acpi, it's
not even a property of the machine type. If it was, query-machines
would be the natural owner of the flag.
Perhaps query-machines is still the proper owner. The value of
wakeup-suspend-support would have to depend on -no-acpi for the machine
types that honor it. Not ideal; I'd prefer MachineInfo to be static.
Tolerable? I guess that's also a libvirt question.
- [Qemu-devel] [PATCH v7 0/3] wakeup-from-suspend and system_wakeup changes, Daniel Henrique Barboza, 2018/05/17
- [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target, Daniel Henrique Barboza, 2018/05/17
- Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target, Eduardo Habkost, 2018/05/21
- Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target, Daniel Henrique Barboza, 2018/05/21
- Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target, Eduardo Habkost, 2018/05/21
- Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target, Markus Armbruster, 2018/05/23
- Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target, Eduardo Habkost, 2018/05/23
- Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target, Daniel Henrique Barboza, 2018/05/23
- Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target, Markus Armbruster, 2018/05/23
- Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target, Eduardo Habkost, 2018/05/24
- Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target, Markus Armbruster, 2018/05/25
- Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target, Eduardo Habkost, 2018/05/25