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

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