Quoting Fiona Ebner (2025-02-20 10:03:09) > Am 11.02.25 um 17:07 schrieb Daniel Kral: > > Allow a caller to specify the volume's intended content type and assert > > whether the specified content type may be stored on the specified > > storage before allocating any volume. > > > > Signed-off-by: Daniel Kral <d.k...@proxmox.com> > > --- > > changes since v1: > > - add assertion at `vdisk_alloc` instead of wrapper `alloc_volume_disk` > > > > src/PVE/Storage.pm | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm > > index 3776565..96d4e41 100755 > > --- a/src/PVE/Storage.pm > > +++ b/src/PVE/Storage.pm > > @@ -1059,6 +1059,13 @@ Specifies the name of the new volume. > > > > If undefined, the name will be generated with > > C<PVE::Storage::Plugin::find_free_diskname>. > > > > +=item * C<< $vtype => $string >> > > + > > +Specifies the content type of the new volume, which can be one of > > C<'images'>, C<'rootdir'>, > > +C<'vztmpl'>, C<'iso'>, C<'backup'>, C<'snippets'> or C<'import'>. > > Hmm, vdisk_alloc() is only for allocating guest and import images. So I > think the other content types should be prohibited here (can still be > extended later if it ever comes up). > > We plan to better distinguish between 'rootdir' and 'images' in the > future, so I think we should aim to make the $vtype parameter even > required here. Not quite possible yet, because that would require > breaking 'pvesm alloc', but we can postpone that part for PVE 9 and have > all other callers already use it. > > So my question is if we should rather make it a required parameter > already rather than putting it into $opts? I mean while supporting > passing in undef too, until we are ready to require it for 'pvesm alloc' > too. > > @Fabian sounds good to you too?
we could also make it required for real in vdisk_alloc, and make `pvesm alloc` auto-select one of the allowed ones based on the storage config and error out if the storage doesn't support either rootdir or images? or use a different "magic" placeholder value like "any" - using undef is a bit opaque and could happen by accident, although it is not very likely for this hash ;) then when we introduce properly scoped volume names, we can replace that fallback logic in `pvesm alloc` with properly setting *either* rootdir or images, depending on what gets allocated? OTOH, vdisk_alloc doesn't have too call sites anyway, so doing $opts now, and then updating those when it becomes a regular argument would also work fine I think.. the only downside to that approach is that we might miss setting the option for newly introduce callers in the meantime.. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
