Re: [Qemu-devel] [RFC][PATCH v2 00/11] QEMU Guest Agent: QMP-based host/guest communication (virtagent)

[Michael Roth <mdroth@linux.vnet.ibm.com>]

On 05/03/2011 09:12 AM, Jes Sorensen wrote:
> On 05/03/11 15:53, Michael Roth wrote:
>>>
>>> IMHO it is better to disable the commands rather than just logging, but
>>> either way should allow it to drop in.
>>
>> Kinda agree, but logging seems to be the real dependency. With the
>> server-side timeouts now in place even doing stuff like fopen/fwrite is
>> permitted (it would just timeout if it blocked too long). It's the
>> logging stuff that we don't really have a way to recover from, because
>> it's not run in a thread we can just nuke after a certain amount of time.
>>
>> Even when we're not frozen, we can't guarantee an fopen/fwrite/fread
>> will succeed, so failures shouldn't be too much of a surprise since they
>> need to be handled anyway. And determining whether or not a command
>> should be marked as executable during a freeze is somewhat nebulous
>> (fopen might work for read-only access, but hang for write access when
>> O_CREATE is set, fwrite might succeed if it doesn't require a flush,
>> etc), plus internal things like logging need to be taken into account.
>>
>> So, for now at least I think it's a reasonable way to do it.
>
> Please be very careful with any fwrite() calls - it's not just logging.
> Any writes to the frozen file systems will result in the caller being
> put to sleep until the file system is unfrozen. It won't timeout, and
> the agent will be stuck hanging in that call.
>
> It's fun playing with the fsfreeze stuff on your desktop system - doing
> it in an xterm has 'interesting' effects..... :)
>
> This is why I prefer the 'disable function' rather 'disable logging'
> approach.
>

I'll review the code to make sure, but what I mean is that the actual 
daemon only ever does disk i/o when it's doing logging. All the RPCs are 
handled in a separate worker thread, so if they do something fubar we 
should still be able to recover (the timeouts are done by 
pthread_cancel()'ing the worker thread). So things should be recoverable 
so long as we don't log when frozen. RPCs succeeding or failing we don't 
care too much about, since they'll just result in a nice "command timed 
out" response to the user eventually.

Checking logging_enabled is more to ensure we don't let the user exploit 
the fs_freeze side-effects to do sensitive stuff under the guest owner's 
nose (not that we couldn't anyway from host, but it may important for 
piece of mind about have an externally accessible/privileged agent in 
your guest).

works_under_fsfreeze is harder to determine beforehand, unless maybe you 
flag any command that did i/o other than logging...but there are 
actually use cases where that would be too restrictive. If they're using 
fwrite to pipe output through a character device, for instance, or 
twiddling knobs in sysfs. Better to keep the same semantics as you'd 
have when "physically" on the machine, when possible.

Will need to do some good testing though, fsfreeze is definitely a 
strange state to be in :)

>>> Sorry for the late reply, been a bit swamped here.
>>
>> No problem I have your patches in my tree now. They still need a little
>> bit of love and testing but I should be able to get them out on the list
>> shortly.
>
> Sounds great!
>
> Cheers,
> Jes