From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 2EF2C1FF16B
for <inbox@lore.proxmox.com>; Thu, 3 Apr 2025 15:07:53 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 83D9E1D57;
Thu, 3 Apr 2025 15:07:40 +0200 (CEST)
Message-ID: <1e85aa62-dd7d-4b33-b754-3e7207ee69a5@proxmox.com>
Date: Thu, 3 Apr 2025 15:07:06 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird Beta
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20250311132055.2826686-1-d.csapak@proxmox.com>
<20250311132055.2826686-9-d.csapak@proxmox.com>
<72026c72-5ac0-4e76-8387-c95137529e46@proxmox.com>
Content-Language: en-US
From: Dominik Csapak <d.csapak@proxmox.com>
In-Reply-To: <72026c72-5ac0-4e76-8387-c95137529e46@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.021 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
RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
Validity was blocked. See
https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
information.
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH qemu-server v7 6/9] api: enable live
migration for marked mapped pci devices
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>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.proxmox.com>
On 4/3/25 15:00, Thomas Lamprecht wrote:
> Am 11.03.25 um 14:20 schrieb Dominik Csapak:
>> They have to be marked as 'live-migration-capable' in the mapping
>> config, and the driver and qemu must support it.
>>
>> For the gui checks, we now return the whole object of the mapped
>> resources, which includes info like the name and if it's marked as
>> live-migration capable. (while deprecating the old 'mapped-resource'
>> return value, since that returns strictly less information)
>>
>> Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
>> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> PVE/API2/Qemu.pm | 8 +++++++-
>> PVE/QemuMigrate.pm | 17 ++++++++++++-----
>> 2 files changed, 19 insertions(+), 6 deletions(-)
>>
>> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
>> index 060bca8b..15bf7548 100644
>> --- a/PVE/API2/Qemu.pm
>> +++ b/PVE/API2/Qemu.pm
>> @@ -4665,13 +4665,18 @@ __PACKAGE__->register_method({
>> },
>> description => "List local resources (e.g. pci, usb) that block migration."
>> },
>> + # FIXME: remove with 9.0
>> 'mapped-resources' => {
>> type => 'array',
>> items => {
>> type => 'string',
>> description => "A mapped resource",
>> },
>> - description => "List of mapped resources e.g. pci, usb"
>> + description => "List of mapped resources e.g. pci, usb. Deprecated, use 'mapped-resource-info' instead."
>> + },
>> + 'mapped-resource-info' => {
>> + type => 'object',
>> + description => "Object of mapped resources with additional information such if they're live migratable.",
>
> Where is the schema/format for this? New stuff really should be documented
> here, I mean, have you not sent enough schema updates to unlock basic features
> in the rust based PVE api crate used in PDM? ;-)
Yes, you're right. I'll send a follow up for that if that's fine with you.
>
>> },
>> },
>> },
>> @@ -4737,6 +4742,7 @@ __PACKAGE__->register_method({
>>
>> $res->{local_resources} = $local_resources;
>> $res->{'mapped-resources'} = [ sort keys $mapped_resources->%* ];
>> + $res->{'mapped-resource-info'} = $mapped_resources;
>
> would be IMO nicer to add returning that info upfront and then combine this
> with patch with 4/9 to have two patches that each do one thing, and not a
> split/mix.
>
> But I probably will just apply this as is with trying to bend the commit
> message from implementation details to the effect of the patch, will not
> be ideal, but I want to finish this finally..
If you want I can try to rework this part of the series, since it does make
sense like you described, but I'm not sure I could finish that today.
>
> No offense to anybody, but IMO does not pain the best picture considering
> this has two R-b tags..
>
>>
>> return $res;
>>
>> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
>> index 2153ac42..6909fc82 100644
>> --- a/PVE/QemuMigrate.pm
>> +++ b/PVE/QemuMigrate.pm
>> @@ -243,11 +243,18 @@ sub prepare {
>>
>> if (scalar(keys $mapped_res->%*)) {
>> my $missing_mappings = $missing_mappings_by_node->{$self->{node}};
>> - if ($running) {
>> - my $mapped_text = join(", ", sort keys $mapped_res->%*);
>> - die "can't migrate running VM which uses mapped devices: $mapped_text\n";
>> - } elsif (scalar($missing_mappings->@*)) {
>> - die "can't migrate to '$self->{node}': missing mapped devices " . join(", ", $missing_mappings->@*) . "\n";
>> + my $missing_live_mappings = [];
>> + for my $key (sort keys $mapped_res->%*) {
>> + my $res = $mapped_res->{$key};
>> + my $name = "$key:$res->{name}";
>> + push $missing_live_mappings->@*, $name if !$res->{'live-migration'};
>> + }
>> + if (scalar($missing_mappings->@*)) {
>> + my $missing = join(", ", $missing_mappings->@*);
>> + die "can't migrate to '$self->{node}': missing mapped devices $missing\n";
>> + } elsif ($running && scalar($missing_live_mappings->@*)) {
>> + my $missing = join(", ", $missing_live_mappings->@*);
>> + die "can't live migrate running VM which uses following mapped devices: $missing\n";
>> } else {
>> $self->log('info', "migrating VM which uses mapped local devices");
>> }
>
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel