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 3/4] check_local_resources: add more info per mapped device
Date: Fri, 22 Mar 2024 14:37:27 +0100 [thread overview]
Message-ID: <a96145e1-bf35-4627-9442-ba07e8b7fe54@proxmox.com> (raw)
In-Reply-To: <20240318111836.1947580-6-d.csapak@proxmox.com>
Am 18.03.24 um 12:18 schrieb Dominik Csapak:
> such as the mapping name and if it's marked for live-migration (pci only)
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> PVE/API2/Qemu.pm | 2 +-
> PVE/QemuMigrate.pm | 5 +++--
> PVE/QemuServer.pm | 10 ++++++----
> 3 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 497987ff..4ecaeb91 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -4516,7 +4516,7 @@ __PACKAGE__->register_method({
> $res->{local_disks} = [ values %$local_disks ];;
>
> $res->{local_resources} = $local_resources;
> - $res->{'mapped-resources'} = $mapped_resources;
> + $res->{'mapped-resources'} = [ map { "$_->{key}" } $mapped_resources->@* ];
Or should it become a hash? Then you could use 'keys' instead of map and
a 'key' property.
>
> return $res;
>
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index 8d9b35ae..6fe8157d 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -232,7 +232,7 @@ sub prepare {
> my ($loc_res, $mapped_res, $missing_mappings_by_node) = PVE::QemuServer::check_local_resources($conf, 1);
> my $blocking_resources = [];
> for my $res ($loc_res->@*) {
> - if (!grep($res, $mapped_res->@*)) {
Seems like this is currently broken. I.e. it should be $res eq $_
> + if (!grep { $_->{key} eq $res } $mapped_res->@*) {
> push $blocking_resources->@*, $res;
> }
> }
> @@ -246,8 +246,9 @@ sub prepare {
>
> if (scalar($mapped_res->@*)) {
> my $missing_mappings = $missing_mappings_by_node->{$self->{node}};
> + my $mapped_text = join(", ", map { $_->{key} } $mapped_res->@*);
> if ($running) {
> - die "can't migrate running VM which uses mapped devices: " . join(", ", $mapped_res->@*) . "\n";
> + die "can't migrate running VM which uses mapped devices: " . $mapped_text . "\n";
Style nit: no need for the concatenation anymore, can just use the
variable inside the string.
> } elsif (scalar($missing_mappings->@*)) {
> die "can't migrate to '$self->{node}': missing mapped devices " . join(", ", $missing_mappings->@*) . "\n";
> } else {
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 6e2c8052..ef3aee20 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -2600,14 +2600,16 @@ sub check_local_resources {
> next if $entry->{host} && $entry->{host} =~ m/^spice$/i;
> if ($entry->{mapping}) {
> $add_missing_mapping->('usb', $k, $entry->{mapping});
> - push @$mapped_res, $k;
> + push @$mapped_res, { key => $k, device => $entry->{mapping} };
Should we name the second key 'name'/'mapping'/'id' instead of 'device'?
> }
> }
> if ($k =~ m/^hostpci/) {
> my $entry = parse_property_string('pve-qm-hostpci', $conf->{$k});
> - if ($entry->{mapping}) {
> - $add_missing_mapping->('pci', $k, $entry->{mapping});
> - push @$mapped_res, $k;
> + if (my $name = $entry->{mapping}) {
> + $add_missing_mapping->('pci', $k, $name);
> + my $mapped_device = { key => $k, device => $name };
> + $mapped_device->{'live-migration'} = 1 if $pci_map->{ids}->{$name}->{'live-migration-capable'};
Style nit: line too long
> + push @$mapped_res, $mapped_device;
> }
> }
> # sockets are safe: they will recreated be on the target side post-migrate
next prev parent reply other threads:[~2024-03-22 13:37 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-18 11:18 [pve-devel] [PATCH guest-common/qemu-server/manager/docs] enable experimental support for pci live migration Dominik Csapak
2024-03-18 11:18 ` [pve-devel] [PATCH guest-common 1/2] mapping: pci: add 'live-migration-capable' flag to mappings Dominik Csapak
2024-03-22 13:37 ` Fiona Ebner
2024-04-02 9:30 ` Dominik Csapak
2024-04-10 10:09 ` Fiona Ebner
2024-03-18 11:18 ` [pve-devel] [PATCH guest-common 2/2] mapping: pci: optionally return the config in 'find_on_current_node' Dominik Csapak
2024-03-22 13:38 ` Fiona Ebner
2024-04-02 9:32 ` Dominik Csapak
2024-03-18 11:18 ` [pve-devel] [PATCH qemu-server 1/4] usb: fix undef error on string match Dominik Csapak
2024-03-22 13:36 ` Fiona Ebner
2024-03-18 11:18 ` [pve-devel] [PATCH qemu-server 2/4] pci: set 'enable-migration' to on for live-migration marked mapped devices Dominik Csapak
2024-03-18 11:18 ` [pve-devel] [PATCH qemu-server 3/4] check_local_resources: add more info per mapped device Dominik Csapak
2024-03-22 13:37 ` Fiona Ebner [this message]
2024-04-02 9:35 ` Dominik Csapak
2024-03-18 11:18 ` [pve-devel] [PATCH qemu-server 4/4] api: enable live migration for marked mapped pci devices Dominik Csapak
2024-03-22 13:37 ` Fiona Ebner
2024-04-02 9:36 ` Dominik Csapak
2024-03-18 11:18 ` [pve-devel] [PATCH docs 1/2] qm: resource mapping: add description for `mdev` option Dominik Csapak
2024-03-18 11:18 ` [pve-devel] [PATCH docs 2/2] qm: resource mapping: document `live-migration-capable` setting Dominik Csapak
2024-03-18 11:18 ` [pve-devel] [PATCH manager 1/1] ui: allow configuring and live migration of mapped pci resources 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=a96145e1-bf35-4627-9442-ba07e8b7fe54@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