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.eb...@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 _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel