public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Dominik Csapak <d.csapak@proxmox.com>
Subject: Re: [pve-devel] [PATCH qemu-server v6 4/6] api: create: implement extracting disks when needed for import-from
Date: Mon, 18 Nov 2024 14:31:18 +0100	[thread overview]
Message-ID: <a919d4ff-22bd-413a-aa68-0d04c34a9d30@proxmox.com> (raw)
In-Reply-To: <20241115151749.633407-17-d.csapak@proxmox.com>

Am 15.11.24 um 16:17 schrieb Dominik Csapak:
> @@ -416,6 +427,23 @@ my sub create_disks : prototype($$$$$$$$$$) {
>  		my ($source_storage, $source_volid) = PVE::Storage::parse_volume_id($source, 1);
>  
>  		if ($source_storage) { # PVE-managed volume
> +		    my ($vtype, undef, undef, undef, undef, undef, $fmt)
> +			= PVE::Storage::parse_volname($storecfg, $source);
> +		    my $needs_extraction = PVE::QemuServer::Helpers::needs_extraction($vtype, $fmt);
> +		    if ($needs_extraction) {
> +			print "extracting $source\n";
> +			my $extracted_volid
> +			     = PVE::GuestImport::extract_disk_from_import_file($source, $vmid);
> +			print "finished extracting to $extracted_volid\n";
> +			push @$vollist, $extracted_volid;
> +			$source = $extracted_volid;
> +
> +			my (undef, undef, undef, $parent)
> +			    = PVE::Storage::volume_size_info($storecfg, $source);
> +			die "importing from extracted images with backing file ($parent) not allowed\n"
> +			    if $parent;
> +		    }
> +
>  		    if ($live_import && $ds ne 'efidisk0') {
>  			my $path = PVE::Storage::path($storecfg, $source)
>  			    or die "failed to get a path for '$source'\n";

Below here is a $source = $path

> @@ -424,9 +452,11 @@ my sub create_disks : prototype($$$$$$$$$$) {
>  
>  			die "could not get file size of $source\n" if !$size;
>  			$live_import_mapping->{$ds} = {
> -			    path => $source,
> +			    path => $path,

So this doesn't change anything. It's nicer to read though :P

>  			    format => $source_format,
>  			};
> +			$live_import_mapping->{$ds}->{'delete-after-finish'} = $source

But here you already have $path assigned to $source rather than the
original volume ID. Doesn't vdisk_free() fail later then?

> +			    if $needs_extraction;
>  		    } else {
>  			my $dest_info = {
>  			    vmid => $vmid,
> @@ -438,8 +468,14 @@ my sub create_disks : prototype($$$$$$$$$$) {
>  			$dest_info->{efisize} = PVE::QemuServer::get_efivars_size($conf, $disk)
>  			    if $ds eq 'efidisk0';
>  
> -			($dst_volid, $size) = eval {
> -			    $import_from_volid->($storecfg, $source, $dest_info, $vollist);
> +			eval {
> +			    ($dst_volid, $size)
> +				= $import_from_volid->($storecfg, $source, $dest_info, $vollist);
> +
> +			    # remove extracted volumes after importing
> +			    PVE::Storage::vdisk_free($storecfg, $source) if $needs_extraction;
> +			    print "cleaned up extracted image $source\n";
> +			    @$vollist = grep { $_ ne $source } @$vollist;
>  			};
>  			die "cannot import from '$source' - $@" if $@;
>  		    }
> @@ -1964,7 +2000,6 @@ my $update_vm_api  = sub {
>  
>  		    assert_scsi_feature_compatibility($opt, $conf, $storecfg, $param->{$opt})
>  			if $opt =~ m/^scsi\d+$/;
> -
>  		    my (undef, $created_opts) = create_disks(
>  			$rpcenv,
>  			$authuser,

Unrelated hunk should not be here



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


  reply	other threads:[~2024-11-18 13:31 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-15 15:17 [pve-devel] [PATCH storage/qemu-server/manager v6] implement ova/ovf import for file based storages Dominik Csapak
2024-11-15 15:17 ` [pve-devel] [PATCH storage v6 01/12] copy OVF.pm from qemu-server Dominik Csapak
2024-11-17 15:50   ` [pve-devel] applied: " Thomas Lamprecht
2024-11-15 15:17 ` [pve-devel] [PATCH storage v6 02/12] plugin: dir: implement import content type Dominik Csapak
2024-11-18 12:16   ` Fiona Ebner
2024-11-15 15:17 ` [pve-devel] [PATCH storage v6 03/12] plugin: dir: handle ova files for import Dominik Csapak
2024-11-18 12:17   ` Fiona Ebner
2024-11-15 15:17 ` [pve-devel] [PATCH storage v6 04/12] ovf: improve and simplify path checking code Dominik Csapak
2024-11-18 12:25   ` Fiona Ebner
2024-11-15 15:17 ` [pve-devel] [PATCH storage v6 05/12] ovf: implement parsing the ostype Dominik Csapak
2024-11-15 15:17 ` [pve-devel] [PATCH storage v6 06/12] ovf: implement parsing out firmware type Dominik Csapak
2024-11-15 15:17 ` [pve-devel] [PATCH storage v6 07/12] ovf: implement rudimentary boot order Dominik Csapak
2024-11-15 15:17 ` [pve-devel] [PATCH storage v6 08/12] ovf: implement parsing nics Dominik Csapak
2024-11-15 15:17 ` [pve-devel] [PATCH storage v6 09/12] api: allow ova upload/download Dominik Csapak
2024-11-18 12:42   ` Fiona Ebner
2024-11-15 15:17 ` [pve-devel] [PATCH storage v6 10/12] plugin: enable import for nfs/btrfs/cifs/cephfs/glusterfs Dominik Csapak
2024-11-15 15:17 ` [pve-devel] [PATCH storage v6 11/12] add 'import' content type to 'check_volume_access' Dominik Csapak
2024-11-18 12:58   ` Fiona Ebner
2024-11-15 15:17 ` [pve-devel] [PATCH storage v6 12/12] plugin: file_size_info: don't ignore base path with whitespace Dominik Csapak
2024-11-17 15:16   ` Thomas Lamprecht
2024-11-18  7:42     ` Dominik Csapak
2024-11-18  7:48       ` Thomas Lamprecht
2024-11-15 15:17 ` [pve-devel] [PATCH qemu-server v6 1/6] disk import: add additional safeguards for imported image files Dominik Csapak
2024-11-18 13:08   ` Fiona Ebner
2024-11-15 15:17 ` [pve-devel] [PATCH qemu-server v6 2/6] api: delete unused OVF.pm Dominik Csapak
2024-11-17 15:18   ` [pve-devel] applied: " Thomas Lamprecht
2024-11-15 15:17 ` [pve-devel] [PATCH qemu-server v6 3/6] use OVF from Storage Dominik Csapak
2024-11-17 17:42   ` Thomas Lamprecht
2024-11-15 15:17 ` [pve-devel] [PATCH qemu-server v6 4/6] api: create: implement extracting disks when needed for import-from Dominik Csapak
2024-11-18 13:31   ` Fiona Ebner [this message]
2024-11-18 13:36     ` Dominik Csapak
2024-11-15 15:17 ` [pve-devel] [PATCH qemu-server v6 5/6] api: create: add 'import-extraction-storage' parameter Dominik Csapak
2024-11-17 16:13   ` Thomas Lamprecht
2024-11-15 15:17 ` [pve-devel] [PATCH qemu-server v6 6/6] api: check untrusted image files for import content type Dominik Csapak
2024-11-18 14:48   ` Fiona Ebner
2024-11-15 15:17 ` [pve-devel] [PATCH manager v6 1/9] ui: fix special 'import' icon for non-esxi storages Dominik Csapak
2024-11-17 16:21   ` [pve-devel] applied: " Thomas Lamprecht
2024-11-18  8:47     ` Dominik Csapak
2024-11-18  9:56       ` Thomas Lamprecht
2024-11-15 15:17 ` [pve-devel] [PATCH manager v6 2/9] ui: guest import: add ova-needs-extracting warning text Dominik Csapak
2024-11-17 16:29   ` Thomas Lamprecht
2024-11-15 15:17 ` [pve-devel] [PATCH manager v6 3/9] ui: enable import content type for relevant storages Dominik Csapak
2024-11-15 15:17 ` [pve-devel] [PATCH manager v6 4/9] ui: enable upload/download/remove buttons for 'import' type storages Dominik Csapak
2024-11-15 15:17 ` [pve-devel] [PATCH manager v6 5/9] ui: disable 'import' button for non importable formats Dominik Csapak
2024-11-15 15:17 ` [pve-devel] [PATCH manager v6 6/9] ui: import: improve rendering of volume names Dominik Csapak
2024-11-15 15:17 ` [pve-devel] [PATCH manager v6 7/9] ui: guest import: add storage selector for ova extraction storage Dominik Csapak
2024-11-17 16:31   ` Thomas Lamprecht
2024-11-15 15:17 ` [pve-devel] [PATCH manager v6 8/9] ui: guest import: change icon/text for non-esxi import storage Dominik Csapak
2024-11-15 15:17 ` [pve-devel] [PATCH manager v6 9/9] ui: import: show size for dir-based storages Dominik Csapak
2024-11-17 16:37 ` [pve-devel] [PATCH storage/qemu-server/manager v6] implement ova/ovf import for file based storages Thomas Lamprecht
2024-11-18 13:06 ` Lukas Wagner
2024-11-18 13:18   ` Dominik Csapak
2024-11-18 13:39     ` Lukas Wagner
2024-11-18 13:44       ` Dominik Csapak
2024-11-18 13:53         ` Dominik Csapak
2024-11-19  8:15           ` Lukas Wagner
2024-11-19  8:44             ` Dominik Csapak
2024-11-19  8:48             ` Thomas Lamprecht
2024-11-20 16:32               ` Gilberto Ferreira via pve-devel
2024-11-20 16:57                 ` Gilberto Ferreira via pve-devel
2024-11-21  8:24                   ` Dominik Csapak
2024-11-21 12:05                     ` Gilberto Ferreira via pve-devel
2024-11-21 12:23                       ` Gilberto Ferreira via pve-devel
2024-11-21 12:34                         ` Fabian Grünbichler
2024-11-18 14:35 ` Daniel Herzig
2024-11-18 15:01   ` Daniel Herzig
2024-11-18 15:33 ` Dominik Csapak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a919d4ff-22bd-413a-aa68-0d04c34a9d30@proxmox.com \
    --to=f.ebner@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal