Re: [pve-devel] [RFC v10 qemu-server 6/7] api: support VM disk import

[Fabian Ebner <f.ebner@proxmox.com>]

Am 13.01.22 um 11:08 schrieb Fabian Ebner:
> +
> +	    if (my $source = delete $disk->{'import-from'}) {

I'm adding a comment here in v11, because otherwise it's not clear where 
volume activation happens:
+               # abs_filesystem_path also calls activate_volume when 
$source is a volid

I'm also adding "The source should not be actively used by another 
process!" to the description of the import-from parameter in v11.

> +		$source = PVE::Storage::abs_filesystem_path($storecfg, $source, 1);

But there are a couple of issues here:

1. There's no protection against using a source volume that's actively 
used by a guest/other operation. While it's not possible to detect in 
general, I wonder if we should behave more like a full clone and lock 
the owning VM?

1a. we could check if the volume is referenced in the config/snapshots, 
but migration picks up everything, so it might be preferable not to.

1b. the volume might be configured in a VM that doesn't own it...

2. Related: avoiding concurrent activation of volumes on a shared LVM.

3. Related: cannot deactivate any volumes as the might be used by 
something else.

4. abs_filesystem_path does not work for RBD when krbd=0, because the 
plugin produces an "rbd:XYZ" path and the -f || -b check doesn't like 
that. But full clone does work, passing the volid to qemu_img_convert 
and that's likely what we should do here as well, when we are dealing 
with an existing volid rather than an absolute path.

5. Add your own ;)

TL;DR: I'd like to behave much more like full clone, when we are dealing 
with a volid rather than an absolute path.

> +		my $src_size = PVE::Storage::file_size_info($source);
> +		die "Could not get file size of $source" if !defined($src_size);
> +
> +		my (undef, $dst_volid) = PVE::QemuServer::ImportDisk::do_import(