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

[Michael Roth]

On 04/21/2011 04:46 AM, Jes Sorensen wrote:
> On 04/18/11 17:02, Michael Roth wrote:
> > These apply on top of Anthony's glib tree, commit 
> > 03d5927deb5e6baebaade1b4c8ff2428a85e125c currently, and can also be obtained 
> > from:
> > git://repo.or.cz/qemu/mdroth.git qga_v2
> >
> > Patches 1-8 are general json/QAPI-related fixes. Anthony, please consider 
> > pulling these into your glib tree. The json fix-ups may need further 
> > evaluation, but I'm confident they're at least an improvement. The QAPI ones 
> > are mostly trivial fix-ups.
> >
> > Changes since V1:
> >
> > - Added guest agent worker thread to execute RPCs in the guest. With this in 
> > place we have a reliable timeout mechanism for hung commands, currently set at 
> > 30 seconds.
> > - Add framework for registering init/cleanup routines for stateful RPCs to 
> > clean up after themselves after a timeout.
> > - Added the following RPCs: guest-file-{open,close,read,write,seek}, 
> > guest-shutdown, guest-info, and removed stubs for guest-view-file (now 
> > deprecated)
> > - Added GUEST_AGENT_UP/GUEST_AGENT_DOWN QMP events
> > - Switched to a TCP-style host-initiated 3-way handshake for channel 
> > negotiation, this simplifies client negotiation/interaction over the wire
> > - Added configurable log level/log file/pid file options for guest agent
> > - Various fixes for bugs/memory leaks and checkpatch.pl fixups
> >
> > ISSUES/TODOS:
> >
> > - Fix QMP proxy handling of error responses sent by guest agent, currently 
> > ignored
> > - Add unit tests for guest agent wire protocol
> > - Add unit tests for QMP interfaces
> > - Add host-side timeout mechanism for async QMP commands
> > - Return error for guest commands if guest up event has not yet been recieved
> > - Make QMP param names more consistent between related commands
> > - Clean up logging
>
> Hi,
>
> I had a look through this patchset and generally it looks pretty good.
> There were a few nits, and I ignored all the python gibberish to avoid
> getting a headache.
>
> Did you do anything with the fsfreeze patches, or were they dropped in
> the migration to qapi?

They were pending some changes required on the agent side that weren't 
really addressed/doable until this patchset, namely:

1) server-side timeout mechanism to recover from RPCs that can hang 
indefinitely or take a really long time (fsfreeze, fopen, etc), 
currently it's 30 seconds, may need to bump it to 60 for fsfreeze, or 
potentially add an RPC to change the server-side timeout
2) a simple way to temporarily turn off logging so agent doesn't 
deadlock itself
3) a way to register a cleanup handler when a timeout occurs.
4) disabling RPCs where proper accounting/logging is required 
(guest-open-file, guest-shutdown, etc)

#4 isn't implemented...I think this could be done fairly in-evasively 
with something like:

Response important_rpc():
  if (!ga_log("syslog", LEVEL_CRITICAL, "important stuff happening"))
    return ERROR_LOGGING_CURRENTLY_DISABLED
  ...

bool ga_log(log_domain, level, msg):
  if (log_domain == "syslog")
    if (!logging_enabled && is_critical(log_level))
      return False;
    syslog(msg, ...)
  else
    if (logging_enabled)
      normallog(msg, ...)
  return True

With that I think we could actually drop the fsfreeze stuff in. Thoughts?

> Cheers,
> Jes