[libvirt] [PATCH] conf: Generate agent socket path if missing

[Peter Krempa]

On 02/15/13 09:53, Michal Privoznik wrote:
> It's not desired to force users imagine path for a socket they
> are not even supposed to connect to. On the other hand, we
> already have a release where the qemu agent socket path is
> exposed to XML, so we cannot silently drop it from there.
> The new path is generated in form:
>
> $LOCALSTATEDIR/lib/libvirt/qemu/$domain.agent
> ---
>
> This makes the qemu agent to be generated at domain startup phase.
> So until that, we generate XML without any path, e.g.:
>
>    <channel type='unix'>
>      <source mode='connect'/>
>      <target type='virtio' name='org.qemu.guest_agent.0'/>
>      <address type='virtio-serial' controller='0' bus='0' port='1'/>
>    </channel>
>
> The other possibility is to generate path during XML parse phase, however this
> expose something we are lacking for years - callbacks to fill in default values

Yep, that is missing. Without that you will fill the path only when the 
machine is started. This isn't ideal. I have the same issue with 
recording the default network model in the XML.

> for not configured ones. The aim so to make libvirt accept this snippet as
> valid qemu agent config:
>
>    <channel type='unix'>
>      <target type='virtio' name='org.qemu.guest_agent.0'/>
>    </channel>
>
>   src/conf/domain_conf.c  | 37 ++++++++++++++++++++-----------------
>   src/qemu/qemu_process.c | 30 ++++++++++++++++++++++++++++--
>   2 files changed, 48 insertions(+), 19 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7a2b012..6245fec 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6038,7 +6038,10 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
>           break;
>
>       case VIR_DOMAIN_CHR_TYPE_UNIX:
> -        if (path == NULL) {
> +        /* path is not required in special case of qemu guest agent */
> +        if (path == NULL &&
> +            chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
> +            STRNEQ(chr_def->target.name, "org.qemu.guest_agent.0")) {

Hm. Not entirely elegant, but this is a corner case, so it's not worth 
refactoring other use of the unix chardev backend.

>               virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                              _("Missing source path attribute for char device"));
>               goto error;
> @@ -6135,7 +6138,6 @@ virDomainChrDefParseXML(virCapsPtr caps,
>       char *type = NULL;
>       const char *nodeName;
>       virDomainChrDefPtr def;
> -    int remaining;
>       bool seenTarget = false;
>
>       if (!(def = virDomainChrDefNew()))

> @@ -13307,10 +13308,12 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
>           break;
>
>       case VIR_DOMAIN_CHR_TYPE_UNIX:
> +

This newline isn't present in the other "case"s. Don't add it.

>           virBufferAsprintf(buf, "      <source mode='%s'",
>                             def->data.nix.listen ? "bind" : "connect");
> -        virBufferEscapeString(buf, " path='%s'/>\n",
> +        virBufferEscapeString(buf, " path='%s'",
>                                 def->data.nix.path);
> +        virBufferAddLit(buf, "/>\n");
>           break;
>       }
>