[PATCH 05/15] conf: domain_addr: Refactor hash usage in zpci reservation code

[Peter Krempa]

On Thu, Oct 22, 2020 at 13:44:10 +0200, Bjoern Walk wrote:
> Peter Krempa <pkrempa at 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 at 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!