public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "DERUMIER, Alexandre" <Alexandre.DERUMIER@groupe-cyllene.com>
To: "pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH many] add cluster-wide hardware device mapping
Date: Tue, 2 Aug 2022 15:59:46 +0000	[thread overview]
Message-ID: <5f273fac-cc2a-e9e3-16a4-a3c02a137b73@groupe-cyllene.com> (raw)
In-Reply-To: <20220719114639.3035048-1-d.csapak@proxmox.com>

many thanks for this patch series.

I had a student at the previous training needing this feature.

I'll be usefull in the future to get mdev vgpu live migration (It's not 
yet working with qemu 6.2)


I'll try to test it next week when I'll back from holiday.



Le 19/07/22 à 13:46, Dominik Csapak a écrit :
> this series aims to add a cluster-wide device mapping for pci and usb devices.
> so that an admin can configure a device to be availble for migration and
> configuring for uses that are non-root
>
> built-in are some additional safety checks in contrast to current
> passthrough, e.g. if pci addresses shift, with the mapping
> we can detect that and prevent a vm to boot with the wrong device
> (in most cases, there are some edge cases when one has multiple
> of the same device, e.g. the same gpu, that we cannot detect)
>
> a few pain points that are probably worth discussing/thinking about:
> (i did not really get feedback on my last RFC on this)
> * the config format
>      i changed to a json backed config, since it makes handling it much
>      easier (since we have a id -> nodenames -> mapping relation that
>      we cannot easily represent with a section config). some
>      (small) parts are written from scratch (update/createSchema for
>      instance) but we would have to do that anyway
>
>      if wanted i can make the section config work, but it makes the
>      handling quite a big uglier (for example, we have name the usb/pci
>      properties differently because the section config cannnot have
>      different formats for different sections)
>
> * getting the cluster wide info
>      the configuring of mappings is all done via node specific api paths,
>      but i created a cluster wide api path that returns the overall
>      structure for easy consumption from the gui. to get the remaining
>      data from the other nodes, i let the gui make an api call
>      for each node.
>
>      alternatively we could distribute the necessary info via pmxcfs,
>      but we'd have to broadcast basically the whole pci listing for all
>      nodes in a relatively short interval, only for it to be extremly
>      seldomly used (when looking at the cluster wide hardware
>      mappings...)
>
> * some minor things that can be improved are how the gui looks/behaves:
>      - 'add new' and 'add mapping' are probably to similar, but i did
>        not come up with really better alternatives
>      - i find the tree of entry -> node-mappings nice, but there may be
>        an even better representation?
>      - position in cluster menu is probably not optimal
>        (but where to put it?)
>
> changes from the rfc:
> * new cluster wide gui instead of node-local one (removed that, since
>    it's not necessary when we have a cluster-wide one)
> * uses json instead of a section config
> * api is quite different overall, i split the type into its own level
>    for configuring, similar to what we do in pbs
>    (e.g. /nodes/NODENAME/hardware/mapping/usb/)
> * fixed quite some bugs the rfc had
> * added patch for handling the gui with limited permissions better
> * added a 'comment' field for mappings
>
> dependencies are ofc:
>
>      manager depends on qemu-server,pve-access-control,pve-common
>      qemu-server depends on pve-access-control,pve-common
>      pve-common depends on pve-cluster
>
> pve-cluster:
>
> Dominik Csapak (1):
>    add nodes/hardware-map.conf
>
>   data/PVE/Cluster.pm | 1 +
>   data/src/status.c   | 1 +
>   2 files changed, 2 insertions(+)
>
> pve-access-control:
>
> Dominik Csapak (2):
>    PVE/AccessControl: add Hardware.* privileges and /hardware/ paths
>    PVE/RPCEnvironment: add helper for checking hw permissions
>
>   src/PVE/AccessControl.pm  | 13 +++++++++++++
>   src/PVE/RPCEnvironment.pm |  9 +++++++++
>   2 files changed, 22 insertions(+)
>
> pve-common:
>
> Dominik Csapak (1):
>    add PVE/HardwareMap
>
>   src/Makefile           |   1 +
>   src/PVE/HardwareMap.pm | 363 +++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 364 insertions(+)
>   create mode 100644 src/PVE/HardwareMap.pm
>
> qemu-server:
>
> Dominik Csapak (7):
>    PVE/QemuServer: allow mapped usb devices in config
>    PVE/QemuServer: allow mapped pci deviced in config
>    PVE/API2/Qemu: add permission checks for mapped usb devices
>    PVE/API2/Qemu: add permission checks for mapped pci devices
>    PVE/QemuServer: extend 'check_local_resources' for mapped resources
>    PVE/API2/Qemu: migrate preconditions: use new check_local_resources
>      info
>    PVE/QemuMigrate: check for mapped resources on migration
>
>   PVE/API2/Qemu.pm      | 108 ++++++++++++++++++++++++++++++++++++++----
>   PVE/QemuMigrate.pm    |  13 ++++-
>   PVE/QemuServer.pm     |  38 ++++++++++++++-
>   PVE/QemuServer/PCI.pm |  20 +++++++-
>   PVE/QemuServer/USB.pm |  21 +++++++-
>   5 files changed, 185 insertions(+), 15 deletions(-)
>
> pve-manager:
>
> Dominik Csapak (12):
>    PVE/API2/Hardware: add Mapping.pm
>    PVE/API2/Cluster: add Hardware mapping list api call
>    ui: form/USBSelector: make it more flexible with nodename
>    ui: form: add PCIMapSelector
>    ui: form: add USBMapSelector
>    ui: qemu/PCIEdit: rework panel to add a mapped configuration
>    ui: qemu/USBEdit: add 'mapped' device case
>    ui: add window/PCIEdit: edit window for pci mappings
>    ui: add window/USBEdit: edit window for usb mappings
>    ui: add dc/HardwareView: a CRUD interface for hardware mapping
>    ui: window/Migrate: allow mapped devices
>    ui: improve permission handling for hardware
>
>   PVE/API2/Cluster.pm                 |   8 +
>   PVE/API2/Cluster/Hardware.pm        | 117 +++++
>   PVE/API2/Cluster/Makefile           |   1 +
>   PVE/API2/Hardware.pm                |   6 +
>   PVE/API2/Hardware/Makefile          |   1 +
>   PVE/API2/Hardware/Mapping.pm        | 708 ++++++++++++++++++++++++++++
>   www/manager6/Makefile               |   5 +
>   www/manager6/data/PermPathStore.js  |   1 +
>   www/manager6/dc/Config.js           |  18 +-
>   www/manager6/dc/HardwareView.js     | 314 ++++++++++++
>   www/manager6/form/PCIMapSelector.js |  95 ++++
>   www/manager6/form/PCISelector.js    |  18 +-
>   www/manager6/form/USBMapSelector.js |  73 +++
>   www/manager6/form/USBSelector.js    |  33 +-
>   www/manager6/qemu/HardwareView.js   |  17 +-
>   www/manager6/qemu/PCIEdit.js        | 231 ++++++---
>   www/manager6/qemu/USBEdit.js        |  34 +-
>   www/manager6/window/Migrate.js      |  37 +-
>   www/manager6/window/PCIEdit.js      | 323 +++++++++++++
>   www/manager6/window/USBEdit.js      | 248 ++++++++++
>   20 files changed, 2185 insertions(+), 103 deletions(-)
>   create mode 100644 PVE/API2/Cluster/Hardware.pm
>   create mode 100644 PVE/API2/Hardware/Mapping.pm
>   create mode 100644 www/manager6/dc/HardwareView.js
>   create mode 100644 www/manager6/form/PCIMapSelector.js
>   create mode 100644 www/manager6/form/USBMapSelector.js
>   create mode 100644 www/manager6/window/PCIEdit.js
>   create mode 100644 www/manager6/window/USBEdit.js
>


      parent reply	other threads:[~2022-08-02 15:59 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-19 11:46 Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH cluster 1/1] add nodes/hardware-map.conf Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH access-control 1/2] PVE/AccessControl: add Hardware.* privileges and /hardware/ paths Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH access-control 2/2] PVE/RPCEnvironment: add helper for checking hw permissions Dominik Csapak
