[pve-devel] [PATCH] remove running from Storage and check it in QemuServer

[Dietmar Maurer]

>  # Note about locking: we use flock on the config file protect
> @@ -3777,7 +3779,7 @@ sub qemu_volume_snapshot {
>  
>      my $running = check_running($vmid);
>  
> -    return if !PVE::Storage::volume_snapshot($storecfg, $volid, $snap,
> $running);
> +    PVE::Storage::volume_snapshot($storecfg, $volid, $snap) if
> storage_support_snapshop($volid, $storecfg);

s/storage_support_snapshop/storage_support_snapshot/

But the name is misleading. The question is if 'qemu' does the snapshot, or if
we
need to do it ourselves. Maybesomething like: do_snapshots_with_qemu()

>  
>      return if !$running;
>  
> @@ -5772,6 +5774,23 @@ my $savevm_wait = sub {
>      }
>  };
>  
> +sub storage_support_snapshot {
> +    my ($volid, $storecfg) = @_;

I would reorder the arguments:

 my ($storecfg, $volid) = @_;

> +
> +    my $storage_name = PVE::Storage::parse_volume_id($volid);
> +
> +    my $ret = undef;  
what for?
> +    if ($snap_storage->{$storecfg->{ids}->{$storage_name}->{type}} ){
> +	$ret = 1;
return 1;
> +    }
> +
> +    if ($volid =~ m/\.(qcow2|qed)$/){
> +	$ret = 1;
return 1;
> +    }
> +
> +    return $ret;
return undef;
> +}
> +
>  sub snapshot_create {
>      my ($vmid, $snapname, $save_vmstate, $comment) = @_;
>  
> -- 
> 2.1.4
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>