[pve-devel] [RFC storage 3/16] volume_import: Use a new name when the the name to import with already exists
Fabian Grünbichler
f.gruenbichler at proxmox.com
Wed Feb 5 11:49:55 CET 2020
On January 29, 2020 2:30 pm, Fabian Ebner wrote:
> The ID of the new volume is returned and pvesm import prints it. This is
> useful for migration, since the storage on the target might already contain
> unused/orphaned disks.
this is dangerous, and should be explicitly requested. (see patch #6)
as-is, this can lead to a repeated "allocate and fully replicate into
new volume" cycle when replication meets an orphaned disk with clashing
volid.
>
> Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
> ---
>
> Breaks the current migration in QEMU/LXC if there is a collision,
> since the code doesn't die anymore and the config is not updated
> with the new name, i.e. needs patches 1,5,12 and 13.
>
> PVE/CLI/pvesm.pm | 13 ++++++++-----
> PVE/Storage/LVMPlugin.pm | 14 +++++++++-----
> PVE/Storage/Plugin.pm | 12 ++++++++----
> PVE/Storage/ZFSPoolPlugin.pm | 10 ++++++----
> 4 files changed, 31 insertions(+), 18 deletions(-)
>
> diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
> index 510faba..7d8547b 100755
> --- a/PVE/CLI/pvesm.pm
> +++ b/PVE/CLI/pvesm.pm
> @@ -299,7 +299,7 @@ __PACKAGE__->register_method ({
> },
> },
> },
> - returns => { type => 'null' },
> + returns => { type => 'string' },
> code => sub {
> my ($param) = @_;
>
> @@ -352,11 +352,11 @@ __PACKAGE__->register_method ({
> my $cfg = PVE::Storage::config();
> my $volume = $param->{volume};
> my $delete = $param->{'delete-snapshot'};
> - PVE::Storage::volume_import($cfg, $infh, $volume, $param->{format},
> + my $volid = PVE::Storage::volume_import($cfg, $infh, $volume, $param->{format},
> $param->{base}, $param->{'with-snapshots'});
> - PVE::Storage::volume_snapshot_delete($cfg, $volume, $delete)
> + PVE::Storage::volume_snapshot_delete($cfg, $volid, $delete)
> if defined($delete);
> - return;
> + return $volid;
> }
> });
>
> @@ -777,7 +777,10 @@ our $cmddef = {
> path => [ __PACKAGE__, 'path', ['volume']],
> extractconfig => [__PACKAGE__, 'extractconfig', ['volume']],
> export => [ __PACKAGE__, 'export', ['volume', 'format', 'filename']],
> - import => [ __PACKAGE__, 'import', ['volume', 'format', 'filename']],
> + import => [ __PACKAGE__, 'import', ['volume', 'format', 'filename'], {}, sub {
> + my $volid = shift;
> + print "successfully imported '$volid'\n";
> + }],
> };
>
> 1;
> diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
> index f02c110..6c575f2 100644
> --- a/PVE/Storage/LVMPlugin.pm
> +++ b/PVE/Storage/LVMPlugin.pm
> @@ -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}) {
> + 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)
> @@ -661,6 +664,7 @@ sub volume_import {
> warn $@ if $@;
> die $err;
> }
> + return "$storeid:$volname";
> }
>
> sub volume_import_write {
> diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
> index 0c39cbd..358a831 100644
> --- a/PVE/Storage/Plugin.pm
> +++ b/PVE/Storage/Plugin.pm
> @@ -1198,16 +1198,19 @@ sub volume_import {
> # Check for an existing file first since interrupting alloc_image doesn't
> # free it.
> my $file = $class->path($scfg, $volname, $storeid);
> - die "file '$file' already exists\n" if -e $file;
> + if (-e $file) {
> + 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)
> @@ -1227,6 +1230,7 @@ sub volume_import {
> warn $@ if $@;
> die $err;
> }
> + return "$storeid:$volname";
> }
>
> sub volume_import_formats {
> diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
> index d72ee16..6394692 100644
> --- a/PVE/Storage/ZFSPoolPlugin.pm
> +++ b/PVE/Storage/ZFSPoolPlugin.pm
> @@ -750,15 +750,17 @@ 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) {
> + 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") };
> @@ -771,7 +773,7 @@ sub volume_import {
> die $err;
> }
>
> - return;
> + return "$storeid:$dataset";
> }
>
> sub volume_import_formats {
> --
> 2.20.1
>
>
> _______________________________________________
> 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