On 05/20/2010 03:57 PM, Chris Lalancette wrote:
> We've been running into a lot of situations where
> virGetHostname() is returning "localhost", where a plain
> gethostname() would have returned the correct thing. This
> is because virGetHostname() is *always* trying to canonicalize
> the name returned from gethostname(), even when it doesn't
> have to.
>
> This patch changes virGetHostname so that if the value returned
> from gethostname() is already FQDN or localhost, it returns
> that string directly. If the value returned from gethostname()
> is a shortened hostname, then we try to canonicalize it. If
> that succeeds, we returned the canonicalized hostname. If
> that fails, and/or returns "localhost", then we just return
> the original string we got from gethostname() and hope for
> the best.
>
> Note that after this patch it is up to clients to check whether
> "localhost" is an allowed return value. The only place
> where it's currently not is in qemu migration.
>
> Signed-off-by: Chris Lalancette <clalance@xxxxxxxxxx>
> ---
> src/libvirt_private.syms | 1 -
> src/qemu/qemu_driver.c | 15 +++++--
> src/util/util.c | 94 ++++++++++++++++++++++++---------------------
> src/util/util.h | 1 -
> 4 files changed, 60 insertions(+), 51 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 104278f..130a2a1 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -653,7 +653,6 @@ virExecDaemonize;
> virSetCloseExec;
> virSetNonBlock;
> virFormatMacAddr;
> -virGetHostnameLocalhost;
> virGetHostname;
> virParseMacAddr;
> virFileDeletePid;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 0af64c7..cfa5964 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -10017,7 +10017,7 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
> virDomainDefPtr def = NULL;
> virDomainObjPtr vm = NULL;
> int this_port;
> - char *hostname;
> + char *hostname = NULL;
> char migrateFrom [64];
> const char *p;
> virDomainEventPtr event = NULL;
> @@ -10057,9 +10057,15 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
> if (port == QEMUD_MIGRATION_NUM_PORTS) port = 0;
>
> /* Get hostname */
> - if ((hostname = virGetHostnameLocalhost(0)) == NULL)
> + if ((hostname = virGetHostname(NULL)) == NULL)
> goto cleanup;
>
> + if (STRPREFIX(hostname, "localhost")) {
> + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("hostname on destination resolved to localhost, but migration requires an FQDN"));
> + goto cleanup;
> + }
> +
> /* XXX this really should have been a properly well-formed
> * URI, but we can't add in tcp:// now without breaking
> * compatability with old targets. We at least make the
> @@ -10067,7 +10073,6 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
> */
> /* Caller frees */
> internalret = virAsprintf(uri_out, "tcp:%s:%d", hostname, this_port);
> - VIR_FREE(hostname);
> if (internalret < 0) {
> virReportOOMError();
> goto cleanup;
> @@ -10171,10 +10176,10 @@ endjob:
> vm = NULL;
>
> cleanup:
> + VIR_FREE(hostname);
> virDomainDefFree(def);
> - if (ret != 0) {
> + if (ret != 0)
> VIR_FREE(*uri_out);
> - }
> if (vm)
> virDomainObjUnlock(vm);
> if (event)
> diff --git a/src/util/util.c b/src/util/util.c
> index e937d39..930bfac 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -2367,11 +2367,31 @@ char *virIndexToDiskName(int idx, const char *prefix)
> # define AI_CANONIDN 0
> #endif
>
> -char *virGetHostnameLocalhost(int allow_localhost)
> +/* Who knew getting a hostname could be so delicate. In Linux (and Unices
> + * in general), many things depend on "hostname" returning a value that will
> + * resolve one way or another. In the modern world where networks frequently
> + * come and go this is often being hard-coded to resolve to "localhost". If
> + * it *doesn't* resolve to localhost, then we would prefer to have the FQDN.
> + * That leads us to 3 possibilities:
> + *
> + * 1) gethostname() returns an FQDN (not localhost) - we return the string
> + * as-is, it's all of the information we want
> + * 2) gethostname() returns "localhost" - we return localhost; doing further
> + * work to try to resolve it is pointless
> + * 3) gethostname() returns a shortened hostname - in this case, we want to
> + * try to resolve this to a fully-qualified name. Therefore we pass it
> + * to getaddrinfo(). There are two possible responses:
> + * a) getaddrinfo() resolves to a FQDN - return the FQDN
> + * b) getaddrinfo() resolves to localhost - in this case, the data we got
> + * from gethostname() is actually more useful than what we got from
> + * getaddrinfo(). Return the value from gethostname() and hope for
> + * the best.
> + */
> +char *virGetHostname(virConnectPtr conn ATTRIBUTE_UNUSED)
> {
Hmm, why isn't one of the fallback options:
if (conn)
hostname = parse_uri(conn.get_uri())
if hostname != "localhost":
return hostname
Seems like if MigratePrepare2 dconn is the remote connection, we are
guaranteed to have a resolvable hostname in the URI (well, resolvable to
the source connection at least).
- Cole
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list