virBitmapCopy has a failure condition, which is impossible to meet when creating a new copy. Copy the contents directly to make it obvious that virBitmapNewCopy can't fail. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 74d1883d94..c62438a3a6 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -655,20 +655,14 @@ virBitmapParseUnlimited(const char *str) * virBitmapNewCopy: * @src: the source bitmap. * - * Makes a copy of bitmap @src. - * - * returns the copied bitmap on success, or NULL otherwise. Caller - * should call virBitmapFree to free the returned bitmap. + * Returns a copy of bitmap @src. */ virBitmapPtr virBitmapNewCopy(virBitmapPtr src) { virBitmapPtr dst = virBitmapNew(src->nbits); - if (virBitmapCopy(dst, src) != 0) { - virBitmapFree(dst); - return NULL; - } + memcpy(dst->map, src->map, src->map_len * sizeof(src->map[0])); return dst; } -- 2.26.2

The function will not fail any more. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/capabilities.c | 3 +-- src/conf/domain_conf.c | 7 +------ src/conf/numa_conf.c | 4 ++-- src/qemu/qemu_driver.c | 13 +++---------- src/qemu/qemu_hotplug.c | 3 +-- src/qemu/qemu_migration_cookie.c | 3 --- src/qemu/qemu_process.c | 3 +-- src/test/test_driver.c | 5 +---- src/util/virstoragefile.c | 5 ++--- 9 files changed, 12 insertions(+), 34 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 4a85c63628..da2633761d 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -1823,8 +1823,7 @@ virCapabilitiesInitResctrlMemory(virCapsPtr caps) if (virResctrlInfoGetMemoryBandwidth(caps->host.resctrl, bank->level, &node->control) > 0) { node->id = bank->id; - if (!(node->cpus = virBitmapNewCopy(bank->cpus))) - goto cleanup; + node->cpus = virBitmapNewCopy(bank->cpus); if (VIR_APPEND_ELEMENT(caps->host.memBW.nodes, caps->host.memBW.nnodes, node) < 0) { diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2fbfd949fd..f716338efd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -20928,12 +20928,7 @@ virDomainResctrlNew(xmlNodePtr node, if (VIR_ALLOC(resctrl) < 0) goto cleanup; - if (!(resctrl->vcpus = virBitmapNewCopy(vcpus))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("failed to copy 'vcpus'")); - goto cleanup; - } - + resctrl->vcpus = virBitmapNewCopy(vcpus); resctrl->alloc = virObjectRef(alloc); ret = g_steal_pointer(&resctrl); diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index b5ddf2c134..6653ba05a6 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -582,8 +582,8 @@ virDomainNumatuneSet(virDomainNumaPtr numa, if (nodeset) { virBitmapFree(numa->memory.nodeset); - if (!(numa->memory.nodeset = virBitmapNewCopy(nodeset))) - return -1; + numa->memory.nodeset = virBitmapNewCopy(nodeset); + if (placement == -1) placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c23dca9970..542eabf05d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4543,8 +4543,7 @@ qemuDomainPinVcpuLive(virDomainObjPtr vm, goto cleanup; } - if (!(tmpmap = virBitmapNewCopy(cpumap))) - goto cleanup; + tmpmap = virBitmapNewCopy(cpumap); if (!(str = virBitmapFormat(cpumap))) goto cleanup; @@ -4782,10 +4781,7 @@ qemuDomainPinEmulator(virDomainPtr dom, goto endjob; virBitmapFree(def->cputune.emulatorpin); - def->cputune.emulatorpin = NULL; - - if (!(def->cputune.emulatorpin = virBitmapNewCopy(pcpumap))) - goto endjob; + def->cputune.emulatorpin = virBitmapNewCopy(pcpumap); if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0) goto endjob; @@ -4802,10 +4798,7 @@ qemuDomainPinEmulator(virDomainPtr dom, if (persistentDef) { virBitmapFree(persistentDef->cputune.emulatorpin); - persistentDef->cputune.emulatorpin = NULL; - - if (!(persistentDef->cputune.emulatorpin = virBitmapNewCopy(pcpumap))) - goto endjob; + persistentDef->cputune.emulatorpin = virBitmapNewCopy(pcpumap); ret = virDomainDefSave(persistentDef, driver->xmlopt, cfg->configDir); goto endjob; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 86ccb9bac4..82845736b1 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -6424,8 +6424,7 @@ qemuDomainFilterHotplugVcpuEntities(virDomainDefPtr def, ssize_t next = -1; size_t i; - if (!(map = virBitmapNewCopy(vcpus))) - return NULL; + map = virBitmapNewCopy(vcpus); /* make sure that all selected vcpus are in the correct state */ while ((next = virBitmapNextSetBit(map, next)) >= 0) { diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 4b73a98f7e..afbb3526e9 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -568,9 +568,6 @@ qemuMigrationCookieAddCaps(qemuMigrationCookiePtr mig, mig->caps->automatic = qemuMigrationParamsGetAlwaysOnCaps(party); - if (!mig->caps->supported || !mig->caps->automatic) - return -1; - mig->flags |= QEMU_MIGRATION_COOKIE_CAPS; return 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 8814e61314..8d28d67692 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2562,8 +2562,7 @@ qemuProcessInitCpuAffinity(virDomainObjPtr vm) if (virNumaNodesetToCPUset(nodeset, &cpumapToSet) < 0) return -1; } else if (vm->def->cputune.emulatorpin) { - if (!(cpumapToSet = virBitmapNewCopy(vm->def->cputune.emulatorpin))) - return -1; + cpumapToSet = virBitmapNewCopy(vm->def->cputune.emulatorpin); } else { settingAll = true; if (qemuProcessGetAllCpuAffinity(&cpumapToSet) < 0) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index af8fd03308..7824e44ed9 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2743,10 +2743,7 @@ testDomainPinEmulator(virDomainPtr dom, } virBitmapFree(def->cputune.emulatorpin); - def->cputune.emulatorpin = NULL; - - if (!(def->cputune.emulatorpin = virBitmapNewCopy(pcpumap))) - goto cleanup; + def->cputune.emulatorpin = virBitmapNewCopy(pcpumap); ret = 0; cleanup: diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index a669b4ca99..459a7be5e4 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2422,9 +2422,8 @@ virStorageSourceCopy(const virStorageSource *src, !(def->srcpool = virStorageSourcePoolDefCopy(src->srcpool))) return NULL; - if (src->features && - !(def->features = virBitmapNewCopy(src->features))) - return NULL; + if (src->features) + def->features = virBitmapNewCopy(src->features); if (src->encryption && !(def->encryption = virStorageEncryptionCopy(src->encryption))) -- 2.26.2

There are only 3 places using the function. Two can use virBitmapNewCopy directly. In case of the qemu capabilities code we need to free the old bitmap first. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/capabilities.c | 3 +-- src/qemu/qemu_capabilities.c | 3 ++- src/util/virresctrl.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index da2633761d..0ba7323ee8 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -1589,8 +1589,7 @@ virCapabilitiesHostNUMAInitFake(virCapsHostNUMAPtr caps) cpus[cid].die_id = 0; cpus[cid].socket_id = s; cpus[cid].core_id = c; - cpus[cid].siblings = virBitmapNew(ncpus); - virBitmapCopy(cpus[cid].siblings, siblings); + cpus[cid].siblings = virBitmapNewCopy(siblings); cid++; } diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e3a934363b..29cb78a388 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1932,7 +1932,8 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps) ret->ctime = qemuCaps->ctime; - virBitmapCopy(ret->flags, qemuCaps->flags); + virBitmapFree(ret->flags); + ret->flags = virBitmapNewCopy(qemuCaps->flags); ret->version = qemuCaps->version; ret->kvmVersion = qemuCaps->kvmVersion; diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index dabfa9990e..aea3fe8687 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -1158,9 +1158,9 @@ virResctrlAllocUpdateMask(virResctrlAllocPtr alloc, return -1; if (!a_type->masks[cache]) - a_type->masks[cache] = virBitmapNew(virBitmapSize(mask)); + a_type->masks[cache] = virBitmapNewCopy(mask); - return virBitmapCopy(a_type->masks[cache], mask); + return 0; } -- 2.26.2

The function is now unused. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virbitmap.c | 23 ----------------------- src/util/virbitmap.h | 6 ------ 3 files changed, 30 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 01ae3cad93..7cf8bea962 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1668,7 +1668,6 @@ virAuthConfigNewData; virBitmapClearAll; virBitmapClearBit; virBitmapClearBitExpand; -virBitmapCopy; virBitmapCountBits; virBitmapDataFormat; virBitmapEqual; diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index c62438a3a6..3666033bac 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -100,29 +100,6 @@ virBitmapFree(virBitmapPtr bitmap) } -/** - * virBitmapCopy: - * @dst: destination bitmap - * @src: source bitmap - * - * Copies contents of @src to @dst. @dst must have the same size as @src. - * Returns -1 if the size is not the same or 0 on success. - */ -int -virBitmapCopy(virBitmapPtr dst, - virBitmapPtr src) -{ - if (dst->nbits != src->nbits) { - errno = EINVAL; - return -1; - } - - memcpy(dst->map, src->map, src->map_len * sizeof(src->map[0])); - - return 0; -} - - /** * virBitmapSetBit: * @bitmap: Pointer to bitmap diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index cd5caa6f61..67a433672d 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -39,12 +39,6 @@ virBitmapPtr virBitmapNew(size_t size); */ void virBitmapFree(virBitmapPtr bitmap); -/* - * Copy all bits from @src to @dst. The bitmap sizes - * must be the same - */ -int virBitmapCopy(virBitmapPtr dst, virBitmapPtr src); - /* * Set bit position @b in @bitmap */ -- 2.26.2

If we use g_new0 there's no need for the 'cleanup' label as there's nothing to fail after the allocation. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f716338efd..0d93ae3341 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -20900,7 +20900,6 @@ virDomainResctrlNew(xmlNodePtr node, unsigned int flags) { virDomainResctrlDefPtr resctrl = NULL; - virDomainResctrlDefPtr ret = NULL; g_autofree char *vcpus_str = NULL; g_autofree char *alloc_id = NULL; @@ -20923,18 +20922,13 @@ virDomainResctrlNew(xmlNodePtr node, } if (virResctrlAllocSetID(alloc, alloc_id) < 0) - goto cleanup; - - if (VIR_ALLOC(resctrl) < 0) - goto cleanup; + return NULL; + resctrl = g_new0(virDomainResctrlDef, 1); resctrl->vcpus = virBitmapNewCopy(vcpus); resctrl->alloc = virObjectRef(alloc); - ret = g_steal_pointer(&resctrl); - cleanup: - virDomainResctrlDefFree(resctrl); - return ret; + return g_steal_pointer(&resctrl); } -- 2.26.2

After recent refactors the function can be refactored to remove the 'cleanup' label by using autoptr for the 'map' variable. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 82845736b1..dbaf3b7666 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -6419,13 +6419,10 @@ qemuDomainFilterHotplugVcpuEntities(virDomainDefPtr def, { qemuDomainVcpuPrivatePtr vcpupriv; virDomainVcpuDefPtr vcpu; - virBitmapPtr map = NULL; - virBitmapPtr ret = NULL; + g_autoptr(virBitmap) map = virBitmapNewCopy(vcpus); ssize_t next = -1; size_t i; - map = virBitmapNewCopy(vcpus); - /* make sure that all selected vcpus are in the correct state */ while ((next = virBitmapNextSetBit(map, next)) >= 0) { if (!(vcpu = virDomainDefGetVcpu(def, next))) @@ -6434,13 +6431,13 @@ qemuDomainFilterHotplugVcpuEntities(virDomainDefPtr def, if (vcpu->online == state) { virReportError(VIR_ERR_INVALID_ARG, _("vcpu '%zd' is already in requested state"), next); - goto cleanup; + return NULL; } if (vcpu->online && !vcpu->hotpluggable) { virReportError(VIR_ERR_INVALID_ARG, _("vcpu '%zd' can't be hotunplugged"), next); - goto cleanup; + return NULL; } } @@ -6457,7 +6454,7 @@ qemuDomainFilterHotplugVcpuEntities(virDomainDefPtr def, virReportError(VIR_ERR_INVALID_ARG, _("vcpu '%zd' belongs to a larger hotpluggable entity, " "but siblings were not selected"), next); - goto cleanup; + return NULL; } for (i = next + 1; i < next + vcpupriv->vcpus; i++) { @@ -6467,7 +6464,7 @@ qemuDomainFilterHotplugVcpuEntities(virDomainDefPtr def, "hotpluggable entity '%zd-%zd' which was " "partially selected"), i, next, next + vcpupriv->vcpus - 1); - goto cleanup; + return NULL; } /* clear the subthreads */ @@ -6475,11 +6472,7 @@ qemuDomainFilterHotplugVcpuEntities(virDomainDefPtr def, } } - ret = g_steal_pointer(&map); - - cleanup: - virBitmapFree(map); - return ret; + return g_steal_pointer(&map); } -- 2.26.2