On Thu, Mar 14, 2013 at 10:31 PM, Peter Crosthwaite
<peter.crosthwa...@xilinx.com> wrote:
> Hi Michael, Anthony,
>
> On Tue, Mar 12, 2013 at 10:53 AM, Michael Roth
> <mdr...@linux.vnet.ibm.com> wrote:
>> Hosts hold on to handles provided by guest-file-open for periods that can
>> span beyond the life of the qemu-ga process that issued them. Since these
>> are issued starting from 0 on every restart, we run the risk of issuing
>> duplicate handles after restarts/reboots.
>>
>> As a result, users with a stale copy of these handles may end up
>> reading/writing corrupted data due to their existing handles effectively
>> being re-assigned to an unexpected file or offset.
>>
>> We unfortunately do not issue handles as strings, but as integers, so a
>> solution such as using UUIDs can't be implemented without introducing a
>> new interface.
>>
>> As a workaround, we fix this by implementing a persistent key-value store
>> that will be used to track the value of the last handle that was issued
>> across restarts/reboots to avoid issuing duplicates.
>>
>> The store is automatically written to the same directory we currently
>> set via --statedir to track fsfreeze state, and so should be applicable
>> for stable releases where this flag is supported.
>>
>> A follow-up can use this same store for handling fsfreeze state, but
>> that change is cosmetic and left out for now.
>>
>> Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com>
>> Cc: qemu-sta...@nongnu.org
>>
>> * fixed guest_file_handle_add() return value from uint64_t to int64_t
>> ---
>> qga/commands-posix.c | 25 +++++--
>> qga/guest-agent-core.h | 1 +
>> qga/main.c | 184
>> ++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 204 insertions(+), 6 deletions(-)
>>
>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>> index 7a0202e..1c2aff3 100644
>> --- a/qga/commands-posix.c
>> +++ b/qga/commands-posix.c
> [snip]
>> + g_assert(pstate);
>> + g_assert(keyfile);
>> + /* if any fields are missing, either because the file was tampered with
>> + * by agents of chaos, or because the field wasn't present at the time
>> the
>> + * file was created, the best we can ever do is start over with the
>> default
>> + * values. so load them now, and ignore any errors in accessing
>> key-value
>> + * pairs
>> + */
>> + set_persistent_state_defaults(pstate);
>> +
>> + if (g_key_file_has_key(keyfile, "global", "fd_counter", NULL)) {
>> + pstate->fd_counter =
>> + g_key_file_get_int64(keyfile, "global", "fd_counter", NULL);
>
> This function (and friends) doesn't exist in glib pre v2.26, breaking
> the build for a few distros (RHEL 5.6 and Ubuntu 10.04 are two that I
> am building with). Are we mandating glib > 2.26 as of this series (and
> should configure be updated) or do we need an alternate implementation
> of this code?
Sigh...that certainly wasn't the intention, and something I specifically tried
to avoid doing. GKeyFile was added in 2.6 according to the documentation:
https://developer.gnome.org/glib/2.31/glib-Key-value-file-parser.html
It seems that it may specifically be g_key_file_get_int64() which is
causing the 2.26 dependency.
Would you mind testing with get_int64/set_int64 swapped out for
get_integer/set_integer? If that does the trick I can send an updated
patch tomorrow.
>
> Regards,
> peter
>
