Bug 856422

The guest agent has this comment:

    /* if we're frozen, don't exit unless we're absolutely forced to,
     * because it's basically impossible for graceful exit to complete
     * unless all log/pid files are on unfreezable filesystems. there's
     * also a very likely chance killing the agent before unfreezing
     * the filesystems is a mistake (or will be viewed as one later).
     */

It makes me think that making the frozen status persistent is a mistake.  When starting the agent we can assume that the filesystems are not frozen.

I've thought about this and my conclusion is that this behavior is the best we can do.

qemu-ga is faced with two mutual exclusive scenarios (that is, fixing one breaks the other):

 1. The FS is frozen and qemu-ga is killed

 2. The FS is frozen and the guest is restarted (this bz)

The first issue is somewhat serious. If qemu-ga is killed and is unable to remember that the FS was frozen, then two things would happen. Firstly, it would get blocked while starting up. Secondly, it's very unlikely that the FS would get correctly thawed by another application, this means that a reboot would certainly be needed.

To prevent this situation, qemu-ga creates a "frozen status" file before freezing the FS. On start up, this file is checked and if it exists then qemu-ga knows the FS was frozen.

This solution causes item 2 above, because if the guest is restarted while the FS is frozen, the file will still exist when qemu restarts. There's a comment in the code which talks about this:

    /* check if a previous instance of qemu-ga exited with filesystems' state
     * marked as frozen. this could be a stale value (a non-qemu-ga process
     * or reboot may have since unfrozen them), but better to require an
     * uneeded unfreeze than to risk hanging on start-up
     */ 

I agree with the last statement, an additional call to fsfreeze-thaw shouldn't be a big deal for mngt apps (while getting qemu-ga hung up is).

An idea to improve this would be to add an ioctl() to the kernel to allow applications to get freeze status. This way qemu-ga could drop the state file for newer kernels. The problem with this approach is that it has to be accepted upstream, and the state file would still be needed for older kernels.

Having said all this, I'm willing to close this as WONTFIX, unless we want implement the idea above or someone has strong arguments against the current behavior.

