On Wed, Oct 24, 2018 at 11:43 PM +0200, "Daniel P. Berrangé" <berrange@xxxxxxxxxx> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
