From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 3D7CA7E1CE for ; Wed, 10 Nov 2021 08:41:22 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2BC4E1796C for ; Wed, 10 Nov 2021 08:40:52 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (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 E31181795F for ; Wed, 10 Nov 2021 08:40:50 +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 B43C642910 for ; Wed, 10 Nov 2021 08:40:44 +0100 (CET) To: pve-devel@lists.proxmox.com, =?UTF-8?Q?Fabian_Gr=c3=bcnbichler?= References: <20211105130359.40803-1-f.gruenbichler@proxmox.com> <20211105130359.40803-20-f.gruenbichler@proxmox.com> <06755979-9217-f572-384a-2825631f4f8f@proxmox.com> From: Fabian Ebner Message-ID: <97332e83-f7e1-aa6e-3932-2ba9de52b5cc@proxmox.com> Date: Wed, 10 Nov 2021 08:40:39 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: <06755979-9217-f572-384a-2825631f4f8f@proxmox.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 1.051 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -1.678 Looks like a legit reply (A) 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. [vmid.storage] Subject: Re: [pve-devel] [PATCH qemu-server 07/10] mtunnel: add API endpoints X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Nov 2021 07:41:22 -0000 Am 09.11.21 um 13:46 schrieb Fabian Ebner: > Am 05.11.21 um 14:03 schrieb Fabian Grünbichler: ---snip--- >>   use IO::Socket::IP; >> +use IO::Socket::UNIX; >> +use IPC::Open3; >> +use JSON; >> +use MIME::Base64; Forgot to ask: is this import needed or a left-over from development? ---snip--- > >> + >> +            my $migration_snapshot; >> +            if ($scfg->{type} eq 'zfspool' || $scfg->{type} eq >> 'btrfs') { >> +            $migration_snapshot = '__migration__'; >> +            } >> + >> +            my $volid = "$storeid:$volname"; >> + >> +            # find common import/export format, taken from PVE::Storage >> +            my @import_formats = >> PVE::Storage::volume_import_formats($state->{storecfg}, $volid, >> $migration_snapshot, undef, $with_snapshots); >> +            my @export_formats = >> PVE::Tools::split_list($params->{'export-formats'}); >> +            my %import_hash = map { $_ => 1 } @import_formats; >> +            my @common = grep { $import_hash{$_} } @export_formats; >> +            die "no matching import/export format found for storage >> '$storeid'\n" >> +            if !@common; >> +            $format = $common[0]; >> + >> +            my $input = IO::File->new(); >> +            my $info = IO::File->new(); >> +            my $unix = "/run/qemu-server/$vmid.storage"; >> + >> +            my $import_cmd = ['pvesm', 'import', $volid, $format, >> "unix://$unix", '-with-snapshots', $with_snapshots]; >> +            if ($params->{'allow-rename'}) { >> +            push @$import_cmd, '-allow-rename', >> $params->{'allow-rename'}; >> +            } >> +            if ($migration_snapshot) { >> +            push @$import_cmd, '-delete-snapshot', $migration_snapshot; > > Missing '-snapshot $migration_snapshot'? While the parameter is ignored > by our ZFSPoolPlugin, the BTRFSPlugin aborts if it's not specified > AFAICS. And external plugins might require it too. That is, for the 'btrfs' format. In the patch with the export command, a snapshot is only used for ZFS, so it would already fail on export for BTRFS with 'btrfs' format. For external plugins we also don't use a migration snapshot in storage_migrate(), so please disregard that part. > > In general, we'll need to be careful not to introduce mismatches between > the import and the export parameters. Might it be better if the client > would pass along (most of) the parameters for the import command (which > basically is how it's done for the existing storage_migrate)? > On the other hand, that would require being very careful with input validation. ---snip---