[libvirt] [PATCH/GSoC] Use virGetLastErrorMessage() rather than open code it
getting err using virGetLastError() and then retrieving message from err make developer have to test the value of err and err->message and default to self-defined unkown error message. It's better to avoid it and using uniform virGetLastErrorMessage --- daemon/libvirtd.c | 8 +--- examples/object-events/event-test.c | 9 ++--- src/bhyve/bhyve_driver.c | 3 +- src/libvirt.c | 3 +- src/libxl/libxl_domain.c | 3 +- src/libxl/libxl_driver.c | 3 +- src/locking/lock_daemon.c | 8 +--- src/logging/log_daemon.c | 8 +--- src/lxc/lxc_container.c | 8 +--- src/lxc/lxc_controller.c | 7 +--- src/lxc/lxc_domain.c | 3 +- src/lxc/lxc_process.c | 6 +-- src/network/bridge_driver.c | 3 +- src/node_device/node_device_hal.c | 3 +- src/rpc/virnettlscontext.c | 3 +- src/secret/secret_driver.c | 6 +-- src/storage/storage_driver.c | 16 ++------ src/uml/uml_driver.c | 3 +- src/util/iohelper.c | 8 +--- src/util/virhook.c | 3 +- src/util/virhostdev.c | 20 ++++----- src/util/virpci.c | 4 +- tests/commandtest.c | 81 +++++++++++++------------------------ tests/libvirtdconftest.c | 3 +- tests/openvzutilstest.c | 6 +-- tests/securityselinuxlabeltest.c | 6 +-- tests/securityselinuxtest.c | 6 +-- tests/virnettlscontexttest.c | 3 +- 28 files changed, 75 insertions(+), 168 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 3d38a46..e526e55 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1273,12 +1273,8 @@ int main(int argc, char **argv) { /* Read the config file if it exists*/ if (remote_config_file && daemonConfigLoadFile(config, remote_config_file, implicit_conf) < 0) { - virErrorPtr err = virGetLastError(); - if (err && err->message) - VIR_ERROR(_("Can't load config file: %s: %s"), - err->message, remote_config_file); - else - VIR_ERROR(_("Can't load config file: %s"), remote_config_file); + VIR_ERROR(_("Can't load config file: %s: %s"), + virGetLastErrorMessage(), remote_config_file); exit(EXIT_FAILURE); } diff --git a/examples/object-events/event-test.c b/examples/object-events/event-test.c index 7be1d21..4a4ef86 100644 --- a/examples/object-events/event-test.c +++ b/examples/object-events/event-test.c @@ -654,9 +654,8 @@ int main(int argc, char **argv) } if (virEventRegisterDefaultImpl() < 0) { - virErrorPtr err = virGetLastError(); fprintf(stderr, "Failed to register event implementation: %s\n", - err && err->message ? err->message: "Unknown error"); + virGetLastErrorMessage()); goto cleanup; } @@ -794,17 +793,15 @@ int main(int argc, char **argv) goto cleanup; if (virConnectSetKeepAlive(dconn, 5, 3) < 0) { - virErrorPtr err = virGetLastError(); fprintf(stderr, "Failed to start keepalive protocol: %s\n", - err && err->message ? err->message : "Unknown error"); + virGetLastErrorMessage()); run = 0; } while (run) { if (virEventRunDefaultImpl() < 0) { - virErrorPtr err = virGetLastError(); fprintf(stderr, "Failed to run event loop: %s\n", - err && err->message ? err->message : "Unknown error"); + virGetLastErrorMessage()); } } diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 9219890..6f2423c 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -88,9 +88,8 @@ bhyveAutostartDomain(virDomainObjPtr vm, void *opaque) ret = virBhyveProcessStart(data->conn, data->driver, vm, VIR_DOMAIN_RUNNING_BOOTED, 0); if (ret < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to autostart VM '%s': %s"), - vm->def->name, err ? err->message : _("unknown error")); + vm->def->name, virGetLastErrorMessage()); } } virObjectUnlock(vm); diff --git a/src/libvirt.c b/src/libvirt.c index dd58e9c..99b1c47 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -777,10 +777,9 @@ virStateInitialize(bool privileged, if (virStateDriverTab[i]->stateInitialize(privileged, callback, opaque) < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Initialization of %s state driver failed: %s"), virStateDriverTab[i]->name, - err && err->message ? err->message : _("Unknown problem")); + virGetLastErrorMessage()); return -1; } } diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index c8d09b1..a814ae5 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -514,9 +514,8 @@ libxlDomainShutdownThread(void *opaque) libxlDomainDestroyInternal(driver, vm); libxlDomainCleanup(driver, vm); if (libxlDomainStart(driver, vm, false, -1) < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to restart VM '%s': %s"), - vm->def->name, err ? err->message : _("unknown error")); + vm->def->name, virGetLastErrorMessage()); } endjob: diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 87ec5a5..767ebbc 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -324,10 +324,9 @@ libxlAutostartDomain(virDomainObjPtr vm, if (vm->autostart && !virDomainObjIsActive(vm) && libxlDomainStart(driver, vm, false, -1) < 0) { - err = virGetLastError(); VIR_ERROR(_("Failed to autostart VM '%s': %s"), vm->def->name, - err ? err->message : _("unknown error")); + virGetLastErrorMessage()); goto endjob; } diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 973e691..b755a02 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -1266,12 +1266,8 @@ int main(int argc, char **argv) { /* Read the config file if it exists*/ if (remote_config_file && virLockDaemonConfigLoadFile(config, remote_config_file, implicit_conf) < 0) { - virErrorPtr err = virGetLastError(); - if (err && err->message) - VIR_ERROR(_("Can't load config file: %s: %s"), - err->message, remote_config_file); - else - VIR_ERROR(_("Can't load config file: %s"), remote_config_file); + VIR_ERROR(_("Can't load config file: %s: %s"), + virGetLastErrorMessage(), remote_config_file); exit(EXIT_FAILURE); } diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 68f0647..83f0475 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -1023,12 +1023,8 @@ int main(int argc, char **argv) { /* Read the config file if it exists*/ if (remote_config_file && virLogDaemonConfigLoadFile(config, remote_config_file, implicit_conf) < 0) { - virErrorPtr err = virGetLastError(); - if (err && err->message) - VIR_ERROR(_("Can't load config file: %s: %s"), - err->message, remote_config_file); - else - VIR_ERROR(_("Can't load config file: %s"), remote_config_file); + VIR_ERROR(_("Can't load config file: %s: %s"), + virGetLastErrorMessage(), remote_config_file); exit(EXIT_FAILURE); } diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 348bbfb..4daba3a 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2290,12 +2290,8 @@ static int lxcContainerChild(void *data) if (ret != 0) { VIR_DEBUG("Tearing down container"); - virErrorPtr err = virGetLastError(); - if (err && err->message) - fprintf(stderr, "%s\n", err->message); - else - fprintf(stderr, "%s\n", - _("Unknown failure in libvirt_lxc startup")); + fprintf(stderr, "Failure in libvirt_lxc startup%s\n", + virGetLastErrorMessage()); } virCommandFree(cmd); diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 8b5ec4c..b20a46f 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -2731,12 +2731,7 @@ int main(int argc, char *argv[]) cleanup: if (rc < 0) { - virErrorPtr err = virGetLastError(); - if (err && err->message) - fprintf(stderr, "%s\n", err->message); - else - fprintf(stderr, "%s\n", - _("Unknown failure in libvirt_lxc startup")); + fprintf(stderr, "Failure in libvirt_lxc startup: %s\n", virGetLastErrorMessage()); } virPidFileDelete(LXC_STATE_DIR, name); diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index 3177a62..5267797 100644 --- a/src/lxc/lxc_domain.c +++ b/src/lxc/lxc_domain.c @@ -221,8 +221,7 @@ virLXCDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, unsigned long long thepid; if (virXPathULongLong("string(./init[1]/@pid)", ctxt, &thepid) < 0) { - virErrorPtr err = virGetLastError(); - VIR_WARN("Failed to load init pid from state %s", err ? err->message : "null"); + VIR_WARN("Failed to load init pid from state %s", virGetLastErrorMessage()); priv->initpid = 0; } else { priv->initpid = thepid; diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 5e0bbe2..6f6df84 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -757,10 +757,9 @@ static void virLXCProcessMonitorInitNotify(virLXCMonitorPtr mon ATTRIBUTE_UNUSED priv->initpid = initpid; if (virLXCProcessGetNsInode(initpid, "pid", &inode) < 0) { - virErrorPtr err = virGetLastError(); VIR_WARN("Cannot obtain pid NS inode for %llu: %s", (unsigned long long)initpid, - err && err->message ? err->message : "<unknown>"); + virGetLastErrorMessage()); virResetLastError(); } virDomainAuditInit(vm, initpid, inode); @@ -1619,10 +1618,9 @@ virLXCProcessAutostartDomain(virDomainObjPtr vm, VIR_DOMAIN_RUNNING_BOOTED); virDomainAuditStart(vm, "booted", ret >= 0); if (ret < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to autostart VM '%s': %s"), vm->def->name, - err ? err->message : ""); + virGetLastErrorMessage()); } else { virObjectEventPtr event = virDomainEventLifecycleNewFromObj(vm, diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index a09a7e4..f82ad24 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -694,9 +694,8 @@ networkStateInitialize(bool privileged, #ifdef HAVE_FIREWALLD if (!(sysbus = virDBusGetSystemBus())) { - virErrorPtr err = virGetLastError(); VIR_WARN("DBus not available, disabling firewalld support " - "in bridge_network_driver: %s", err->message); + "in bridge_network_driver: %s", virGetLastErrorMessage()); } else { /* add matches for * NameOwnerChanged on org.freedesktop.DBus for firewalld start/stop diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 6d18a87..6ddfad0 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -641,9 +641,8 @@ nodeStateInitialize(bool privileged ATTRIBUTE_UNUSED, dbus_error_init(&err); if (!(sysbus = virDBusGetSystemBus())) { - virErrorPtr verr = virGetLastError(); VIR_ERROR(_("DBus not available, disabling HAL driver: %s"), - verr->message); + virGetLastErrorMessage()); ret = 0; goto failure; } diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 947038d..6e78623 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -1141,8 +1141,7 @@ int virNetTLSContextCheckCertificate(virNetTLSContextPtr ctxt, virObjectLock(ctxt); virObjectLock(sess); if (virNetTLSContextValidCertificate(ctxt, sess) < 0) { - virErrorPtr err = virGetLastError(); - VIR_WARN("Certificate check failed %s", err && err->message ? err->message : "<unknown>"); + VIR_WARN("Certificate check failed %s", virGetLastErrorMessage()); if (ctxt->requireValidCert) { virReportError(VIR_ERR_AUTH_FAILED, "%s", _("Failed to verify peer's certificate")); diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 4d15797..b9c82c6 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -484,11 +484,9 @@ secretLoadAllConfigs(virSecretObjPtr *dest, VIR_FREE(base64name); if (!(secret = secretLoad(&list, de->d_name, path, base64path))) { - virErrorPtr err = virGetLastError(); - VIR_ERROR(_("Error reading secret: %s"), - err != NULL ? err->message: _("unknown error")); - virResetError(err); + virGetLastErrorMessage()); + virResetLastError(); VIR_FREE(path); VIR_FREE(base64path); continue; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 1d96618..ccf1fc1 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -97,10 +97,8 @@ storagePoolUpdateState(virStoragePoolObjPtr pool) active = false; if (backend->checkPool && backend->checkPool(pool, &active) < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to initialize storage pool '%s': %s"), - pool->def->name, err ? err->message : - _("no error message found")); + pool->def->name, virGetLastErrorMessage()); goto error; } @@ -111,12 +109,10 @@ storagePoolUpdateState(virStoragePoolObjPtr pool) if (active) { virStoragePoolObjClearVols(pool); if (backend->refreshPool(NULL, pool) < 0) { - virErrorPtr err = virGetLastError(); if (backend->stopPool) backend->stopPool(NULL, pool); VIR_ERROR(_("Failed to restart storage pool '%s': %s"), - pool->def->name, err ? err->message : - _("no error message found")); + pool->def->name, virGetLastErrorMessage()); goto error; } } @@ -175,10 +171,8 @@ storageDriverAutostart(void) !virStoragePoolObjIsActive(pool)) { if (backend->startPool && backend->startPool(conn, pool) < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to autostart storage pool '%s': %s"), - pool->def->name, err ? err->message : - _("no error message found")); + pool->def->name, virGetLastErrorMessage()); virStoragePoolObjUnlock(pool); continue; } @@ -194,14 +188,12 @@ storageDriverAutostart(void) if (!stateFile || virStoragePoolSaveState(stateFile, pool->def) < 0 || backend->refreshPool(conn, pool) < 0) { - virErrorPtr err = virGetLastError(); if (stateFile) unlink(stateFile); if (backend->stopPool) backend->stopPool(conn, pool); VIR_ERROR(_("Failed to autostart storage pool '%s': %s"), - pool->def->name, err ? err->message : - _("no error message found")); + pool->def->name, virGetLastErrorMessage()); } else { pool->active = true; } diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 84e1df8..923c3f6 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -188,9 +188,8 @@ umlAutostartDomain(virDomainObjPtr vm, ret = umlStartVMDaemon(data->conn, data->driver, vm, false); virDomainAuditStart(vm, "booted", ret >= 0); if (ret < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to autostart VM '%s': %s"), - vm->def->name, err ? err->message : _("unknown error")); + vm->def->name, virGetLastErrorMessage()); } else { virObjectEventPtr event = virDomainEventLifecycleNewFromObj(vm, diff --git a/src/util/iohelper.c b/src/util/iohelper.c index 8a3c377..9fe0f81 100644 --- a/src/util/iohelper.c +++ b/src/util/iohelper.c @@ -310,12 +310,6 @@ main(int argc, char **argv) return 0; error: - err = virGetLastError(); - if (err) { - fprintf(stderr, "%s: %s\n", program_name, err->message); - } else { - fprintf(stderr, _("%s: unknown failure with %s\n"), - program_name, path); - } + fprintf(stderr, "%s: %s\n", program_name, virGetLastErrorMessage()); exit(EXIT_FAILURE); } diff --git a/src/util/virhook.c b/src/util/virhook.c index ba50598..d37d6da 100644 --- a/src/util/virhook.c +++ b/src/util/virhook.c @@ -297,9 +297,8 @@ virHookCall(int driver, ret = virCommandRun(cmd, NULL); if (ret < 0) { /* Convert INTERNAL_ERROR into known error. */ - virErrorPtr err = virGetLastError(); virReportError(VIR_ERR_HOOK_SCRIPT_FAILED, "%s", - err ? err->message : _("unknown error")); + virGetLastErrorMessage()); } virCommandFree(cmd); diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index b397b79..42c146a 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -801,10 +801,9 @@ virHostdevReattachPCIDevice(virHostdevManagerPtr mgr, VIR_DEBUG("Reattaching PCI device %s", virPCIDeviceGetName(actual)); if (virPCIDeviceReattach(actual, mgr->activePCIHostdevs, mgr->inactivePCIHostdevs) < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to re-attach PCI device: %s"), - err ? err->message : _("unknown error")); - virResetError(err); + virGetLastErrorMessage()); + virResetLastError(); } } @@ -829,10 +828,9 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr, virObjectLock(mgr->inactivePCIHostdevs); if (!(pcidevs = virHostdevGetPCIHostDeviceList(hostdevs, nhostdevs))) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to allocate PCI device list: %s"), - err ? err->message : _("unknown error")); - virResetError(err); + virGetLastErrorMessage()); + virResetLastError(); goto cleanup; } @@ -883,10 +881,9 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr, if (!actual || virPCIDeviceListAdd(mgr->inactivePCIHostdevs, actual) < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to add PCI device %s to the inactive list"), - err ? err->message : _("unknown error")); - virResetError(err); + virGetLastErrorMessage()); + virResetLastError(); } } @@ -928,10 +925,9 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr, VIR_DEBUG("Resetting PCI device %s", virPCIDeviceGetName(pci)); if (virPCIDeviceReset(pci, mgr->activePCIHostdevs, mgr->inactivePCIHostdevs) < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to reset PCI device: %s"), - err ? err->message : _("unknown error")); - virResetError(err); + virGetLastErrorMessage()); + virResetLastError(); } } diff --git a/src/util/virpci.c b/src/util/virpci.c index f7921f8..2349d7f 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -989,12 +989,10 @@ virPCIDeviceReset(virPCIDevicePtr dev, ret = virPCIDeviceTrySecondaryBusReset(dev, fd, inactiveDevs); if (ret < 0) { - virErrorPtr err = virGetLastError(); virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to reset PCI device %s: %s"), dev->name, - err ? err->message : - _("no FLR, PM reset or bus reset available")); + virGetLastErrorMessage()); } cleanup: diff --git a/tests/commandtest.c b/tests/commandtest.c index cf5f44a..6430e20 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -178,8 +178,7 @@ static int test2(const void *unused ATTRIBUTE_UNUSED) int ret; if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -190,8 +189,7 @@ static int test2(const void *unused ATTRIBUTE_UNUSED) } if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -218,8 +216,7 @@ static int test3(const void *unused ATTRIBUTE_UNUSED) VIR_COMMAND_PASS_FD_CLOSE_PARENT); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -260,8 +257,7 @@ static int test4(const void *unused ATTRIBUTE_UNUSED) virCommandDaemonize(cmd); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -294,8 +290,7 @@ static int test5(const void *unused ATTRIBUTE_UNUSED) virCommandAddEnvPassCommon(cmd); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -318,8 +313,7 @@ static int test6(const void *unused ATTRIBUTE_UNUSED) virCommandAddEnvPassBlockSUID(cmd, "DOESNOTEXIST", NULL); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -343,8 +337,7 @@ static int test7(const void *unused ATTRIBUTE_UNUSED) virCommandAddEnvPassBlockSUID(cmd, "DOESNOTEXIST", NULL); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -368,8 +361,7 @@ static int test8(const void *unused ATTRIBUTE_UNUSED) virCommandAddEnvPair(cmd, "USER", "test"); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -406,8 +398,7 @@ static int test9(const void *unused ATTRIBUTE_UNUSED) } if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -432,8 +423,7 @@ static int test10(const void *unused ATTRIBUTE_UNUSED) virCommandAddArgSet(cmd, args); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -456,8 +446,7 @@ static int test11(const void *unused ATTRIBUTE_UNUSED) virCommandPtr cmd = virCommandNewArgs(args); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -478,8 +467,7 @@ static int test12(const void *unused ATTRIBUTE_UNUSED) virCommandSetInputBuffer(cmd, "Hello World\n"); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -506,8 +494,7 @@ static int test13(const void *unused ATTRIBUTE_UNUSED) virCommandSetOutputBuffer(cmd, &outactual); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } if (!outactual) @@ -559,8 +546,7 @@ static int test14(const void *unused ATTRIBUTE_UNUSED) virCommandSetErrorBuffer(cmd, &erractual); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } if (!outactual || !erractual) @@ -573,8 +559,7 @@ static int test14(const void *unused ATTRIBUTE_UNUSED) virCommandSetOutputBuffer(cmd, &jointactual); virCommandSetErrorBuffer(cmd, &jointactual); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } if (!jointactual) @@ -620,8 +605,7 @@ static int test15(const void *unused ATTRIBUTE_UNUSED) virCommandSetUmask(cmd, 002); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -651,8 +635,7 @@ static int test16(const void *unused ATTRIBUTE_UNUSED) virCommandAddArg(cmd, "G H"); if ((outactual = virCommandToString(cmd)) == NULL) { - virErrorPtr err = virGetLastError(); - printf("Cannot convert to string: %s\n", err->message); + printf("Cannot convert to string: %s\n", virGetLastErrorMessage()); goto cleanup; } if ((fd = open(abs_builddir "/commandhelper.log", @@ -697,8 +680,7 @@ static int test17(const void *unused ATTRIBUTE_UNUSED) } if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -720,8 +702,7 @@ static int test17(const void *unused ATTRIBUTE_UNUSED) } if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -756,8 +737,7 @@ static int test18(const void *unused ATTRIBUTE_UNUSED) alarm(5); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } alarm(0); @@ -798,8 +778,7 @@ static int test19(const void *unused ATTRIBUTE_UNUSED) alarm(5); if (virCommandRunAsync(cmd, &pid) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -848,8 +827,7 @@ static int test20(const void *unused ATTRIBUTE_UNUSED) virCommandSetInputBuffer(cmd, buf); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -891,8 +869,7 @@ static int test21(const void *unused ATTRIBUTE_UNUSED) virCommandDoAsyncIO(cmd); if (virCommandRunAsync(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -930,8 +907,7 @@ test22(const void *unused ATTRIBUTE_UNUSED) cmd = virCommandNewArgList("/bin/sh", "-c", "exit 3", NULL); if (virCommandRun(cmd, &status) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } if (status != 3) { @@ -941,8 +917,7 @@ test22(const void *unused ATTRIBUTE_UNUSED) virCommandRawStatus(cmd); if (virCommandRun(cmd, &status) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } if (!WIFEXITED(status) || WEXITSTATUS(status) != 3) { @@ -960,8 +935,7 @@ test22(const void *unused ATTRIBUTE_UNUSED) virCommandRawStatus(cmd); if (virCommandRun(cmd, &status) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } if (!WIFSIGNALED(status) || WTERMSIG(status) != SIGKILL) { @@ -1057,8 +1031,7 @@ static int test24(const void *unused ATTRIBUTE_UNUSED) virCommandPassListenFDs(cmd); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } diff --git a/tests/libvirtdconftest.c b/tests/libvirtdconftest.c index 61d861d..06830bb 100644 --- a/tests/libvirtdconftest.c +++ b/tests/libvirtdconftest.c @@ -212,8 +212,7 @@ mymain(void) } if (virFileReadAll(filename, 1024*1024, &filedata) < 0) { - virErrorPtr err = virGetLastError(); - fprintf(stderr, "Cannot load %s for testing: %s", filename, err->message); + fprintf(stderr, "Cannot load %s for testing: %s", filename, virGetLastErrorMessage()); ret = -1; goto cleanup; } diff --git a/tests/openvzutilstest.c b/tests/openvzutilstest.c index ccde636..57a8601 100644 --- a/tests/openvzutilstest.c +++ b/tests/openvzutilstest.c @@ -110,16 +110,14 @@ testReadNetworkConf(const void *data ATTRIBUTE_UNUSED) def->os.type = VIR_DOMAIN_OSTYPE_EXE; if (openvzReadNetworkConf(def, 1) < 0) { - err = virGetLastError(); - fprintf(stderr, "ERROR: %s\n", err != NULL ? err->message : "<unknown>"); + fprintf(stderr, "ERROR: %s\n", virGetLastErrorMessage()); goto cleanup; } actual = virDomainDefFormat(def, NULL, VIR_DOMAIN_DEF_FORMAT_INACTIVE); if (actual == NULL) { - err = virGetLastError(); - fprintf(stderr, "ERROR: %s\n", err != NULL ? err->message : "<unknown>"); + fprintf(stderr, "ERROR: %s\n", virGetLastErrorMessage()); goto cleanup; } diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c index c82b3f2..f6caa30 100644 --- a/tests/securityselinuxlabeltest.c +++ b/tests/securityselinuxlabeltest.c @@ -332,8 +332,7 @@ testSELinuxLabeling(const void *opaque) } VIR_FREE(files); if (ret < 0) { - virErrorPtr err = virGetLastError(); - VIR_TEST_VERBOSE("%s\n", err ? err->message : "<unknown>"); + VIR_TEST_VERBOSE("%s\n", virGetLastErrorMessage()); } return ret; } @@ -354,9 +353,8 @@ mymain(void) if (!(mgr = virSecurityManagerNew("selinux", "QEMU", VIR_SECURITY_MANAGER_DEFAULT_CONFINED | VIR_SECURITY_MANAGER_PRIVILEGED))) { - virErrorPtr err = virGetLastError(); VIR_TEST_VERBOSE("Unable to initialize security driver: %s\n", - err->message); + virGetLastErrorMessage()); return EXIT_FAILURE; } diff --git a/tests/securityselinuxtest.c b/tests/securityselinuxtest.c index 49694f3..3423e66 100644 --- a/tests/securityselinuxtest.c +++ b/tests/securityselinuxtest.c @@ -230,8 +230,7 @@ testSELinuxGenLabel(const void *opaque) goto cleanup; if (virSecurityManagerGenLabel(data->mgr, def) < 0) { - virErrorPtr err = virGetLastError(); - fprintf(stderr, "Cannot generate label: %s\n", err->message); + fprintf(stderr, "Cannot generate label: %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -275,9 +274,8 @@ mymain(void) if (!(mgr = virSecurityManagerNew("selinux", "QEMU", VIR_SECURITY_MANAGER_DEFAULT_CONFINED | VIR_SECURITY_MANAGER_PRIVILEGED))) { - virErrorPtr err = virGetLastError(); fprintf(stderr, "Unable to initialize security driver: %s\n", - err->message); + virGetLastErrorMessage()); return EXIT_FAILURE; } diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c index a3e24a3..d33b896 100644 --- a/tests/virnettlscontexttest.c +++ b/tests/virnettlscontexttest.c @@ -90,13 +90,12 @@ static int testTLSContextInit(const void *opaque) goto cleanup; } } else { - virErrorPtr err = virGetLastError(); if (!data->expectFail) { VIR_WARN("Unexpected failure %s against %s", data->cacrt, data->crt); goto cleanup; } - VIR_DEBUG("Got error %s", err ? err->message : "<unknown>"); + VIR_DEBUG("Got error %s", virGetLastErrorMessage()); } ret = 0; -- 2.7.4
On 03/25/2016 05:05 AM, Hui Yiqun wrote:
getting err using virGetLastError() and then retrieving message from err make developer have to test the value of err and err->message and default to self-defined unkown error message.
It's better to avoid it and using uniform virGetLastErrorMessage
Thanks for the patch! There's several compiler warnings after this though, which by default are errors for libvirt. They are of the form: error: unused variable 'err' [-Werror=unused-variable] . I won't tell you where :) Check the patch for any places that the 'virErrorPtr err' definition wasn't deleted, and if there's no other uses in the function, delete the definition. Make sure you aren't passing --disable-werror to ./configure (--enable-werror is the default) Also make sure to run 'make syntax-check', it will flag a couple style issues you need to fix. A few more comments below
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 3d38a46..e526e55 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1273,12 +1273,8 @@ int main(int argc, char **argv) { /* Read the config file if it exists*/ if (remote_config_file && daemonConfigLoadFile(config, remote_config_file, implicit_conf) < 0) { - virErrorPtr err = virGetLastError(); - if (err && err->message) - VIR_ERROR(_("Can't load config file: %s: %s"), - err->message, remote_config_file); - else - VIR_ERROR(_("Can't load config file: %s"), remote_config_file); + VIR_ERROR(_("Can't load config file: %s: %s"), + virGetLastErrorMessage(), remote_config_file); exit(EXIT_FAILURE); }
This and (and the other places like it) changes the error message slightly, but it's a good change. Just wanted to point it out
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 348bbfb..4daba3a 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2290,12 +2290,8 @@ static int lxcContainerChild(void *data)
if (ret != 0) { VIR_DEBUG("Tearing down container"); - virErrorPtr err = virGetLastError(); - if (err && err->message) - fprintf(stderr, "%s\n", err->message); - else - fprintf(stderr, "%s\n", - _("Unknown failure in libvirt_lxc startup")); + fprintf(stderr, "Failure in libvirt_lxc startup%s\n", + virGetLastErrorMessage()); }
virCommandFree(cmd);
You dropped _() which ensures the string is translated. And you need a separator between virGetLastErrorMessage() and the root string: _("Unknown failure in libvirt_lxc startup: %s\n")
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 8b5ec4c..b20a46f 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -2731,12 +2731,7 @@ int main(int argc, char *argv[])
cleanup: if (rc < 0) { - virErrorPtr err = virGetLastError(); - if (err && err->message) - fprintf(stderr, "%s\n", err->message); - else - fprintf(stderr, "%s\n", - _("Unknown failure in libvirt_lxc startup")); + fprintf(stderr, "Failure in libvirt_lxc startup: %s\n", virGetLastErrorMessage()); }
Similarly this dropped the _() usage, please re-add it
diff --git a/src/util/iohelper.c b/src/util/iohelper.c index 8a3c377..9fe0f81 100644 --- a/src/util/iohelper.c +++ b/src/util/iohelper.c @@ -310,12 +310,6 @@ main(int argc, char **argv) return 0;
error: - err = virGetLastError(); - if (err) { - fprintf(stderr, "%s: %s\n", program_name, err->message); - } else { - fprintf(stderr, _("%s: unknown failure with %s\n"), - program_name, path); - } + fprintf(stderr, "%s: %s\n", program_name, virGetLastErrorMessage()); exit(EXIT_FAILURE); }
This one is a bit unclear but I'd suggest _("%s: failure with %s: %s"), program_name, path, virGetLastErrorMessage()
diff --git a/src/util/virpci.c b/src/util/virpci.c index f7921f8..2349d7f 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -989,12 +989,10 @@ virPCIDeviceReset(virPCIDevicePtr dev, ret = virPCIDeviceTrySecondaryBusReset(dev, fd, inactiveDevs);
if (ret < 0) { - virErrorPtr err = virGetLastError(); virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to reset PCI device %s: %s"), dev->name, - err ? err->message : - _("no FLR, PM reset or bus reset available")); + virGetLastErrorMessage()); }
Hmm this case is kind of legitimate. The pattern is a bit weird though, maybe it makes more sense to clean up the code above it. I suggest dropping this change from the patch since it may deserve a bit more work. Thanks, Cole
getting err using virGetLastError() and then retrieving message from err asks developers to test the value of err and err->message and default to self-defined unkown error message. It's better to avoid it and use uniform virGetLastErrorMessage --- daemon/libvirtd.c | 8 +--- examples/object-events/event-test.c | 9 ++--- src/bhyve/bhyve_driver.c | 3 +- src/libvirt.c | 3 +- src/libxl/libxl_domain.c | 3 +- src/libxl/libxl_driver.c | 3 +- src/locking/lock_daemon.c | 8 +--- src/logging/log_daemon.c | 8 +--- src/lxc/lxc_container.c | 8 +--- src/lxc/lxc_controller.c | 8 +--- src/lxc/lxc_domain.c | 3 +- src/lxc/lxc_process.c | 6 +-- src/network/bridge_driver.c | 3 +- src/node_device/node_device_hal.c | 3 +- src/rpc/virnettlscontext.c | 3 +- src/secret/secret_driver.c | 6 +-- src/storage/storage_driver.c | 16 ++------ src/uml/uml_driver.c | 3 +- src/util/iohelper.c | 10 +---- src/util/virhook.c | 3 +- src/util/virhostdev.c | 20 ++++----- tests/commandtest.c | 81 +++++++++++++------------------------ tests/libvirtdconftest.c | 3 +- tests/openvzutilstest.c | 7 +--- tests/securityselinuxlabeltest.c | 9 ++--- tests/securityselinuxtest.c | 6 +-- tests/virnettlscontexttest.c | 3 +- 27 files changed, 77 insertions(+), 169 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 3d38a46..e526e55 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1273,12 +1273,8 @@ int main(int argc, char **argv) { /* Read the config file if it exists*/ if (remote_config_file && daemonConfigLoadFile(config, remote_config_file, implicit_conf) < 0) { - virErrorPtr err = virGetLastError(); - if (err && err->message) - VIR_ERROR(_("Can't load config file: %s: %s"), - err->message, remote_config_file); - else - VIR_ERROR(_("Can't load config file: %s"), remote_config_file); + VIR_ERROR(_("Can't load config file: %s: %s"), + virGetLastErrorMessage(), remote_config_file); exit(EXIT_FAILURE); } diff --git a/examples/object-events/event-test.c b/examples/object-events/event-test.c index 7be1d21..4a4ef86 100644 --- a/examples/object-events/event-test.c +++ b/examples/object-events/event-test.c @@ -654,9 +654,8 @@ int main(int argc, char **argv) } if (virEventRegisterDefaultImpl() < 0) { - virErrorPtr err = virGetLastError(); fprintf(stderr, "Failed to register event implementation: %s\n", - err && err->message ? err->message: "Unknown error"); + virGetLastErrorMessage()); goto cleanup; } @@ -794,17 +793,15 @@ int main(int argc, char **argv) goto cleanup; if (virConnectSetKeepAlive(dconn, 5, 3) < 0) { - virErrorPtr err = virGetLastError(); fprintf(stderr, "Failed to start keepalive protocol: %s\n", - err && err->message ? err->message : "Unknown error"); + virGetLastErrorMessage()); run = 0; } while (run) { if (virEventRunDefaultImpl() < 0) { - virErrorPtr err = virGetLastError(); fprintf(stderr, "Failed to run event loop: %s\n", - err && err->message ? err->message : "Unknown error"); + virGetLastErrorMessage()); } } diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 9219890..6f2423c 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -88,9 +88,8 @@ bhyveAutostartDomain(virDomainObjPtr vm, void *opaque) ret = virBhyveProcessStart(data->conn, data->driver, vm, VIR_DOMAIN_RUNNING_BOOTED, 0); if (ret < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to autostart VM '%s': %s"), - vm->def->name, err ? err->message : _("unknown error")); + vm->def->name, virGetLastErrorMessage()); } } virObjectUnlock(vm); diff --git a/src/libvirt.c b/src/libvirt.c index dd58e9c..99b1c47 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -777,10 +777,9 @@ virStateInitialize(bool privileged, if (virStateDriverTab[i]->stateInitialize(privileged, callback, opaque) < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Initialization of %s state driver failed: %s"), virStateDriverTab[i]->name, - err && err->message ? err->message : _("Unknown problem")); + virGetLastErrorMessage()); return -1; } } diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index c8d09b1..a814ae5 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -514,9 +514,8 @@ libxlDomainShutdownThread(void *opaque) libxlDomainDestroyInternal(driver, vm); libxlDomainCleanup(driver, vm); if (libxlDomainStart(driver, vm, false, -1) < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to restart VM '%s': %s"), - vm->def->name, err ? err->message : _("unknown error")); + vm->def->name, virGetLastErrorMessage()); } endjob: diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 87ec5a5..767ebbc 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -324,10 +324,9 @@ libxlAutostartDomain(virDomainObjPtr vm, if (vm->autostart && !virDomainObjIsActive(vm) && libxlDomainStart(driver, vm, false, -1) < 0) { - err = virGetLastError(); VIR_ERROR(_("Failed to autostart VM '%s': %s"), vm->def->name, - err ? err->message : _("unknown error")); + virGetLastErrorMessage()); goto endjob; } diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 973e691..b755a02 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -1266,12 +1266,8 @@ int main(int argc, char **argv) { /* Read the config file if it exists*/ if (remote_config_file && virLockDaemonConfigLoadFile(config, remote_config_file, implicit_conf) < 0) { - virErrorPtr err = virGetLastError(); - if (err && err->message) - VIR_ERROR(_("Can't load config file: %s: %s"), - err->message, remote_config_file); - else - VIR_ERROR(_("Can't load config file: %s"), remote_config_file); + VIR_ERROR(_("Can't load config file: %s: %s"), + virGetLastErrorMessage(), remote_config_file); exit(EXIT_FAILURE); } diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 68f0647..83f0475 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -1023,12 +1023,8 @@ int main(int argc, char **argv) { /* Read the config file if it exists*/ if (remote_config_file && virLogDaemonConfigLoadFile(config, remote_config_file, implicit_conf) < 0) { - virErrorPtr err = virGetLastError(); - if (err && err->message) - VIR_ERROR(_("Can't load config file: %s: %s"), - err->message, remote_config_file); - else - VIR_ERROR(_("Can't load config file: %s"), remote_config_file); + VIR_ERROR(_("Can't load config file: %s: %s"), + virGetLastErrorMessage(), remote_config_file); exit(EXIT_FAILURE); } diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 348bbfb..eb17459 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -2290,12 +2290,8 @@ static int lxcContainerChild(void *data) if (ret != 0) { VIR_DEBUG("Tearing down container"); - virErrorPtr err = virGetLastError(); - if (err && err->message) - fprintf(stderr, "%s\n", err->message); - else - fprintf(stderr, "%s\n", - _("Unknown failure in libvirt_lxc startup")); + fprintf(stderr, _("Failure in libvirt_lxc startup%s\n"), + virGetLastErrorMessage()); } virCommandFree(cmd); diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 8b5ec4c..72b984b 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -2731,12 +2731,8 @@ int main(int argc, char *argv[]) cleanup: if (rc < 0) { - virErrorPtr err = virGetLastError(); - if (err && err->message) - fprintf(stderr, "%s\n", err->message); - else - fprintf(stderr, "%s\n", - _("Unknown failure in libvirt_lxc startup")); + fprintf(stderr, _("Failure in libvirt_lxc startup: %s\n"), + virGetLastErrorMessage()); } virPidFileDelete(LXC_STATE_DIR, name); diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c index 3177a62..5267797 100644 --- a/src/lxc/lxc_domain.c +++ b/src/lxc/lxc_domain.c @@ -221,8 +221,7 @@ virLXCDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, unsigned long long thepid; if (virXPathULongLong("string(./init[1]/@pid)", ctxt, &thepid) < 0) { - virErrorPtr err = virGetLastError(); - VIR_WARN("Failed to load init pid from state %s", err ? err->message : "null"); + VIR_WARN("Failed to load init pid from state %s", virGetLastErrorMessage()); priv->initpid = 0; } else { priv->initpid = thepid; diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 5e0bbe2..6f6df84 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -757,10 +757,9 @@ static void virLXCProcessMonitorInitNotify(virLXCMonitorPtr mon ATTRIBUTE_UNUSED priv->initpid = initpid; if (virLXCProcessGetNsInode(initpid, "pid", &inode) < 0) { - virErrorPtr err = virGetLastError(); VIR_WARN("Cannot obtain pid NS inode for %llu: %s", (unsigned long long)initpid, - err && err->message ? err->message : "<unknown>"); + virGetLastErrorMessage()); virResetLastError(); } virDomainAuditInit(vm, initpid, inode); @@ -1619,10 +1618,9 @@ virLXCProcessAutostartDomain(virDomainObjPtr vm, VIR_DOMAIN_RUNNING_BOOTED); virDomainAuditStart(vm, "booted", ret >= 0); if (ret < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to autostart VM '%s': %s"), vm->def->name, - err ? err->message : ""); + virGetLastErrorMessage()); } else { virObjectEventPtr event = virDomainEventLifecycleNewFromObj(vm, diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index a09a7e4..f82ad24 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -694,9 +694,8 @@ networkStateInitialize(bool privileged, #ifdef HAVE_FIREWALLD if (!(sysbus = virDBusGetSystemBus())) { - virErrorPtr err = virGetLastError(); VIR_WARN("DBus not available, disabling firewalld support " - "in bridge_network_driver: %s", err->message); + "in bridge_network_driver: %s", virGetLastErrorMessage()); } else { /* add matches for * NameOwnerChanged on org.freedesktop.DBus for firewalld start/stop diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 6d18a87..6ddfad0 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -641,9 +641,8 @@ nodeStateInitialize(bool privileged ATTRIBUTE_UNUSED, dbus_error_init(&err); if (!(sysbus = virDBusGetSystemBus())) { - virErrorPtr verr = virGetLastError(); VIR_ERROR(_("DBus not available, disabling HAL driver: %s"), - verr->message); + virGetLastErrorMessage()); ret = 0; goto failure; } diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 947038d..6e78623 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -1141,8 +1141,7 @@ int virNetTLSContextCheckCertificate(virNetTLSContextPtr ctxt, virObjectLock(ctxt); virObjectLock(sess); if (virNetTLSContextValidCertificate(ctxt, sess) < 0) { - virErrorPtr err = virGetLastError(); - VIR_WARN("Certificate check failed %s", err && err->message ? err->message : "<unknown>"); + VIR_WARN("Certificate check failed %s", virGetLastErrorMessage()); if (ctxt->requireValidCert) { virReportError(VIR_ERR_AUTH_FAILED, "%s", _("Failed to verify peer's certificate")); diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 4d15797..b9c82c6 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -484,11 +484,9 @@ secretLoadAllConfigs(virSecretObjPtr *dest, VIR_FREE(base64name); if (!(secret = secretLoad(&list, de->d_name, path, base64path))) { - virErrorPtr err = virGetLastError(); - VIR_ERROR(_("Error reading secret: %s"), - err != NULL ? err->message: _("unknown error")); - virResetError(err); + virGetLastErrorMessage()); + virResetLastError(); VIR_FREE(path); VIR_FREE(base64path); continue; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 1d96618..ccf1fc1 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -97,10 +97,8 @@ storagePoolUpdateState(virStoragePoolObjPtr pool) active = false; if (backend->checkPool && backend->checkPool(pool, &active) < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to initialize storage pool '%s': %s"), - pool->def->name, err ? err->message : - _("no error message found")); + pool->def->name, virGetLastErrorMessage()); goto error; } @@ -111,12 +109,10 @@ storagePoolUpdateState(virStoragePoolObjPtr pool) if (active) { virStoragePoolObjClearVols(pool); if (backend->refreshPool(NULL, pool) < 0) { - virErrorPtr err = virGetLastError(); if (backend->stopPool) backend->stopPool(NULL, pool); VIR_ERROR(_("Failed to restart storage pool '%s': %s"), - pool->def->name, err ? err->message : - _("no error message found")); + pool->def->name, virGetLastErrorMessage()); goto error; } } @@ -175,10 +171,8 @@ storageDriverAutostart(void) !virStoragePoolObjIsActive(pool)) { if (backend->startPool && backend->startPool(conn, pool) < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to autostart storage pool '%s': %s"), - pool->def->name, err ? err->message : - _("no error message found")); + pool->def->name, virGetLastErrorMessage()); virStoragePoolObjUnlock(pool); continue; } @@ -194,14 +188,12 @@ storageDriverAutostart(void) if (!stateFile || virStoragePoolSaveState(stateFile, pool->def) < 0 || backend->refreshPool(conn, pool) < 0) { - virErrorPtr err = virGetLastError(); if (stateFile) unlink(stateFile); if (backend->stopPool) backend->stopPool(conn, pool); VIR_ERROR(_("Failed to autostart storage pool '%s': %s"), - pool->def->name, err ? err->message : - _("no error message found")); + pool->def->name, virGetLastErrorMessage()); } else { pool->active = true; } diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 84e1df8..923c3f6 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -188,9 +188,8 @@ umlAutostartDomain(virDomainObjPtr vm, ret = umlStartVMDaemon(data->conn, data->driver, vm, false); virDomainAuditStart(vm, "booted", ret >= 0); if (ret < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to autostart VM '%s': %s"), - vm->def->name, err ? err->message : _("unknown error")); + vm->def->name, virGetLastErrorMessage()); } else { virObjectEventPtr event = virDomainEventLifecycleNewFromObj(vm, diff --git a/src/util/iohelper.c b/src/util/iohelper.c index 8a3c377..21a4452 100644 --- a/src/util/iohelper.c +++ b/src/util/iohelper.c @@ -219,7 +219,6 @@ int main(int argc, char **argv) { const char *path; - virErrorPtr err; unsigned long long offset; unsigned long long length; int oflags = -1; @@ -310,12 +309,7 @@ main(int argc, char **argv) return 0; error: - err = virGetLastError(); - if (err) { - fprintf(stderr, "%s: %s\n", program_name, err->message); - } else { - fprintf(stderr, _("%s: unknown failure with %s\n"), - program_name, path); - } + fprintf(stderr, _("%s: failure with %s: %s"), + program_name, path, virGetLastErrorMessage()); exit(EXIT_FAILURE); } diff --git a/src/util/virhook.c b/src/util/virhook.c index ba50598..d37d6da 100644 --- a/src/util/virhook.c +++ b/src/util/virhook.c @@ -297,9 +297,8 @@ virHookCall(int driver, ret = virCommandRun(cmd, NULL); if (ret < 0) { /* Convert INTERNAL_ERROR into known error. */ - virErrorPtr err = virGetLastError(); virReportError(VIR_ERR_HOOK_SCRIPT_FAILED, "%s", - err ? err->message : _("unknown error")); + virGetLastErrorMessage()); } virCommandFree(cmd); diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c index b397b79..42c146a 100644 --- a/src/util/virhostdev.c +++ b/src/util/virhostdev.c @@ -801,10 +801,9 @@ virHostdevReattachPCIDevice(virHostdevManagerPtr mgr, VIR_DEBUG("Reattaching PCI device %s", virPCIDeviceGetName(actual)); if (virPCIDeviceReattach(actual, mgr->activePCIHostdevs, mgr->inactivePCIHostdevs) < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to re-attach PCI device: %s"), - err ? err->message : _("unknown error")); - virResetError(err); + virGetLastErrorMessage()); + virResetLastError(); } } @@ -829,10 +828,9 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr, virObjectLock(mgr->inactivePCIHostdevs); if (!(pcidevs = virHostdevGetPCIHostDeviceList(hostdevs, nhostdevs))) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to allocate PCI device list: %s"), - err ? err->message : _("unknown error")); - virResetError(err); + virGetLastErrorMessage()); + virResetLastError(); goto cleanup; } @@ -883,10 +881,9 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr, if (!actual || virPCIDeviceListAdd(mgr->inactivePCIHostdevs, actual) < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to add PCI device %s to the inactive list"), - err ? err->message : _("unknown error")); - virResetError(err); + virGetLastErrorMessage()); + virResetLastError(); } } @@ -928,10 +925,9 @@ virHostdevReAttachPCIDevices(virHostdevManagerPtr mgr, VIR_DEBUG("Resetting PCI device %s", virPCIDeviceGetName(pci)); if (virPCIDeviceReset(pci, mgr->activePCIHostdevs, mgr->inactivePCIHostdevs) < 0) { - virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to reset PCI device: %s"), - err ? err->message : _("unknown error")); - virResetError(err); + virGetLastErrorMessage()); + virResetLastError(); } } diff --git a/tests/commandtest.c b/tests/commandtest.c index cf5f44a..6430e20 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -178,8 +178,7 @@ static int test2(const void *unused ATTRIBUTE_UNUSED) int ret; if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -190,8 +189,7 @@ static int test2(const void *unused ATTRIBUTE_UNUSED) } if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -218,8 +216,7 @@ static int test3(const void *unused ATTRIBUTE_UNUSED) VIR_COMMAND_PASS_FD_CLOSE_PARENT); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -260,8 +257,7 @@ static int test4(const void *unused ATTRIBUTE_UNUSED) virCommandDaemonize(cmd); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -294,8 +290,7 @@ static int test5(const void *unused ATTRIBUTE_UNUSED) virCommandAddEnvPassCommon(cmd); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -318,8 +313,7 @@ static int test6(const void *unused ATTRIBUTE_UNUSED) virCommandAddEnvPassBlockSUID(cmd, "DOESNOTEXIST", NULL); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -343,8 +337,7 @@ static int test7(const void *unused ATTRIBUTE_UNUSED) virCommandAddEnvPassBlockSUID(cmd, "DOESNOTEXIST", NULL); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -368,8 +361,7 @@ static int test8(const void *unused ATTRIBUTE_UNUSED) virCommandAddEnvPair(cmd, "USER", "test"); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -406,8 +398,7 @@ static int test9(const void *unused ATTRIBUTE_UNUSED) } if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -432,8 +423,7 @@ static int test10(const void *unused ATTRIBUTE_UNUSED) virCommandAddArgSet(cmd, args); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -456,8 +446,7 @@ static int test11(const void *unused ATTRIBUTE_UNUSED) virCommandPtr cmd = virCommandNewArgs(args); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -478,8 +467,7 @@ static int test12(const void *unused ATTRIBUTE_UNUSED) virCommandSetInputBuffer(cmd, "Hello World\n"); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); virCommandFree(cmd); return -1; } @@ -506,8 +494,7 @@ static int test13(const void *unused ATTRIBUTE_UNUSED) virCommandSetOutputBuffer(cmd, &outactual); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } if (!outactual) @@ -559,8 +546,7 @@ static int test14(const void *unused ATTRIBUTE_UNUSED) virCommandSetErrorBuffer(cmd, &erractual); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } if (!outactual || !erractual) @@ -573,8 +559,7 @@ static int test14(const void *unused ATTRIBUTE_UNUSED) virCommandSetOutputBuffer(cmd, &jointactual); virCommandSetErrorBuffer(cmd, &jointactual); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } if (!jointactual) @@ -620,8 +605,7 @@ static int test15(const void *unused ATTRIBUTE_UNUSED) virCommandSetUmask(cmd, 002); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -651,8 +635,7 @@ static int test16(const void *unused ATTRIBUTE_UNUSED) virCommandAddArg(cmd, "G H"); if ((outactual = virCommandToString(cmd)) == NULL) { - virErrorPtr err = virGetLastError(); - printf("Cannot convert to string: %s\n", err->message); + printf("Cannot convert to string: %s\n", virGetLastErrorMessage()); goto cleanup; } if ((fd = open(abs_builddir "/commandhelper.log", @@ -697,8 +680,7 @@ static int test17(const void *unused ATTRIBUTE_UNUSED) } if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -720,8 +702,7 @@ static int test17(const void *unused ATTRIBUTE_UNUSED) } if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -756,8 +737,7 @@ static int test18(const void *unused ATTRIBUTE_UNUSED) alarm(5); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } alarm(0); @@ -798,8 +778,7 @@ static int test19(const void *unused ATTRIBUTE_UNUSED) alarm(5); if (virCommandRunAsync(cmd, &pid) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -848,8 +827,7 @@ static int test20(const void *unused ATTRIBUTE_UNUSED) virCommandSetInputBuffer(cmd, buf); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -891,8 +869,7 @@ static int test21(const void *unused ATTRIBUTE_UNUSED) virCommandDoAsyncIO(cmd); if (virCommandRunAsync(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -930,8 +907,7 @@ test22(const void *unused ATTRIBUTE_UNUSED) cmd = virCommandNewArgList("/bin/sh", "-c", "exit 3", NULL); if (virCommandRun(cmd, &status) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } if (status != 3) { @@ -941,8 +917,7 @@ test22(const void *unused ATTRIBUTE_UNUSED) virCommandRawStatus(cmd); if (virCommandRun(cmd, &status) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } if (!WIFEXITED(status) || WEXITSTATUS(status) != 3) { @@ -960,8 +935,7 @@ test22(const void *unused ATTRIBUTE_UNUSED) virCommandRawStatus(cmd); if (virCommandRun(cmd, &status) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } if (!WIFSIGNALED(status) || WTERMSIG(status) != SIGKILL) { @@ -1057,8 +1031,7 @@ static int test24(const void *unused ATTRIBUTE_UNUSED) virCommandPassListenFDs(cmd); if (virCommandRun(cmd, NULL) < 0) { - virErrorPtr err = virGetLastError(); - printf("Cannot run child %s\n", err->message); + printf("Cannot run child %s\n", virGetLastErrorMessage()); goto cleanup; } diff --git a/tests/libvirtdconftest.c b/tests/libvirtdconftest.c index 61d861d..06830bb 100644 --- a/tests/libvirtdconftest.c +++ b/tests/libvirtdconftest.c @@ -212,8 +212,7 @@ mymain(void) } if (virFileReadAll(filename, 1024*1024, &filedata) < 0) { - virErrorPtr err = virGetLastError(); - fprintf(stderr, "Cannot load %s for testing: %s", filename, err->message); + fprintf(stderr, "Cannot load %s for testing: %s", filename, virGetLastErrorMessage()); ret = -1; goto cleanup; } diff --git a/tests/openvzutilstest.c b/tests/openvzutilstest.c index ccde636..d747165 100644 --- a/tests/openvzutilstest.c +++ b/tests/openvzutilstest.c @@ -75,7 +75,6 @@ testReadNetworkConf(const void *data ATTRIBUTE_UNUSED) int result = -1; virDomainDefPtr def = NULL; char *actual = NULL; - virErrorPtr err = NULL; const char *expected = "<domain type='openvz'>\n" " <uuid>00000000-0000-0000-0000-000000000000</uuid>\n" @@ -110,16 +109,14 @@ testReadNetworkConf(const void *data ATTRIBUTE_UNUSED) def->os.type = VIR_DOMAIN_OSTYPE_EXE; if (openvzReadNetworkConf(def, 1) < 0) { - err = virGetLastError(); - fprintf(stderr, "ERROR: %s\n", err != NULL ? err->message : "<unknown>"); + fprintf(stderr, "ERROR: %s\n", virGetLastErrorMessage()); goto cleanup; } actual = virDomainDefFormat(def, NULL, VIR_DOMAIN_DEF_FORMAT_INACTIVE); if (actual == NULL) { - err = virGetLastError(); - fprintf(stderr, "ERROR: %s\n", err != NULL ? err->message : "<unknown>"); + fprintf(stderr, "ERROR: %s\n", virGetLastErrorMessage()); goto cleanup; } diff --git a/tests/securityselinuxlabeltest.c b/tests/securityselinuxlabeltest.c index c82b3f2..7bb5de0 100644 --- a/tests/securityselinuxlabeltest.c +++ b/tests/securityselinuxlabeltest.c @@ -331,10 +331,8 @@ testSELinuxLabeling(const void *opaque) VIR_FREE(files[i].context); } VIR_FREE(files); - if (ret < 0) { - virErrorPtr err = virGetLastError(); - VIR_TEST_VERBOSE("%s\n", err ? err->message : "<unknown>"); - } + if (ret < 0) + VIR_TEST_VERBOSE("%s\n", virGetLastErrorMessage()); return ret; } @@ -354,9 +352,8 @@ mymain(void) if (!(mgr = virSecurityManagerNew("selinux", "QEMU", VIR_SECURITY_MANAGER_DEFAULT_CONFINED | VIR_SECURITY_MANAGER_PRIVILEGED))) { - virErrorPtr err = virGetLastError(); VIR_TEST_VERBOSE("Unable to initialize security driver: %s\n", - err->message); + virGetLastErrorMessage()); return EXIT_FAILURE; } diff --git a/tests/securityselinuxtest.c b/tests/securityselinuxtest.c index 49694f3..3423e66 100644 --- a/tests/securityselinuxtest.c +++ b/tests/securityselinuxtest.c @@ -230,8 +230,7 @@ testSELinuxGenLabel(const void *opaque) goto cleanup; if (virSecurityManagerGenLabel(data->mgr, def) < 0) { - virErrorPtr err = virGetLastError(); - fprintf(stderr, "Cannot generate label: %s\n", err->message); + fprintf(stderr, "Cannot generate label: %s\n", virGetLastErrorMessage()); goto cleanup; } @@ -275,9 +274,8 @@ mymain(void) if (!(mgr = virSecurityManagerNew("selinux", "QEMU", VIR_SECURITY_MANAGER_DEFAULT_CONFINED | VIR_SECURITY_MANAGER_PRIVILEGED))) { - virErrorPtr err = virGetLastError(); fprintf(stderr, "Unable to initialize security driver: %s\n", - err->message); + virGetLastErrorMessage()); return EXIT_FAILURE; } diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c index a3e24a3..d33b896 100644 --- a/tests/virnettlscontexttest.c +++ b/tests/virnettlscontexttest.c @@ -90,13 +90,12 @@ static int testTLSContextInit(const void *opaque) goto cleanup; } } else { - virErrorPtr err = virGetLastError(); if (!data->expectFail) { VIR_WARN("Unexpected failure %s against %s", data->cacrt, data->crt); goto cleanup; } - VIR_DEBUG("Got error %s", err ? err->message : "<unknown>"); + VIR_DEBUG("Got error %s", virGetLastErrorMessage()); } ret = 0; -- 2.8.0
When sending a v2, please give a description of what changed compared to the previous patch version, after the --- break so it doesn't show up in the commit message. For example: On 04/04/2016 06:49 AM, Hui Yiqun wrote:
getting err using virGetLastError() and then retrieving message from err asks developers to test the value of err and err->message and default to self-defined unkown error message.
It's better to avoid it and use uniform virGetLastErrorMessage ---
v2: * Fixed the FOO * Replaces the BAZ * etc
daemon/libvirtd.c | 8 +--- examples/object-events/event-test.c | 9 ++--- src/bhyve/bhyve_driver.c | 3 +- src/libvirt.c | 3 +- src/libxl/libxl_domain.c | 3 +- src/libxl/libxl_driver.c | 3 +- src/locking/lock_daemon.c | 8 +--- src/logging/log_daemon.c | 8 +--- src/lxc/lxc_container.c | 8 +--- src/lxc/lxc_controller.c | 8 +--- src/lxc/lxc_domain.c | 3 +- src/lxc/lxc_process.c | 6 +-- src/network/bridge_driver.c | 3 +- src/node_device/node_device_hal.c | 3 +- src/rpc/virnettlscontext.c | 3 +- src/secret/secret_driver.c | 6 +-- src/storage/storage_driver.c | 16 ++------ src/uml/uml_driver.c | 3 +- src/util/iohelper.c | 10 +---- src/util/virhook.c | 3 +- src/util/virhostdev.c | 20 ++++----- tests/commandtest.c | 81 +++++++++++++------------------------ tests/libvirtdconftest.c | 3 +- tests/openvzutilstest.c | 7 +--- tests/securityselinuxlabeltest.c | 9 ++--- tests/securityselinuxtest.c | 6 +-- tests/virnettlscontexttest.c | 3 +- 27 files changed, 77 insertions(+), 169 deletions(-)
There's still at least one build error 'error: unused variable 'err'' in here. Please review the code carefully, or only send the patch targetting the drivers that you can actually build test. Thanks, Cole
2016-04-08 23:12 GMT+08:00 Cole Robinson <crobinso@redhat.com>:
When sending a v2, please give a description of what changed compared to the previous patch version, after the --- break so it doesn't show up in the commit message. For example:
Nice proposal. Thanks.
On 04/04/2016 06:49 AM, Hui Yiqun wrote:
getting err using virGetLastError() and then retrieving message from err asks developers to test the value of err and err->message and default to self-defined unkown error message.
It's better to avoid it and use uniform virGetLastErrorMessage ---
v2: * Fixed the FOO * Replaces the BAZ * etc
daemon/libvirtd.c | 8 +--- examples/object-events/event-test.c | 9 ++--- src/bhyve/bhyve_driver.c | 3 +- src/libvirt.c | 3 +- src/libxl/libxl_domain.c | 3 +- src/libxl/libxl_driver.c | 3 +- src/locking/lock_daemon.c | 8 +--- src/logging/log_daemon.c | 8 +--- src/lxc/lxc_container.c | 8 +--- src/lxc/lxc_controller.c | 8 +--- src/lxc/lxc_domain.c | 3 +- src/lxc/lxc_process.c | 6 +-- src/network/bridge_driver.c | 3 +- src/node_device/node_device_hal.c | 3 +- src/rpc/virnettlscontext.c | 3 +- src/secret/secret_driver.c | 6 +-- src/storage/storage_driver.c | 16 ++------ src/uml/uml_driver.c | 3 +- src/util/iohelper.c | 10 +---- src/util/virhook.c | 3 +- src/util/virhostdev.c | 20 ++++----- tests/commandtest.c | 81 +++++++++++++------------------------ tests/libvirtdconftest.c | 3 +- tests/openvzutilstest.c | 7 +--- tests/securityselinuxlabeltest.c | 9 ++--- tests/securityselinuxtest.c | 6 +-- tests/virnettlscontexttest.c | 3 +- 27 files changed, 77 insertions(+), 169 deletions(-)
There's still at least one build error 'error: unused variable 'err'' in here. Please review the code carefully, or only send the patch targetting the drivers that you can actually build test.
It's strange that it could be build on my machine and `make syntax-check` also passed.
Thanks, Cole
On 04/10/2016 10:40 PM, 惠轶群 wrote:
2016-04-08 23:12 GMT+08:00 Cole Robinson <crobinso@redhat.com>:
When sending a v2, please give a description of what changed compared to the previous patch version, after the --- break so it doesn't show up in the commit message. For example:
Nice proposal. Thanks.
On 04/04/2016 06:49 AM, Hui Yiqun wrote:
getting err using virGetLastError() and then retrieving message from err asks developers to test the value of err and err->message and default to self-defined unkown error message.
It's better to avoid it and use uniform virGetLastErrorMessage ---
v2: * Fixed the FOO * Replaces the BAZ * etc
daemon/libvirtd.c | 8 +--- examples/object-events/event-test.c | 9 ++--- src/bhyve/bhyve_driver.c | 3 +- src/libvirt.c | 3 +- src/libxl/libxl_domain.c | 3 +- src/libxl/libxl_driver.c | 3 +- src/locking/lock_daemon.c | 8 +--- src/logging/log_daemon.c | 8 +--- src/lxc/lxc_container.c | 8 +--- src/lxc/lxc_controller.c | 8 +--- src/lxc/lxc_domain.c | 3 +- src/lxc/lxc_process.c | 6 +-- src/network/bridge_driver.c | 3 +- src/node_device/node_device_hal.c | 3 +- src/rpc/virnettlscontext.c | 3 +- src/secret/secret_driver.c | 6 +-- src/storage/storage_driver.c | 16 ++------ src/uml/uml_driver.c | 3 +- src/util/iohelper.c | 10 +---- src/util/virhook.c | 3 +- src/util/virhostdev.c | 20 ++++----- tests/commandtest.c | 81 +++++++++++++------------------------ tests/libvirtdconftest.c | 3 +- tests/openvzutilstest.c | 7 +--- tests/securityselinuxlabeltest.c | 9 ++--- tests/securityselinuxtest.c | 6 +-- tests/virnettlscontexttest.c | 3 +- 27 files changed, 77 insertions(+), 169 deletions(-)
There's still at least one build error 'error: unused variable 'err'' in here. Please review the code carefully, or only send the patch targetting the drivers that you can actually build test.
It's strange that it could be build on my machine and `make syntax-check` also passed.
Make sure you configured with --enable-werror, otherwise it might just be a non-fatal warning that is lost in the compiler output - Cole
participants (3)
-
Cole Robinson -
Hui Yiqun -
惠轶群