[pve-devel] [PATCH v2 storage 16/28] pvesm import: introduce allow_rename parameter

Fabian Ebner f.ebner at proxmox.com
Tue Feb 25 13:50:28 CET 2020


On 2/25/20 12:33 PM, Fabian Grünbichler wrote:
> see previous patch. this is a second break because of the new pvesm
> parameter.
> 

I think I'll squash the whole allow-rename + return-volume-id together,
since it should be a single change from the API point of view.

> On February 24, 2020 1:44 pm, Fabian Ebner wrote:
>> which can be used to avoid aborting on collision.
>>
>> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
>> ---
>>   PVE/CLI/pvesm.pm             |  9 ++++++++-
>>   PVE/Storage.pm               |  8 ++++----
>>   PVE/Storage/LVMPlugin.pm     | 15 +++++++++------
>>   PVE/Storage/Plugin.pm        | 13 +++++++++----
>>   PVE/Storage/ZFSPoolPlugin.pm | 11 +++++++----
>>   5 files changed, 37 insertions(+), 19 deletions(-)
>>
>> diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
>> index c55cdd0..57aac14 100755
>> --- a/PVE/CLI/pvesm.pm
>> +++ b/PVE/CLI/pvesm.pm
>> @@ -297,6 +297,13 @@ __PACKAGE__->register_method ({
>>   		maxLength => 80,
>>   		optional => 1,
>>   	    },
>> +	    'allow-rename' => {
>> +		description => "Choose a new volume ID if the requested " .
>> +		  "volume ID already exists, instead of throwing an error.",
>> +		type => 'boolean',
>> +		optional => 1,
>> +		default => 0,
>> +	    },
>>   	},
>>       },
>>       returns => { type => 'string' },
>> @@ -353,7 +360,7 @@ __PACKAGE__->register_method ({
>>   	my $volume = $param->{volume};
>>   	my $delete = $param->{'delete-snapshot'};
>>   	my $imported_volid = PVE::Storage::volume_import($cfg, $infh, $volume, $param->{format},
>> -	    $param->{base}, $param->{'with-snapshots'});
>> +	    $param->{base}, $param->{'with-snapshots'}, $param->{'allow-rename'});
>>   	PVE::Storage::volume_snapshot_delete($cfg, $imported_volid, $delete)
>>   	    if defined($delete);
>>   	return $imported_volid;
>> diff --git a/PVE/Storage.pm b/PVE/Storage.pm
>> index cb07066..bed6e81 100755
>> --- a/PVE/Storage.pm
>> +++ b/PVE/Storage.pm
>> @@ -39,11 +39,11 @@ use PVE::Storage::DRBDPlugin;
>>   use PVE::Storage::PBSPlugin;
>>   
>>   # Storage API version. Icrement it on changes in storage API interface.
>> -use constant APIVER => 3;
>> +use constant APIVER => 4;
>>   # Age is the number of versions we're backward compatible with.
>>   # This is like having 'current=APIVER' and age='APIAGE' in libtool,
>>   # see https://www.gnu.org/software/libtool/manual/html_node/Libtool-versioning.html
>> -use constant APIAGE => 2;
>> +use constant APIAGE => 3;
>>   
>>   # load standard plugins
>>   PVE::Storage::DirPlugin->register();
>> @@ -1432,14 +1432,14 @@ sub volume_export {
>>   }
>>   
>>   sub volume_import {
>> -    my ($cfg, $fh, $volid, $format, $base_snapshot, $with_snapshots) = @_;
>> +    my ($cfg, $fh, $volid, $format, $base_snapshot, $with_snapshots, $allow_rename) = @_;
>>   
>>       my ($storeid, $volname) = parse_volume_id($volid, 1);
>>       die "cannot import into volume '$volid'\n" if !$storeid;
>>       my $scfg = storage_config($cfg, $storeid);
>>       my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
>>       return $plugin->volume_import($scfg, $storeid, $fh, $volname, $format,
>> -                                  $base_snapshot, $with_snapshots) // $volid;
>> +                                  $base_snapshot, $with_snapshots, $allow_rename) // $volid;
>>   }
>>   
>>   sub volume_export_formats {
>> diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
>> index fcd48ef..3212a2d 100644
>> --- a/PVE/Storage/LVMPlugin.pm
>> +++ b/PVE/Storage/LVMPlugin.pm
>> @@ -624,7 +624,7 @@ sub volume_import_formats {
>>   }
>>   
>>   sub volume_import {
>> -    my ($class, $scfg, $storeid, $fh, $volname, $format, $base_snapshot, $with_snapshots) = @_;
>> +    my ($class, $scfg, $storeid, $fh, $volname, $format, $base_snapshot, $with_snapshots, $allow_rename) = @_;
>>       die "volume import format $format not available for $class\n"
>>   	if $format ne 'raw+size';
>>       die "cannot import volumes together with their snapshots in $class\n"
>> @@ -638,17 +638,20 @@ sub volume_import {
>>   
>>       my $vg = $scfg->{vgname};
>>       my $lvs = lvm_list_volumes($vg);
>> -    die "volume $vg/$volname already exists\n"
>> -	if $lvs->{$vg}->{$volname};
>> +    if ($lvs->{$vg}->{$volname}) {
>> +	die "volume $vg/$volname already exists\n" if !$allow_rename;
>> +	warn "volume $vg/$volname already exists - importing with a different name\n";
>> +	$name = undef;
>> +    }
>>   
>>       my ($size) = PVE::Storage::Plugin::read_common_header($fh);
>>       $size = int($size/1024);
>>   
>>       eval {
>>   	my $allocname = $class->alloc_image($storeid, $scfg, $vmid, 'raw', $name, $size);
>> -	if ($allocname ne $volname) {
>> -	    my $oldname = $volname;
>> -	    $volname = $allocname; # Let the cleanup code know what to free
>> +	my $oldname = $volname;
>> +	$volname = $allocname;
>> +	if (defined($name) && $allocname ne $oldname) {
>>   	    die "internal error: unexpected allocated name: '$allocname' != '$oldname'\n";
>>   	}
>>   	my $file = $class->path($scfg, $volname, $storeid)
>> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
>> index 0a35ce5..59660d9 100644
>> --- a/PVE/Storage/Plugin.pm
>> +++ b/PVE/Storage/Plugin.pm
>> @@ -1191,7 +1191,7 @@ sub volume_export_formats {
>>   
>>   # Import data from a stream, creating a new or replacing or adding to an existing volume.
>>   sub volume_import {
>> -    my ($class, $scfg, $storeid, $fh, $volname, $format, $base_snapshot, $with_snapshots) = @_;
>> +    my ($class, $scfg, $storeid, $fh, $volname, $format, $base_snapshot, $with_snapshots, $allow_rename) = @_;
>>   
>>       die "volume import format '$format' not available for $class\n"
>>   	if $format !~ /^(raw|tar|qcow2|vmdk)\+size$/;
>> @@ -1214,15 +1214,20 @@ sub volume_import {
>>       # free it.
>>       my $file = $class->path($scfg, $volname, $storeid);
>>       die "file '$file' already exists\n" if -e $file;
>> +    if (-e $file) {
>> +	die "file '$file' already exists\n" if !$allow_rename;
>> +	warn "file '$file' already exists - importing with a different name\n";
>> +	$name = undef;
>> +    }
>>   
>>       my ($size) = read_common_header($fh);
>>       $size = int($size/1024);
>>   
>>       eval {
>>   	my $allocname = $class->alloc_image($storeid, $scfg, $vmid, $file_format, $name, $size);
>> -	if ($allocname ne $volname) {
>> -	    my $oldname = $volname;
>> -	    $volname = $allocname; # Let the cleanup code know what to free
>> +	my $oldname = $volname;
>> +	$volname = $allocname;
>> +	if (defined($name) && $allocname ne $oldname) {
>>   	    die "internal error: unexpected allocated name: '$allocname' != '$oldname'\n";
>>   	}
>>   	my $file = $class->path($scfg, $volname, $storeid)
>> diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
>> index b618e06..b4fd65f 100644
>> --- a/PVE/Storage/ZFSPoolPlugin.pm
>> +++ b/PVE/Storage/ZFSPoolPlugin.pm
>> @@ -743,7 +743,7 @@ sub volume_export_formats {
>>   }
>>   
>>   sub volume_import {
>> -    my ($class, $scfg, $storeid, $fh, $volname, $format, $base_snapshot, $with_snapshots) = @_;
>> +    my ($class, $scfg, $storeid, $fh, $volname, $format, $base_snapshot, $with_snapshots, $allow_rename) = @_;
>>   
>>       die "unsupported import stream format for $class: $format\n"
>>   	if $format ne 'zfs';
>> @@ -752,15 +752,18 @@ sub volume_import {
>>       die "internal error: invalid file handle for volume_import\n"
>>   	if !defined($fd);
>>   
>> -    my $dataset = ($class->parse_volname($volname))[1];
>> +    my (undef, $dataset, $vmid) = $class->parse_volname($volname);
>>       my $zfspath = "$scfg->{pool}/$dataset";
>>       my $suffix = defined($base_snapshot) ? "\@$base_snapshot" : '';
>>       my $exists = 0 == run_command(['zfs', 'get', '-H', 'name', $zfspath.$suffix],
>>   			     noerr => 1, errfunc => sub {});
>>       if (defined($base_snapshot)) {
>>   	die "base snapshot '$zfspath\@$base_snapshot' doesn't exist\n" if !$exists;
>> -    } else {
>> -	die "volume '$zfspath' already exists\n" if $exists;
>> +    } elsif ($exists) {
>> +	die "volume '$zfspath' already exists\n" if !$allow_rename;
>> +	warn "volume '$zfspath' already exists - importing with a different name\n";
>> +	$dataset = $class->find_free_diskname($storeid, $scfg, $vmid, $format);
>> +	$zfspath = "$scfg->{pool}/$dataset";
>>       }
>>   
>>       eval { run_command(['zfs', 'recv', '-F', '--', $zfspath], input => "<&$fd") };
>> -- 
>> 2.20.1
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel at pve.proxmox.com
>> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 



More information about the pve-devel mailing list