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.

On 2/20/25 10:36, Fiona Ebner wrote:
It does seem natural that callers that check for a content type being
supported actually want to do something with that content type too and
actually care about the content type being "ready"/"available". The only
cases I would imagine is checks about the storage config whether it
would be supported in principle, but if we ever have one of those, we
just don't need to use the helper.

Yes, I'd double check that when working on the v3, but as far as I've seen how they current have been used and going through the cases above it'd sound like they were used like this before anyway.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to