2022-08-01 12:01   ` Fabian Grünbichler
2022-08-09  6:55     ` Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH common 1/1] add PVE/HardwareMap Dominik Csapak
     [not found]   ` <<20220719114639.3035048-5-d.csapak@proxmox.com>
2022-08-01 12:58     ` Fabian Grünbichler
2022-08-09  7:29       ` Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH qemu-server 1/7] PVE/QemuServer: allow mapped usb devices in config Dominik Csapak
     [not found]   ` <<20220719114639.3035048-6-d.csapak@proxmox.com>
2022-08-01 12:59     ` Fabian Grünbichler
2022-07-19 11:46 ` [pve-devel] [PATCH qemu-server 2/7] PVE/QemuServer: allow mapped pci deviced " Dominik Csapak
     [not found]   ` <<20220719114639.3035048-7-d.csapak@proxmox.com>
2022-08-01 12:59     ` Fabian Grünbichler
2022-07-19 11:46 ` [pve-devel] [PATCH qemu-server 3/7] PVE/API2/Qemu: add permission checks for mapped usb devices Dominik Csapak
     [not found]   ` <<20220719114639.3035048-8-d.csapak@proxmox.com>
2022-08-01 13:01     ` Fabian Grünbichler
2022-08-09  7:32       ` Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH qemu-server 4/7] PVE/API2/Qemu: add permission checks for mapped pci devices Dominik Csapak
     [not found]   ` <<20220719114639.3035048-9-d.csapak@proxmox.com>
2022-08-01 13:01     ` Fabian Grünbichler
2022-07-19 11:46 ` [pve-devel] [PATCH qemu-server 5/7] PVE/QemuServer: extend 'check_local_resources' for mapped resources Dominik Csapak
     [not found]   ` <<<20220719114639.3035048-10-d.csapak@proxmox.com>
