Re: [PATCH 05/15] conf: domain_addr: Refactor hash usage in zpci reservation code
On Thu, Oct 22, 2020 at 13:44:10 +0200, Bjoern Walk wrote:
Peter Krempa <pkrempa(a)redhat.com> [2020-10-22, 11:34AM +0200]:
> Rewrite using GHashTable which already has interfaces for using a number
> as hash key.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> src/conf/domain_addr.c | 123 +++++++++--------------------------------
> src/conf/domain_addr.h | 4 +-
> 2 files changed, 27 insertions(+), 100 deletions(-)
[...]
> @@ -123,40 +122,19 @@
virDomainZPCIAddressAssignFid(virHashTablePtr set,
>
>
> static void
> -virDomainZPCIAddressReleaseId(virHashTablePtr set,
> - virZPCIDeviceAddressID *id,
> - const char *name)
> +virDomainZPCIAddressReleaseId(GHashTable *set,
> + virZPCIDeviceAddressID *id)
> {
> if (!id->isSet)
> return;
>
> - if (virHashRemoveEntry(set, &id->value) < 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("Release %s %o failed"),
> - name, id->value);
> - }
> + g_hash_table_remove(set, &id->value);
Here I'm not so sure, IIUC the original code reported an error when the
value was not found in the hash table. This will be dropped with the new
code. But since this should only occur in pathological cases anyways, so
Specifically, here the error is only logged, but the caller doesn't take
any action/return any value.
That means that there wouldn't be any immediate notification of the
user. It's very unlikely that the error will be noticed in logs.
g_hash_table_remove returns whether it released the value or not, but I
don't feel it's worth keeping it.
I guess this is fine.
Thanks!