On Wed, Oct 24, 2018 at 11:43 PM +0200, "Daniel P. Berrangé" <berrange@redhat.com> wrote:
On Thu, Sep 20, 2018 at 07:44:46PM +0200, Marc Hartmayer wrote:
For a domain definition there are numerous calls of virQEMUCapsCacheLookup (the same applies to the domain start). This slows down the process since virQEMUCapsCacheLookup validates that the QEMU capabilitites are still valid (among other things, a fork is done for this if the user for the QEMU processes is 'qemu'). Therefore let's reduce the number of virQEMUCapsCacheLookup calls whenever possible and reasonable.
In addition to the speed up, there is the risk that virQEMUCapsCacheLookup returns different QEMU capabilities during a task if, for example, the QEMU binary has changed during the task.
The correct way would be:
- get the QEMU capabilities only once per task via virQEMUCapsCacheLookup - do the task with these QEMU capabilities
or
- abort the task after a cache invalidation
Note: With this patch series the behavior is still not (completely) fixed, but the performance has been significantly improved. In a quick test this gave a speed up of factor 4 for a simple define/undefine loop.
In general, the more devices a domain has, the more drastic the overhead becomes (because a cache validation is performed for each device).
IIUC from your KVM Forum presentation, the overhead of the cache lookup is almost entirely coming from the fork() call triggered by the single call
kvmUsable = virFileAccessibleAs("/dev/kvm", R_OK | W_OK, priv->runUid, priv->runGid) == 0;
Rather than the major refactor of the way the parse callbacks work, we could instead just optimize this call to avoid the repeated forks.
Even if we reduced the number of calls to the cache, it is still somewhat overkill to be checking /dev/kvm via fork() every time. eg even if you reduced it to just a single cache lookup, if you run virDomainDefine for 100 domains in parallel it is still going to do 100 forks.
We could optimize this by jcalling virFileAccessibleAs once and storing the result in a global. Then just do a plain stat() call in process to check the st_ctime field for changes. We only need re-run the heavy virFileAccessibleAs check if st_ctime has changed (indicating a owner/group/acl change).
Okay, if that’s fine I’ll add this as an additional patch to this series. Because the various virQEMUCapsCacheLookup calls still seem to be wrong to me. Marc
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 :|
-- Kind regards / Beste Grüße Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294