[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: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target |
Date: |
Mon, 21 May 2018 17:26:16 -0300 |
User-agent: |
Mutt/1.9.2 (2017-12-15) |
On Mon, May 21, 2018 at 04:46:36PM -0300, Daniel Henrique Barboza wrote:
>
>
> On 05/21/2018 03:14 PM, Eduardo Habkost wrote:
> > > 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.
> > It depends when libvirt is going to query it. Is it OK to only
> > query it after the VM is already up and running? If it is, then
> > we can simply expose it as a read-only property of the machine
> > object.
> >
> > Or, if we don't want to rely on qom-get as a stable API, we can
> > add a new query command (query-machine? query-power-management?)
> >
> In the first version this logic was included in a new query command called
> "query-wakeup-from-suspend-support":
>
> https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg00889.html
>
> In that review it was suggested that this logic could be a flag in either
> query-target
> or query-machines API. Before sending the v2 I sent the following comment:
>
> "After investigating, I think that it's simpler to hook the wakeup support
> info into
> TargetInfo than MachineInfo, given that the detection I'm using for this new
> property
> is based on the current runtime state. Hooking into MachineInfo would
> require to
> change the MachineClass to add a new property, then setting it up for the
> machines
> that have the wakeup support (only x86 so far). Definitely doable, but if we
> don't
> have any favorites between MachineInfo and TargetInfo I'd rather pick the
> simpler
> route.
>
> So, if no one objects, I'll rework this series by putting the logic inside
> query-target
> instead of a new API."
Apologies for not noticing this series months ago. :(
>
> Since no objection was made back then, this logic was put into query-target
> starting
> in v2. Still, I don't have any favorites though: query-target looks ok,
> query-machine
> looks ok and a new API looks ok too. It's all about what makes (more) sense
> in the
> management level, I think.
I understand the original objection from Eric: having to add a
new command for every runtime flag we want to expose to the user
looks wrong to me.
However, extending query-machines and query-target looks wrong
too, however. query-target looks wrong because this not a
property of the target. query-machines is wrong because this is
not a static property of the machine-type, but of the running
machine instance.
Can we have a new query command that could be an obvious
container for simple machine capabilities that are not static? A
name like "query-machine" would be generic enough for that, I
guess.
Markus, Eric, what do you think?
--
Eduardo
- [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, 2018/05/18
- 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 <=
- 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
- Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target, Markus Armbruster, 2018/05/28
- Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target, Eduardo Habkost, 2018/05/29
[Qemu-devel] [PATCH v7 2/3] qga: update guest-suspend-ram and guest-suspend-hybrid descriptions, Daniel Henrique Barboza, 2018/05/17