Re: [Qemu-devel] [libvirt] [PATCH RFC 0/4] Allow hibernation on guests

[Michal Privoznik]

On 30.01.2012 15:44, Luiz Capitulino wrote:
> On Mon, 30 Jan 2012 07:54:56 -0600
> Anthony Liguori <address@hidden> wrote:
> 
>> On 01/30/2012 06:57 AM, Luiz Capitulino wrote:
>>> On Thu, 26 Jan 2012 16:57:01 -0600
>>> Anthony Liguori<address@hidden>  wrote:
>>>
>>>> On 01/26/2012 01:35 PM, Luiz Capitulino wrote:
>>>>> On Thu, 26 Jan 2012 08:18:03 -0700
>>>>> Eric Blake<address@hidden>   wrote:
>>>>>
>>>>>> [adding qemu-devel]
>>>>>>
>>>>>> On 01/26/2012 07:46 AM, Daniel P. Berrange wrote:
>>>>>>>> One thing, that you'll probably notice is this
>>>>>>>> 'set-support-level' command. Basically, it tells GA what qemu version
>>>>>>>> is it running on. Ideally, this should be done as soon as
>>>>>>>> GA starts up. However, that cannot be determined from outside
>>>>>>>> world as GA doesn't emit any events yet.
>>>>>>>> Ideally^2 this command should be left out as it should be qemu
>>>>>>>> who tells its own agent this kind of information.
>>>>>>>> Anyway, I was going to call this command in qemuProcess{Startup,
>>>>>>>> Reconnect,Attach}, but it won't work. We need to un-pause guest CPUs
>>>>>>>> so guest can boot and start GA, but that implies returning from 
>>>>>>>> qemuProcess*.
>>>>>>>>
>>>>>>>> So I am setting this just before 'guest-suspend' command, as
>>>>>>>> there is one more thing about GA. It is unable to remember anything
>>>>>>>> upon its restart (GA process). Which has BTW show flaw
>>>>>>>> in our current code with FS freeze&   thaw. If we freeze guest
>>>>>>>> FS, and somebody restart GA, the simple FS Thaw will not succeed as
>>>>>>>> GA thinks FS are not frozen. But that's a different cup of tea.
>>>>>>>>
>>>>>>>> Because of what written above, we need to call set-level
>>>>>>>> on every suspend.
>>>>>>>
>>>>>>>
>>>>>>> IMHO all this says that the 'set-level' command is a conceptually
>>>>>>> unfixably broken design&   should be killed in QEMU before it turns
>>>>>>> into an even bigger mess.
>>>>>
>>>>> Can you elaborate on this? Michal and I talked on irc about making the
>>>>> compatibility level persistent, would that help?
>>>>>
>>>>>>> Once we're in a situation where we need to call 'set-level' prior
>>>>>>> to every single invocation, you might as well just allow the QEMU
>>>>>>> version number to be passed in directly as an arg to the command
>>>>>>> you are running directly thus avoiding this horrificness.
>>>>>>
>>>>>> Qemu folks, would you care to chime in on this?
>>>>>>
>>>>>> Exactly how is the set-level command supposed to work?  As I understand
>>>>>> it, the goal is that if the guest has qemu-ga 1.1 installed, but is
>>>>>> being run by qemu 1.0, then we want to ensure that any guest agent
>>>>>> command supported by qemu-ga 1.1 but requiring features of qemu not
>>>>>> present in qemu 1.0 will be properly rejected.
>>>>>
>>>>> Not exactly, the default support of qemu-ga is qemu 1.0. This means that 
>>>>> by
>>>>> default qemu-ga will only support qemu 1.0 even when running on qemu 2.0. 
>>>>> This
>>>>> way the set-support-level command allows you to specify that qemu 2.0 
>>>>> features
>>>>> are supported.
>>>>
>>>> Version numbers are meaningless.  What happens when a bunch of features get
>>>> backported by RHEL such that qemu-ga 1.0 ends up being a frankenstein 
>>>> version of
>>>> 2.0?
>>>>
>>>> The feature negotiation mechanism we have in QMP is the existence of a 
>>>> command.
>>>>    If we're in a position where we're trying to disable part of a command, 
>>>> it
>>>> simply means that we should have multiple commands such that we can just 
>>>> remove
>>>> the disabled part entirely.
>>>
>>> You may have a point that we shouldn't be using the version number for that,
>>> but just switching to multiple commands doesn't solve the fundamental 
>>> problem.
>>>
>>> The fundamental problem is that, S3 in current (and old) qemu has two known 
>>> bugs:
>>>
>>>   1. The screen is left black after S3 (it's a bug in seabios)
>>>   2. QEMU resumes the guest immediately (Gerd posted patches to address 
>>> this)
>>>
>>> We're going to address both issues in 1.1. However, if qemu-ga is installed 
>>> in
>>> an old qemu and S3 is used, the bugs will be triggered.
>>
>> It's a management tool problem.
>>
>> Before a management tool issues a command, it should query the existence of 
>> the 
>> command to determine whether this version of QEMU has that capability.  If 
>> the 
>> tool needs to use two commands, it should query the existence of both of 
>> them.
>>
>> In this case, the management tool needs a qemu-ga command *and* a QEMU 
>> command 
>> (to resume from suspend) so it should query both of them.
>>
>> Obviously, we wouldn't have a resume-from-suspend command in QEMU unless it 
>> S3 
>> worked in QEMU as expected.
> 
> That's right, it's a coincidence, but I don't see why this wouldn't work.
> 
> I think we should do the following then:
> 
>  1. Drop the set-support-level command
>  2. Split the guest-suspend command into guest-suspend-ram, 
> guest-suspend-hybrid,
>     guest-suspend-disk
>  3. Libvirt should query for _QEMU_'s 'wakeup' command before issuing
>     the guest-suspend-ram command
> 
> Michal, Michael, do you agree?

I don't see into qemu internals, but <like> to dropping
set-support-level for sure. From my POV, splitting may solve the
problem. But how hard would it be to fix suspend to ram in qemu, so
libvirt doesn't have to query for wakeup command? It would be nice if we
have easy to use interface.

Michal