(In reply to comment #3)
> I've thought about this and my conclusion is that this behavior is the best
> we can do.
> 
> qemu-ga is faced with two mutual exclusive scenarios (that is, fixing one
> breaks the other):
> 
>  1. The FS is frozen and qemu-ga is killed
> 
>  2. The FS is frozen and the guest is restarted (this bz)
> 
> The first issue is somewhat serious. If qemu-ga is killed and is unable to
> remember that the FS was frozen, then two things would happen. Firstly, it
> would get blocked while starting up. Secondly, it's very unlikely that the
> FS would get correctly thawed by another application, this means that a
> reboot would certainly be needed.
> 
> To prevent this situation, qemu-ga creates a "frozen status" file before
> freezing the FS. On start up, this file is checked and if it exists then
> qemu-ga knows the FS was frozen.

qemu-ga shouldn't keep any state.  It should just be a daemon in the guest, which gets restarted upon exit.  All state should be maintained in the host.  When qemu-ga (re-)starts, it queries the host for current state.  If the host says the guest fs is frozen, qemu-ga should then set its internal state accordingly.

(In reply to comment #4)

> qemu-ga shouldn't keep any state.  It should just be a daemon in the guest,
> which gets restarted upon exit.  All state should be maintained in the host.
> When qemu-ga (re-)starts, it queries the host for current state.  If the
> host says the guest fs is frozen, qemu-ga should then set its internal state
> accordingly.

This is exactly what I proposed in the part you removed:

"""
An idea to improve this would be to add an ioctl() to the kernel to allow applications to get freeze status. This way qemu-ga could drop the state file for newer kernels. The problem with this approach is that it has to be accepted upstream, and the state file would still be needed for older kernels.
"""

Searching around, I found this:

http://www.linuxplumbersconf.org/2011/ocw/system/presentations/873/original/plumbers2011-newfsfreezeapi.pdf

Which is _exactly_ what we're discussing here, it even mentions a guest agent as the main target of such API.

However, that API doesn't seem to be merged yet. I'll check it in more detail, but if it's not merged than the best we can do is to move this to 6.5. And having the state is unavoidable for guest kernels that support freezing but are unable to provide freeze status to applications.

Btw, there's a possible workaround for this issue in 6.4.

Currently, qemu-ga stores its state file in /tmp. We have bug 831174, asking to move it to /var/run instead. This would workaround this issue as /var/run should be cleaned on boot.

So, my recommendation for this bz is:

 1. Fix bug 831174 and backport it to 6.4, this should fix this bz

 2. Create two new bzs for 7.0: one for the kernel asking for the API mentioned in the last comment; the other for qemu asking to change qemu-ga to make use of the new API

(In reply to comment #5)
> (In reply to comment #4)
> 
> > qemu-ga shouldn't keep any state.  It should just be a daemon in the guest,
> > which gets restarted upon exit.  All state should be maintained in the host.
> > When qemu-ga (re-)starts, it queries the host for current state.  If the
> > host says the guest fs is frozen, qemu-ga should then set its internal state
> > accordingly.
> 
> This is exactly what I proposed in the part you removed:
> 
> """
> An idea to improve this would be to add an ioctl() to the kernel to allow
> applications to get freeze status. This way qemu-ga could drop the state
> file for newer kernels. The problem with this approach is that it has to be
> accepted upstream, and the state file would still be needed for older
> kernels.
> """

No, I didn't suggest that.  I said qemu-ga should query the *host*, not the guest kernel.  Querying the guest kernel for this specific use-case (fs frozen?) seems OK, but the host can give qemu-ga much more information than just fs frozen status, and that approach is much more flexible.

I honestly think this is a bad idea. This seems quite complicated for the problem at hand, as we'll have to add code to qemu-ga to be able to make requests to mngt apps. mngt apps would also need to learn how to respond to qemu-ga requests, and mngt apps would have to track the fs state themselves, eg. listen for guest reboot event (but what if the event is lost?) account for N guests, etc...

The solution for this problem is the new fsfreeze API being pushed upstream by Fernando Vazquez. It's really designed to solve this specific problem.

There are two separate problems:

1. qemu-ga restarts while executing some command from host,
2. qemu-ga loses state, as a result of a crash while not executing anything.

For fsfreeze status, you're right that the guest kernel should be queried.  However, qemu-ga should query that status on every invocation.  There could be many other such states qemu-ga should query for.

However, for other use-cases, we will need to implement some solution where the qemu-ga has to query the host and decide how to proceed from its restart.  Storing state files within the guest seems more error-prone in that regard.

I get your point*, but I feel this is too sophisticated for the problem at hand, where querying the guest (trivial) and failing back to a state file in the guest (already implemented and this bz's issue shouldn't happen if we use /var/run) suffices.

I think we should reserve this solution for a case where it's the best or our only option.


 * Actually, the host might have similar issues too. If you want the mngt app to    this, then the mngt app can crash too. Having qemu-ga access the host directly will rise security issues. So I'm not 100% sure this is really a good solution.

Can reproduce this issue with qemu-kvm-0.12.1.2-2.320.el6 qemu-guest-agent-0.12.1.2-2.320.


verify this issue with qemu-kvm-0.12.1.2-2.328.el6 qemu-guest-agent-0.12.1.2-2.328

steps:
1./usr/libexec/qemu-kvm -M rhel6.4.0 -cpu SandyBridge -enable-kvm -m 2048 -smp 2,sockets=2,cores=1,threads=1 -usb -device usb-tablet,id=input0 -name fsfreeze -uuid 1fcee025-70c9-4e1e-8d99-d4afdc974635 -drive file=rhel6.3-64.backup.qcow2,format=qcow2,if=none,id=drive-disk,cache=none,werror=stop,rerror=stop,aio=native -device virtio-blk-pci,drive=drive-disk,bootindex=1 -netdev tap,id=hostnet0,script=/etc/qemu-ifup -device virtio-net-pci,netdev=hostnet0,id=virtio-net-pci0,mac=98:3B:CB:2E:91:A9,bus=pci.0,addr=0x5 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 -vnc :1 -serial unix:/tmp/ttyS0,server,nowait -boot menu=on -monitor stdio -chardev socket,path=/tmp/qga.sock,server,nowait,id=qga0 -device virtio-serial -device virtserialport,chardev=qga0,name=org.qemu.guest_agent.0

2.execute qemu-agent in guest
#qemu-ga -m virtio-serial -p /dev/virtio-ports/org.qemu.guest_agent.0 

3.host:
# nc -U /tmp/qga.sock readline 
{"execute":"guest-fsfreeze-status"}
{"return": "thawed"}
{"execute":"guest-fsfreeze-freeze"}
{"return": 2}
{"execute":"guest-fsfreeze-status"}
{"return": "frozen"}

4. system_reset guest via monitor

5. touch a file after guest boot successfully

6. re-check guest status in host
# nc -U /tmp/qga.sock readline 
{"execute":"guest-fsfreeze-status"}
{"return": "thawed"}

base on testing result above, this bug is fixed.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2013-0527.html