[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 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
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.

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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]