Bug https://bugzilla.redhat.com/show_bug.cgi?id=689374 reported libvirtd crash during error dispatch. The reason is that libvirtd uses remoteDispatchConnError() with non-NULL conn parameter which means that virConnGetLastError() is used instead of its thread safe replacement virGetLastError(). So when several libvirtd threads are reporting errors at the same time, the errors can get mixed or corrupted or in case of bad luck libvirtd itself crashes. Since Daniel B. is going to rewrite this code from scratch on top of his RPC infrastructure, I tried to come up with a minimal fix. Thus, remoteDispatchConnError() now just ignores its conn argument and always calls virGetLastError(). However, several callers had to be touched as well, since no libvirt API is allowed to be called before dispatching the error. Doing so would reset the error and we would have nothing to dispatch. As a result of that, the code is not very nice but that doesn't really make daemon/remote.c worse than it is now :-) And it will all die soon, which is good. The bug report also contains a reproducer in C which detects both mixed up error messages and libvirtd crash. Before this patch, I was able to crash libvirtd in about 20 seconds up to 3 minutes depending on number of CPU cores (more is better) and luck. --- daemon/dispatch.c | 8 +-- daemon/remote.c | 238 +++++++++++++++++++++++++++------------------------- 2 files changed, 126 insertions(+), 120 deletions(-) diff --git a/daemon/dispatch.c b/daemon/dispatch.c index dc3b48a..4814017 100644 --- a/daemon/dispatch.c +++ b/daemon/dispatch.c @@ -113,14 +113,10 @@ void remoteDispatchOOMError (remote_error *rerr) void remoteDispatchConnError (remote_error *rerr, - virConnectPtr conn) + virConnectPtr conn ATTRIBUTE_UNUSED) { - virErrorPtr verr; + virErrorPtr verr = virGetLastError(); - if (conn) - verr = virConnGetLastError(conn); - else - verr = virGetLastError(); if (verr) remoteDispatchCopyError(rerr, verr); else diff --git a/daemon/remote.c b/daemon/remote.c index a8fef4d..4b42ed2 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -757,8 +757,8 @@ remoteDispatchDomainGetSchedulerType (struct qemud_server *server ATTRIBUTE_UNUS type = virDomainGetSchedulerType (dom, &nparams); if (type == NULL) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } @@ -801,9 +801,9 @@ remoteDispatchDomainGetSchedulerParameters (struct qemud_server *server ATTRIBUT r = virDomainGetSchedulerParameters (dom, params, &nparams); if (r == -1) { + remoteDispatchConnError(rerr, conn); virDomainFree(dom); VIR_FREE(params); - remoteDispatchConnError(rerr, conn); return -1; } @@ -908,12 +908,13 @@ remoteDispatchDomainSetSchedulerParameters (struct qemud_server *server ATTRIBUT } r = virDomainSetSchedulerParameters (dom, params, nparams); - virDomainFree(dom); VIR_FREE(params); if (r == -1) { remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } + virDomainFree(dom); return 0; } @@ -939,8 +940,8 @@ remoteDispatchDomainBlockStats (struct qemud_server *server ATTRIBUTE_UNUSED, path = args->path; if (virDomainBlockStats (dom, path, &stats, sizeof stats) == -1) { - virDomainFree (dom); remoteDispatchConnError(rerr, conn); + virDomainFree (dom); return -1; } virDomainFree (dom); @@ -975,8 +976,8 @@ remoteDispatchDomainInterfaceStats (struct qemud_server *server ATTRIBUTE_UNUSED path = args->path; if (virDomainInterfaceStats (dom, path, &stats, sizeof stats) == -1) { - virDomainFree (dom); remoteDispatchConnError(rerr, conn); + virDomainFree (dom); return -1; } virDomainFree (dom); @@ -1026,12 +1027,13 @@ remoteDispatchDomainMemoryStats (struct qemud_server *server ATTRIBUTE_UNUSED, } nr_stats = virDomainMemoryStats (dom, stats, args->maxStats, 0); - virDomainFree (dom); if (nr_stats == -1) { VIR_FREE(stats); remoteDispatchConnError(rerr, conn); + virDomainFree (dom); return -1; } + virDomainFree (dom); /* Allocate return buffer */ if (VIR_ALLOC_N(ret->stats.stats_val, args->maxStats) < 0) { @@ -1092,8 +1094,8 @@ remoteDispatchDomainBlockPeek (struct qemud_server *server ATTRIBUTE_UNUSED, if (virDomainBlockPeek (dom, path, offset, size, ret->buffer.buffer_val, flags) == -1) { /* free (ret->buffer.buffer_val); - caller frees */ - virDomainFree (dom); remoteDispatchConnError(rerr, conn); + virDomainFree (dom); return -1; } virDomainFree (dom); @@ -1141,8 +1143,8 @@ remoteDispatchDomainMemoryPeek (struct qemud_server *server ATTRIBUTE_UNUSED, if (virDomainMemoryPeek (dom, offset, size, ret->buffer.buffer_val, flags) == -1) { /* free (ret->buffer.buffer_val); - caller frees */ - virDomainFree (dom); remoteDispatchConnError(rerr, conn); + virDomainFree (dom); return -1; } virDomainFree (dom); @@ -1168,8 +1170,8 @@ remoteDispatchDomainAttachDevice (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainAttachDevice (dom, args->xml) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -1194,8 +1196,8 @@ remoteDispatchDomainAttachDeviceFlags (struct qemud_server *server ATTRIBUTE_UNU } if (virDomainAttachDeviceFlags (dom, args->xml, args->flags) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -1220,8 +1222,8 @@ remoteDispatchDomainUpdateDeviceFlags (struct qemud_server *server ATTRIBUTE_UNU } if (virDomainUpdateDeviceFlags (dom, args->xml, args->flags) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -1246,8 +1248,8 @@ remoteDispatchDomainCreate (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainCreate (dom) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -1272,8 +1274,8 @@ remoteDispatchDomainCreateWithFlags (struct qemud_server *server ATTRIBUTE_UNUSE } if (virDomainCreateWithFlags (dom, args->flags) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } @@ -1346,8 +1348,8 @@ remoteDispatchDomainDestroy (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainDestroy (dom) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -1372,8 +1374,8 @@ remoteDispatchDomainDetachDevice (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainDetachDevice (dom, args->xml) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } @@ -1399,8 +1401,8 @@ remoteDispatchDomainDetachDeviceFlags (struct qemud_server *server ATTRIBUTE_UNU } if (virDomainDetachDeviceFlags (dom, args->xml, args->flags) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } @@ -1428,8 +1430,8 @@ remoteDispatchDomainDumpXml (struct qemud_server *server ATTRIBUTE_UNUSED, /* remoteDispatchClientRequest will free this. */ ret->xml = virDomainGetXMLDesc (dom, args->flags); if (!ret->xml) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -1497,8 +1499,8 @@ remoteDispatchDomainGetAutostart (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainGetAutostart (dom, &ret->autostart) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -1524,8 +1526,8 @@ remoteDispatchDomainGetInfo (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainGetInfo (dom, &info) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } @@ -1559,8 +1561,8 @@ remoteDispatchDomainGetMaxMemory (struct qemud_server *server ATTRIBUTE_UNUSED, ret->memory = virDomainGetMaxMemory (dom); if (ret->memory == 0) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -1586,8 +1588,8 @@ remoteDispatchDomainGetMaxVcpus (struct qemud_server *server ATTRIBUTE_UNUSED, ret->num = virDomainGetMaxVcpus (dom); if (ret->num == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -1614,8 +1616,8 @@ remoteDispatchDomainGetSecurityLabel(struct qemud_server *server ATTRIBUTE_UNUSE memset(&seclabel, 0, sizeof seclabel); if (virDomainGetSecurityLabel(dom, &seclabel) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } @@ -1686,8 +1688,8 @@ remoteDispatchDomainGetOsType (struct qemud_server *server ATTRIBUTE_UNUSED, /* remoteDispatchClientRequest will free this */ ret->type = virDomainGetOSType (dom); if (ret->type == NULL) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -1737,10 +1739,10 @@ remoteDispatchDomainGetVcpus (struct qemud_server *server ATTRIBUTE_UNUSED, info, args->maxinfo, cpumaps, args->maplen); if (info_len == -1) { + remoteDispatchConnError(rerr, conn); VIR_FREE(info); VIR_FREE(cpumaps); virDomainFree(dom); - remoteDispatchConnError(rerr, conn); return -1; } @@ -1794,8 +1796,8 @@ remoteDispatchDomainGetVcpusFlags (struct qemud_server *server ATTRIBUTE_UNUSED, ret->num = virDomainGetVcpusFlags (dom, args->flags); if (ret->num == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -1877,11 +1879,12 @@ remoteDispatchDomainMigratePerform (struct qemud_server *server ATTRIBUTE_UNUSED args->cookie.cookie_len, args->uri, args->flags, dname, args->resource); - virDomainFree (dom); if (r == -1) { remoteDispatchConnError(rerr, conn); + virDomainFree (dom); return -1; } + virDomainFree (dom); return 0; } @@ -2013,8 +2016,8 @@ remoteDispatchDomainMigratePrepareTunnel(struct qemud_server *server ATTRIBUTE_U args->flags, dname, args->resource, args->dom_xml); if (r == -1) { - remoteFreeClientStream(client, stream); remoteDispatchConnError(rerr, conn); + remoteFreeClientStream(client, stream); return -1; } @@ -2175,8 +2178,8 @@ remoteDispatchDomainPinVcpu (struct qemud_server *server ATTRIBUTE_UNUSED, (unsigned char *) args->cpumap.cpumap_val, args->cpumap.cpumap_len); if (rv == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -2201,8 +2204,8 @@ remoteDispatchDomainReboot (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainReboot (dom, args->flags) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -2245,8 +2248,8 @@ remoteDispatchDomainResume (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainResume (dom) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -2271,8 +2274,8 @@ remoteDispatchDomainSave (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainSave (dom, args->to) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -2297,8 +2300,8 @@ remoteDispatchDomainCoreDump (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainCoreDump (dom, args->to, args->flags) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -2323,8 +2326,8 @@ remoteDispatchDomainSetAutostart (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainSetAutostart (dom, args->autostart) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -2349,8 +2352,8 @@ remoteDispatchDomainSetMaxMemory (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainSetMaxMemory (dom, args->memory) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -2375,8 +2378,8 @@ remoteDispatchDomainSetMemory (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainSetMemory (dom, args->memory) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -2401,8 +2404,8 @@ remoteDispatchDomainSetMemoryFlags (struct qemud_server *server ATTRIBUTE_UNUSED } if (virDomainSetMemoryFlags (dom, args->memory, args->flags) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -2491,12 +2494,13 @@ remoteDispatchDomainSetMemoryParameters(struct qemud_server *server } r = virDomainSetMemoryParameters(dom, params, nparams, flags); - virDomainFree(dom); VIR_FREE(params); if (r == -1) { remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } + virDomainFree(dom); return 0; } @@ -2541,9 +2545,9 @@ remoteDispatchDomainGetMemoryParameters(struct qemud_server *server r = virDomainGetMemoryParameters(dom, params, &nparams, flags); if (r == -1) { + remoteDispatchConnError(rerr, conn); virDomainFree(dom); VIR_FREE(params); - remoteDispatchConnError(rerr, conn); return -1; } /* In this case, we need to send back the number of parameters @@ -2701,12 +2705,13 @@ remoteDispatchDomainSetBlkioParameters(struct qemud_server *server } r = virDomainSetBlkioParameters(dom, params, nparams, flags); - virDomainFree(dom); VIR_FREE(params); if (r == -1) { remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } + virDomainFree(dom); return 0; } @@ -2751,9 +2756,9 @@ remoteDispatchDomainGetBlkioParameters(struct qemud_server *server r = virDomainGetBlkioParameters(dom, params, &nparams, flags); if (r == -1) { + remoteDispatchConnError(rerr, conn); virDomainFree(dom); VIR_FREE(params); - remoteDispatchConnError(rerr, conn); return -1; } /* In this case, we need to send back the number of parameters @@ -2847,8 +2852,8 @@ remoteDispatchDomainSetVcpus (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainSetVcpus (dom, args->nvcpus) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -2873,8 +2878,8 @@ remoteDispatchDomainSetVcpusFlags (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainSetVcpusFlags (dom, args->nvcpus, args->flags) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -2899,8 +2904,8 @@ remoteDispatchDomainShutdown (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainShutdown (dom) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -2925,8 +2930,8 @@ remoteDispatchDomainSuspend (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainSuspend (dom) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -2951,8 +2956,8 @@ remoteDispatchDomainUndefine (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainUndefine (dom) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -3044,8 +3049,8 @@ remoteDispatchDomainManagedSave (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainManagedSave (dom, args->flags) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -3071,8 +3076,8 @@ remoteDispatchDomainHasManagedSaveImage (struct qemud_server *server ATTRIBUTE_U ret->ret = virDomainHasManagedSaveImage (dom, args->flags); if (ret->ret == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -3097,8 +3102,8 @@ remoteDispatchDomainManagedSaveRemove (struct qemud_server *server ATTRIBUTE_UNU } if (virDomainManagedSaveRemove (dom, args->flags) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } virDomainFree(dom); @@ -3157,8 +3162,8 @@ remoteDispatchNetworkCreate (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virNetworkCreate (net) == -1) { - virNetworkFree(net); remoteDispatchConnError(rerr, conn); + virNetworkFree(net); return -1; } virNetworkFree(net); @@ -3227,8 +3232,8 @@ remoteDispatchNetworkDestroy (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virNetworkDestroy (net) == -1) { - virNetworkFree(net); remoteDispatchConnError(rerr, conn); + virNetworkFree(net); return -1; } virNetworkFree(net); @@ -3255,8 +3260,8 @@ remoteDispatchNetworkDumpXml (struct qemud_server *server ATTRIBUTE_UNUSED, /* remoteDispatchClientRequest will free this. */ ret->xml = virNetworkGetXMLDesc (net, args->flags); if (!ret->xml) { - virNetworkFree(net); remoteDispatchConnError(rerr, conn); + virNetworkFree(net); return -1; } virNetworkFree(net); @@ -3281,8 +3286,8 @@ remoteDispatchNetworkGetAutostart (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virNetworkGetAutostart (net, &ret->autostart) == -1) { - virNetworkFree(net); remoteDispatchConnError(rerr, conn); + virNetworkFree(net); return -1; } virNetworkFree(net); @@ -3309,8 +3314,8 @@ remoteDispatchNetworkGetBridgeName (struct qemud_server *server ATTRIBUTE_UNUSED /* remoteDispatchClientRequest will free this. */ ret->name = virNetworkGetBridgeName (net); if (!ret->name) { - virNetworkFree(net); remoteDispatchConnError(rerr, conn); + virNetworkFree(net); return -1; } virNetworkFree(net); @@ -3379,8 +3384,8 @@ remoteDispatchNetworkSetAutostart (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virNetworkSetAutostart (net, args->autostart) == -1) { - virNetworkFree(net); remoteDispatchConnError(rerr, conn); + virNetworkFree(net); return -1; } virNetworkFree(net); @@ -3405,8 +3410,8 @@ remoteDispatchNetworkUndefine (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virNetworkUndefine (net) == -1) { - virNetworkFree(net); remoteDispatchConnError(rerr, conn); + virNetworkFree(net); return -1; } virNetworkFree(net); @@ -3642,8 +3647,8 @@ remoteDispatchInterfaceGetXmlDesc (struct qemud_server *server ATTRIBUTE_UNUSED, /* remoteDispatchClientRequest will free this. */ ret->xml = virInterfaceGetXMLDesc (iface, args->flags); if (!ret->xml) { - virInterfaceFree(iface); remoteDispatchConnError(rerr, conn); + virInterfaceFree(iface); return -1; } virInterfaceFree(iface); @@ -3690,8 +3695,8 @@ remoteDispatchInterfaceUndefine (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virInterfaceUndefine (iface) == -1) { - virInterfaceFree(iface); remoteDispatchConnError(rerr, conn); + virInterfaceFree(iface); return -1; } virInterfaceFree(iface); @@ -3716,8 +3721,8 @@ remoteDispatchInterfaceCreate (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virInterfaceCreate (iface, args->flags) == -1) { - virInterfaceFree(iface); remoteDispatchConnError(rerr, conn); + virInterfaceFree(iface); return -1; } virInterfaceFree(iface); @@ -3742,8 +3747,8 @@ remoteDispatchInterfaceDestroy (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virInterfaceDestroy (iface, args->flags) == -1) { - virInterfaceFree(iface); remoteDispatchConnError(rerr, conn); + virInterfaceFree(iface); return -1; } virInterfaceFree(iface); @@ -4656,8 +4661,8 @@ remoteDispatchStoragePoolCreate (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virStoragePoolCreate (pool, args->flags) == -1) { - virStoragePoolFree(pool); remoteDispatchConnError(rerr, conn); + virStoragePoolFree(pool); return -1; } virStoragePoolFree(pool); @@ -4726,8 +4731,8 @@ remoteDispatchStoragePoolBuild (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virStoragePoolBuild (pool, args->flags) == -1) { - virStoragePoolFree(pool); remoteDispatchConnError(rerr, conn); + virStoragePoolFree(pool); return -1; } virStoragePoolFree(pool); @@ -4753,8 +4758,8 @@ remoteDispatchStoragePoolDestroy (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virStoragePoolDestroy (pool) == -1) { - virStoragePoolFree(pool); remoteDispatchConnError(rerr, conn); + virStoragePoolFree(pool); return -1; } virStoragePoolFree(pool); @@ -4779,8 +4784,8 @@ remoteDispatchStoragePoolDelete (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virStoragePoolDelete (pool, args->flags) == -1) { - virStoragePoolFree(pool); remoteDispatchConnError(rerr, conn); + virStoragePoolFree(pool); return -1; } virStoragePoolFree(pool); @@ -4805,8 +4810,8 @@ remoteDispatchStoragePoolRefresh (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virStoragePoolRefresh (pool, args->flags) == -1) { - virStoragePoolFree(pool); remoteDispatchConnError(rerr, conn); + virStoragePoolFree(pool); return -1; } virStoragePoolFree(pool); @@ -4832,8 +4837,8 @@ remoteDispatchStoragePoolGetInfo (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virStoragePoolGetInfo (pool, &info) == -1) { - virStoragePoolFree(pool); remoteDispatchConnError(rerr, conn); + virStoragePoolFree(pool); return -1; } @@ -4867,8 +4872,8 @@ remoteDispatchStoragePoolDumpXml (struct qemud_server *server ATTRIBUTE_UNUSED, /* remoteDispatchClientRequest will free this. */ ret->xml = virStoragePoolGetXMLDesc (pool, args->flags); if (!ret->xml) { - virStoragePoolFree(pool); remoteDispatchConnError(rerr, conn); + virStoragePoolFree(pool); return -1; } virStoragePoolFree(pool); @@ -4893,8 +4898,8 @@ remoteDispatchStoragePoolGetAutostart (struct qemud_server *server ATTRIBUTE_UNU } if (virStoragePoolGetAutostart (pool, &ret->autostart) == -1) { - virStoragePoolFree(pool); remoteDispatchConnError(rerr, conn); + virStoragePoolFree(pool); return -1; } virStoragePoolFree(pool); @@ -4965,11 +4970,12 @@ remoteDispatchStoragePoolLookupByVolume (struct qemud_server *server ATTRIBUTE_U } pool = virStoragePoolLookupByVolume (vol); - virStorageVolFree(vol); if (pool == NULL) { remoteDispatchConnError(rerr, conn); + virStorageVolFree(vol); return -1; } + virStorageVolFree(vol); make_nonnull_storage_pool (&ret->pool, pool); virStoragePoolFree(pool); @@ -4994,8 +5000,8 @@ remoteDispatchStoragePoolSetAutostart (struct qemud_server *server ATTRIBUTE_UNU } if (virStoragePoolSetAutostart (pool, args->autostart) == -1) { - virStoragePoolFree(pool); remoteDispatchConnError(rerr, conn); + virStoragePoolFree(pool); return -1; } virStoragePoolFree(pool); @@ -5020,8 +5026,8 @@ remoteDispatchStoragePoolUndefine (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virStoragePoolUndefine (pool) == -1) { - virStoragePoolFree(pool); remoteDispatchConnError(rerr, conn); + virStoragePoolFree(pool); return -1; } virStoragePoolFree(pool); @@ -5163,11 +5169,12 @@ remoteDispatchStorageVolCreateXml (struct qemud_server *server ATTRIBUTE_UNUSED, } vol = virStorageVolCreateXML (pool, args->xml, args->flags); - virStoragePoolFree(pool); if (vol == NULL) { remoteDispatchConnError(rerr, conn); + virStoragePoolFree(pool); return -1; } + virStoragePoolFree(pool); make_nonnull_storage_vol (&ret->vol, vol); virStorageVolFree(vol); @@ -5194,19 +5201,21 @@ remoteDispatchStorageVolCreateXmlFrom (struct qemud_server *server ATTRIBUTE_UNU clonevol = get_nonnull_storage_vol (conn, args->clonevol); if (clonevol == NULL) { - virStoragePoolFree(pool); remoteDispatchConnError(rerr, conn); + virStoragePoolFree(pool); return -1; } newvol = virStorageVolCreateXMLFrom (pool, args->xml, clonevol, args->flags); - virStorageVolFree(clonevol); - virStoragePoolFree(pool); if (newvol == NULL) { remoteDispatchConnError(rerr, conn); + virStorageVolFree(clonevol); + virStoragePoolFree(pool); return -1; } + virStorageVolFree(clonevol); + virStoragePoolFree(pool); make_nonnull_storage_vol (&ret->vol, newvol); virStorageVolFree(newvol); @@ -5231,8 +5240,8 @@ remoteDispatchStorageVolDelete (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virStorageVolDelete (vol, args->flags) == -1) { - virStorageVolFree(vol); remoteDispatchConnError(rerr, conn); + virStorageVolFree(vol); return -1; } virStorageVolFree(vol); @@ -5290,8 +5299,8 @@ remoteDispatchStorageVolGetInfo (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virStorageVolGetInfo (vol, &info) == -1) { - virStorageVolFree(vol); remoteDispatchConnError(rerr, conn); + virStorageVolFree(vol); return -1; } @@ -5324,8 +5333,8 @@ remoteDispatchStorageVolDumpXml (struct qemud_server *server ATTRIBUTE_UNUSED, /* remoteDispatchClientRequest will free this. */ ret->xml = virStorageVolGetXMLDesc (vol, args->flags); if (!ret->xml) { - virStorageVolFree(vol); remoteDispatchConnError(rerr, conn); + virStorageVolFree(vol); return -1; } virStorageVolFree(vol); @@ -5353,8 +5362,8 @@ remoteDispatchStorageVolGetPath (struct qemud_server *server ATTRIBUTE_UNUSED, /* remoteDispatchClientRequest will free this. */ ret->name = virStorageVolGetPath (vol); if (!ret->name) { - virStorageVolFree(vol); remoteDispatchConnError(rerr, conn); + virStorageVolFree(vol); return -1; } virStorageVolFree(vol); @@ -5381,11 +5390,12 @@ remoteDispatchStorageVolLookupByName (struct qemud_server *server ATTRIBUTE_UNUS } vol = virStorageVolLookupByName (pool, args->name); - virStoragePoolFree(pool); if (vol == NULL) { remoteDispatchConnError(rerr, conn); + virStoragePoolFree(pool); return -1; } + virStoragePoolFree(pool); make_nonnull_storage_vol (&ret->vol, vol); virStorageVolFree(vol); @@ -5622,8 +5632,8 @@ remoteDispatchNodeDeviceNumOfCaps (struct qemud_server *server ATTRIBUTE_UNUSED, ret->num = virNodeDeviceNumOfCaps(dev); if (ret->num < 0) { - virNodeDeviceFree(dev); remoteDispatchConnError(rerr, conn); + virNodeDeviceFree(dev); return -1; } @@ -5668,8 +5678,8 @@ remoteDispatchNodeDeviceListCaps (struct qemud_server *server ATTRIBUTE_UNUSED, virNodeDeviceListCaps (dev, ret->names.names_val, args->maxnames); if (ret->names.names_len == -1) { - virNodeDeviceFree(dev); remoteDispatchConnError(rerr, conn); + virNodeDeviceFree(dev); VIR_FREE(ret->names.names_val); return -1; } @@ -5698,8 +5708,8 @@ remoteDispatchNodeDeviceDettach (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virNodeDeviceDettach(dev) == -1) { - virNodeDeviceFree(dev); remoteDispatchConnError(rerr, conn); + virNodeDeviceFree(dev); return -1; } @@ -5727,8 +5737,8 @@ remoteDispatchNodeDeviceReAttach (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virNodeDeviceReAttach(dev) == -1) { - virNodeDeviceFree(dev); remoteDispatchConnError(rerr, conn); + virNodeDeviceFree(dev); return -1; } @@ -5756,8 +5766,8 @@ remoteDispatchNodeDeviceReset (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virNodeDeviceReset(dev) == -1) { - virNodeDeviceFree(dev); remoteDispatchConnError(rerr, conn); + virNodeDeviceFree(dev); return -1; } @@ -5808,8 +5818,8 @@ remoteDispatchNodeDeviceDestroy(struct qemud_server *server ATTRIBUTE_UNUSED, } if (virNodeDeviceDestroy(dev) == -1) { - virNodeDeviceFree(dev); remoteDispatchConnError(rerr, conn); + virNodeDeviceFree(dev); return -1; } @@ -6189,8 +6199,8 @@ static int remoteDispatchDomainIsActive(struct qemud_server *server ATTRIBUTE_UN ret->active = virDomainIsActive(domain); if (ret->active < 0) { - virDomainFree(domain); remoteDispatchConnError(err, conn); + virDomainFree(domain); return -1; } @@ -6217,8 +6227,8 @@ static int remoteDispatchDomainIsPersistent(struct qemud_server *server ATTRIBUT ret->persistent = virDomainIsPersistent(domain); if (ret->persistent < 0) { - virDomainFree(domain); remoteDispatchConnError(err, conn); + virDomainFree(domain); return -1; } @@ -6245,8 +6255,8 @@ static int remoteDispatchDomainIsUpdated(struct qemud_server *server ATTRIBUTE_U ret->updated = virDomainIsUpdated(domain); if (ret->updated < 0) { - virDomainFree(domain); remoteDispatchConnError(err, conn); + virDomainFree(domain); return -1; } @@ -6273,8 +6283,8 @@ static int remoteDispatchInterfaceIsActive(struct qemud_server *server ATTRIBUTE ret->active = virInterfaceIsActive(iface); if (ret->active < 0) { - virInterfaceFree(iface); remoteDispatchConnError(err, conn); + virInterfaceFree(iface); return -1; } @@ -6301,8 +6311,8 @@ static int remoteDispatchNetworkIsActive(struct qemud_server *server ATTRIBUTE_U ret->active = virNetworkIsActive(network); if (ret->active < 0) { - virNetworkFree(network); remoteDispatchConnError(err, conn); + virNetworkFree(network); return -1; } @@ -6329,8 +6339,8 @@ static int remoteDispatchNetworkIsPersistent(struct qemud_server *server ATTRIBU ret->persistent = virNetworkIsPersistent(network); if (ret->persistent < 0) { - virNetworkFree(network); remoteDispatchConnError(err, conn); + virNetworkFree(network); return -1; } @@ -6357,8 +6367,8 @@ static int remoteDispatchStoragePoolIsActive(struct qemud_server *server ATTRIBU ret->active = virStoragePoolIsActive(pool); if (ret->active < 0) { - virStoragePoolFree(pool); remoteDispatchConnError(err, conn); + virStoragePoolFree(pool); return -1; } @@ -6385,8 +6395,8 @@ static int remoteDispatchStoragePoolIsPersistent(struct qemud_server *server ATT ret->persistent = virStoragePoolIsPersistent(pool); if (ret->persistent < 0) { - virStoragePoolFree(pool); remoteDispatchConnError(err, conn); + virStoragePoolFree(pool); return -1; } @@ -6481,8 +6491,8 @@ remoteDispatchDomainGetJobInfo (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainGetJobInfo (dom, &info) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } @@ -6523,8 +6533,8 @@ remoteDispatchDomainAbortJob (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainAbortJob (dom) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } @@ -6552,8 +6562,8 @@ remoteDispatchDomainMigrateSetMaxDowntime(struct qemud_server *server ATTRIBUTE_ } if (virDomainMigrateSetMaxDowntime(dom, args->downtime, args->flags) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } @@ -6610,8 +6620,8 @@ remoteDispatchDomainSnapshotCreateXml (struct qemud_server *server ATTRIBUTE_UNU snapshot = virDomainSnapshotCreateXML(domain, args->xml_desc, args->flags); if (snapshot == NULL) { - virDomainFree(domain); remoteDispatchConnError(rerr, conn); + virDomainFree(domain); return -1; } @@ -6652,12 +6662,12 @@ remoteDispatchDomainSnapshotDumpXml (struct qemud_server *server ATTRIBUTE_UNUSE rc = 0; cleanup: + if (rc < 0) + remoteDispatchConnError(rerr, conn); if (snapshot) virDomainSnapshotFree(snapshot); if (domain) virDomainFree(domain); - if (rc < 0) - remoteDispatchConnError(rerr, conn); return rc; } @@ -6681,8 +6691,8 @@ remoteDispatchDomainSnapshotNum (struct qemud_server *server ATTRIBUTE_UNUSED, ret->num = virDomainSnapshotNum(domain, args->flags); if (ret->num == -1) { - virDomainFree(domain); remoteDispatchConnError(rerr, conn); + virDomainFree(domain); return -1; } @@ -6726,9 +6736,9 @@ remoteDispatchDomainSnapshotListNames (struct qemud_server *server ATTRIBUTE_UNU args->nameslen, args->flags); if (ret->names.names_len == -1) { + remoteDispatchConnError(rerr, conn); virDomainFree(domain); VIR_FREE(ret->names.names_val); - remoteDispatchConnError(rerr, conn); return -1; } @@ -6757,8 +6767,8 @@ remoteDispatchDomainSnapshotLookupByName (struct qemud_server *server ATTRIBUTE_ snapshot = virDomainSnapshotLookupByName(domain, args->name, args->flags); if (snapshot == NULL) { - virDomainFree(domain); remoteDispatchConnError(rerr, conn); + virDomainFree(domain); return -1; } @@ -6790,8 +6800,8 @@ remoteDispatchDomainHasCurrentSnapshot(struct qemud_server *server ATTRIBUTE_UNU result = virDomainHasCurrentSnapshot(domain, args->flags); if (result < 0) { - virDomainFree(domain); remoteDispatchConnError(rerr, conn); + virDomainFree(domain); return -1; } @@ -6822,8 +6832,8 @@ remoteDispatchDomainSnapshotCurrent(struct qemud_server *server ATTRIBUTE_UNUSED snapshot = virDomainSnapshotCurrent(domain, args->flags); if (snapshot == NULL) { - virDomainFree(domain); remoteDispatchConnError(rerr, conn); + virDomainFree(domain); return -1; } @@ -6862,12 +6872,12 @@ remoteDispatchDomainRevertToSnapshot (struct qemud_server *server ATTRIBUTE_UNUS rc = 0; cleanup: + if (rc < 0) + remoteDispatchConnError(rerr, conn); if (snapshot) virDomainSnapshotFree(snapshot); if (domain) virDomainFree(domain); - if (rc < 0) - remoteDispatchConnError(rerr, conn); return rc; } @@ -6899,12 +6909,12 @@ remoteDispatchDomainSnapshotDelete (struct qemud_server *server ATTRIBUTE_UNUSED rc = 0; cleanup: + if (rc < 0) + remoteDispatchConnError(rerr, conn); if (snapshot) virDomainSnapshotFree(snapshot); if (domain) virDomainFree(domain); - if (rc < 0) - remoteDispatchConnError(rerr, conn); return rc; } @@ -7069,8 +7079,8 @@ remoteDispatchNwfilterUndefine (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virNWFilterUndefine (nwfilter) == -1) { - virNWFilterFree(nwfilter); remoteDispatchConnError(rerr, conn); + virNWFilterFree(nwfilter); return -1; } virNWFilterFree(nwfilter); @@ -7132,8 +7142,8 @@ remoteDispatchNwfilterGetXmlDesc (struct qemud_server *server ATTRIBUTE_UNUSED, /* remoteDispatchClientRequest will free this. */ ret->xml = virNWFilterGetXMLDesc (nwfilter, args->flags); if (!ret->xml) { - virNWFilterFree(nwfilter); remoteDispatchConnError(rerr, conn); + virNWFilterFree(nwfilter); return -1; } virNWFilterFree(nwfilter); @@ -7180,8 +7190,8 @@ remoteDispatchDomainGetBlockInfo (struct qemud_server *server ATTRIBUTE_UNUSED, } if (virDomainGetBlockInfo (dom, args->path, &info, args->flags) == -1) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); return -1; } @@ -7213,8 +7223,8 @@ qemuDispatchMonitorCommand (struct qemud_server *server ATTRIBUTE_UNUSED, if (virDomainQemuMonitorCommand(domain, args->cmd, &ret->result, args->flags) == -1) { - virDomainFree(domain); remoteDispatchConnError(rerr, conn); + virDomainFree(domain); return -1; } @@ -7257,15 +7267,15 @@ remoteDispatchDomainOpenConsole(struct qemud_server *server ATTRIBUTE_UNUSED, stream->st, args->flags); if (r == -1) { + remoteDispatchConnError(rerr, conn); virDomainFree(dom); remoteFreeClientStream(client, stream); - remoteDispatchConnError(rerr, conn); return -1; } if (remoteAddClientStream(client, stream, 1) < 0) { - virDomainFree(dom); remoteDispatchConnError(rerr, conn); + virDomainFree(dom); virStreamAbort(stream->st); remoteFreeClientStream(client, stream); return -1; -- 1.7.4.1