[pve-devel] [qemu-server] fix #1829 check cloud-init disk format.

[Wolfgang Bumiller]

On Tue, Jul 03, 2018 at 05:05:54PM +0200, Wolfgang Link wrote:
> Directory storage support raw and qcow2 format.
> It is not good to implicitly set the cloud-init format
> and ignore the selected format.
> 
> Signed-off-by: Wolfgang Link <w.link at proxmox.com>
> ---
>  PVE/API2/Qemu.pm | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index c15c71f..26ace09 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -149,12 +149,14 @@ my $create_disks = sub {
>  	    die "no storage ID specified (and no default storage)\n" if !$storeid;
>  	    my $scfg = PVE::Storage::storage_config($storecfg, $storeid);
>  	    my $name = "vm-$vmid-cloudinit";
> -	    my $fmt = undef;
> +	    my $fmt = $disk->{format};
>  	    if ($scfg->{path}) {
> -		$name .= ".qcow2";
> -		$fmt = 'qcow2';
> +		die "Format \"$fmt\" is not supported\n"
> +		    if $fmt !~ m/[raw|qcow2]/;

Wrong regex.

IIRC we said that we eg. don't want vmdk here, so that's why you're
doing this, but I find this is unnecessary work... Can we just skip the
check?

> +		$name .= ".$fmt";

BTW. @Thomas, @Dietmar:
Why do we even need to add the extension? This is a storage-dependent
thing, vdisk_alloc() currently actually verifies that the extension is
there for storages which need it, but instead, it could just add it,
because here we need to needlessly distinguish between storage types for
a function which returns the actually allocated volid anyway.

We could make the extension optional and simplify code like this...
 => QemuConfig.pm
    > $name .= ".raw" if $scfg->{path}; # add filename extension for file base storage
 => API2/Storage/Content.pm
    `create` takes a 'format' option and a filename, and verifies that
    the extension matches the option if both exist.
    (Weirdly the parameter is called `filename` actually...)
 => QemuServer.pm
    clone_disk has a cloud-init case where it appends .qcow2 (which btw.
    I think will fail with .raw as well currently...)


>  	    }else{
> -		$fmt = 'raw';
> +		die "Format \"$fmt\" is not supported\n"
> +		    if $fmt eq 'raw';
>  	    }
>  	    # FIXME: Reasonable size? qcow2 shouldn't grow if the space isn't used anyway?
>  	    my $cloudinit_iso_size = 5; # in MB
> -- 
> 2.11.0