Re: [Qemu-devel] [PATCH v1] qga: Add 'guest-get-fqdn' command

[Vinzenz Feenstra]

> On Mar 27, 2017, at 12:44 PM, Sameeh Jubran <address@hidden> wrote:
> 
> 
> 
> On Thu, Mar 23, 2017 at 9:51 PM, Vinzenz 'evilissimo' Feenstra 
> <address@hidden <mailto:address@hidden>> wrote:
> From: Vinzenz Feenstra <address@hidden <mailto:address@hidden>>
> 
> Retrieving the guest OS fully qualified domain name (FQDN) is a very
> useful feature for virtual management systems. This information can help
> to have more user friendly VM access details, instead of an IP there
> would be the FQDN. Also the FQDN reported can be used to have automated
> checks for valid SSL certificates.
> 
> virsh # qemu-agent-command F25 '{ "execute": "guest-get-fqdn" }'
> {"return":{"fqdn":"F25.lab.evilissimo.net <http://f25.lab.evilissimo.net/>"}}
> 
> Signed-off-by: Vinzenz Feenstra <address@hidden <mailto:address@hidden>>
> ---
>  qga/commands.c       | 11 +++++++++++
>  qga/qapi-schema.json | 25 +++++++++++++++++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/qga/commands.c b/qga/commands.c
> index 4d92946..61577af 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -499,3 +499,14 @@ int ga_parse_whence(GuestFileWhence *whence, Error 
> **errp)
>      error_setg(errp, "invalid whence code %"PRId64, whence->u.value);
>      return -1;
>  }
> +
> +GuestFQDN *qmp_guest_get_fqdn(Error **err)
> +{
> +    GuestFQDN *result = NULL;
> +    gchar const *hostname = g_get_host_name();
> According to glib documentation on "g_get_host_name" function:
> "The returned name is not necessarily a fully-qualified domain name, or even 
> present in DNS or some other name service at all. It need not even be unique 
> on your local network or site, but usually it is."
> I think the command name should be changed to something like 
> "guest_get_machine_name" and not fqdn as this is not always true!

I prefer the `guest_get_fqdn` name over the machine name, because on best 
effort this is, what
this method is trying to achieve. The machine name could be without domain 
part, and yes, this
could be what it returns, again it’s on best effort to get a FQDN.

> 
> Moreover the documentation states that:
> "If no name can be determined, a default fixed string "localhost" is 
> returned."
> I think we should handle this case and return and informative error.

Having a fallback which returns ‘localhost' is pretty much a good thing in my 
opinion.
We should document it, yes, but I wouldn’t report an error here. Especially 
what do you want
to report? Returning ‘localhost’ is a value and a report of an error at the 
same time.

I will leave this up for discussion and will follow up with an improved 
documentation of the
functionality.

> +    if (hostname != NULL) {
> +        result = g_new0(GuestFQDN, 1);
> +        result->fqdn = g_strdup(hostname);
> +    }
> +    return result;
> +}
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index a02dbf2..0a2c0a4 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -1042,3 +1042,28 @@
>    'data':    { 'path': 'str', '*arg': ['str'], '*env': ['str'],
>                 '*input-data': 'str', '*capture-output': 'bool' },
>    'returns': 'GuestExec' }
> +
> +
> +
> +##
> +# @GuestFQDN:
> +# @fqdn: Fully qualified domain name of the guest OS
> +#
> +# Since: 2.10
> +##
> +{ 'struct': 'GuestFQDN',
> +  'data':   { 'fqdn': 'str' } }
> +
> +
> +##
> +# @guest-get-fqdn:
> +#
> +# Request the FQDN (Fully Qualified Domain Name) of the guest operating 
> system
> +#
> +# Returns: FQDN on success
> +#
> +# Since: 2.10
> +##
> +{ 'command': 'guest-get-fqdn',
> +  'returns': 'GuestFQDN' }
> +
> --
> 2.9.3
> 
> 
> 
> 
> 
> -- 
> Respectfully,
> Sameeh Jubran
> Linkedin <https://il.linkedin.com/pub/sameeh-jubran/87/747/a8a>
> Software Engineer @ Daynix <http://www.daynix.com/>.