On 2/20/25 11:48, Fiona Ebner wrote:
Am 20.02.25 um 11:40 schrieb Fabian Grünbichler:
Quoting Fiona Ebner (2025-02-20 10:03:09)
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?
Sounds good to me. I'm in favor of making it required already, since we
already need versioned breaks for the series.
Sounds like a good plan, then I'll move the $vtype parameter into the
signature itself in a v3 and implement the behavior suggested by
@Fabian, thanks!
Thinking about the changes for PVE 9, I'd best retrieve the type of the
resource behind $vmid with `PVE::Cluster::get_vmlist()` to select
between the two content types, right?
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
