Re: [pve-devel] [PATCH pve-storage 02/10] common: add qemu_img_create an preallocation_cmd_option

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

> Alexandre Derumier via pve-devel <pve-devel@lists.proxmox.com> hat am 04.07.2025 08:44 CEST geschrieben:
> Signed-off-by: Alexandre Derumier <alexandre.derumier@groupe-cyllene.com>
> ---
>  src/PVE/Storage/Common.pm | 52 +++++++++++++++++++++++++++++++++++++++
>  src/PVE/Storage/Plugin.pm | 47 +----------------------------------
>  2 files changed, 53 insertions(+), 46 deletions(-)
> 
> diff --git a/src/PVE/Storage/Common.pm b/src/PVE/Storage/Common.pm
> index 89a70f4..29f2e52 100644
> --- a/src/PVE/Storage/Common.pm
> +++ b/src/PVE/Storage/Common.pm
> @@ -5,12 +5,26 @@ use warnings;
>  
>  use PVE::JSONSchema;
>  use PVE::Syscall;
> +use PVE::Tools qw(run_command);
>  
>  use constant {
>      FALLOC_FL_KEEP_SIZE => 0x01, # see linux/falloc.h
>      FALLOC_FL_PUNCH_HOLE => 0x02, # see linux/falloc.h
>  };
>  
> +our $QCOW2_PREALLOCATION = {
> +    off => 1,
> +    metadata => 1,
> +    falloc => 1,
> +    full => 1,
> +};
> +
> +our $RAW_PREALLOCATION = {
> +    off => 1,
> +    falloc => 1,
> +    full => 1,
> +};

these should probably stay in Plugin.pm

> +
>  =pod
>  
>  =head1 NAME
> @@ -110,4 +124,42 @@ sub deallocate : prototype($$$) {
>      }
>  }
>  
> +sub preallocation_cmd_option {

this as well, since it is storage config dependent

> +    my ($scfg, $fmt) = @_;
> +
> +    my $prealloc = $scfg->{preallocation};
> +
> +    if ($fmt eq 'qcow2') {
> +        $prealloc = $prealloc // 'metadata';
> +
> +        die "preallocation mode '$prealloc' not supported by format '$fmt'\n"
> +            if !$QCOW2_PREALLOCATION->{$prealloc};
> +
> +        return "preallocation=$prealloc";
> +    } elsif ($fmt eq 'raw') {
> +        $prealloc = $prealloc // 'off';
> +        $prealloc = 'off' if $prealloc eq 'metadata';
> +
> +        die "preallocation mode '$prealloc' not supported by format '$fmt'\n"
> +            if !$RAW_PREALLOCATION->{$prealloc};
> +
> +        return "preallocation=$prealloc";
> +    }
> +
> +    return;
> +}
> +
> +sub qemu_img_create {
> +    my ($scfg, $fmt, $size, $path) = @_;

then we could lose the $scfg here, and instead add an $options (that the
later patch can then extend further).

> +
> +    my $cmd = ['/usr/bin/qemu-img', 'create'];
> +
> +    my $prealloc_opt = preallocation_cmd_option($scfg, $fmt);
> +    push @$cmd, '-o', $prealloc_opt if defined($prealloc_opt);
> +
> +    push @$cmd, '-f', $fmt, $path, "${size}K";
> +
> +    run_command($cmd, errmsg => "unable to create image");
> +}
> +
>  1;
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index c2f376b..80bb077 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -38,19 +38,6 @@ our @SHARED_STORAGE = (
>      'iscsi', 'nfs', 'cifs', 'rbd', 'cephfs', 'iscsidirect', 'zfs', 'drbd', 'pbs',
>  );
>  
> -our $QCOW2_PREALLOCATION = {
> -    off => 1,
> -    metadata => 1,
> -    falloc => 1,
> -    full => 1,
> -};
> -
> -our $RAW_PREALLOCATION = {
> -    off => 1,
> -    falloc => 1,
> -    full => 1,
> -};

because we don't know whether somebody relies on this being here..

> -
>  our $MAX_VOLUMES_PER_GUEST = 1024;
>  
>  cfs_register_file(
> @@ -606,31 +593,6 @@ sub parse_config {
>      return $cfg;
>  }
>  
> -sub preallocation_cmd_option {
> -    my ($scfg, $fmt) = @_;
> -
> -    my $prealloc = $scfg->{preallocation};
> -
> -    if ($fmt eq 'qcow2') {
> -        $prealloc = $prealloc // 'metadata';
> -
> -        die "preallocation mode '$prealloc' not supported by format '$fmt'\n"
> -            if !$QCOW2_PREALLOCATION->{$prealloc};
> -
> -        return "preallocation=$prealloc";
> -    } elsif ($fmt eq 'raw') {
> -        $prealloc = $prealloc // 'off';
> -        $prealloc = 'off' if $prealloc eq 'metadata';
> -
> -        die "preallocation mode '$prealloc' not supported by format '$fmt'\n"
> -            if !$RAW_PREALLOCATION->{$prealloc};
> -
> -        return "preallocation=$prealloc";
> -    }
> -
> -    return;
> -}
> -
>  # Storage implementation
>  
>  # called during addition of storage (before the new storage config got written)
> @@ -969,14 +931,7 @@ sub alloc_image {
>          umask $old_umask;
>          die $err if $err;
>      } else {
> -        my $cmd = ['/usr/bin/qemu-img', 'create'];
> -
> -        my $prealloc_opt = preallocation_cmd_option($scfg, $fmt);
> -        push @$cmd, '-o', $prealloc_opt if defined($prealloc_opt);
> -
> -        push @$cmd, '-f', $fmt, $path, "${size}K";
> -
> -        eval { run_command($cmd, errmsg => "unable to create image"); };
> +        eval { PVE::Storage::Common::qemu_img_create($scfg, $fmt, $size, $path) };
>          if ($@) {
>              unlink $path;
>              rmdir $imagedir;
> -- 
> 2.39.5


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