On 8/11/14 16:36 , "Eric Blake" <eblake@xxxxxxxxxx> 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@xxxxxxx>
>> ---
>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list