[libvirt] [PATCH 4/8] conf: Allow NULL for virDomainDeviceInfoCallback

[Andrea Bolognani]

On Mon, 2019-05-20 at 14:59 +0200, Ján Tomko wrote:
> On Mon, May 20, 2019 at 01:37:49PM +0200, Andrea Bolognani wrote:
> > The virDomainDeviceInfoIterate() function was initially
> > written with the expectation that all devices would embed a
> > virDomainDeviceInfo, and thus the user-provided callback
> > would never be passed NULL; however, that doesn't really
> > represent reality, as multiple devices don't have any
> > virDomainDeviceInfo associated with them.
> 
> virDomainDeviceInfoIterate is specifically meant for iterating
> over device infos.
> 
> Commit 88d24aaccc1e31ff1ce682f9496cf08cc7f7c216 :
>     conf: domain: Introduce virDomainDeviceIterateFlags
> 
> documented this function as iterating over devices (but did not
> implement this for every device) and then
> 
> commit dd45c2710f6fd2d4f8a47f97960532d0e0091e7d
>     conf: domain: gfx: Iterate over graphics devices when doing validation
> 
> added one of them.

Yeah, I'm aware of the history here, and the next patch partially
reverts the second commit.

> Of course, there is a huge overlap between iterating over all devices
> and just those with an info, but since the callers do request *Info*
> I don't think they should expect it to be NULL.

I realize that. I even toyed with the idea of renaming
virDomainDeviceInfoIterate() to virDomainDeviceIterate() to get rid
of the inconsistency, but I didn't feel like it would be worth the
churn and though that documenting the expectations would be enough.
I'd be fine renaming it, though.

Personally I don't think adding a new virDomainDeviceIterateFlags
for each virDomainDeviceInfo-less device class is a good solution,
as it leaves the door open to wiring up a validate callback that
looks like it would be called but actually isn't: this is what
caused dd45c27, and what happened to me while I was moving the
validation code for Intel IOMMU from qemu_command to domain_conf.

-- 
Andrea Bolognani / Red Hat / Virtualization