Re: [libvirt] [PATCH v1 01/37] viralloc: Report OOM error on failure

Similarly to VIR_STRDUP, we want the OOM error to be reported in VIR_ALLOC and friends. --- po/POTFILES.in | 1 + python/libvirt-override.c | 102 +++++++++---------- src/conf/domain_conf.c | 9 +- src/conf/network_conf.c | 34 ++----- src/conf/node_device_conf.c | 2 +- src/esx/esx_vi.c | 2 +- src/lxc/lxc_process.c | 4 +- src/security/security_manager.c | 4 +- src/security/security_selinux.c | 4 +- src/util/viralloc.c | 165 ++++++++++++++++++++++++++----- src/util/viralloc.h | 213 ++++++++++++++++++++++++++++++++-------- src/util/virbuffer.c | 8 +- src/util/vircgroup.c | 2 +- src/util/virerror.c | 4 +- src/util/virpci.c | 2 +- src/util/virthreadpthread.c | 2 +- tests/commandhelper.c | 1 + tests/networkxml2conftest.c | 2 + tests/test_conf.c | 2 +- tests/xmconfigtest.c | 2 + 20 files changed, 401 insertions(+), 164 deletions(-)

diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 5c5586d..6b6e77b 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 64fd581..d6799d2 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c

@@ -3995,11 +3978,8 @@ virNetworkDefUpdateDNSTxt(virNetworkDefPtr def, /* add to beginning/end of list */ if (VIR_INSERT_ELEMENT(dns->txts, command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST - ? 0 : dns->ntxts, dns->ntxts, txt) < 0) { - virReportOOMError(); + ? 0 : dns->ntxts, dns->ntxts, txt, true) < 0)

