qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 4/9] qemu-ga: use key-value store to avoid recyc


From: Peter Crosthwaite
Subject: Re: [Qemu-devel] [PATCH 4/9] qemu-ga: use key-value store to avoid recycling fd handles after restart
Date: Fri, 15 Mar 2013 15:09:54 +1000

On Fri, Mar 15, 2013 at 1:52 PM, Michael Roth <address@hidden> wrote:
> On Thu, Mar 14, 2013 at 10:31 PM, Peter Crosthwaite
> <address@hidden> wrote:
>> Hi Michael, Anthony,
>>
>> On Tue, Mar 12, 2013 at 10:53 AM, Michael Roth
>> <address@hidden> 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 <address@hidden>
>>> Cc: address@hidden
>>>
>>> * 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.
>

That did the trick,

Patch up on list if you want to sign it off or suggested-by it.

Regards,
Peter


>>
>> Regards,
>> peter
>>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]