From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 40BC91FF135 for ; Thu, 02 Jul 2026 14:46:16 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 6601321418; Thu, 02 Jul 2026 14:46:15 +0200 (CEST) Date: Thu, 2 Jul 2026 14:46:11 +0200 From: Wolfgang Bumiller To: "Max R. Carrara" Subject: Re: [PATCH pve-storage v1 38/54] test: list volumes: document behavior wrt. undeclared content types Message-ID: References: <20260422111322.257380-1-m.carrara@proxmox.com> <20260422111322.257380-39-m.carrara@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260422111322.257380-39-m.carrara@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1782996365341 X-SPAM-LEVEL: Spam detection results: 0 DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: T6P34XHOXFMATBPTEXKI2YT2YAZKGBM4 X-Message-ID-Hash: T6P34XHOXFMATBPTEXKI2YT2YAZKGBM4 X-MailFrom: w.bumiller@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: pve-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Wed, Apr 22, 2026 at 01:13:04PM +0200, Max R. Carrara wrote: > Any content types / volume types that are passed to > `PVE::Storage::Plugin->list_volumes()` are still taken into account by > the method, even if the passed storage config does not declare them in > its `content` key. In other words, the `content` property is ignored > when listing volumes through `list_volumes()`. > > Or to express this more explicitly, consider a directory storage with > `path` set to `/mnt/example` and `content` set to `iso,vztmpl`. Now, > if there are files in `/mnt/example/snippets` for some reason and the > `list_volumes()` method is called with `['snippets']` for the volume > type list parameter, then the volume info hashes for the files in > `/mnt/example/snippets` are included in the output. > > Document this through a test case. Did you find any rationale for this? Because I think the better solution here would be for list_volumes() to just either return nothing or die in this case (but dying could have some unwanted effects in the rest of the code base, so an empty list is safer). > > Signed-off-by: Max R. Carrara > --- > src/test/list_volumes_test.pm | 109 ++++++++++++++++++++++++++++++++++ > 1 file changed, 109 insertions(+) > > diff --git a/src/test/list_volumes_test.pm b/src/test/list_volumes_test.pm > index ca3bbac7..455fb227 100644 > --- a/src/test/list_volumes_test.pm > +++ b/src/test/list_volumes_test.pm > @@ -909,6 +909,115 @@ my $test_param_list = [ > }, > ], > }, > + { > + description => > + "VMID: none, volume info is still returned for content types that are not declared", > + storeid => $DEFAULT_STOREID, > + scfg => { > + type => 'dir', > + path => $DEFAULT_STORAGE_PATH, > + shared => 0, > + content => {}, # note how no content types are declared here > + }, > + vmid => undef, > + vtypes => ['images', 'rootdir', 'vztmpl', 'iso', 'backup', 'snippets', 'import'], > + cases => [ > + { > + file => "$DEFAULT_STORAGE_PATH/images/16110/vm-16110-disk-0.qcow2", > + expected => { > + content => 'images', > + ctime => $DEFAULT_CTIME, > + format => 'qcow2', > + parent => undef, > + size => $DEFAULT_SIZE, > + used => $DEFAULT_USED, > + vmid => '16110', > + volid => 'local:16110/vm-16110-disk-0.qcow2', > + }, > + }, > + { > + file => "$DEFAULT_STORAGE_PATH/images/1234/vm-1234-disk-0.qcow2", > + parent => '../ssss/base-4321-disk-0.qcow2', > + expected => { > + content => 'images', > + ctime => $DEFAULT_CTIME, > + format => 'qcow2', > + parent => '../ssss/base-4321-disk-0.qcow2', > + size => $DEFAULT_SIZE, > + used => $DEFAULT_USED, > + vmid => '1234', > + volid => 'local:1234/vm-1234-disk-0.qcow2', > + }, > + }, > + { > + file => "$DEFAULT_STORAGE_PATH/images/16112/vm-16112-disk-0.raw", > + expected => { > + content => 'rootdir', > + ctime => $DEFAULT_CTIME, > + format => 'raw', > + parent => undef, > + size => $DEFAULT_SIZE, > + used => $DEFAULT_USED, > + vmid => '16112', > + volid => 'local:16112/vm-16112-disk-0.raw', > + }, > + }, > + { > + file => > + "$DEFAULT_STORAGE_PATH/template/cache/alpine-3.10-default_20190626_amd64.tar.xz", > + expected => { > + content => 'vztmpl', > + ctime => $DEFAULT_CTIME, > + format => 'txz', > + size => $DEFAULT_SIZE, > + volid => 'local:vztmpl/alpine-3.10-default_20190626_amd64.tar.xz', > + }, > + }, > + { > + file => "$DEFAULT_STORAGE_PATH/template/iso/archlinux-2020.02.01-x86_64.iso", > + expected => { > + content => 'iso', > + ctime => $DEFAULT_CTIME, > + format => 'iso', > + size => $DEFAULT_SIZE, > + volid => 'local:iso/archlinux-2020.02.01-x86_64.iso', > + }, > + }, > + { > + file => > + "$DEFAULT_STORAGE_PATH/dump/vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo", > + expected => { > + content => 'backup', > + ctime => 1585604370, > + format => 'tar.lzo', > + size => $DEFAULT_SIZE, > + subtype => 'lxc', > + vmid => '16112', > + volid => 'local:backup/vzdump-lxc-16112-2020_03_30-21_39_30.tar.lzo', > + }, > + }, > + { > + file => "$DEFAULT_STORAGE_PATH/snippets/hookscript.pl", > + expected => { > + content => 'snippets', > + ctime => $DEFAULT_CTIME, > + format => 'snippet', > + size => $DEFAULT_SIZE, > + volid => 'local:snippets/hookscript.pl', > + }, > + }, > + { > + file => "$DEFAULT_STORAGE_PATH/import/import.ova", > + expected => { > + content => 'import', > + ctime => $DEFAULT_CTIME, > + format => 'ova', > + size => $DEFAULT_SIZE, > + volid => 'local:import/import.ova', > + }, > + }, > + ], > + }, > ]; > > # provide static vmlist for tests > -- > 2.47.3 > > > > > --