2022-08-01 13:02     ` Fabian Grünbichler
2022-07-19 11:46 ` [pve-devel] [PATCH qemu-server 6/7] PVE/API2/Qemu: migrate preconditions: use new check_local_resources info Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH qemu-server 7/7] PVE/QemuMigrate: check for mapped resources on migration Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH manager 01/12] PVE/API2/Hardware: add Mapping.pm Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH manager 02/12] PVE/API2/Cluster: add Hardware mapping list api call Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH manager 03/12] ui: form/USBSelector: make it more flexible with nodename Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH manager 04/12] ui: form: add PCIMapSelector Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH manager 05/12] ui: form: add USBMapSelector Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH manager 06/12] ui: qemu/PCIEdit: rework panel to add a mapped configuration Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH manager 07/12] ui: qemu/USBEdit: add 'mapped' device case Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH manager 08/12] ui: add window/PCIEdit: edit window for pci mappings Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH manager 09/12] ui: add window/USBEdit: edit window for usb mappings Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH manager 10/12] ui: add dc/HardwareView: a CRUD interface for hardware mapping Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH manager 11/12] ui: window/Migrate: allow mapped devices Dominik Csapak
2022-07-19 11:46 ` [pve-devel] [PATCH manager 12/12] ui: improve permission handling for hardware Dominik Csapak
2022-07-19 13:26 ` [pve-devel] [PATCH many] add cluster-wide hardware device mapping Dominik Csapak
     [not found]   ` <mailman.329.1658406652.464.pve-devel@lists.proxmox.com>
2022-07-21 14:48     ` Dominik Csapak
2022-08-02 15:59 ` DERUMIER, Alexandre [this message]

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=5f273fac-cc2a-e9e3-16a4-a3c02a137b73@groupe-cyllene.com \
    --to=alexandre.derumier@groupe-cyllene.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