Re: [pve-devel] [PATCH storage 1/2] zfs: only use cache when listing images locally

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On November 7, 2022 12:00 pm, Fiona Ebner wrote:
> The plugin for remote ZFS storages currently also uses the same
> list_images() as the plugin for local ZFS storages. The issue with
> this is that there is only one cache which does not remember the
> target host where the information originated.
> 
> Simply restrict the cache to be used for the local ZFS plugin only. An
> alternative solution would be to use a cache for each target host, but
> that seems a bit more involved and could still be added in the future.

wouldn't it be sufficient to just do

$cache->{zfs}->{$storeid}

when filling/querying the cache, and combining that with *always* listing only
the storage-relevant pool?

the only case where we actually benefit from listing *all* zfs volumes/datasets
is when
- there are multiple storages configured referencing overlapping parts of the
ZFS hierarchy
- vdisk_list is called with a volume_list with multiple such storages being part
of the set, or with $vmid but no $storeid (rescan, or purging unreferenced guest
disks on guest removal)

in practice, it likely doesn't make much difference since ZFS should cache the
metadata for the overlapping parts in memory anyway (given that we'd then call
'zfs list' in a loop with different starting points).

whereas, for most regular cases listing happens without a cache anyway (or with
a cache, but only a single storage involved), so there is no benefit in querying
volumes belonging to other storages since we are not interested in them anyway.

sidenote: it seems like vdisk_list's volume_list is not used anywhere as parameter?

> 
> For example, issues can happen during rescan:
> 1. Via list_images() and zfs_list_zvol(), ZFSPlugin.pm's zfs_request()
>    is executed for a remote ZFS.
> 2. $cache->{zfs} is set to the result of scanning the images there.
> 3. Another list_images() for a local ZFS storage happens and uses the
>    cache with the wrong information.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>