From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Dominik Csapak <d.csapak@proxmox.com>
Subject: Re: [pve-devel] [PATCH guest-common v5 3/3] mapping: remove find_on_current_node
Date: Wed, 22 Jan 2025 16:10:22 +0100 [thread overview]
Message-ID: <125cb79d-e157-4376-bd27-50cb1267de27@proxmox.com> (raw)
In-Reply-To: <26c867c9-83a9-4a23-8a1f-7cce319d9d3f@proxmox.com>
Am 22.01.25 um 09:16 schrieb Dominik Csapak:
> On 1/20/25 15:51, Dominik Csapak wrote:
>> they only have one user each (where we can inline the implementation).
>> It's easy enough to recreate should we need to.
>>
>
> turns out i forgot that we added a second user of the pci function in pve-manager
>
> we still need to adapt the qemu-server side code still, so this would have one user after
> again...
>
> i could still do the changes similar to this version (remove the find_on_current_node here,
> add a new sub in qemu-server) but add a new patch for pve-manager that makes
> use of the new qemu-server sub
>
> alternatively we could omit this patch and simply change the one place in qemu-server
> where find_on_current_node is not enough
>
> seems variant 2 is less breakage & work, any input on this @thomas?
> (I'm asking you because you started to review the patches in v5)
Yeah, just skipping applying this patch here seems the least amount of
work for now.
Especially as this seems just to be for refactoring reasons, or is there
any other reason this is done? I checked the patches for further modifications
to these methods that require qemu-server specific code or the like in
subsequent patches and could not find any; but I might have overlooked
something and possibly also just forgot the discussion on this.
That said, moving can be fine for me, just would try to limit work required
to do it if there's really little benefit of doing so and if there's any
other reason it naturally would be good to have that stated in the commit
message, even if it's to help to make planned future changes easier.
>
> but I'll wait with a v6 until i get more feedback on this series
> (at least a user on the bugzilla reported that it works correct except the VFIO state in the
> migration log)
I skimmed over the patches and most seem OK to me from a higher level
perspective, just found one small potential patch ordering thing that
might be improved (will reply to the respective patch), so I'd like to
see some testing/review from Christoph here – ideally that results in
a slightly polished v6 with review/test trailers included that can be
directly rolled out.
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
next prev parent reply other threads:[~2025-01-22 15:10 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-20 14:51 [pve-devel] [PATCH guest-common/qemu-server/manager/docs v5 0/3] implement experimental vgpu live migration Dominik Csapak
2025-01-20 14:51 ` [pve-devel] [PATCH guest-common v5 1/3] mapping: pci: check the mdev configuration on the device too Dominik Csapak
2025-01-20 14:51 ` [pve-devel] [PATCH guest-common v5 2/3] mapping: pci: add 'live-migration-capable' flag to mappings Dominik Csapak
2025-01-20 14:51 ` [pve-devel] [PATCH guest-common v5 3/3] mapping: remove find_on_current_node Dominik Csapak
2025-01-22 8:16 ` Dominik Csapak
2025-01-22 15:10 ` Thomas Lamprecht [this message]
2025-01-20 14:51 ` [pve-devel] [PATCH qemu-server v5 01/11] usb: mapping: move implementation of find_on_current_node here Dominik Csapak
2025-01-20 14:51 ` [pve-devel] [PATCH qemu-server v5 02/11] pci: " Dominik Csapak
2025-02-11 12:45 ` Christoph Heiss
2025-02-13 9:30 ` Dominik Csapak
2025-01-20 14:51 ` [pve-devel] [PATCH qemu-server v5 03/11] pci: mapping: check mdev config against hardware Dominik Csapak
2025-01-20 14:51 ` [pve-devel] [PATCH qemu-server v5 04/11] vm stop-cleanup: allow callers to decide error behavior Dominik Csapak
2025-01-20 14:51 ` [pve-devel] [PATCH qemu-server v5 05/11] migrate: call vm_stop_cleanup after stopping in phase3_cleanup Dominik Csapak
2025-01-20 14:51 ` [pve-devel] [PATCH qemu-server v5 06/11] pci: set 'enable-migration' to on for live-migration marked mapped devices Dominik Csapak
2025-01-20 14:51 ` [pve-devel] [PATCH qemu-server v5 07/11] check_local_resources: add more info per mapped device and return as hash Dominik Csapak
2025-01-20 14:51 ` [pve-devel] [PATCH qemu-server v5 08/11] api: enable live migration for marked mapped pci devices Dominik Csapak
2025-01-20 14:51 ` [pve-devel] [PATCH qemu-server v5 09/11] api: include not mapped resources for running vms in migrate preconditions Dominik Csapak
2025-01-20 14:51 ` [pve-devel] [PATCH qemu-server v5 10/11] tests: cfg2cmd: fix mdev tests Dominik Csapak
2025-01-22 15:10 ` Thomas Lamprecht
2025-01-20 14:51 ` [pve-devel] [PATCH qemu-server v5 11/11] migration: show vfio state transferred too Dominik Csapak
2025-01-20 14:51 ` [pve-devel] [PATCH manager v5 1/5] mapping: pci: include mdev in config checks Dominik Csapak
2025-01-20 14:51 ` [pve-devel] [PATCH manager v5 2/5] bulk migrate: improve precondition checks Dominik Csapak
2025-02-11 12:50 ` Christoph Heiss
2025-01-20 14:51 ` [pve-devel] [PATCH manager v5 3/5] bulk migrate: include checks for live-migratable local resources Dominik Csapak
2025-01-20 14:52 ` [pve-devel] [PATCH manager v5 4/5] ui: adapt migration window to precondition api change Dominik Csapak
2025-01-20 14:52 ` [pve-devel] [PATCH manager v5 5/5] fix #5175: ui: allow configuring and live migration of mapped pci resources Dominik Csapak
2025-01-20 14:52 ` [pve-devel] [PATCH docs v5 1/2] qm: resource mapping: add description for `mdev` option Dominik Csapak
2025-01-20 14:52 ` [pve-devel] [PATCH docs v5 2/2] qm: resource mapping: document `live-migration-capable` setting Dominik Csapak
2025-02-11 12:57 ` [pve-devel] [PATCH guest-common/qemu-server/manager/docs v5 0/3] implement experimental vgpu live migration Christoph Heiss
2025-03-05 10:34 ` Eneko Lacunza via pve-devel
2025-03-06 11:06 ` Dominik Csapak
2025-03-06 14:42 ` Eneko Lacunza via pve-devel
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=125cb79d-e157-4376-bd27-50cb1267de27@proxmox.com \
--to=t.lamprecht@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