[pve-devel] [RFC storage 6/16] pvesm import: allow specifying storage+vmid instead of full volumeid
Fabian Ebner
f.ebner at proxmox.com
Wed Jan 29 14:30:04 CET 2020
Extends the API so that 'volume' can also only be a storage identifier. In
that case the VMID needs to be specified as well. In 'import_volume' a new
name for the allocation is determined. This is useful for migration where
the storage on the target is a different type, since the volume ID might
look differently. E.g. 'mydir:102/vm-102-disk-0.raw' on a 'dir'
storage would be 'mylvm:vm-102-disk-0' on an 'lvm' storage.
Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
---
An alternative approach would be to translate the volids as mentioned
in the cover letter.
It's not a pretty interface, but to not break backwards compatibility
the 'volume' parameter needs to be non-optional (or is there a way to get
around this?) and so it is a hybrid of either full ID or only storage ID.
PVE/CLI/pvesm.pm | 30 +++++++++++++++++++++++----
PVE/Storage.pm | 6 ++----
PVE/Storage/LVMPlugin.pm | 28 ++++++++++++++++----------
PVE/Storage/Plugin.pm | 39 ++++++++++++++++++++++--------------
PVE/Storage/ZFSPoolPlugin.pm | 32 ++++++++++++++++++++---------
5 files changed, 91 insertions(+), 44 deletions(-)
diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
index 74294b4..d91bc31 100755
--- a/PVE/CLI/pvesm.pm
+++ b/PVE/CLI/pvesm.pm
@@ -261,7 +261,9 @@ __PACKAGE__->register_method ({
additionalProperties => 0,
properties => {
volume => {
- description => "Volume identifier",
+ description => "Either a complete volume identifier of the form " .
+ "'storage:volume_name' or just the name of a storage. In the " .
+ "latter case the parameter vmid is required.",
type => 'string',
completion => \&PVE::Storage::complete_volume,
},
@@ -297,6 +299,11 @@ __PACKAGE__->register_method ({
maxLength => 80,
optional => 1,
},
+ vmid => get_standard_option('pve-vmid', {
+ description => "Only needed when no volume name is specified.",
+ optional => 1,
+ completion => \&PVE::Cluster::complete_vmid,
+ }),
},
},
returns => { type => 'string' },
@@ -350,10 +357,25 @@ __PACKAGE__->register_method ({
}
my $cfg = PVE::Storage::config();
- my $volume = $param->{volume};
+ my $storeid_or_volid = $param->{volume};
+ my $vmid = $param->{vmid};
+
+ my ($storeid, $volname) = PVE::Storage::parse_volume_id($storeid_or_volid, 1);
+ if (!defined($volname) || !defined($storeid)) { # assume it's only a storeid
+ $storeid = PVE::JSONSchema::parse_storage_id($storeid_or_volid);
+ $volname = undef;
+ die "vmid needs to be specified if there is no volume name\n"
+ if !defined($vmid);
+ }
+
+ if (defined($vmid) && defined($volname)) {
+ warn "ignoring parameter vmid='$vmid' since a volume name was specified\n";
+ $vmid = undef;
+ }
+
my $delete = $param->{'delete-snapshot'};
- my $volid = PVE::Storage::volume_import($cfg, $infh, $volume, $param->{format},
- $param->{base}, $param->{'with-snapshots'});
+ my $volid = PVE::Storage::volume_import($cfg, $infh, $storeid, $volname,
+ $param->{format}, $param->{base}, $param->{'with-snapshots'}, $vmid);
PVE::Storage::volume_snapshot_delete($cfg, $volid, $delete)
if defined($delete);
return $volid;
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index 57d723d..7d18b11 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -1420,14 +1420,12 @@ sub volume_export {
}
sub volume_import {
- my ($cfg, $fh, $volid, $format, $base_snapshot, $with_snapshots) = @_;
+ my ($cfg, $fh, $storeid, $volname, $format, $base_snapshot, $with_snapshots, $vmid) = @_;
- 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);
+ $base_snapshot, $with_snapshots, $vmid);
}
sub volume_export_formats {
diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
index 6c575f2..7250876 100644
--- a/PVE/Storage/LVMPlugin.pm
+++ b/PVE/Storage/LVMPlugin.pm
@@ -624,26 +624,32 @@ 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, $vmid) = @_;
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"
if $with_snapshots;
die "cannot import an incremental stream in $class\n" if defined($base_snapshot);
- my ($vtype, $name, $vmid, $basename, $basevmid, $isBase, $file_format) =
- $class->parse_volname($volname);
- die "cannot import format $format into a file of format $file_format\n"
- if $file_format ne 'raw';
+ my $name;
- my $vg = $scfg->{vgname};
- my $lvs = lvm_list_volumes($vg);
+ if (defined($volname)) {
+ (undef, $name, $vmid, undef, undef, undef, my $file_format) =
+ $class->parse_volname($volname);
+ die "cannot import format $format into a file of format $file_format\n"
+ if $file_format ne 'raw';
- if ($lvs->{$vg}->{$volname}) {
- warn "volume $vg/$volname already exists - importing with a different name\n";
- $name = undef;
+ my $vg = $scfg->{vgname};
+ my $lvs = lvm_list_volumes($vg);
+
+ if ($lvs->{$vg}->{$volname}) {
+ warn "volume $vg/$volname already exists - importing with a different name\n";
+ $name = undef;
+ }
}
+ die "no vmid specified\n" if !defined($vmid);
+
my ($size) = PVE::Storage::Plugin::read_common_header($fh);
$size = int($size/1024);
@@ -651,7 +657,7 @@ sub volume_import {
my $allocname = $class->alloc_image($storeid, $scfg, $vmid, 'raw', $name, $size);
my $oldname = $volname;
$volname = $allocname;
- if (defined($name) && $allocname ne $oldname) {
+ if (defined($name) && defined($oldname) && $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 358a831..4e0ae8f 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -1176,7 +1176,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, $vmid) = @_;
die "volume import format '$format' not available for $class\n"
if $format !~ /^(raw|tar|qcow2|vmdk)\+size$/;
@@ -1187,22 +1187,31 @@ sub volume_import {
die "format $format cannot be imported with snapshots\n"
if $with_snapshots && ($data_format eq 'raw' || $data_format eq 'tar');
- my ($vtype, $name, $vmid, $basename, $basevmid, $isBase, $file_format) =
- $class->parse_volname($volname);
+ my $name;
+ my $file_format;
+
+ if (defined($volname)) {
+ (undef, $name, $vmid, undef, undef, undef, $file_format) =
+ $class->parse_volname($volname);
+
+ # XXX: Should we bother with conversion routines at this level? This won't
+ # happen without manual CLI usage, so for now we just error out...
+ die "cannot import format $format into a file of format $file_format\n"
+ if $data_format ne $file_format && !($data_format eq 'tar' && $file_format eq 'subvol');
- # XXX: Should we bother with conversion routines at this level? This won't
- # happen without manual CLI usage, so for now we just error out...
- die "cannot import format $format into a file of format $file_format\n"
- if $data_format ne $file_format && !($data_format eq 'tar' && $file_format eq 'subvol');
-
- # Check for an existing file first since interrupting alloc_image doesn't
- # free it.
- my $file = $class->path($scfg, $volname, $storeid);
- if (-e $file) {
- warn "file '$file' already exists - importing with a different name\n";
- $name = undef;
+ # Check for an existing file first since interrupting alloc_image doesn't
+ # free it.
+ my $file = $class->path($scfg, $volname, $storeid);
+ if (-e $file) {
+ warn "file '$file' already exists - importing with a different name\n";
+ $name = undef;
+ }
+ } else {
+ $file_format = ($data_format eq 'tar') ? 'subvol' : $data_format;
}
+ die "no vmid specified\n" if !defined($vmid);
+
my ($size) = read_common_header($fh);
$size = int($size/1024);
@@ -1210,7 +1219,7 @@ sub volume_import {
my $allocname = $class->alloc_image($storeid, $scfg, $vmid, $file_format, $name, $size);
my $oldname = $volname;
$volname = $allocname;
- if (defined($name) && $allocname ne $oldname) {
+ if (defined($name) && defined($oldname) && $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 6394692..0ef6768 100644
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -741,7 +741,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, $vmid) = @_;
die "unsupported import stream format for $class: $format\n"
if $format ne 'zfs';
@@ -750,15 +750,27 @@ sub volume_import {
die "internal error: invalid file handle for volume_import\n"
if !defined($fd);
- 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;
- } elsif ($exists) {
- warn "volume '$zfspath' already exists - importing with a different name\n";
+ die "base snapshot was specified, but no volume name\n"
+ if defined($base_snapshot) && !defined($volname);
+
+ my $zfspath;
+ my $dataset;
+
+ if (defined($volname)) {
+ (undef, $dataset, $vmid) = $class->parse_volname($volname);
+ $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;
+ } 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";
+ }
+ } else {
+ die "no vmid specified\n" if !defined($vmid);
$dataset = $class->find_free_diskname($storeid, $scfg, $vmid, $format);
$zfspath = "$scfg->{pool}/$dataset";
}
--
2.20.1
More information about the pve-devel
mailing list