Re: [libvirt] [PATCH 1/4] QEMU guest agent support
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