Andrea Bolognani <abologna@redhat.com> [2020-06-25, 07:43PM +0200]:
From 82197ea6d794144e51b72f97df2315a65134b385 Mon Sep 17 00:00:00 2001 From: Andrea Bolognani <abologna@redhat.com> Date: Thu, 25 Jun 2020 18:37:27 +0200 Subject: [libvirt PATCH 1/2] conf: Move id->{value,isSet} handling further down the stack
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/domain_addr.c | 61 ++++++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 26 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 90ed31ef4e..493b155129 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -33,20 +33,27 @@ VIR_LOG_INIT("conf.domain_addr");
static int virDomainZPCIAddressReserveId(virHashTablePtr set, - unsigned int id, + virZPCIDeviceAddressID *id, const char *name) { - if (virHashLookup(set, &id)) { + if (!id->isSet) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("No zPCI %s to reserve"), + name);
Maybe it's better to fail silently here (or not fail at all and just make it no-op)? This is more of an assert that concerns the developer and not something the user can understand.
+ return -1; + } + + if (virHashLookup(set, &id->value)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("zPCI %s %o is already reserved"), - name, id); + name, id->value); return -1; }
- if (virHashAddEntry(set, &id, (void*)1) < 0) { + if (virHashAddEntry(set, &id->value, (void*)1) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to reserve %s %o"), - name, id); + name, id->value); return -1; }
[...]
static void virDomainZPCIAddressReleaseId(virHashTablePtr set, - unsigned int *id, + virZPCIDeviceAddressID *id, const char *name) { - if (virHashRemoveEntry(set, id) < 0) { + if (!id->isSet) + return; + + if (virHashRemoveEntry(set, &id->value) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Release %s %o failed"), - name, *id); + name, id->value); }
- *id = 0; + id->value = 0; + id->isSet = false;
Not sure if I don't want to leave this to the caller. 99% of time it shouldn't matter because the released device is deleted from the domain anyways, but this tight coupling would prevent hypothetical re-use of the id. Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> -- IBM Systems Linux on Z & Virtualization Development -------------------------------------------------- IBM Deutschland Research & Development GmbH Schönaicher Str. 220, 71032 Böblingen Phone: +49 7031 16 1819 -------------------------------------------------- Vorsitzende des Aufsichtsrats: Gregor Pillen Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294