From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <f.ebner@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) server-digest SHA256)
(No client certificate requested)
by lists.proxmox.com (Postfix) with ESMTPS id 152759B718
for <pve-devel@lists.proxmox.com>; Thu, 25 May 2023 10:15:10 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id F251E27E35
for <pve-devel@lists.proxmox.com>; Thu, 25 May 2023 10:15:09 +0200 (CEST)
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))
(No client certificate requested)
by firstgate.proxmox.com (Proxmox) with ESMTPS
for <pve-devel@lists.proxmox.com>; Thu, 25 May 2023 10:15:09 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 3F6C747071
for <pve-devel@lists.proxmox.com>; Thu, 25 May 2023 10:15:09 +0200 (CEST)
Message-ID: <ad5a6260-af0e-a18e-585e-e3d6459d7d36@proxmox.com>
Date: Thu, 25 May 2023 10:15:08 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
Thunderbird/102.10.0
Content-Language: en-US
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Aaron Lauterer <a.lauterer@proxmox.com>
References: <20230512124043.888785-1-a.lauterer@proxmox.com>
<20230512124043.888785-4-a.lauterer@proxmox.com>
From: Fiona Ebner <f.ebner@proxmox.com>
In-Reply-To: <20230512124043.888785-4-a.lauterer@proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.005 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
NICE_REPLY_A -0.107 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
T_SCC_BODY_TEXT_LINE -0.01 -
Subject: Re: [pve-devel] [PATCH qemu-server v2 3/6] migration: fail when
aliased volume is detected
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: Thu, 25 May 2023 08:15:10 -0000
Am 12.05.23 um 14:40 schrieb Aaron Lauterer:
> Aliased volumes (referencing the same disk image multiple times) can
> lead to unexpected behavior in a migration.
>
> Therefore, stop the migration in such a case.
>
> The check works by comparing the path returned by the storage plugin.
> This means that we should be able to catch the common situations where
> it can happen:
>
> * by referencing the same volid multiple times
This situation is not actually caught by the patch? If it's the same
volid, it will resolve to the same path, so it will just execute
$path_to_volid->{$path}->{$volid} = 1;
multiple times and there won't be any detection of multi-reference, or
what am I missing? Just incrementing the counter instead also doesn't
work, because e.g. multi-reference is fine when done by snapshots.
Maybe there should be a helper just for the currently attached disks?
E.g. iterate over the currently attached disks with
QemuConfig->foreach_volume() and for each volume, resolve the path and
count. If any path is referenced multiple times, complain. To not lose
the volids while counting, you could e.g. "count" with
push $path_to_volid->{$path}->@*, $volid;
The helper can then be called during migration additionally to your
current patch here.
And the helper can also be re-used during VM start (maybe only warn
there to allow for exotic use-cases?) and I'd expect it to already catch
most problematic situations there.
> @@ -397,6 +397,8 @@ sub scan_local_volumes {
> die "owned by other VM (owner = VM $owner)\n"
> if !$owner || ($owner != $vmid);
>
> + $path_to_volid->{$path}->{$volid} = 1;
> +
> return if $attr->{is_vmstate};
>
> if (defined($snaprefs)) {
> @@ -443,6 +445,12 @@ sub scan_local_volumes {
> }
> });
>
> + for my $path (keys %$path_to_volid) {
> + my @volids = keys %{$path_to_volid->{$path}};
> + die "detected not supported aliased volumes: '" . join("', '", @volids) . "'"
> + if (scalar @volids > 1);
> + }
> +
> foreach my $vol (sort keys %$local_volumes) {
> my $type = $replicatable_volumes->{$vol} ? 'local, replicated' : 'local';
> my $ref = $local_volumes->{$vol}->{ref};