Re: [PATCH] qga: Use gethostname() instead of g_get_host_name()

[Philippe Mathieu-Daudé]

Hi Michal,

On 6/16/20 10:34 AM, Michal Privoznik wrote:
> Problem with g_get_host_name() is that on the first call it saves
> the hostname into a global variable and from then on, every
> subsequent call returns the saved hostname. Even if the hostname
> changes. This doesn't play nicely with guest agent, because if
> the hostname is acquired before the guest is set up (e.g. on the
> first boot, or before DHCP) we will report old, invalid hostname.
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1845127
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  qga/commands.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 52 insertions(+), 4 deletions(-)
> 
> diff --git a/qga/commands.c b/qga/commands.c
> index efc8b90281..ce3c2041a6 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -512,14 +512,62 @@ int ga_parse_whence(GuestFileWhence *whence, Error 
> **errp)
>      return -1;
>  }
>  
> +#ifndef HOST_NAME_MAX
> +# ifdef _POSIX_HOST_NAME_MAX
> +#  define HOST_NAME_MAX _POSIX_HOST_NAME_MAX
> +# else
> +#  define HOST_NAME_MAX 255
> +# endif
> +#endif
> +
>  GuestHostName *qmp_guest_get_host_name(Error **errp)
>  {
>      GuestHostName *result = NULL;
> -    gchar const *hostname = g_get_host_name();
> -    if (hostname != NULL) {
> -        result = g_new0(GuestHostName, 1);
> -        result->host_name = g_strdup(hostname);
> +    g_autofree char *hostname = NULL;
> +
> +    /*
> +     * We want to avoid using g_get_host_name() because that
> +     * caches the result and we wouldn't reflect changes in the
> +     * host name.
> +     */

I see there is only one g_get_host_name() call in the
codebase, but can we have a generic qemu_get_host_name()
helper implemented in util/oslib-*c instead?

> +
> +#ifndef G_OS_WIN32
> +    long len = -1;
> +
> +#ifdef _SC_HOST_NAME_MAX
> +    len = sysconf(_SC_HOST_NAME_MAX);
> +#endif /* _SC_HOST_NAME_MAX */
> +
> +    if (len < 0) {
> +        len = HOST_NAME_MAX;
>      }
> +
> +    hostname = g_malloc0(len + 1);
> +
> +    if (gethostname(hostname, len) < 0) {
> +        return NULL;
> +    }
> +
> +#else /* G_OS_WIN32 */
> +
> +    wchar_t tmp[MAX_COMPUTERNAME_LENGTH + 1];
> +    DWORD size = G_N_ELEMENTS(tmp);
> +
> +    if (GetComputerNameW(tmp, &size) != 0) {
> +        /*
> +         * Indeed, on Windows retval of zero means failure
> +         * and nonzero means success.
> +         */
> +        hostname = g_utf16_to_utf8(tmp, size, NULL, NULL, NULL);
> +    }
> +#endif /* G_OS_WIN32 */
> +
> +    if (!hostname) {
> +        hostname = g_strdup("localhost");
> +    }
> +
> +    result = g_new0(GuestHostName, 1);
> +    result->host_name = g_steal_pointer(&hostname);
>      return result;
>  }
>  
>