On Wed, Nov 14, 2018 at 02:22:12PM -0500, John Ferlan wrote: > > > On 11/14/18 4:25 AM, Daniel P. Berrangé wrote: > > On Tue, Nov 13, 2018 at 03:21:03PM -0500, John Ferlan wrote: > >> Sending as an RFC primarily because I'm looking for whether either > >> or both mechanisms in the series is more or less desired. Likewise, > >> if it's felt that the current process of telling customers to just > >> delete the cache is acceptible, then so be it. If there's other ideas > >> I'm willing to give them a go too. I did consider adding a virsh > >> option to "virsh capabilities" (still possible) and/or a virt-admin > >> option to force the refresh. These just were "easier" and didn't > >> require an API adjustment to implement. > >> > >> Patch1 is essentially a means to determine if the kernel config > >> was changed to allow nested virtualization and to force a refresh > >> of the capabilities in that case. Without doing so the CPU settings > >> for a guest may not add the vmx=on depending on configuration and > >> for the user that doesn't make sense. There is a private bz on this > >> so I won't bother posting it. > >> > >> Patch2 and Patch3 make use of the 'service libvirtd reload' function > >> in order to invalidate all the entries in the internal QEMU capabilities > >> hash table and then to force a reread. This perhaps has downsides related > >> to guest usage and previous means to use reload and not refresh if a guest > >> was running. On the other hand, we tell people to just clear the QEMU > >> capabilities cache (e.g. rm /var/cache/libvirt/qemu/capabilities/*.xml) > >> and restart libvirtd, so in essence, the same result. It's not clear > >> how frequently this is used (it's essentially a SIGHUP to libvirtd). > > > > IMHO the fact that we cache stuff should be completely invisible outside > > of libvirt. Sure we've had some bugs in this area, but they are not very > > frequent so I'm not enthusiastic to expose any knob to force rebuild beyond > > just deleting files. > > > > OK - so that more or less obviates patch2 and patch3... > > Of course the fact that we cache stuff hasn't been completely invisible > and telling someone to fix the problem by "simply" removing the cache > files and pointing them to the cache location seems a bit "awkward" once > you figure out that is the problem of course. Many times it's just not > intuitively obvious! > > OTOH, baking in the idea that a "reload" could remove the chance that > caching was the problem could be useful. I guess it just felt like it > was a perhaps less used option. Assuming of course most would use > stop/start or restart instead. Looking at this from a more general POV, cache invalidation bugs are just one of many different bugs that can & have impacted libvirt over the years. Adding a force reload API is essentially saying we want to have virConnectWhackPossibleBug() because we think we might have a future bug. I don't think that is a good design precedent or rationale - essentially its admitting failure. If caching really were so terribly implemented that this is considered needed, then I'd argue caching should be deleted. I don't think we are in such a bad case though - the kind of problems have been fairly niche in impact. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
