From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <f.gruenbichler@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits))
(No client certificate requested)
by lists.proxmox.com (Postfix) with ESMTPS id C13656356D
for <pve-devel@lists.proxmox.com>; Wed, 10 Feb 2021 10:41:09 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id B7E60EF1F
for <pve-devel@lists.proxmox.com>; Wed, 10 Feb 2021 10:41:09 +0100 (CET)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
[212.186.127.180])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256)
(No client certificate requested)
by firstgate.proxmox.com (Proxmox) with ESMTPS id CEC20EF15
for <pve-devel@lists.proxmox.com>; Wed, 10 Feb 2021 10:41:07 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 94E2F4415E
for <pve-devel@lists.proxmox.com>; Wed, 10 Feb 2021 10:41:07 +0100 (CET)
Date: Wed, 10 Feb 2021 10:40:56 +0100
From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20210205100442.28163-1-d.jaeger@proxmox.com>
In-Reply-To: <20210205100442.28163-1-d.jaeger@proxmox.com>
MIME-Version: 1.0
User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid)
Message-Id: <1612945336.hphk4jq6ac.astroid@nora.none>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.026 Adjusted score from AWL reputation of From: address
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/,
medium trust
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
information. [ovf.pm, qemu.pm, proxmox.com, qemuserver.pm]
Subject: Re: [pve-devel] [PATCH v4 qemu-server] Add API for disk & VM import
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Wed, 10 Feb 2021 09:41:09 -0000
high level review: this is starting to take shape :)
I wonder whether it doesn't make sense to add proper permissions=20
already? most of it is handled in PVE::Storage::check_volume_access=20
or check_storage_access in this API module already.. the latter could be=20
extended to handle the new special syntax similary to NEW_DISK_RE, and=20
we'd basically have almost no special casing left..
I'm also a bit on the fence whether we really want a separate API call=20
or just a new parameter for the importvm feature. with the switch to a=20
single parameter for the key to source mapping and some of the changes=20
below it might be rather straight-forward - I might try to take your=20
next version and refactor it to be in-line with the regular create_vm to=20
see which variant looks better. just as a heads-up ;)
On February 5, 2021 11:04 am, Dominic J=C3=A4ger wrote:
> Extend qm importdisk/importovf functionality to the API.
> qm can be adapted to use this later.
>=20
> Signed-off-by: Dominic J=C3=A4ger <d.jaeger@proxmox.com>
> ---
> Biggest v3->v4 changes:
> * New code instead of bloating update_vm_api
> * Don't change anything in the existing schema, use new parameter "diskim=
ages"
> * Because this can happen later:
> - Only root can use this
> - Don't touch qm (yet)
>=20
>=20
> PVE/API2/Qemu.pm | 375 +++++++++++++++++++++++++++++++++++++++++-
> PVE/QemuServer.pm | 16 +-
> PVE/QemuServer/OVF.pm | 10 +-
> 3 files changed, 394 insertions(+), 7 deletions(-)
>=20
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 3571f5e..1ed763b 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -45,7 +45,6 @@ BEGIN {
> }
> }
> =20
> -use Data::Dumper; # fixme: remove
> =20
> use base qw(PVE::RESTHandler);
> =20
> @@ -4325,4 +4324,378 @@ __PACKAGE__->register_method({
> return PVE::QemuServer::Cloudinit::dump_cloudinit_config($conf, $param-=
>{vmid}, $param->{type});
> }});
> =20
> +# Raise exception if $format is not supported by $storageid
> +my $check_format_is_supported =3D sub {
> + my ($format, $storageid) =3D @_;
> +
> + return if !$format;
> +
> + my $store_conf =3D PVE::Storage::config();
it probably makes sense to pass this in as parameter, all call sites=20
probably have it already ;)
> + my (undef, $valid_formats) =3D PVE::Storage::storage_default_format(=
$store_conf, $storageid);
> + my $supported =3D grep { $_ eq $format } @$valid_formats;
> +
> + if (!$supported) {
> + raise_param_exc({format =3D> "$format is not supported on storage $stor=
ageid"});
I think a regular die here makes more sense (one of the two call-sites=20
you introduce can have a different parameter than 'format' as source of=20
the value being checked - that would be rather confusing..). it might=20
make it usable for other call-sites as well then. where applicable, you=20
can always raise a param exception with $@ as message ;)
> + }
> +};
> +
> +# paths are returned as is
> +# volids are returned as paths
> +#
> +# Also checks if $original actually exists
> +my $convert_to_path =3D sub {
> + my ($original) =3D @_;
> + my $volid_as_path =3D eval { # Nonempty iff $original_source is a volid
> + PVE::Storage::path(PVE::Storage::config(), $original);
> + };
> + my $result =3D $volid_as_path || $original ;
> + if (!-e $result) {
> + die "Could not import because source '$original' does not exist!";
> + }
> + return $result;
> +};
what does this do that PVE::Storage::abs_filesystem_path doesn't already=20
do (except having 'import' in the error message ;))?
ah, further down below I see that -b might be missing for raw block=20
devices.. maybe it makes sense to allow those in that helper as well?=20
optionally? call-sites would need to be checked..
> +
> +# vmid ... target VM ID
> +# source ... absolute path of the source image (volid must be converted =
before)
that restriction is not actually needed, see below
> +# storage ... target storage for the disk image
> +# format ... target format for the disk image (optional)
> +#
> +# returns ... volid of the allocated disk image (e.g. local-lvm:vm-100-d=
isk-2)
> +my $import_disk_image =3D sub {
> + my ($param) =3D @_;
> + my $vmid =3D $param->{vmid};
> + my $requested_format =3D $param->{format};
> + my $storage =3D $param->{storage};
> + my $source =3D $param->{source};
> +
> + my $vm_conf =3D PVE::QemuConfig->load_config($vmid);
could be passed in, the call sites already have it.. and it might allow=20
to pull in some of the surrounding codes at the call sites that is=20
similar (config updating, property string building) into this helper
> + my $store_conf =3D PVE::Storage::config();
could also be passed in here
> + if (!$source) {
> + die "It is necessary to pass the source parameter";
> + }
just a check for definedness
> + if ($source !~ m!^/!) {
> + die "source must be an absolute path but is $source";
> + }
> + if (!-e $source) {
> + die "Could not import because source $source does not exist!";
> + }
and another call to abs_filesystem_path would do all these checks, and=20
support passing in regular volumes as source as well.
> + if (!$storage) {
> + die "It is necessary to pass the storage parameter";
> + }
call PVE::Storage::storage_config() here to get both that and the check=20
that the storage exists..
> +
> + print "Importing disk image '$source'...\n";
> +
> + my $src_size =3D PVE::Storage::file_size_info($source);
> + if (!defined($src_size)) {
> + die "Could not get file size of $source";
> + } elsif (!$src_size) {
> + die "Size of file $source is 0";
> + } elsif ($src_size=3D=3D1) {
> + die "Cannot import a directory";
can't happen if abs_filesystem_path was used above - that only works for=20
volumes or files ;)
> + }
> +
> + $check_format_is_supported->($requested_format, $storage);
> +
> + my $dst_format =3D PVE::QemuServer::resolve_dst_disk_format(
> + $store_conf, $storage, undef, $requested_format);
> + my $dst_volid =3D PVE::Storage::vdisk_alloc($store_conf, $storage,
> + $vmid, $dst_format, undef, $src_size / 1024);
> +
> + eval {
> + local $SIG{INT} =3D
> + local $SIG{TERM} =3D
> + local $SIG{QUIT} =3D
> + local $SIG{HUP} =3D
> + local $SIG{PIPE} =3D sub { die "Interrupted by signal $!\n"; };
> +
> + my $zeroinit =3D PVE::Storage::volume_has_feature($store_conf,
> + 'sparseinit', $dst_volid);
> +
> + PVE::Storage::activate_volumes($store_conf, [$dst_volid]);
> + PVE::QemuServer::qemu_img_convert($source, $dst_volid,
> + $src_size, undef, $zeroinit);
> + PVE::Storage::deactivate_volumes($store_conf, [$dst_volid]);
> +
> + };
> + if (my $err =3D $@) {
> + eval { PVE::Storage::vdisk_free($store_conf, $dst_volid) };
> + warn "Cleanup of $dst_volid failed: $@ \n" if $@;
> +
> + die "Importing disk '$source' failed: $err\n" if $err;
> + }
> +
> + return $dst_volid;
> +};
> +
> +__PACKAGE__->register_method ({
> + name =3D> 'importdisk',
> + path =3D> '{vmid}/importdisk',
> + method =3D> 'POST',
> + protected =3D> 1, # for worker upid file
huh? no - because we need elevated privileges to allocate disks on=20
storages..
> + proxyto =3D> 'node',
> + description =3D> "Import an external disk image into a VM. The image=
format ".
> + "has to be supported by qemu-img.",
> + parameters =3D> {
> + additionalProperties =3D> 0,
> + properties =3D> {
> + node =3D> get_standard_option('pve-node'),
> + vmid =3D> get_standard_option('pve-vmid',
> + {completion =3D> \&PVE::QemuServer::complete_vmid}),
> + source =3D> {
> + description =3D> "Disk image to import. Can be a volid ".
> + "(local-lvm:vm-104-disk-0), an image on a PVE storage ".
> + "(local:104/toImport.raw) or (for root only) an absolute ".
how does the second work here? and the whole thing is root-only, so that=20
qualifier at the end is redundant ;)
> + "path on the server.",
> + type =3D> 'string',
> + format =3D> 'pve-volume-id-or-absolute-path',
> + },
> + device =3D> {
> + type =3D> 'string',
> + description =3D> "Bus/Device type of the new disk (e.g. 'ide0', ".
> + "'scsi2'). Will add the image as unused disk if omitted.",
> + enum =3D> [PVE::QemuServer::Drive::valid_drive_names()],
> + optional =3D> 1,
> + },
> + device_options =3D> {
> + type =3D> 'string',
> + description =3D> "Options to set for the new disk ".
> + "(e.g. 'discard=3Don,backup=3D0')",
> + optional =3D> 1,
> + requires =3D> 'device',
> + },
> + storage =3D> get_standard_option('pve-storage-id', {
> + description =3D> "The storage to which the image will be imported to."=
,
> + completion =3D> \&PVE::QemuServer::complete_storage,
> + }),
> + format =3D> {
> + type =3D> 'string',
> + description =3D> 'Target format.',
> + enum =3D> [ 'raw', 'qcow2', 'vmdk' ],
> + optional =3D> 1,
> + },
not directly related to this series, but this enum is copied all over=20
the place and might be defined in one place as standard option=20
(qm-new-disk-format ?)
> + digest =3D> get_standard_option('pve-config-digest'),
> + },
> + },
> + returns =3D> { type =3D> 'string'},
> + code =3D> sub {
> + my ($param) =3D @_;
> + my $vmid =3D extract_param($param, 'vmid');
> + my $node =3D extract_param($param, 'node');
> + my $original_source =3D extract_param($param, 'source');
not sure why this gets this name, it's just passed once and not used=20
afterwards.. also, see comments above about how to make this more=20
flexible..
> + my $digest =3D extract_param($param, 'digest');
> + my $device_options =3D extract_param($param, 'device_options');
IMHO this one needs to be parsed - e.g., by adding a fake volume with=20
the syntax used in importvm at the front and parsing it according to the=20
disk schema..
both to catch bogus stuff early, and to make the handling below more=20
robust w.r.t. future changes..
> + my $device =3D extract_param($param, 'device');
> + my $storecfg =3D PVE::Storage::config();
> + my $storeid =3D extract_param($param, 'storage');
> +
> + my $rpcenv =3D PVE::RPCEnvironment::get();
> + my $authuser =3D $rpcenv->get_user();
> +
> + my $format_explicit =3D extract_param($param, 'format');
> + my $format_device_option;
> + if ($device_options) {
> + $device_options =3D~ m/format=3D([^,]*)/;
> + $format_device_option =3D $1;
e.g. here we'd just check $device_options->{format}
> + if ($format_explicit && $format_device_option) {
> + raise_param_exc({format =3D> "Disk format may be specified only once!"=
});
format already specified in device_options?
> + }
> + }
> + my $format =3D $format_explicit || $format_device_option;
since both of those are not needed after this point, this could just be=20
returned / set by the above if, dropping the extra variables in the=20
outer scope..
> + $check_format_is_supported->($format, $storeid);
> +
> + my $locked =3D sub {
> + my $conf =3D PVE::QemuConfig->load_config($vmid);
> + PVE::Tools::assert_if_modified($conf->{digest}, $digest);
> +
> + if ($device && $conf->{$device}) {
> + die "Could not import because device $device is already in ".
> + "use in VM $vmid. Choose a different device!";
both of these checks might make sense outside of the worker already to=20
give immediate feedback in the API response..
> + }
> +
> + my $imported_volid =3D $import_disk_image->({
> + vmid =3D> $vmid,
> + source =3D> $convert_to_path->($original_source),
> + storage =3D> $storeid,
> + format =3D> $format,
> + });
so this could become=20
my $target =3D {
storage =3D> $storeid,
format =3D> $format,
options =3D> $device_options,
device =3D> $device,
};
$import_disk_image($storecfg, $vmid, $conf, $source, $target)
> +
> + my $volid =3D $imported_volid;
> + if ($device) {
> + # Attach with specified options
> + $volid .=3D ",${device_options}" if $device_options;
> + } else {
> + # Add as unused to config
> + $device =3D PVE::QemuConfig->add_unused_volume($conf, $imported_volid)=
;
> + }
> + $update_vm_api->({
> + node =3D> $node,
> + vmid =3D> $vmid,
> + $device =3D> $volid,
> + });
and all of that could move inside the helper
> + };
> + my $worker =3D sub {
> + PVE::QemuConfig->lock_config_full($vmid, 1, $locked);
> + };
> + return $rpcenv->fork_worker('importdisk', $vmid, $authuser, $worker);
> + }});
> +
> +__PACKAGE__->register_method({
> + name =3D> 'importvm',
> + path =3D> '{vmid}/importvm',
> + method =3D> 'POST',
> + description =3D> "Import a VM from existing disk images.",
> + protected =3D> 1,
> + proxyto =3D> 'node',
> + parameters =3D> {
> + additionalProperties =3D> 0,
> + properties =3D> PVE::QemuServer::json_config_properties(
> + {
> + node =3D> get_standard_option('pve-node'),
> + vmid =3D> get_standard_option('pve-vmid', { completion =3D> \&PVE::Clu=
ster::complete_next_vmid }),
> + diskimages =3D> {
> + description =3D> "Mapping of devices to disk images." .
> + "For example, scsi0:/mnt/nfs/image1.vmdk,scsi1:/mnt/nfs/image2",
> + type =3D> 'string',
> + },
> + start =3D> {
> + optional =3D> 1,
> + type =3D> 'boolean',
> + default =3D> 0,
> + description =3D> "Start VM after it was imported successfully.",
> + },
> + }),
> + },
> + returns =3D> {
> + type =3D> 'string',
> + },
> + code =3D> sub {
> + my ($param) =3D @_;
> + my $node =3D extract_param($param, 'node');
> + my $vmid =3D extract_param($param, 'vmid');
> +
> + my $rpcenv =3D PVE::RPCEnvironment::get();
> + my $authuser =3D $rpcenv->get_user();
> + my $storecfg =3D PVE::Storage::config();
> +
> + PVE::Cluster::check_cfs_quorum();
> +
> + my $diskimages_string =3D extract_param($param, 'diskimages');
> + my @diskimage_pairs =3D split(',', $diskimages_string);
> +
> + my $use_import =3D sub {
> + my ($opt) =3D @_;
> + return 0 if $opt eq 'efidisk0';
> + return PVE::QemuServer::Drive::is_valid_drivename($opt);
> + };
> +
> + my $msg =3D "There must be exactly as many devices specified as there "=
.
> + " are devices in the diskimage parameter.\n For example for " .
> + "--scsi0 local-lvm:0,discard=3Don --scsi1 local:0,cache=3Dunsafe " =
.
> + "there must be --diskimages scsi0=3D/source/path,scsi1=3D/other/pat=
h";
> + my $device_count =3D grep { $use_import->($_) } keys %$param;
why though? isn't it sufficient to say there must be a matching device for=20
each diskimages entry and vice-versa?
while doing the matching you could also check that the file exists (as=20
per the comments above ;))
> +
> + my $diskimages_count =3D @diskimage_pairs;
> + if ($device_count !=3D $diskimages_count) {
> + raise_param_exc({diskimages =3D> $msg});
> + }
> +
> + my $diskimages =3D {};
> + foreach ( @diskimage_pairs ) {
> + my ($device, $diskimage) =3D split('=3D', $_);
> + $diskimages->{$device} =3D $diskimage;
> + }
> +
> + my $worker =3D sub {
> + eval { PVE::QemuConfig->create_and_lock_config($vmid, 0, 'import') =
};
> + die "Unable to create config for VM import: $@" if $@;
should happen outside the worker (so that a VMID clash is detected=20
early). inside the worker you just lock and load, then check that the=20
'import' lock is still there..
we could also filter out disk parameters, and call update_vm_api with=20
the rest here (those should be fast, but potentially in the future could=20
run into permission issues that would be nice to return early before=20
doing a massive disk conversion that has to be undone afterwards..
> +
> + my @volids_of_imported =3D ();
> + eval { foreach my $opt (keys %$param) {
this could then just iterate over the disk parameters
> + next if ($opt eq 'start');
> +
> + my $updated_value;
> + if ($use_import->($opt)) {
> + # $opt is bus/device like ide0, scsi5
> +
> + my $device =3D PVE::QemuServer::parse_drive($opt, $param->{$opt});
$drive? $device is used in this very patch for something else ;)
> + raise_param_exc({ $opt =3D> "Unable to parse drive options" })
> + if !$device;
> +
> + my $source_path =3D $convert_to_path->($diskimages->{$opt});
see other comments regarding this
> +
> + $param->{$opt} =3D~ m/format=3D([^,]*)/;
> + my $format =3D $1;
parse!
also, if we allow mixing imports and newly-allocated empty or=20
pre-existing volumes, check whether it's actually one we want to import=20
here ;)
> +
> + my $imported_volid =3D $import_disk_image->({
> + vmid =3D> $vmid,
> + source =3D> $source_path,
> + device =3D> $opt,
> + storage =3D> (split ':', $device->{file})[0],
parse!
> + format =3D> $format,
> + });
also see comment for the other call site about this helper's signature..
> + push @volids_of_imported, $imported_volid;
> +
> + # $param->{opt} has all required options but also dummy
> + # import 0 instead of the image
> + # for example, local-lvm:0,discard=3Don,mbps_rd=3D100
> + my $volid =3D $param->{$opt};
> + # Replace 0 with allocated volid, for example
> + # local-lvm:vm-100-disk-2,discard=3Don,mbps_rd=3D100
> + $volid =3D~ s/^.*?,/$imported_volid,/;
parse/print, move into helper..
> +
> + $updated_value =3D $volid;
> + } else {
> + $updated_value =3D $param->{$opt};
> + }
> + $update_vm_api->(
> + {
> + node =3D> $node,
> + vmid =3D> $vmid,
> + $opt =3D> $updated_value,
> + skiplock =3D> 1,
could just pass in the whole disk parameters instead, if we do the rest=20
early..
> + },
> + 1, # avoid nested workers that only do a short operation
> + );
> + }};
> +
> + my $conf =3D PVE::QemuConfig->load_config($vmid);
> + my $bootdevs =3D PVE::QemuServer::get_default_bootdevices($conf);
> + $update_vm_api->(
> + {
> + node =3D> $node,
> + vmid =3D> $vmid,
> + boot =3D> PVE::QemuServer::print_bootorder($bootdevs),
> + skiplock =3D> 1,
> + },
> + 1,
> + );
should only be done if boot is not already set..
> +
> + my $err =3D $@;
> + if ($err) {
> + foreach my $volid (@volids_of_imported) {
> + eval { PVE::Storage::vdisk_free($storecfg, $volid) };
> + warn $@ if $@;
> + }
> +
> + eval {
> + my $conffile =3D PVE::QemuConfig->config_file($vmid);
> + unlink($conffile) or die "Failed to remove config file: $!\n";
> + };
> + warn $@ if $@;
> +
> + die $err;
> + }
> +
> + eval { PVE::QemuConfig->remove_lock($vmid, 'import') };
> + warn $@ if $@;
> +
> + if ($param->{start}) {
> + PVE::QemuServer::vm_start($storecfg, $vmid);
> + }
> + };
> +
> + return $rpcenv->fork_worker('importvm', $vmid, $authuser, $worker);
> + }});
> +
> +
> 1;
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 9c65d76..c02f5eb 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -300,7 +300,7 @@ my $confdesc =3D {
> optional =3D> 1,
> type =3D> 'string',
> description =3D> "Lock/unlock the VM.",
> - enum =3D> [qw(backup clone create migrate rollback snapshot snapshot-de=
lete suspending suspended)],
> + enum =3D> [qw(backup clone create migrate rollback snapshot snapshot-de=
lete suspending suspended import)],
> },
> cpulimit =3D> {
> optional =3D> 1,
> @@ -998,6 +998,18 @@ sub verify_volume_id_or_qm_path {
> return $volid;
> }
> =20
> +PVE::JSONSchema::register_format('pve-volume-id-or-absolute-path', \&ver=
ify_volume_id_or_absolute_path);
> +sub verify_volume_id_or_absolute_path {
> + my ($volid, $noerr) =3D @_;
> +
> + # Exactly these 2 are allowed in id_or_qm_path but should not be all=
owed here
> + if ($volid eq 'none' || $volid eq 'cdrom') {
> + return undef if $noerr;
> + die "Invalid format! Should be volume ID or absolute path.";
> + }
> + return verify_volume_id_or_qm_path($volid, $noerr);
> +}
> +
> my $usb_fmt =3D {
> host =3D> {
> default_key =3D> 1,
> @@ -6659,7 +6671,7 @@ sub qemu_img_convert {
> $src_path =3D PVE::Storage::path($storecfg, $src_volid, $snapname);
> $src_is_iscsi =3D ($src_path =3D~ m|^iscsi://|);
> $cachemode =3D 'none' if $src_scfg->{type} eq 'zfspool';
> - } elsif (-f $src_volid) {
> + } elsif (-f $src_volid || -b _) { # -b required to import from LVM i=
mages
> $src_path =3D $src_volid;
> if ($src_path =3D~ m/\.($PVE::QemuServer::Drive::QEMU_FORMAT_RE)$/) {
> $src_format =3D $1;
> diff --git a/PVE/QemuServer/OVF.pm b/PVE/QemuServer/OVF.pm
> index c76c199..36b7fff 100644
> --- a/PVE/QemuServer/OVF.pm
> +++ b/PVE/QemuServer/OVF.pm
> @@ -87,7 +87,7 @@ sub id_to_pve {
> =20
> # returns two references, $qm which holds qm.conf style key/values, and =
\@disks
> sub parse_ovf {
> - my ($ovf, $debug) =3D @_;
> + my ($ovf, $debug, $ignore_size) =3D @_;
> =20
> my $dom =3D XML::LibXML->load_xml(location =3D> $ovf, no_blanks =3D>=
1);
> =20
> @@ -220,9 +220,11 @@ ovf:Item[rasd:InstanceID=3D'%s']/rasd:ResourceType",=
$controller_id);
> die "error parsing $filepath, file seems not to exist at $backing_f=
ile_path\n";
> }
> =20
> - my $virtual_size;
> - if ( !($virtual_size =3D PVE::Storage::file_size_info($backing_file_pat=
h)) ) {
> - die "error parsing $backing_file_path, size seems to be $virtual_si=
ze\n";
> + my $virtual_size =3D 0;
> + if (!$ignore_size) { # Not possible if manifest is uploaded in web gui
> + if ( !($virtual_size =3D PVE::Storage::file_size_info($backing_file=
_path)) ) {
> + die "error parsing $backing_file_path: Could not get file size info: $=
@\n";
> + }
> }
> =20
> $pve_disk =3D {
> --=20
> 2.20.1
>=20
>=20
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>=20
=