goto cleanup; - } - } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) { if (foundIdx == dns->ntxts) { diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 3209604..7406652 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1025,7 +1025,7 @@ virNodeDevCapPciDevIommuGroupParseXML(xmlXPathContextPtr ctxt, pciAddr->function = addr.function; if (VIR_APPEND_ELEMENT(data->pci_dev.iommuGroupDevices, data->pci_dev.nIommuGroupDevices, - pciAddr) < 0) { + pciAddr, false) < 0) {

virReportOOMError(); goto cleanup; }

diff --git a/src/security/security_manager.c b/src/security/security_manager.c index f7c5c2e..f5ea73a 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -487,10 +487,8 @@ int virSecurityManagerGenLabel(virSecurityManagerPtr mgr, /* The seclabel must be added to @vm prior calling domainGenSecurityLabel * which may require seclabel to be presented already */ if (generated && - VIR_APPEND_ELEMENT(vm->seclabels, vm->nseclabels, seclabel) < 0) { - virReportOOMError(); + VIR_APPEND_ELEMENT(vm->seclabels, vm->nseclabels, seclabel, true) < 0)

goto cleanup; - } if (sec_managers[i]->drv->domainGenSecurityLabel(sec_managers[i], vm) < 0) { if (VIR_DELETE_ELEMENT(vm->seclabels,

diff --git a/src/util/viralloc.c b/src/util/viralloc.c index 8d6a7e6..16afac7 100644 --- a/src/util/viralloc.c +++ b/src/util/viralloc.c

@@ -132,15 +150,28 @@ int virAlloc(void *ptrptr, size_t size) * @ptrptr: pointer to pointer for address of allocated memory * @size: number of bytes to allocate * @count: number of elements to allocate + * @report: whether to report OOM error, if there is one + * @domcode: error domain code + * @filename: caller's filename + * @funcname: caller's funcname + * @linenr: caller's line number * * Allocate an array of memory 'count' elements long, * each with 'size' bytes. Return the address of the * allocated memory in 'ptrptr'. The newly allocated - * memory is filled with zeros. + * memory is filled with zeros. In case of OOM error + * and @report is true, the error is reported.

* * Returns -1 on failure to allocate, zero on success

@@ -160,16 +194,29 @@ int virAllocN(void *ptrptr, size_t size, size_t count) * @ptrptr: pointer to pointer for address of allocated memory * @size: number of bytes to allocate * @count: number of elements in array + * @report: whether to report OOM error, if there is one + * @domcode: error domain code + * @filename: caller's filename + * @funcname: caller's funcname + * @linenr: caller's line number * * Resize the block of memory in 'ptrptr' to be an array of * 'count' elements, each 'size' bytes in length. Update 'ptrptr' * with the address of the newly allocated memory. On failure, * 'ptrptr' is not changed and still points to the original memory * block. Any newly allocated memory in 'ptrptr' is uninitialized. + * In case of OOM error and @report is true, the error is reported.

* * Returns -1 on failure to allocate, zero on success

@@ -194,24 +246,41 @@ int virReallocN(void *ptrptr, size_t size, size_t count) * @size: number of bytes per element * @countptr: pointer to number of elements in array * @add: number of elements to add + * @report: whether to report OOM error, if there is one + * @domcode: error domain code + * @filename: caller's filename + * @funcname: caller's funcname + * @linenr: caller's line number * * Resize the block of memory in 'ptrptr' to be an array of * '*countptr' + 'add' elements, each 'size' bytes in length. * Update 'ptrptr' and 'countptr' with the details of the newly * allocated memory. On failure, 'ptrptr' and 'countptr' are not * changed. Any newly allocated memory in 'ptrptr' is zero-filled. + * In case of OOM error and @report is true, the error is reported.

* * Returns -1 on failure to allocate, zero on success

@@ -226,22 +295,38 @@ int virExpandN(void *ptrptr, size_t size, size_t *countptr, size_t add) * @allocptr: pointer to number of elements allocated in array * @count: number of elements currently used in array * @add: minimum number of additional elements to support in array + * @report: whether to report OOM error, if there is one + * @domcode: error domain code + * @filename: caller's filename + * @funcname: caller's funcname + * @linenr: caller's line number * * If 'count' + 'add' is larger than '*allocptr', then resize the * block of memory in 'ptrptr' to be an array of at least 'count' + * 'add' elements, each 'size' bytes in length. Update 'ptrptr' and * 'allocptr' with the details of the newly allocated memory. On * failure, 'ptrptr' and 'allocptr' are not changed. Any newly - * allocated memory in 'ptrptr' is zero-filled. + * allocated memory in 'ptrptr' is zero-filled. In case of OOM error + * and @report is true, the error is reported.

@@ -301,7 +393,8 @@ void virShrinkN(void *ptrptr, size_t size, size_t *countptr, size_t toremove) * allocated memory in *ptrptr and the new size in *countptr. If * newelems is NULL, the new elements at ptrptr[at] are instead filled * with zero. at must be between [0,*countptr], except that -1 is - * treated the same as *countptr for convenience. + * treated the same as *countptr for convenience. In case of OOM error + * and @report is true, the error is reported.

@@ -405,11 +510,20 @@ virDeleteElementsN(void *ptrptr, size_t size, size_t at, * The caller of this type of API is expected to know the length of * the array that will be returned and allocate a suitable buffer to * contain the returned data. C99 refers to these variable length - * objects as structs containing flexible array members. + * objects as structs containing flexible array members. In case of + * OOM error and @report is true, the error is reported. *

diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 35d3a37..f0dba49 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h

-# define VIR_INSERT_ELEMENT(ptr, at, count, newelem) \ +# define VIR_INSERT_ELEMENT(ptr, at, count, newelem, report) \ virInsertElementsN(&(ptr), sizeof(*(ptr)), at, &(count), \ - VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), true, false) -# define VIR_INSERT_ELEMENT_COPY(ptr, at, count, newelem) \ + VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), true, false, \ + report, VIR_FROM_THIS, __FILE__, __FUNCTION__, __LINE__) +# define VIR_INSERT_ELEMENT_COPY(ptr, at, count, newelem, report) \ virInsertElementsN(&(ptr), sizeof(*(ptr)), at, &(count), \ - VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), false, false) -# define VIR_INSERT_ELEMENT_INPLACE(ptr, at, count, newelem) \ + VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), false, false, \ + report, VIR_FROM_THIS, __FILE__, __FUNCTION__, __LINE__) +# define VIR_INSERT_ELEMENT_INPLACE(ptr, at, count, newelem, report) \ virInsertElementsN(&(ptr), sizeof(*(ptr)), at, &(count), \ - VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), true, true) -# define VIR_INSERT_ELEMENT_COPY_INPLACE(ptr, at, count, newelem) \ + VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), true, true, \ + report, VIR_FROM_THIS, __FILE__, __FUNCTION__, __LINE__) +# define VIR_INSERT_ELEMENT_COPY_INPLACE(ptr, at, count, newelem, report) \ virInsertElementsN(&(ptr), sizeof(*(ptr)), at, &(count), \ - VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), false, true) + VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), false, true, \ + report, VIR_FROM_THIS, __FILE__, __FUNCTION__, __LINE__) /** * VIR_APPEND_ELEMENT: @@ -272,6 +379,7 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1); * @newelem: the new element to move into place (*not* a pointer to * the element, but the element itself). * (the original will be zeroed out if successful) + * @report: whether report OOM error on failure

* * Re-allocate an array of 'count' elements, each sizeof(*ptr) bytes * long, to be 'count' + 1 elements long, then copy the item from

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index e0b25ed..13fd162 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1171,7 +1171,7 @@ static int virCgroupPartitionEscape(char **path) if ((rc = virCgroupPartitionNeedsEscaping(*path)) <= 0) return rc; - if (VIR_INSERT_ELEMENT(*path, 0, len, escape) < 0) + if (VIR_INSERT_ELEMENT(*path, 0, len, escape, false) < 0)

return -ENOMEM; return 0;

diff --git a/src/util/virpci.c b/src/util/virpci.c index 54f7715..9ed257e 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2009,7 +2009,7 @@ virPCIGetIOMMUGroupAddressesAddOne(virPCIDeviceAddressPtr newDevAddr, void *opaq *copyAddr = *newDevAddr; if (VIR_APPEND_ELEMENT(*addrList->iommuGroupDevices, - *addrList->nIommuGroupDevices, copyAddr) < 0) { + *addrList->nIommuGroupDevices, copyAddr, false) < 0) { virReportOOMError();

goto cleanup; }