[pve-devel] [PATCH pve-storage v2 2/5] introduce helpers for content type assertions of storages and volumes

[Fiona Ebner]

Am 20.02.25 um 13:53 schrieb Daniel Kral:
> On 2/20/25 10:36, Fiona Ebner wrote:
>> Not necessarily? What about qm showcmd <ID>? Question is, do we want to
>> do storage-related checks there or not? If we already check for the
>> content type, I don't see much reason not to check for storage being
>> enabled either.
>>
>> It could be seen as surprising, because it's just building the command,
>> why would it do storage checks? But if we follow that rationale, it
>> shouldn't do either of the checks.
>>
>> I'm fine with keeping/adding the checks, because we already do that for
>> other things while building the command too. Otherwise, we'd need some
>> nocheck flag or similar. I have no strong opinion against that either.
>> Just nobody complained yet about this I guess and there's nothing really
>> wrong with having the semantics be getting a "checked" command.
> 
> Right, there's also `qm showcmd`, I only thought about the
> vm_start_nolock case here! I guess the best thing would be to move it
> there, or would an extra iteration through the disks like in cfg2cmd be
> too expensive there? Otherwise I'll let it be in cfg2cmd.

>From my side, it's fine to do the checks even in the "qm showcmd" case.
We also do other kinds of checks in config_to_command() already and I
don't think we ever got user complaints about that. It's good to know
even when querying the command when something is misconfigured IMHO.