Re: [libvirt] [PATCH 00/11] Avoid numerous calls of virQEMUCapsCacheLookup

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).

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 :|