On Mon, May 20, 2019 at 03:24:06PM +0200, Andrea Bolognani wrote:
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 think there would be less churn that way, see my proposal: Message-Id: <cover.1558448715.git.jtomko@xxxxxxxxxx> https://www.redhat.com/archives/libvir-list/2019-May/msg00607.html
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,
Agreed.
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.
Jano
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
