[libvirt] [PATCH 0/3] add disk driver metadata_cache_size option

[Kevin Wolf]

Am 01.11.2018 um 12:32 hat Nikolay Shirokovskiy geschrieben:
> Hi, all.
> 
> This is a patch series after offlist agreement on introducing
> metadata-cache-size option for disks. The options itself is described in 2nd
> patch of the series.
> 
> There is a plenty of attempts to add option to set qcow2 metadata cache sizes in
> past, see [1] [2] to name recent that has comments I tried to address or has
> work I reused to some extent.
> 
> I would like to address Peter's comment in [1] that this option is image's
> option rather then disk's here. While this makes sense if we set specific cache
> value that covers disk only partially, here when we have maximum policy that
> covers whole disk it makes sense to set same value for all images. The setted
> value is only upper limit on cache size and thus cache for every image will
> grow proportionally to image data size "visible from top" eventually I guess.
> And these sizes are changing during guest lifetime - thus we need set whole
> disk limit for every image for 'maxium' effect.
> 
> On the other hand if we add policies different from 'maximum' it may require
> per image option. Well I can't imagine name for element for backing chain that
> will have cache size attribute better then 'driver'). And driver is already
> used for top image so I leave it this way.
> 
>   KNOWN ISSUES
> 
> 1. when using -driver to configure disks in command line cache size does not
>    get propagated thru backing chain
> 
> 2. when making external disk snapshot cache size sneak into backing file
>    filename and thus cache size for backing image became remembered there
> 
> 3. blockcommit after such snapshot will not work because libvirt is not ready
>    for backing file name is reported as json sometimes
> 
> Looks like 1 can be addressed in qemu.

I don't think we want to add inheritance of driver-specific options.
Option inheritance is already a bit messy with options that every driver
understands.

And -drive is the obsolete interface anyway, when you use -blockdev you
specify all nodes explicitly.

> The reason for behaviour in 2 I do not understand honestly.

QEMU doesn't recognise that the cache size option doesn't affect which
data you're accessing. Max had a series that should probably fix it.
Maybe we should finally get it merged.

Anyway, I suppose if you use libvirt in -blockdev mode, it doesn't make
a difference anyway, because libvirt should then manually call
blockdev-create for the overlay and therefore specify the backing file
string explicitly.

Can you confirm this in practice?

> And 3 will be naturally addessed after blockjobs starts working with
> blockdev configuration I guess.

Hopefully (Peter?), but depending on 2. it might not even be necessary
if libvirt doesn't explicitly store json: pseudo-filenames.

Kevin