Re: [pve-devel] [PATCH storage v2 1/4] add standard option for VM disk formats in file-based storages

[Fiona Ebner <f.ebner@proxmox.com>]

Am 12.02.25 um 14:02 schrieb Markus Frank:
> Signed-off-by: Markus Frank <m.frank@proxmox.com>
> ---
>  src/PVE/Storage/Plugin.pm | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index 65cf43f..9f21f0b 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -354,6 +354,13 @@ sub verify_format {
>      return $fmt;
>  }
>  
> +PVE::JSONSchema::register_standard_option('pve-storage-qm-image-format', {

Nit: There is no real relation to the qm CLI tool. I'd prefer
pve-vm-image-format.

> +    type => 'string',
> +    enum => ['raw', 'qcow2', 'vmdk'],
> +    description => "Supported VM disk formats in a file-based storage.",

Nit: not all file-based storage might (want to) support all. Maybe
simply "VM image format."?

> +    optional => 1,

Not sure if we want to make it optional by default. Of course, can still
opt-out when needed at the use-site, but might be easier to forget this
way around. Having it be required by default and the use-site opt-out
might be more natural in this case (with future use-sites in mind). That
also makes it explicitly visible at the use site if optional. You
already specify optional => 1 for both use sites, so no change would be
needed there :) All that said, fine by me either way.

> +});
> +
>  PVE::JSONSchema::register_format('pve-storage-options', \&verify_options);
>  sub verify_options {
>      my ($value, $noerr) = @_;


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