[libvirt] [PATCH] qemu: use guest-fsfreeze-freeze-list command if mountpoints to freeze specified

[Tomoki Sekiyama]

On 8/11/14 16:36 , "Eric Blake" <eblake at redhat.com> wrote:

>On 08/08/2014 02:03 PM, Tomoki Sekiyama wrote:
>> A command to freeze a part of mounted file systems is implemented in
>> upstream QEMU-guest-agent with a name of 'guest-fsfreeze-freeze-list'.
>> This fixes the name of the command used to partial fsfreeze in qemu
>>driver
>> when 'mountpoints' option is specified to virDomainFSFreeze API.
>> 
>> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama at hds.com>
>> ---
>>  src/qemu/qemu_agent.c |    2 +-
>>  tests/qemuagenttest.c |    2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>
>Michal already pushed this, but I wish he had waited a bit longer.  You
>are trying to use a qemu-guest-agent command that has just barely been
>added for qemu 2.2 (merge commit 2d591ce in qemu.git).  We tend to avoid
>doing patches to libvirt without guarantees that upstream qemu has
>committed to the interface, and I want to avoid even the slight risk
>that the interface could change in qemu prior to the release candidate
>freeze for qemu 2.2 in a couple months.
>
>> 
>> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
>> index 0421733..a10954a 100644
>> --- a/src/qemu/qemu_agent.c
>> +++ b/src/qemu/qemu_agent.c
>> @@ -1336,7 +1336,7 @@ int qemuAgentFSFreeze(qemuAgentPtr mon, const
>>char **mountpoints,
>>          if (!arg)
>>              return -1;
>>  
>> -        cmd = qemuAgentMakeCommand("guest-fsfreeze-freeze",
>> +        cmd = qemuAgentMakeCommand("guest-fsfreeze-freeze-list",
>
>Worse, this patch causes a regression for guests using qemu-guest-agent
>2.1 (which lacks 'guest-fsfreeze-freeze-list', but has
>'guest-fsfreeze-freeze'), for the case where the user provided NULL for
>the set of mountpoints.  The only proper way to fix this is to do
>conditional calls: if the user supplies NULL for mountpoints,
>unconditionally use the old API; if the user supplies non-NULL
>mountpoints, then use the new command.  Then, depending on whether the

This is what my patch does -- if mountpoints == NULL, it always uses
'guest-fsfreeze-freeze' command, so old qga (<=2.1) still works with
patched libvirt.

>error message that gets generated when qga 2.1 doesn't support the
>command is gross looking (basically, if the error message includes
>"internal error"), then you also need libvirt capability probing so that
>libvirt can detect up front whether qga is new enough, and give a nicer
>error message up front instead of relying on the guest agent to complain.

OK, for nicer error messages, I will post a follow up patch soon.

That would query 'guest-info', and if qga doesn't support
'guest-fsfreeze-freeze-list', gives users an error message that says
something like "this version of qemu guest agent doesn't support
specifying mountpoints to freeze". Does it sound correct?

>I'd like to see a followup patch before libvirt 1.2.8 goes into freeze;
>if we don't get one in time, then I plan on reverting this patch so that
>we don't cause regressions to users of older guest agents.

Thanks,
Tomoki Sekiyama