[libvirt] [PATCH 1/4] QEMU guest agent support

[Michal Privoznik]

On 19.01.2012 23:18, Eric Blake wrote:
> On 01/17/2012 04:44 AM, Michal Privoznik wrote:
>> There is now a standard QEMU guest agent that can be installed
>> and given a virtio serial channel
>>
>>     <channel type='unix'>
>>       <source mode='bind' path='/var/lib/libvirt/qemu/f16x86_64.agent'/>
> 
> Do we really want to be documenting a path in the libvirt-controlled
> hierarchy?
> 
> Or is this something that we should be automatically creating on the
> user's behalf if it is apparent that qemu supports it, rather than
> requiring the user to modify their XML to add it?  At which point, it
> would _not_ be part of 'virsh dumpxml' output, but would only be visible
> in the /var/run/libvirt/qemu/dom.xml runtime state (similar to  how
> <domstatus>/<monitor> is the chardev element automatically created for
> the monitor)?
> 
>>       <target type='virtio' name='org.qemu.guest_agent.0'/>
>>     </channel>
>>

> 
>> +++ b/src/qemu/qemu_process.c
>> @@ -107,6 +107,164 @@ qemuProcessRemoveDomainStatus(struct qemud_driver *driver,
>>  extern struct qemud_driver *qemu_driver;
>>  
>>  /*
>> + * This is a callback registered with a qemuAgentPtr instance,
>> + * and to be invoked when the agent console hits an end of file
>> + * condition, or error, thus indicating VM shutdown should be
>> + * performed
> 
> Does agent shutdown really imply VM shutdown?  I hope not - that should
> be reserved for just monitor EOF.

Yes. You won't receive EOF if you close guest agent. But you'll get EOF
when qemu closes the socket. Even during reboots this socket remains
opened. So it is like monitor.
> 
>> +static virDomainChrSourceDefPtr
>> +qemuFindAgentConfig(virDomainDefPtr def)
>> +{
>> +    virDomainChrSourceDefPtr config = NULL;
>> +    int i;
>> +
>> +    for (i = 0 ; i < def->nchannels ; i++) {
>> +        virDomainChrDefPtr channel = def->channels[i];
>> +
>> +        if (channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO)
>> +            continue;
>> +
>> +        if (STREQ(channel->target.name, "org.qemu.guest_agent.0")) {
>> +            config = &channel->source;
>> +            break;
>> +        }
>> +    }
> 
> This implementation requires users to add the agent in their XML; is
> there any way we can automate the use of an agent without doing this?

I am not sure we want this. Although I am not fully convinced by the
opposite. Thing is, I don't want to enable something by default because
users might have not noticed and things may break for them.
On the other hand, users will definitely benefit from this feature so
making it as easy as possible to enable is the right step.
My initial thought is to have simple one line element:
<guest_agent name="org.qemu.guest_agent.0"/>

with name defaulting to "org.qemu.guest_agent.0" so simply inserting
bare <guest_agent/> will turn everything on?

> Then again, it's not just that qemu has to be new enough to have agent
> support, but it's also that the guest has to have an agent installed.
> Maybe we should consider (as a followup, not for this patch) an XML
> shorthand that lets the user request use of an agent without having to
> specify full details (that is, without having to choose a patch for the
> socket, and without having to know the name "org.qemu.guest_agent.0" for
> the protocol of a channel).
> 
> Overall, I'm looking forward to using the agent (I want to support a new
> flag to snapshot creation that uses the agent's ability to request file
> system queiscing from the guest).
> 

Thanks for review and watch out for v2 :) (as soon as we settle down on
XML part)

Michal