[pve-devel] applied-series: [PATCH v2 storage 1/3] zfs: list: only
Fabian Grünbichler
f.gruenbichler at proxmox.com
Wed Dec 21 11:09:01 CET 2022
thanks!
some possible room for further improvements:
- zfs_list_zvol could add `-d1` to only list $scfg->{pool} and direct children
(then we don't need to filter out any indirect descendants, just the "pool"
itself..)
- $list in zfs_list_zvol could be initialized as `{}`, then we don't need an
explicit fallback in list_images
On December 20, 2022 2:16 pm, Fiona Ebner wrote:
> The plugin for remote ZFS storages currently also uses the same
> list_images() as the plugin for local ZFS storages. There is only
> one cache which does not remember the target host where the
> information originated.
>
> This is problematic for rescan in qemu-server:
> 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.
>
> The only two operations using the cache currently are
> 1. Disk rescan in qemu-server which is affected by the issue. It is
> done as part of (or as a) long-running operations.
> 2. prepare_local_job for pvesr, but the non-local ZFS plugin doesn't
> support replication, so it cannot trigger there. The cache only
> helps if there is a replicated guest with volumes on different
> ZFS storages, but otherwise it will be less efficient than no
> cache or querying every storage by itself.
>
> Fix the issue by making the cache $storeid-specific, which effectively
> makes the cache superfluous, because there is no caller using the same
> $storeid multiple times. As argued above, this should not really hurt
> the callers described above much and actually improves performance for
> all other callers.
>
> Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
> ---
>
> Changes from v1:
> * Always only list images for $scfg->{pool} (and drop patch that
> would only do so for the remote ZFS plugin).
> * This makes the cache useless, so add a patch removing it.
> * Also add a patch for cleanup.
>
> See here for previous discussion:
> https://lists.proxmox.com/pipermail/pve-devel/2022-November/054485.html
>
> PVE/Storage/ZFSPoolPlugin.pm | 25 +++++++++++++++++++------
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
> index e264fde..0f16e7d 100644
> --- a/PVE/Storage/ZFSPoolPlugin.pm
> +++ b/PVE/Storage/ZFSPoolPlugin.pm
> @@ -254,11 +254,12 @@ sub free_image {
> sub list_images {
> my ($class, $storeid, $scfg, $vmid, $vollist, $cache) = @_;
>
> - $cache->{zfs} = $class->zfs_list_zvol($scfg) if !$cache->{zfs};
> - my $zfspool = $scfg->{pool};
> + $cache->{zfs}->{$storeid} = $class->zfs_list_zvol($scfg)
> + if !$cache->{zfs}->{$storeid};
> +
> my $res = [];
>
> - if (my $dat = $cache->{zfs}->{$zfspool}) {
> + if (my $dat = $cache->{zfs}->{$storeid}) {
>
> foreach my $image (keys %$dat) {
>
> @@ -375,20 +376,32 @@ sub zfs_delete_zvol {
> sub zfs_list_zvol {
> my ($class, $scfg) = @_;
>
> - my $text = $class->zfs_request($scfg, 10, 'list', '-o', 'name,volsize,origin,type,refquota', '-t', 'volume,filesystem', '-Hrp');
> + my $text = $class->zfs_request(
> + $scfg,
> + 10,
> + 'list',
> + '-o',
> + 'name,volsize,origin,type,refquota',
> + '-t',
> + 'volume,filesystem',
> + '-Hrp',
> + $scfg->{pool},
> + );
> my $zvols = zfs_parse_zvol_list($text);
> return undef if !$zvols;
>
> my $list = ();
> foreach my $zvol (@$zvols) {
> - my $pool = $zvol->{pool};
> + # The "pool" in $scfg is not the same as ZFS pool, so it's necessary to filter here.
> + next if $scfg->{pool} ne $zvol->{pool};
> +
> my $name = $zvol->{name};
> my $parent = $zvol->{origin};
> if($zvol->{origin} && $zvol->{origin} =~ m/^$scfg->{pool}\/(\S+)$/){
> $parent = $1;
> }
>
> - $list->{$pool}->{$name} = {
> + $list->{$name} = {
> name => $name,
> size => $zvol->{size},
> parent => $parent,
> --
> 2.30.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
More information about the pve-devel
mailing list