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

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

 



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




[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