diff -rup libvirt-0.7.1/src/libvirt.c new/src/libvirt.c --- libvirt-0.7.1/src/libvirt.c 2010-06-03 15:30:32.615164000 -0400 +++ new/src/libvirt.c 2010-06-03 15:33:22.863409000 -0400 @@ -3054,6 +3054,7 @@ virDomainMigrateVersion2 (virDomainPtr d char *cookie = NULL; char *dom_xml = NULL; int cookielen = 0, ret; + virErrorPtr orig_err = NULL; /* Prepare the migration. * @@ -3102,6 +3103,10 @@ virDomainMigrateVersion2 (virDomainPtr d ret = domain->conn->driver->domainMigratePerform (domain, cookie, cookielen, uri, flags, dname, bandwidth); + /* Perform failed. Make sure Finish doesn't overwrite the error */ + if (ret < 0) + orig_err = virSaveLastError(); + /* In version 2 of the migration protocol, we pass the * status code from the sender to the destination host, * so it can do any cleanup if the migration failed. @@ -3111,6 +3116,10 @@ virDomainMigrateVersion2 (virDomainPtr d (dconn, dname, cookie, cookielen, uri, flags, ret); done: + if (orig_err) { + virSetError(orig_err); + virFreeError(orig_err); + } VIR_FREE (uri_out); VIR_FREE (cookie); return ddomain; @@ -3222,7 +3231,7 @@ virDomainMigrate (virDomainPtr domain, error: /* Copy to connection error object for back compatability */ - virSetConnError(domain->conn); + virDispatchError(domain->conn); return NULL; } @@ -3269,8 +3278,7 @@ virDomainMigratePrepare (virConnectPtr d virLibConnError (dconn, VIR_ERR_NO_SUPPORT, __FUNCTION__); error: - /* Copy to connection error object for back compatability */ - virSetConnError(dconn); + virDispatchError(dconn); return -1; } @@ -3318,8 +3326,7 @@ virDomainMigratePerform (virDomainPtr do virLibDomainError (domain, VIR_ERR_NO_SUPPORT, __FUNCTION__); error: - /* Copy to connection error object for back compatability */ - virSetConnError(domain->conn); + virDispatchError(domain->conn); return -1; } @@ -3364,8 +3371,7 @@ virDomainMigrateFinish (virConnectPtr dc virLibConnError (dconn, VIR_ERR_NO_SUPPORT, __FUNCTION__); error: - /* Copy to connection error object for back compatability */ - virSetConnError(dconn); + virDispatchError(dconn); return NULL; } @@ -3416,8 +3422,7 @@ virDomainMigratePrepare2 (virConnectPtr virLibConnError (dconn, VIR_ERR_NO_SUPPORT, __FUNCTION__); error: - /* Copy to connection error object for back compatability */ - virSetConnError(dconn); + virDispatchError(dconn); return -1; } @@ -3464,8 +3469,7 @@ virDomainMigrateFinish2 (virConnectPtr d virLibConnError (dconn, VIR_ERR_NO_SUPPORT, __FUNCTION__); error: - /* Copy to connection error object for back compatability */ - virSetConnError(dconn); + virDispatchError(dconn); return NULL; } diff -rup libvirt-0.7.1/src/libvirt_private.syms new/src/libvirt_private.syms --- libvirt-0.7.1/src/libvirt_private.syms 2010-06-03 15:30:33.051186000 -0400 +++ new/src/libvirt_private.syms 2010-06-03 15:33:22.869400000 -0400 @@ -461,6 +461,7 @@ virRaiseErrorFull; virReportSystemErrorFull; virReportOOMErrorFull; virStrerror; +virSetError; # xml.h diff -rup libvirt-0.7.1/src/qemu_driver.c new/src/qemu_driver.c --- libvirt-0.7.1/src/qemu_driver.c 2010-06-03 15:30:33.111159000 -0400 +++ new/src/qemu_driver.c 2010-06-03 15:35:22.809404000 -0400 @@ -2297,12 +2297,17 @@ static void qemudShutdownVMDaemon(virCon virDomainObjPtr vm) { int ret; int retries = 0; + virErrorPtr orig_err; if (!virDomainIsActive(vm)) return; VIR_DEBUG(_("Shutting down VM '%s'\n"), vm->def->name); + /* This method is routinely used in clean up paths. Disable error + * reporting so we don't squash a legit error. */ + orig_err = virSaveLastError(); + if (virKillProcess(vm->pid, 0) == 0 && virKillProcess(vm->pid, SIGTERM) < 0) virReportSystemError(conn, errno, @@ -2377,6 +2382,11 @@ retry: vm->def->id = -1; vm->newDef = NULL; } + + if (orig_err) { + virSetError(orig_err); + virFreeError(orig_err); + } } @@ -7497,6 +7507,10 @@ qemudDomainMigrateFinish2 (virConnectPtr virDomainObjPtr vm; virDomainPtr dom = NULL; virDomainEventPtr event = NULL; + virErrorPtr orig_err; + + /* Migration failed. Save the current error so nothing squashes it */ + orig_err = virSaveLastError(); qemuDriverLock(driver); vm = virDomainFindByName(&driver->domains, dname); @@ -7540,6 +7554,10 @@ qemudDomainMigrateFinish2 (virConnectPtr } cleanup: + if (orig_err) { + virSetError(orig_err); + virFreeError(orig_err); + } if (vm) virDomainObjUnlock(vm); if (event) diff -rup libvirt-0.7.1/src/util.c new/src/util.c --- libvirt-0.7.1/src/util.c 2010-06-03 15:30:33.064159000 -0400 +++ new/src/util.c 2010-06-03 15:33:22.881417000 -0400 @@ -1764,31 +1764,82 @@ int virDiskNameToIndex(const char *name) #define AI_CANONIDN 0 #endif -char *virGetHostname(void) +/* 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() { int r; char hostname[HOST_NAME_MAX+1], *result; struct addrinfo hints, *info; r = gethostname (hostname, sizeof(hostname)); - if (r == -1) + if (r == -1) { + virReportSystemError(NULL, errno, + "%s", _("failed to determine host name")); return NULL; + } NUL_TERMINATE(hostname); + if (STRPREFIX(hostname, "localhost") || strchr(hostname, '.')) { + /* in this case, gethostname returned localhost (meaning we can't + * do any further canonicalization), or it returned an FQDN (and + * we don't need to do any further canonicalization). Return the + * string as-is; it's up to callers to check whether "localhost" + * is allowed. + */ + result = strdup(hostname); + goto check_and_return; + } + + /* otherwise, it's a shortened, non-localhost, hostname. Attempt to + * canonicalize the hostname by running it through getaddrinfo + */ + memset(&hints, 0, sizeof(hints)); hints.ai_flags = AI_CANONNAME|AI_CANONIDN; hints.ai_family = AF_UNSPEC; r = getaddrinfo(hostname, NULL, &hints, &info); - if (r != 0) - return NULL; - if (info->ai_canonname == NULL) { - freeaddrinfo(info); + if (r != 0) { + ReportError(NULL, VIR_ERR_INTERNAL_ERROR, + _("getaddrinfo failed for '%s': %s"), + hostname, gai_strerror(r)); return NULL; } - /* Caller frees this string. */ - result = strdup (info->ai_canonname); + if (info->ai_canonname == NULL || + STRPREFIX(info->ai_canonname, "localhost")) + /* in this case, we tried to canonicalize and we ended up back with + * localhost. Ignore the canonicalized name and just return the + * original hostname + */ + result = strdup(hostname); + else + /* Caller frees this string. */ + result = strdup (info->ai_canonname); + freeaddrinfo(info); + +check_and_return: + if (result == NULL) + virReportOOMError(NULL); return result; } diff -rup libvirt-0.7.1/src/virterror.c new/src/virterror.c --- libvirt-0.7.1/src/virterror.c 2009-09-14 06:12:53.000000000 -0400 +++ new/src/virterror.c 2010-06-03 15:33:22.886409000 -0400 @@ -287,6 +287,28 @@ virGetLastError(void) } /** + * virSetError: + * + * Set the current error from a previously saved error object + * + * Can be used to re-set an old error, which may have been squashed by + * other functions (like cleanup routines). + * + * Returns 0 on success, 1 on failure + */ +int +virSetError(virErrorPtr newerr) +{ + virErrorPtr err; + err = virGetLastError(); + if (!err) + return -1; + + virResetError(err); + return virCopyError(newerr, err); +} + +/** * virCopyLastError: * @to: target to receive the copy * @@ -596,6 +618,52 @@ virSetConnError(virConnectPtr conn) } } +/** + * virDispatchError: + * @conn: pointer to the hypervisor connection + * + * Internal helper to do final stage of error + * reporting in public APIs. + * + * - Copy the global error to per-connection error if needed + * - Set a generic error message if none is already set + * - Invoke the error callback functions + */ +void +virDispatchError(virConnectPtr conn) +{ + virErrorPtr err = virLastErrorObject(); + virErrorFunc handler = virErrorHandler; + void *userData = virUserData; + + /* Should never happen, but doesn't hurt to check */ + if (!err) + return; + + /* Set a generic error message if none is already set */ + if (err->code == VIR_ERR_OK) + virErrorGenericFailure(err); + + /* Copy the global error to per-connection error if needed */ + if (conn) { + virMutexLock(&conn->lock); + virCopyError(err, &conn->err); + + if (conn->handler != NULL) { + handler = conn->handler; + userData = conn->userData; + } + virMutexUnlock(&conn->lock); + } + + /* Invoke the error callback functions */ + if (handler != NULL) { + (handler)(userData, err); + } else { + virDefaultErrorFunc(err); + } +} + /** @@ -634,8 +702,6 @@ virRaiseErrorFull(virConnectPtr conn, const char *fmt, ...) { virErrorPtr to; - void *userData = virUserData; - virErrorFunc handler = virErrorHandler; char *str; /* @@ -653,18 +719,6 @@ virRaiseErrorFull(virConnectPtr conn, return; /* - * try to find the best place to save and report the error - */ - if (conn != NULL) { - virMutexLock(&conn->lock); - if (conn->handler != NULL) { - handler = conn->handler; - userData = conn->userData; - } - virMutexUnlock(&conn->lock); - } - - /* * formats the message */ if (fmt == NULL) { @@ -683,7 +737,6 @@ virRaiseErrorFull(virConnectPtr conn, /* * Save the information about the error */ - virResetError(to); /* * Delibrately not setting conn, dom & net fields since * they're utterly unsafe @@ -701,14 +754,7 @@ virRaiseErrorFull(virConnectPtr conn, to->int1 = int1; to->int2 = int2; - /* - * now, report it - */ - if (handler != NULL) { - handler(userData, to); - } else { - virDefaultErrorFunc(to); - } + virDispatchError(conn); } /** diff -rup libvirt-0.7.1/src/virterror_internal.h new/src/virterror_internal.h --- libvirt-0.7.1/src/virterror_internal.h 2009-07-23 12:33:02.000000000 -0400 +++ new/src/virterror_internal.h 2010-06-03 15:33:22.890402000 -0400 @@ -89,6 +89,8 @@ void virReportOOMErrorFull(virConnectPtr void virSetGlobalError(void); void virSetConnError(virConnectPtr conn); +int virSetError(virErrorPtr newerr); +void virDispatchError(virConnectPtr conn); const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen); #endif