On 10/06/14 09:44, Igor Mammedov wrote:
> GA was keepeing persistent state info in /var/run/qga.state
> file. However it's lost after every reboot since /var/run
> usually is located on tmpfs or cleaned on start-up.
>
> Fix issue by keeping state file in /var/lib/qemu-ga/
> directory, which is intended for keeping persistent
> local state according to FHS.
>
> Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> ---
> qga/main.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/qga/main.c b/qga/main.c
> index 227f2bd..5afba01 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -45,7 +45,8 @@
>
> #ifndef _WIN32
> #define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0"
> -#define QGA_STATE_RELATIVE_DIR "run"
> +#define QGA_VOLATILE_STATE_RELATIVE_DIR "run"
> +#define QGA_STATE_RELATIVE_DIR "lib/qemu-ga"
> #define QGA_SERIAL_PATH_DEFAULT "/dev/ttyS0"
> #else
> #define QGA_VIRTIO_PATH_DEFAULT "\\\\.\\Global\\org.qemu.guest_agent.0"
> @@ -121,7 +122,7 @@ init_dfl_pathnames(void)
> dfl_pathnames.state_dir = qemu_get_local_state_pathname(
> QGA_STATE_RELATIVE_DIR);
> dfl_pathnames.pidfile = qemu_get_local_state_pathname(
> - QGA_STATE_RELATIVE_DIR G_DIR_SEPARATOR_S "qemu-ga.pid");
> + QGA_VOLATILE_STATE_RELATIVE_DIR G_DIR_SEPARATOR_S "qemu-ga.pid");
> }
>
> static void quit_handler(int sig)
>
I think this patch is right, but perhaps another hunk should be added.
According to commit 39097daf, persistent state should indeed survive
guest reboots. This seems appropriate for at least "fd_counter".
However we store "qga.state.isfrozen" too in the state directory
("state_filepath_isfrozen"). According to commit f789aa7b, that file
should survive qemu-ga crashes and restarts, but I think it shouldn't
survive entire *guest* restarts. (When the guest reboots, its
filesystems certainly won't be frozen, and the "qga.state.isfrozen" will
be stale.)
What we have now is:
- state (including fd_counter and isfrozen status) is lost across guest
reboot. This is wrong for fd_counter, but right for isfrozen.
If you make the state persistent for real, then:
- fd_counter will work more safely, but isfrozen will (theoretically)
regress.
Hence I think that "state_filepath_isfrozen" should also use
QGA_VOLATILE_STATE_RELATIVE_DIR.
And that's a huge mess then, because:
- in Windows guests, we only have one state dir, and
the "state_filepath_isfrozen" assignment is currently guest-type
independent
- in Linux guests, we'd have two state dirs (non-volatile and
volatile), but the user can control only one with the -t option.
I don't know what to recommend for this. Anyway I have some worries
about the 2nd patch, let me continue there.
Thanks
Laszlo
