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(a)hds.com&gt; > --- > 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.