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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, May 22, 2019 at 03:36:59PM +0200, Andrea Bolognani wrote:
On Tue, 2019-05-21 at 16:28 +0200, Ján Tomko wrote:
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:
> > 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

Well of course you're avoiding most of the churn I was mentioning:
you're adding a new function instead of renaming the existing one!
That's cheating ;)

I don't much like the idea of adding a separate function that does
almost the same thing but not quite, because that will ultimately
result in the same situation we have now: someone will add a new
callback and (reasonably) expect it to be called for all devices,
but that won't happen because the original code uses the DeviceInfo
variant instead of the Device one. That's unnecessary friction.


So is having to argue about not putting if (!info) into every single
internal function that should not have been called with a NULL info in
the first place.

Jano

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux