[pve-devel] applied: [PATCH qemu-server] fix #4553: nvidia vgpu: reuse smbios uuid for '-uuid' parameter
applied, thanks
On Mon, Feb 27, 2023 at 04:34:27PM +0100, Dominik Csapak wrote:
> instead of using the mdev uuid. The nvidia driver does not actually care
> that it's the same as the mdev, and in qemu the uuid parameter
> overwrites the smbios1 uuid internally, so we should have been reusing
> that in the first place.
>
> Signed-off-by: Dominik Csapak
> ---
> when i was writing the uuid appending in the first place, i was sure
> that the nvidia driver needed the mdev uuid, but i was wrong
>
> also i wrongly assumed the '-uuid' parameter does not do anything to the
> guest, but it overwrites the smbios uuid. seems i misread the qemu source
> code then.. (the man/help pages are not very helpful in that regard)
>
> PVE/QemuServer.pm | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 096e7f0d..b5836f7a 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -5851,9 +5851,14 @@ sub vm_start_nolock {
> for my $dev ($d->{pciid}->@*) {
> my $info = PVE::QemuServer::PCI::prepare_pci_device($vmid,
> $dev->{id}, $id, $d->{mdev});
>
> - # nvidia grid needs the uuid of the mdev as qemu parameter
> + # nvidia grid needs the qemu parameter '-uuid' set
> + # use smbios uuid or mdev uuid as fallback for that
> if ($d->{mdev} && !defined($uuid) && $info->{vendor} eq '10de')
> {
> - $uuid = PVE::QemuServer::PCI::generate_mdev_uuid($vmid,
> $id);
> + if (defined($conf->{smbios1})) {
> + my $smbios_conf = parse_smbios1($conf->{smbios1});
> + $uuid = $smbios_conf->{uuid} if
> defined($smbios_conf->{uuid});
> + }
> + $uuid = PVE::QemuServer::PCI::generate_mdev_uuid($vmid,
> $id) if !defined($uuid);
> }
> }
> }
> --
> 2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH qemu-server] pci: workaround nvidia driver issue on mdev cleanup
applied, though I'm not too happy about it...
The moment those driver versions that don't cleanup by themselves are
"rare" enough I'd like to just drop the cleanup code from our side!
On Fri, Feb 24, 2023 at 02:04:31PM +0100, Dominik Csapak wrote:
> in some nvidia grid drivers (e.g. 14.4 and 15.x), their kernel module
> tries to clean up the mdev device when the vm is shutdown and if it
> cannot do that (e.g. becaues we already cleaned it up), their removal
> process cancels with an error such that the vgpu does still exist inside
> their book-keeping, but can't be used/recreated/freed until a reboot.
>
> since there seems no obvious way to detect if thats the case besides
> either parsing dmesg (which is racy), or the nvidia kernel module
> version(which i'd rather not do), we simply test the pci device vendor
> for nvidia and add a 10s sleep. that should give the driver enough time
> to clean up and we will not find the path anymore and skip the cleanup.
>
> This way, it works with both the newer and older versions of the driver
> (some of the older drivers are LTS releases, so they're still
> supported).
>
> Signed-off-by: Dominik Csapak
> ---
> PVE/QemuServer.pm | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 40be44db..096e7f0d 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -6161,6 +6161,15 @@ sub cleanup_pci_devices {
> # NOTE: avoid PVE::SysFSTools::pci_cleanup_mdev_device as it
> requires PCI ID and we
> # don't want to break ABI just for this two liner
> my $dev_sysfs_dir = "/sys/bus/mdev/devices/$uuid";
> +
> + # some nvidia vgpu driver versions want to clean the mdevs up
> themselves, and error
> + # out when we do it first. so wait for 10 seconds and then try it
> + my $pciid = $d->{pciid}->[0]->{id};
> + my $info = PVE::SysFSTools::pci_device_info("$pciid");
> + if ($info->{vendor} eq '10de') {
> + sleep 10;
> + }
> +
> PVE::SysFSTools::file_write("$dev_sysfs_dir/remove", "1") if -e
> $dev_sysfs_dir;
> }
> }
> --
> 2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager] ui: resource tree: correctly reinsert item when name changes
if the user has the tree sorted by name, we have to move the items
on a name change, otherwise they'll stay on the old position
Signed-off-by: Dominik Csapak
---
www/manager6/tree/ResourceTree.js | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/www/manager6/tree/ResourceTree.js
b/www/manager6/tree/ResourceTree.js
index 7d7900b59..54c6403d8 100644
--- a/www/manager6/tree/ResourceTree.js
+++ b/www/manager6/tree/ResourceTree.js
@@ -278,7 +278,8 @@ Ext.define('PVE.tree.ResourceTree', {
let groups = me.viewFilter.groups || [];
// explicitly check for node/template, as those are not always
grouping attributes
- let moveCheckAttrs = groups.concat(['node', 'template']);
+ // also check for name for when the tree is sorted by name
+ let moveCheckAttrs = groups.concat(['node', 'template', 'name']);
let filterfn = me.viewFilter.filterfn;
let reselect = false; // for disappeared nodes
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer] fix #4430: add UTC timezone as option to installer
Thanks for the review! On Wed, Mar 15, 2023 at 02:23:09PM +0100, Thomas Lamprecht wrote: > [..] > > What I don't like as much that one has to set the country to the timezone, > which is confusing UX and will be subtle to most. > > Better ways might be: > > - add an explicit "Use UTC" checkbox that grey's out the country/timezone > selection then. Disadvantage here would be taking up extra space and > expanding > the user input amount, which goes a bit against our "as simple as possible" > approach for the installer > > - Add UTC always to the time-zone selection, independent of what country is > selected. This keeps the selection where it belongs, and allows to quickly > change to UTC without any typing/searching required (at least for most > countries) > > From a gut feeling I'd go for the second option, but didn't checked out how > the > implementation would then look like. >From looking at the code while implementing this, the second option shouldn't be that much of a hassle to implement I'd say (and probably less than the first). Sounds like the better option to me too. The first option could also strike some users that this option is permament and cannot be changed after installation, due to the grey'ing out and such, I guess. I send a v2 soon with the second approach implemented, let's see how the look & feel of that is. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH manager] ui: resource tree: correctly reinsert item when name changes
Am 16/03/2023 um 10:03 schrieb Dominik Csapak: > if the user has the tree sorted by name, we have to move the items > on a name change, otherwise they'll stay on the old position > > Signed-off-by: Dominik Csapak > --- > www/manager6/tree/ResourceTree.js | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 installer] fix #4430: add UTC timezone as option to installer
Adds 'Etc/UTC' as option to the timezone selection, regardless of what
country is selected.
The 'Etc/' prefix needs to be stripped for the installation, as this
value is written to /etc/timezone. PVE/PMG/PBS already use 'UTC' without
the prefix, so avoid regressing them.
Signed-off-by: Christoph Heiss
---
Would it be sensible to fix up the timezone change component in the UI
so that it uses 'Etc/UTC' (which is the canonical timezone name for UTC)
as well? As separate patch in the future, of course.
v1: https://lists.proxmox.com/pipermail/pve-devel/2023-March/056202.html
proxinstall | 4
1 file changed, 4 insertions(+)
diff --git a/proxinstall b/proxinstall
index 79abc34..899b534 100755
--- a/proxinstall
+++ b/proxinstall
@@ -2658,6 +2658,7 @@ sub update_zonelist {
$cb->signal_connect('changed' => sub {
$timezone = $cb->get_active_text();
+ $timezone =~ s|Etc/UTC|UTC|;
});
my @za;
@@ -2674,6 +2675,9 @@ sub update_zonelist {
$i++;
}
+# Append Etc/UTC here, so it is always the last item and never the default
for any country.
+$cb->append_text('Etc/UTC');
+
$cb->set_active($ind || 0);
$cb->show;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH guest-common] fix #4572: config: also update volume IDs in pending section
Am 15/03/2023 um 15:44 schrieb Fiona Ebner:
> The method is intended to be used in cases where the volumes actually
> got renamed (e.g. migration). Thus, updating the volume IDs should of
> course also be done for pending changes to avoid changes referring to
> now non-existent volumes or even the wrong existing volume.
>
> Signed-off-by: Fiona Ebner
> ---
> src/PVE/AbstractConfig.pm | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>
applied, thanks!
as talked off-list, I wrapped this and the pre-existing $conf->{snapshot} access
into an if-defined guard, just to be sure as autovivifiaction and undef-warns
can
be both a nuisance.
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH v2 installer] fix #4430: add UTC timezone as option to installer
Am 16/03/2023 um 11:01 schrieb Christoph Heiss: > Adds 'Etc/UTC' as option to the timezone selection, regardless of what > country is selected. > > The 'Etc/' prefix needs to be stripped for the installation, as this > value is written to /etc/timezone. PVE/PMG/PBS already use 'UTC' without > the prefix, so avoid regressing them. > > Signed-off-by: Christoph Heiss > --- > Would it be sensible to fix up the timezone change component in the UI > so that it uses 'Etc/UTC' (which is the canonical timezone name for UTC) > as well? As separate patch in the future, of course. IMO no, the Etc/ is mostly a detail of the IANA zone db format/data, but not really used when talking about UTC or setting the timezone to it. > > v1: https://lists.proxmox.com/pipermail/pve-devel/2023-March/056202.html > > proxinstall | 4 > 1 file changed, 4 insertions(+) > > applied, with a followup that drops the "Etc/" prefix (IMO would mostly confuse some of our users at best), thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH v4 container/manager 0/3] fix #3413: Add `Disconnect` option for LXC networks
Am 22/02/2023 um 13:49 schrieb Christoph Heiss: > Add a `Disconnect` option for network interfaces on LXC containers, much > like it already exists for VMs. This has been requested in #3413 [0] and > seems useful, especially considering we already support the same thing > for VMs. > > One thing to note is that LXC does not seem to support the notion of > setting an interface down. The `flags` property would suggest that this > possible [1], but AFAICS it does not work. I tried setting the value as > empty and to something else than "up" (since that is really the only > supported option [2][3]), which both had absolutely no effect. > > Thus force the host-side link of the container network down and avoid > adding it to the designated bridge if the new option is set, effectively > disconnecting the container network. > > The first patch is cleanup only and does not change anything regarding > functionality. > > Testing > --- > Testing was done by starting a LXC container (w/ and w/o `link_down` > set), checking if the interface has (or not) LOWERLAYERDOWN set inside > the container (`ip address eth0`) and if packet transit works (or not) > using a simple `ping`. Same thing after toggeling the option on the > interface. Further, the interface(s) should (or should not) be listed > in `brctl show`. Same thing was done for hotplugged interfaces to a > running container. > > Also tested with `ifreload -a` (thanks Wolfgang!) thrown in, which did > nothing unexpected: If `link_down` was set, interfaces stayed in > LOWERLAYERDOWN and unplugged from the bridge, and stayed UP and plugged > into the bridge when `link_down` was unset. > > [0] https://bugzilla.proxmox.com/show_bug.cgi?id=3413 > [1] > https://linuxcontainers.org/lxc/manpages/man5/lxc.container.conf.5.html#lbAO > [2] https://github.com/lxc/lxc/blob/08f0e769/src/lxc/confile.c#L453-L467 > [3] https://github.com/lxc/lxc/blob/08f0e769/src/lxc/confile.c#L5933-L5952 > > v1: https://lists.proxmox.com/pipermail/pve-devel/2023-February/055762.html > v2: https://lists.proxmox.com/pipermail/pve-devel/2023-February/055795.html > v3: https://lists.proxmox.com/pipermail/pve-devel/2023-February/055839.html > > pve-container: > > Christoph Heiss (2): > net: Pass network config directly to net_tap_plug() > net: Add `link_down` config to allow setting interfaces as disconnected > > src/PVE/LXC.pm| 37 +++-- > src/PVE/LXC/Config.pm | 6 ++ > src/lxcnetaddbr | 9 + > 3 files changed, 30 insertions(+), 22 deletions(-) > applied above two, with the relevant bits of the cover letter added to the commit message of the second container patch, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-docs v2] update the PCI(e) docs
A little update to the PCI(e) docs with the plan of reworking the PCI
wiki as well.
Along some minor grammar fixes added:
* how to check if kernelmodules are being loaded
* how to check which drivers to blacklist
* how to add softdeps for module loading
* where to find kernel params
Signed-off-by: Noel Ullreich
---
changes from v1:
* fixed spelling mistakes
* reduced code snippets of how to check iommu groupings to one
* moved where to find kernel params to kernel cmdline section
* removed wrong info on display output. will add correct info to
Examples-Wiki
* changed module names to variable-names, so that people can't
blindly copy-paste.
* restructured commit message ;)
qm-pci-passthrough.adoc | 75 ++---
system-booting.adoc | 9 +
2 files changed, 71 insertions(+), 13 deletions(-)
diff --git a/qm-pci-passthrough.adoc b/qm-pci-passthrough.adoc
index df6cf21..3f75ed0 100644
--- a/qm-pci-passthrough.adoc
+++ b/qm-pci-passthrough.adoc
@@ -16,16 +16,17 @@ device anymore on the host or in any other VM.
General Requirements
-Since passthrough is a feature which also needs hardware support, there are
-some requirements to check and preparations to be done to make it work.
-
+Since passthrough is performed on real hardware, it needs to fulfill some
+requirements. A brief overview of these requirements is given below, for more
+information on specific devices, see
+https://pve.proxmox.com/wiki/PCI_Passthrough[PCI Passthrough Examples].
Hardware
Your hardware needs to support `IOMMU` (*I*/*O* **M**emory **M**anagement
**U**nit) interrupt remapping, this includes the CPU and the mainboard.
-Generally, Intel systems with VT-d, and AMD systems with AMD-Vi support this.
+Generally, Intel systems with VT-d and AMD systems with AMD-Vi support this.
But it is not guaranteed that everything will work out of the box, due
to bad hardware implementation and missing or low quality drivers.
@@ -44,8 +45,8 @@ some configuration to enable PCI(e) passthrough.
.IOMMU
-First, you have to enable IOMMU support in your BIOS/UEFI. Usually the
-corresponding setting is called `IOMMU` or `VT-d`,but you should find the exact
+First, you will have to enable IOMMU support in your BIOS/UEFI. Usually the
+corresponding setting is called `IOMMU` or `VT-d`, but you should find the
exact
option name in the manual of your motherboard.
For Intel CPUs, you may also need to enable the IOMMU on the
@@ -72,6 +73,9 @@ hardware IOMMU. To enable these options, add:
to the xref:sysboot_edit_kernel_cmdline[kernel commandline].
+For a complete list of kernel commandline options (of kernel 5.15), see
+https://www.kernel.org/doc/html/v5.15/admin-guide/kernel-parameters.html[kernel.org].
+
.Kernel Modules
You have to make sure the following modules are loaded. This can be achieved by
@@ -92,6 +96,14 @@ After changing anything modules related, you need to refresh
your
# update-initramfs -u -k all
+To check if the modules are being loaded, the output of
+
+
+# lsmod | grep vfio
+
+
+should include the four modules from above.
+
.Finish Configuration
Finally reboot to bring the changes into effect and check that it is indeed
@@ -105,10 +117,11 @@ should display that `IOMMU`, `Directed I/O` or `Interrupt
Remapping` is
enabled, depending on hardware and kernel the exact message can vary.
It is also important that the device(s) you want to pass through
-are in a *separate* `IOMMU` group. This can be checked with:
+are in a *separate* `IOMMU` group. This can be checked with a call to the {pve}
+API:
-# find /sys/kernel/iommu_groups/ -type l
+# pvesh get /nodes/{nodename}/hardware/pci --pci-class-blacklist ""
It is okay if the device is in an `IOMMU` group together with its functions
@@ -159,8 +172,8 @@ PCI(e) card, for example a GPU or a network card.
Host Configuration
^^
-In this case, the host must not use the card. There are two methods to achieve
-this:
+{pve} tries to automatically make the PCI(e) device unavailable for the host.
+However, if this doesn't work, there are two things that can be done:
* pass the device IDs to the options of the 'vfio-pci' modules by adding
+
@@ -175,7 +188,7 @@ the vendor and device IDs obtained by:
# lspci -nn
-* blacklist the driver completely on the host, ensuring that it is free to bind
+* blacklist the driver on the host completely, ensuring that it is free to bind
for passthrough, with
+
@@ -183,11 +196,49 @@ for passthrough, with
+
in a .conf file in */etc/modprobe.d/*.
++
+To find the drivername, execute
++
+
+# lspci -k
+
++
+for example:
++
+
+# lspci -k | grep -A 3 "VGA"
+
++
+will output something similar to
++
+
+01:00.0 VGA compatible controller: NVIDIA Corporation GP108 [GeForce GT 1030]
(rev a1)
+ Subsystem: Micro-Star International Co., Ltd. [MSI] GP108 [GeForce GT
[pve-devel] [PATCH pve-docs v3] update the PCI(e) docs
A little update to the PCI(e) docs with the plan of reworking the PCI
wiki as well.
Along with some minor grammar fixes added:
* how to check if kernelmodules are being loaded
* how to check which drivers to blacklist
* how to add softdeps for module loading
* where to find kernel params
Signed-off-by: Noel Ullreich
---
changes from v1:
* fixed spelling mistakes
* reduced code snippets of how to check iommu groupings to one
* moved where to find kernel params to kernel cmdline section
* removed wrong info on display output. will add correct info to
Examples-Wiki
* changed module names to variable-names, so that people can't
blindly copy-paste.
* restructured commit message ;)
changes from v2:
* while moving where to find the kernel params to the kernel
cmdline section, I forgot to remove it from the pci(e) section
* fixed typo in the link to the kernel param section
qm-pci-passthrough.adoc | 72 +
system-booting.adoc | 9 ++
2 files changed, 68 insertions(+), 13 deletions(-)
diff --git a/qm-pci-passthrough.adoc b/qm-pci-passthrough.adoc
index df6cf21..fc89b6c 100644
--- a/qm-pci-passthrough.adoc
+++ b/qm-pci-passthrough.adoc
@@ -16,16 +16,17 @@ device anymore on the host or in any other VM.
General Requirements
-Since passthrough is a feature which also needs hardware support, there are
-some requirements to check and preparations to be done to make it work.
-
+Since passthrough is performed on real hardware, it needs to fulfill some
+requirements. A brief overview of these requirements is given below, for more
+information on specific devices, see
+https://pve.proxmox.com/wiki/PCI_Passthrough[PCI Passthrough Examples].
Hardware
Your hardware needs to support `IOMMU` (*I*/*O* **M**emory **M**anagement
**U**nit) interrupt remapping, this includes the CPU and the mainboard.
-Generally, Intel systems with VT-d, and AMD systems with AMD-Vi support this.
+Generally, Intel systems with VT-d and AMD systems with AMD-Vi support this.
But it is not guaranteed that everything will work out of the box, due
to bad hardware implementation and missing or low quality drivers.
@@ -44,8 +45,8 @@ some configuration to enable PCI(e) passthrough.
.IOMMU
-First, you have to enable IOMMU support in your BIOS/UEFI. Usually the
-corresponding setting is called `IOMMU` or `VT-d`,but you should find the exact
+First, you will have to enable IOMMU support in your BIOS/UEFI. Usually the
+corresponding setting is called `IOMMU` or `VT-d`, but you should find the
exact
option name in the manual of your motherboard.
For Intel CPUs, you may also need to enable the IOMMU on the
@@ -92,6 +93,14 @@ After changing anything modules related, you need to refresh
your
# update-initramfs -u -k all
+To check if the modules are being loaded, the output of
+
+
+# lsmod | grep vfio
+
+
+should include the four modules from above.
+
.Finish Configuration
Finally reboot to bring the changes into effect and check that it is indeed
@@ -105,10 +114,11 @@ should display that `IOMMU`, `Directed I/O` or `Interrupt
Remapping` is
enabled, depending on hardware and kernel the exact message can vary.
It is also important that the device(s) you want to pass through
-are in a *separate* `IOMMU` group. This can be checked with:
+are in a *separate* `IOMMU` group. This can be checked with a call to the {pve}
+API:
-# find /sys/kernel/iommu_groups/ -type l
+# pvesh get /nodes/{nodename}/hardware/pci --pci-class-blacklist ""
It is okay if the device is in an `IOMMU` group together with its functions
@@ -159,8 +169,8 @@ PCI(e) card, for example a GPU or a network card.
Host Configuration
^^
-In this case, the host must not use the card. There are two methods to achieve
-this:
+{pve} tries to automatically make the PCI(e) device unavailable for the host.
+However, if this doesn't work, there are two things that can be done:
* pass the device IDs to the options of the 'vfio-pci' modules by adding
+
@@ -175,7 +185,7 @@ the vendor and device IDs obtained by:
# lspci -nn
-* blacklist the driver completely on the host, ensuring that it is free to bind
+* blacklist the driver on the host completely, ensuring that it is free to bind
for passthrough, with
+
@@ -183,11 +193,49 @@ for passthrough, with
+
in a .conf file in */etc/modprobe.d/*.
++
+To find the drivername, execute
++
+
+# lspci -k
+
++
+for example:
++
+
+# lspci -k | grep -A 3 "VGA"
+
++
+will output something similar to
++
+
+01:00.0 VGA compatible controller: NVIDIA Corporation GP108 [GeForce GT 1030]
(rev a1)
+ Subsystem: Micro-Star International Co., Ltd. [MSI] GP108 [GeForce GT
1030]
+ Kernel driver in use:
+ Kernel modules:
+
++
+Now we can blacklist the drivers by writing them into a .conf file:
++
+
+echo "blacklist " >> /etc/modprobe.d/b
[pve-devel] [PATCH v2 manager 3/5] api: ceph: add endpoint to fetch config keys
This new endpoint allows to get the values of config keys that are
either set in the config db or the ceph.conf file.
Values that are set in the ceph.conf file have priority over values set
in the conifg db via 'ceph config set'.
Expects the --config-keys parameter as a semicolon separated list of
":" where the section is a section in the ceph.conf
or config db. For example: global:osd_pool_default_size
Signed-off-by: Aaron Lauterer
---
changes since v1:
* use kebab-case parameter names
* use kebab-case for the ceph config parameters, which also are returned
that way
* improve how we parse and merge the config db and ceph.conf file. This
way though, we dont warn if we cannot find a config key.
* renamed regex to make the distinctions clearer
* dropped 'format => string-list' as it didn't work when leaving out
[;, ] from the regex. But we don't need both.
PVE/API2/Ceph/Cfg.pm | 81
1 file changed, 81 insertions(+)
diff --git a/PVE/API2/Ceph/Cfg.pm b/PVE/API2/Ceph/Cfg.pm
index a00ef19c..0caa96d3 100644
--- a/PVE/API2/Ceph/Cfg.pm
+++ b/PVE/API2/Ceph/Cfg.pm
@@ -40,6 +40,7 @@ __PACKAGE__->register_method ({
my $result = [
{ name => 'raw' },
{ name => 'db' },
+ { name => 'value' },
];
return $result;
@@ -114,3 +115,83 @@ __PACKAGE__->register_method ({
return $res;
}});
+
+
+my $SINGLE_CONFIGKEY_RE = qr/[0-9a-z\-_\.]+:[0-9a-zA-Z\-_]+/i;
+my $CONFIGKEYS_RE = qr/^(:?${SINGLE_CONFIGKEY_RE})(:?[;,
]${SINGLE_CONFIGKEY_RE})*$/;
+
+__PACKAGE__->register_method ({
+name => 'value',
+path => 'value',
+method => 'GET',
+proxyto => 'node',
+protected => 1,
+permissions => {
+ check => ['perm', '/', [ 'Sys.Audit' ]],
+},
+description => "Get configured values from either the config file or
config DB.",
+parameters => {
+ additionalProperties => 0,
+ properties => {
+ node => get_standard_option('pve-node'),
+ 'config-keys' => {
+ type => "string",
+ typetext => ":[;:]",
+ pattern => $CONFIGKEYS_RE,
+ description => "List of : items.",
+ }
+ },
+},
+returns => {
+ type => 'object',
+ description => "Contains {section}->{key} children with the values",
+},
+code => sub {
+ my ($param) = @_;
+
+ PVE::Ceph::Tools::check_ceph_inited();
+
+ # Ceph treats '-' and '_' the same in parameter names, stick with '-'
+ my $normalize = sub {
+ my $t = shift;
+ $t =~ s/_/-/g;
+ return $t;
+ };
+
+ my $requested_keys = {};
+ for my $pair (PVE::Tools::split_list($param->{'config-keys'})) {
+ my ($section, $key) = split(":", $pair);
+ $section = $normalize->($section);
+ $key = $normalize->($key);
+
+ $requested_keys->{$section}->{$key} = 1;
+ }
+
+ my $config = {};
+
+ my $rados = PVE::RADOS->new();
+ my $configdb = $rados->mon_command( { prefix => 'config dump', format
=> 'json' });
+ for my $s (@{$configdb}) {
+ my ($section, $name, $value) = $s ->@{'section', 'name', 'value'};
+ my $n_section = $normalize->($section);
+ my $n_name = $normalize->($name);
+
+ $config->{$n_section}->{$n_name} = $value
+ if defined $requested_keys->{$n_section} && $n_name eq $n_name;
+ }
+
+ # read ceph.conf after config db as it has priority if settings are
present in both
+ my $config_file = cfs_read_file('ceph.conf');
+ for my $section (keys %{$config_file}) {
+ my $n_section = $normalize->($section);
+ next if !defined $requested_keys->{$n_section};
+
+ for my $key (keys %{$config_file->{$section}}) {
+ my $n_key = $normalize->($key);
+ $config->{$n_section}->{$n_key} =
$config_file->{$section}->{$key}
+ if $requested_keys->{$n_section}->{$n_key};
+ }
+ }
+
+ return $config;
+}});
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 manager 4/5] fix #2515: ui: ceph pool create: use configured defaults for size and min_size
Instead of hard coded defaults for the size and min_size parameter,
check if we have defaults configured in the ceph.conf or config db and
use those.
There are clusters where different defaults are needed. For example if
the cluster spans two rooms and needs to survive the loss of one. A
size/min_size of 4/2 are common defaults in such a situation.
Signed-off-by: Aaron Lauterer
---
changes since v1:
* set defaultSize and defaultMinSize to undefined in CephPoolInputPanel
* set them in cbindData in PoolEdit (needed when editing existing pool)
* waitMsgTarget when opening pool create window
* guard returned values for config keys in case they are empty
www/manager6/ceph/Pool.js | 63 +++
1 file changed, 51 insertions(+), 12 deletions(-)
diff --git a/www/manager6/ceph/Pool.js b/www/manager6/ceph/Pool.js
index f7a4d9ba..e155a731 100644
--- a/www/manager6/ceph/Pool.js
+++ b/www/manager6/ceph/Pool.js
@@ -7,6 +7,11 @@ Ext.define('PVE.CephPoolInputPanel', {
onlineHelp: 'pve_ceph_pools',
subject: 'Ceph Pool',
+
+// sets default pool sizes
+defaultSize: undefined,
+defaultMinSize: undefined,
+
column1: [
{
xtype: 'pmxDisplayEditField',
@@ -27,7 +32,9 @@ Ext.define('PVE.CephPoolInputPanel', {
name: 'size',
editConfig: {
xtype: 'proxmoxintegerfield',
- value: 3,
+ cbind: {
+ value: (get) => get('defaultSize') ?? 3,
+ },
minValue: 2,
maxValue: 7,
allowBlank: false,
@@ -40,7 +47,6 @@ Ext.define('PVE.CephPoolInputPanel', {
},
},
},
-
},
],
column2: [
@@ -78,9 +84,15 @@ Ext.define('PVE.CephPoolInputPanel', {
xtype: 'proxmoxintegerfield',
fieldLabel: gettext('Min. Size'),
name: 'min_size',
- value: 2,
cbind: {
- minValue: (get) => get('isCreate') ? 2 : 1,
+ value: (get) => get('defaultMinSize') ?? 2,
+ minValue: (get) => {
+ if (Number(get('defaultMinSize')) === 1) {
+ return 1;
+ } else {
+ return get('isCreate') ? 2 : 1;
+ }
+ },
},
maxValue: 7,
allowBlank: false,
@@ -195,6 +207,8 @@ Ext.define('PVE.Ceph.PoolEdit', {
cbindData: {
pool_name: '',
isCreate: (cfg) => !cfg.pool_name,
+ defaultSize: undefined,
+ defaultMinSize: undefined,
},
cbind: {
@@ -216,6 +230,8 @@ Ext.define('PVE.Ceph.PoolEdit', {
pool_name: '{pool_name}',
isErasure: '{isErasure}',
isCreate: '{isCreate}',
+ defaultSize: '{defaultSize}',
+ defaultMinSize: '{defaultMinSize}',
},
}],
});
@@ -388,14 +404,37 @@ Ext.define('PVE.node.Ceph.PoolList', {
{
text: gettext('Create'),
handler: function() {
- Ext.create('PVE.Ceph.PoolEdit', {
- title: gettext('Create') + ': Ceph Pool',
- isCreate: true,
- isErasure: false,
- nodename: nodename,
- autoShow: true,
- listeners: {
- destroy: () => rstore.load(),
+ let keys = [
+ 'global:osd-pool-default-min-size',
+ 'global:osd-pool-default-size',
+ ];
+ let params = {
+ 'config-keys': keys.join(';'),
+ };
+
+ Proxmox.Utils.API2Request({
+ url: '/nodes/localhost/ceph/cfg/value',
+ method: 'GET',
+ params,
+ waitMsgTarget: me.getView(),
+ failure: response =>
Ext.Msg.alert(gettext('Error'), response.htmlStatus),
+ success: function({ result: { data } }) {
+ let global = data.global;
+ let defaultSize =
global?.['osd-pool-default-size'] ?? undefined;
+ let defaultMinSize =
global?.['osd-pool-default-min-size'] ?? undefined;
+
+ Ext.create('PVE.Ceph.PoolEdit', {
+ title: gettext('Create') + ': Ceph Pool',
+ isCreate: true,
+ isErasure: false,
+ defaultSize,
+ defaultMinSize,
+ nodename: nodename,
+ autoShow: true,
+
[pve-devel] [PATCH v2 manager 0/5] rework ceph cfg api & fix 2515 use size defaults
The main goal of this series is to improve the handling of configured default size & min_size values when creating a new Ceph Pool in the GUI. But since that means a new necessary API endpoint, we also rework the Ceph API to make it cleaner. A new API endpoint is used: 'cfg' and the current 'config' and 'configdb' are moved there (first 2 patches). The result is * cfg/ * raw (formerly config) * db (formerly configdb) Then we add a new endpoint 'cfg/value' that allows us to fetch values for config keys that are set either in the config DB of Ceph or in the ceph.conf file. More in each patch. Depending on how we want to handle the API deprecation, we might just want to apply the first two patches with 7.4, even if the actual implementation of the fix itself will have to wait. changes since v1: * 2 new patches to rework API so that the new endpoint has its place * incorporated suggested code changes Aaron Lauterer (5): api: ceph: add ceph/cfg path, deprecate ceph/config and ceph/configdb ui: ceph config: use new ceph/cfg/ API endpoints api: ceph: add endpoint to fetch config keys fix #2515: ui: ceph pool create: use configured defaults for size and min_size ui: ceph pool edit: rework with controller and formulas PVE/API2/Ceph.pm| 15 ++- PVE/API2/Ceph/Cfg.pm| 197 PVE/API2/Ceph/Makefile | 1 + www/manager6/ceph/Config.js | 4 +- www/manager6/ceph/Pool.js | 144 +++--- 5 files changed, 321 insertions(+), 40 deletions(-) create mode 100644 PVE/API2/Ceph/Cfg.pm -- 2.30.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 manager 5/5] ui: ceph pool edit: rework with controller and formulas
instead of relying purely on listeners that then manually change other
components, we can use binds, formulas and a basic controller.
This makes it quite a bit easier to let multiple components react to
changes.
A cbind is used for the size component to set the initial start value.
Other options, like using setValue in the controller init, will trigger
the change listener and therefore can affect the min size without any
user interaction.
Signed-off-by: Aaron Lauterer
---
I left the 'showWarning' as is. They are also used in the
'minSizeLabel' formula and I prefer to have them there in a non-negated
form.
changes since v1:
* moved view between controller and layout
* small code style cleanups
www/manager6/ceph/Pool.js | 83 ---
1 file changed, 59 insertions(+), 24 deletions(-)
diff --git a/www/manager6/ceph/Pool.js b/www/manager6/ceph/Pool.js
index e155a731..d511b1a4 100644
--- a/www/manager6/ceph/Pool.js
+++ b/www/manager6/ceph/Pool.js
@@ -12,6 +12,48 @@ Ext.define('PVE.CephPoolInputPanel', {
defaultSize: undefined,
defaultMinSize: undefined,
+controller: {
+ xclass: 'Ext.app.ViewController',
+
+ init: function(view) {
+ let vm = this.getViewModel();
+ vm.set('size', view.defaultSize ? Number(view.defaultSize) : 3);
+ vm.set('minSize', view.defaultMinSize ? Number(view.defaultMinSize)
: 2);
+ },
+ sizeChange: function(field, val) {
+ let vm = this.getViewModel();
+ let minSize = Math.round(val / 2);
+ if (minSize > 1) {
+ vm.set('minSize', minSize);
+ }
+ vm.set('size', val); // bind does not work in a
pmxDisplayEditField, update manually
+ },
+},
+
+viewModel: {
+ data: {
+ minSize: null,
+ size: null,
+ },
+ formulas: {
+ minSizeLabel: (get) => {
+ if (get('showMinSizeOneWarning') ||
get('showMinSizeHalfWarning')) {
+ return `${gettext('Min. Size')} `;
+ }
+ return gettext('Min. Size');
+ },
+ showMinSizeOneWarning: (get) => get('minSize') === 1,
+ showMinSizeHalfWarning: (get) => {
+ let minSize = get('minSize');
+ let size = get('size');
+ if (minSize === 1) {
+ return false;
+ }
+ return minSize < (size / 2) && minSize !== size;
+ },
+ },
+},
+
column1: [
{
xtype: 'pmxDisplayEditField',
@@ -39,12 +81,7 @@ Ext.define('PVE.CephPoolInputPanel', {
maxValue: 7,
allowBlank: false,
listeners: {
- change: function(field, val) {
- let size = Math.round(val / 2);
- if (size > 1) {
-
field.up('inputpanel').down('field[name=min_size]').setValue(size);
- }
- },
+ change: 'sizeChange',
},
},
},
@@ -82,10 +119,12 @@ Ext.define('PVE.CephPoolInputPanel', {
advancedColumn1: [
{
xtype: 'proxmoxintegerfield',
- fieldLabel: gettext('Min. Size'),
+ bind: {
+ fieldLabel: '{minSizeLabel}',
+ value: '{minSize}',
+ },
name: 'min_size',
cbind: {
- value: (get) => get('defaultMinSize') ?? 2,
minValue: (get) => {
if (Number(get('defaultMinSize')) === 1) {
return 1;
@@ -96,28 +135,24 @@ Ext.define('PVE.CephPoolInputPanel', {
},
maxValue: 7,
allowBlank: false,
- listeners: {
- change: function(field, minSize) {
- let panel = field.up('inputpanel');
- let size = panel.down('field[name=size]').getValue();
-
- let showWarning = minSize < (size / 2) && minSize !== size;
-
- let fieldLabel = gettext('Min. Size');
- if (showWarning) {
- fieldLabel = gettext('Min. Size') + ' ';
- }
-
panel.down('field[name=min_size-warning]').setHidden(!showWarning);
- field.setFieldLabel(fieldLabel);
- },
- },
},
{
xtype: 'displayfield',
- name: 'min_size-warning',
+ bind: {
+ hidden: '{!showMinSizeHalfWarning}',
+ },
+ hidden: true,
userCls: 'pmx-hint',
value: gettext('min_size < size/2 can lead to data loss, incomplete
PGs or unfound objects.'),
+ },
+ {
+ xtype: 'displayfield',
+ bind: {
+ hidden: '{!showMinSizeOneWarning}',
+ },
hidden: true,
+ userCls: 'p
[pve-devel] [PATCH v2 manager 1/5] api: ceph: add ceph/cfg path, deprecate ceph/config and ceph/configdb
Consolidating the different config paths lets us add more as needed
without polluting our API with too many 'configxxx' endpoints.
The config and configdb paths are renamed under the ceph/cfg path:
* config -> raw (returns the ceph.conf file as is)
* configdb -> db (returns the ceph config db contents)
The old paths are still available and need to be dropped at some point.
Signed-off-by: Aaron Lauterer
---
changes since v1:
* add this commit to rework the API
PVE/API2/Ceph.pm | 15 --
PVE/API2/Ceph/Cfg.pm | 116 +
PVE/API2/Ceph/Makefile | 1 +
3 files changed, 129 insertions(+), 3 deletions(-)
create mode 100644 PVE/API2/Ceph/Cfg.pm
diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
index 786a1870..3e3dd399 100644
--- a/PVE/API2/Ceph.pm
+++ b/PVE/API2/Ceph.pm
@@ -18,6 +18,7 @@ use PVE::RPCEnvironment;
use PVE::Storage;
use PVE::Tools qw(run_command file_get_contents file_set_contents
extract_param);
+use PVE::API2::Ceph::Cfg;
use PVE::API2::Ceph::OSD;
use PVE::API2::Ceph::FS;
use PVE::API2::Ceph::MDS;
@@ -30,6 +31,11 @@ use base qw(PVE::RESTHandler);
my $pve_osd_default_journal_size = 1024*5;
+__PACKAGE__->register_method ({
+subclass => "PVE::API2::Ceph::Cfg",
+path => 'cfg',
+});
+
__PACKAGE__->register_method ({
subclass => "PVE::API2::Ceph::OSD",
path => 'osd',
@@ -88,6 +94,7 @@ __PACKAGE__->register_method ({
my $result = [
{ name => 'cmd-safety' },
+ { name => 'cfg' },
{ name => 'config' },
{ name => 'configdb' },
{ name => 'crush' },
@@ -109,6 +116,8 @@ __PACKAGE__->register_method ({
return $result;
}});
+
+# TODO: deprecrated, remove with PVE 8
__PACKAGE__->register_method ({
name => 'config',
path => 'config',
@@ -117,7 +126,7 @@ __PACKAGE__->register_method ({
permissions => {
check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
},
-description => "Get the Ceph configuration file.",
+description => "Get the Ceph configuration file. Deprecated, please use
`/nodes/{node}/ceph/cfg/raw.",
parameters => {
additionalProperties => 0,
properties => {
@@ -135,6 +144,7 @@ __PACKAGE__->register_method ({
}});
+# TODO: deprecrated, remove with PVE 8
__PACKAGE__->register_method ({
name => 'configdb',
path => 'configdb',
@@ -144,7 +154,7 @@ __PACKAGE__->register_method ({
permissions => {
check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
},
-description => "Get the Ceph configuration database.",
+description => "Get the Ceph configuration database. Deprecated, please
use `/nodes/{node}/ceph/cfg/db.",
parameters => {
additionalProperties => 0,
properties => {
@@ -179,7 +189,6 @@ __PACKAGE__->register_method ({
return $res;
}});
-
__PACKAGE__->register_method ({
name => 'init',
path => 'init',
diff --git a/PVE/API2/Ceph/Cfg.pm b/PVE/API2/Ceph/Cfg.pm
new file mode 100644
index ..a00ef19c
--- /dev/null
+++ b/PVE/API2/Ceph/Cfg.pm
@@ -0,0 +1,116 @@
+package PVE::API2::Ceph::Cfg;
+
+use strict;
+use warnings;
+
+use PVE::Ceph::Tools;
+use PVE::Cluster qw(cfs_read_file cfs_write_file);
+use PVE::JSONSchema qw(get_standard_option);
+use PVE::RADOS;
+use PVE::Tools qw(run_command file_get_contents file_set_contents
extract_param);
+
+use base qw(PVE::RESTHandler);
+
+__PACKAGE__->register_method ({
+name => 'index',
+path => '',
+method => 'GET',
+description => "Directory index.",
+permissions => { user => 'all' },
+permissions => {
+ check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
+},
+parameters => {
+ additionalProperties => 0,
+ properties => {
+ node => get_standard_option('pve-node'),
+ },
+},
+returns => {
+ type => 'array',
+ items => {
+ type => "object",
+ properties => {},
+ },
+ links => [ { rel => 'child', href => "{name}" } ],
+},
+code => sub {
+ my ($param) = @_;
+
+ my $result = [
+ { name => 'raw' },
+ { name => 'db' },
+ ];
+
+ return $result;
+}});
+
+__PACKAGE__->register_method ({
+name => 'raw',
+path => 'raw',
+method => 'GET',
+proxyto => 'node',
+permissions => {
+ check => ['perm', '/', [ 'Sys.Audit', 'Datastore.Audit' ], any => 1],
+},
+description => "Get the Ceph configuration file.",
+parameters => {
+ additionalProperties => 0,
+ properties => {
+ node => get_standard_option('pve-node'),
+ },
+},
+returns => { type => 'string' },
+code => sub {
+ my ($param) = @_;
+
+ PVE::Ceph::Tools::check_ceph_inited();
+
+ my $path = PVE::Ceph::Tools::get_config('pve_ceph_cfgpath');
+ return file_get_contents($path);
+
+}});
+
+__PAC
[pve-devel] [PATCH v2 manager 2/5] ui: ceph config: use new ceph/cfg/ API endpoints
Signed-off-by: Aaron Lauterer
---
changes since v1:
* added this patch
www/manager6/ceph/Config.js | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/www/manager6/ceph/Config.js b/www/manager6/ceph/Config.js
index 7f07f15f..d4da20a8 100644
--- a/www/manager6/ceph/Config.js
+++ b/www/manager6/ceph/Config.js
@@ -53,7 +53,7 @@ Ext.define('PVE.node.CephConfigDb', {
throw "no node name specified";
}
- me.store.proxy.url = '/api2/json/nodes/' + nodename + '/ceph/configdb';
+ me.store.proxy.url = '/api2/json/nodes/' + nodename + '/ceph/cfg/db';
me.callParent();
@@ -102,7 +102,7 @@ Ext.define('PVE.node.CephConfig', {
}
Ext.apply(me, {
- url: '/nodes/' + nodename + '/ceph/config',
+ url: '/nodes/' + nodename + '/ceph/cfg/raw',
listeners: {
activate: function() {
me.load();
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH container] lxc start: warn in case of conflicting lxc.idmap entries
On Thu, Feb 23, 2023 at 06:03:02PM +0100, Friedrich Weber wrote:
> Users can customize the mapping between host and container uids/gids
> by providing `lxc.idmap` entries in the container config. The syntax
> is described in lxc.container.conf(5). One source of errors are
> conflicting entries for one or more uid/gids. An example:
>
> ...
> lxc.idmap: u 0 10 65536
> lxc.idmap: u 1000 1000 10
> ...
>
> Assuming `root:1000:10` is correctly added to /etc/subuid, starting
> the container fails with an error that is hard to interpret:
>
> lxc_map_ids: 3701 newuidmap failed to write mapping
> "newuidmap: write to uid_map failed: Invalid argument":
> newuidmap 67993 0 10 65536 1000 1000 10
>
> In order to simplify troubleshooting, validate the mapping before
> starting the container and print a warning if conflicting uid/gid
> mappings are detected. For the above mapping:
>
> lxc.idmap: invalid map entry 'u 1000 1000 10': container uid 1000
> is already mapped by entry 'u 0 10 65536'
>
> The warning appears in the task log and in the output of `pct start`.
>
> A failed validation check only prints a warning instead of erroring
> out, to make sure potentially buggy (or outdated) validation code does
> not prevent containers from starting.
>
> The validation algorithm is quite straightforward and has quadratic
> runtime in the worst case. The kernel allows at most 340 lines in
> uid_map (see user_namespaces(7)), which the algorithm should be able
> to handle in well under 0.5s, but to be safe, skip validation for more
> than 100 entries.
>
> Note that validation does not take /etc/sub{uid,gid} into account,
> which, if misconfigured, could still prevent the container from
> starting with an error like
>
> "newuidmap: uid range [1000-1010) -> [1000-1010) not allowed"
>
> If needed, validating /etc/sub{uid,gid} could be added in the future.
>
> Signed-off-by: Friedrich Weber
> ---
> The warning could of course be even more detailed, e.g., "container uid range
> [1000...1009] is already mapped to [101000...101009] by entry 'u 0 10
> 65536'". But this would require a more sophisticated algorithm, and I'm not
> sure if the added complexity is worth it -- happy to add it if wanted,
> though.
>
> src/PVE/LXC.pm | 97 ++
> src/test/Makefile | 5 +-
> src/test/idmap-test.pm | 197
> src/test/run_idmap_tests.pl | 10 ++
> 4 files changed, 308 insertions(+), 1 deletion(-)
> create mode 100644 src/test/idmap-test.pm
> create mode 100755 src/test/run_idmap_tests.pl
>
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index 54ee0d9..141f195 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -2313,6 +2313,93 @@ sub parse_id_maps {
> return ($id_map, $rootuid, $rootgid);
> }
>
> +# parse custom id mapping and throw a human-readable error if any
> +# host/container uid/gid is mapped twice. note that the algorithm has
> quadratic
> +# worst-case runtime, which may be problematic with >1000 mapping entries.
> +# we're unlikely to exceed this because the kernel only allows 340 entries
> (see
> +# user_namespaces(7)), but if we do, this could be implemented more
> efficiently
> +# (see e.g. "interval trees").
Both seem a bit excessive to me.
Let's look at the data:
We have a set of ranges consisting of a type, 2 starts and a count.
The types are uids and gids, so we can view those as 2 separate
instances of sets of [ct_start, host_start, count].
Since neither the container nor the host sides must overlap we can -
again - view these as separate sets of container side [start, count] and
host side [start, count].
In other words, we can see the entire id map as just 4 sets of [start,
count] ranges which must not overlap.
So I think all we need to do is sort these by the 'start' value, and for
each element make sure that
prevous_start + previous_count <= current_start
And yes, that means we need to sort $id_maps twice, once by ct id, once
by host id, and then iterate and do the above check.
Should be much shorter (and faster).
> +sub validate_id_maps {
> +my ($id_map) = @_;
> +
> +# keep track of already-mapped uids/gid ranges on the container and host
> +# sides. each range is an array [mapped_entry, first_id, last_id], where
> +# mapped_entry is the original config entry (stored for generating error
> +# messages), and [first_id, last_id] is an (inclusive) interval of mapped
> +# ids. Ranges are sorted ascendingly by first_id for more efficient
> +# traversal, and they must not overlap (this would be an invalid
> mapping).
> +my $sides_ranges = {
> + host => { u => [], g => [] },
> + container => { u => [], g => [] },
> +};
> +
> +# try to update the mapping with the requested range.
> +# return (1, undef, undef) on success and (0, $mapped_entry, $id) on
> failure,
> +# where $mapped_entry is t
[pve-devel] applied: [PATCH v4 manager 3/3] lxc: Add `Disconnect` option for network interfaces
applied gui patch, thanks ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH container] lxc start: warn in case of conflicting lxc.idmap entries
Thanks for the review! On 16/03/2023 14:59, Wolfgang Bumiller wrote: Both seem a bit excessive to me. Let's look at the data: We have a set of ranges consisting of a type, 2 starts and a count. The types are uids and gids, so we can view those as 2 separate instances of sets of [ct_start, host_start, count]. Since neither the container nor the host sides must overlap we can - again - view these as separate sets of container side [start, count] and host side [start, count]. In other words, we can see the entire id map as just 4 sets of [start, count] ranges which must not overlap. So I think all we need to do is sort these by the 'start' value, and for each element make sure that prevous_start + previous_count <= current_start And yes, that means we need to sort $id_maps twice, once by ct id, once by host id, and then iterate and do the above check. Should be much shorter (and faster). Yeah, good point, splitting $id_maps into separate uid/gid maps, and then sorting+iterating twice (I'll call this the "sorting algorithm" below) does sound more understandable than the current ad-hoc approach, and faster too. However, one small benefit of iterating over $id_maps in its original order (instead of sorting) is that the error message always references the *first* invalid map entry in the config, e.g. (omitting host uids for clarity) 1) u 1000 <...> 100 2) u 950 <...> 100 3) u 900 <...> 100 4) u 850 <...> 100 The sorting algorithm would error on entry 3, which might suggest to users that entries 1-2 are okay (which they are not). The current algorithm errors on line 2 already. Similar things would happen with interleaved uid/gid mappings, I guess. But I'm not sure if this really matters to users. What do you think? ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH v2 follow up manager 3/5] api: ceph: add endpoint to fetch config keys
This new endpoint allows to get the values of config keys that are
either set in the config db or the ceph.conf file.
Values that are set in the ceph.conf file have priority over values set
in the conifg db via 'ceph config set'.
Expects the --config-keys parameter as a semicolon separated list of
":" where the section is a section in the ceph.conf
or config db. For example: global:osd_pool_default_size
Signed-off-by: Aaron Lauterer
---
I found a small code style issue that was present since the initial
patch. The diff to the original v2 to this follow up is:
--- a/PVE/API2/Ceph/Cfg.pm
+++ b/PVE/API2/Ceph/Cfg.pm
@@ -172,7 +172,7 @@ __PACKAGE__->register_method ({
my $rados = PVE::RADOS->new();
my $configdb = $rados->mon_command( { prefix => 'config dump', format
=> 'json' });
for my $s (@{$configdb}) {
- my ($section, $name, $value) = $s ->@{'section', 'name', 'value'};
+ my ($section, $name, $value) = $s->@{'section', 'name', 'value'};
my $n_section = $normalize->($section);
my $n_name = $normalize->($name);
PVE/API2/Ceph/Cfg.pm | 81
1 file changed, 81 insertions(+)
diff --git a/PVE/API2/Ceph/Cfg.pm b/PVE/API2/Ceph/Cfg.pm
index a00ef19c..6d05db9c 100644
--- a/PVE/API2/Ceph/Cfg.pm
+++ b/PVE/API2/Ceph/Cfg.pm
@@ -40,6 +40,7 @@ __PACKAGE__->register_method ({
my $result = [
{ name => 'raw' },
{ name => 'db' },
+ { name => 'value' },
];
return $result;
@@ -114,3 +115,83 @@ __PACKAGE__->register_method ({
return $res;
}});
+
+
+my $SINGLE_CONFIGKEY_RE = qr/[0-9a-z\-_\.]+:[0-9a-zA-Z\-_]+/i;
+my $CONFIGKEYS_RE = qr/^(:?${SINGLE_CONFIGKEY_RE})(:?[;,
]${SINGLE_CONFIGKEY_RE})*$/;
+
+__PACKAGE__->register_method ({
+name => 'value',
+path => 'value',
+method => 'GET',
+proxyto => 'node',
+protected => 1,
+permissions => {
+ check => ['perm', '/', [ 'Sys.Audit' ]],
+},
+description => "Get configured values from either the config file or
config DB.",
+parameters => {
+ additionalProperties => 0,
+ properties => {
+ node => get_standard_option('pve-node'),
+ 'config-keys' => {
+ type => "string",
+ typetext => ":[;:]",
+ pattern => $CONFIGKEYS_RE,
+ description => "List of : items.",
+ }
+ },
+},
+returns => {
+ type => 'object',
+ description => "Contains {section}->{key} children with the values",
+},
+code => sub {
+ my ($param) = @_;
+
+ PVE::Ceph::Tools::check_ceph_inited();
+
+ # Ceph treats '-' and '_' the same in parameter names, stick with '-'
+ my $normalize = sub {
+ my $t = shift;
+ $t =~ s/_/-/g;
+ return $t;
+ };
+
+ my $requested_keys = {};
+ for my $pair (PVE::Tools::split_list($param->{'config-keys'})) {
+ my ($section, $key) = split(":", $pair);
+ $section = $normalize->($section);
+ $key = $normalize->($key);
+
+ $requested_keys->{$section}->{$key} = 1;
+ }
+
+ my $config = {};
+
+ my $rados = PVE::RADOS->new();
+ my $configdb = $rados->mon_command( { prefix => 'config dump', format
=> 'json' });
+ for my $s (@{$configdb}) {
+ my ($section, $name, $value) = $s->@{'section', 'name', 'value'};
+ my $n_section = $normalize->($section);
+ my $n_name = $normalize->($name);
+
+ $config->{$n_section}->{$n_name} = $value
+ if defined $requested_keys->{$n_section} && $n_name eq $n_name;
+ }
+
+ # read ceph.conf after config db as it has priority if settings are
present in both
+ my $config_file = cfs_read_file('ceph.conf');
+ for my $section (keys %{$config_file}) {
+ my $n_section = $normalize->($section);
+ next if !defined $requested_keys->{$n_section};
+
+ for my $key (keys %{$config_file->{$section}}) {
+ my $n_key = $normalize->($key);
+ $config->{$n_section}->{$n_key} =
$config_file->{$section}->{$key}
+ if $requested_keys->{$n_section}->{$n_key};
+ }
+ }
+
+ return $config;
+}});
--
2.30.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH container] lxc start: warn in case of conflicting lxc.idmap entries
On Thu, Mar 16, 2023 at 04:07:34PM +0100, Friedrich Weber wrote: > Thanks for the review! > > On 16/03/2023 14:59, Wolfgang Bumiller wrote: > > Both seem a bit excessive to me. > > > > Let's look at the data: > > We have a set of ranges consisting of a type, 2 starts and a count. > > The types are uids and gids, so we can view those as 2 separate > > instances of sets of [ct_start, host_start, count]. > > Since neither the container nor the host sides must overlap we can - > > again - view these as separate sets of container side [start, count] and > > host side [start, count]. > > In other words, we can see the entire id map as just 4 sets of [start, > > count] ranges which must not overlap. > > > > So I think all we need to do is sort these by the 'start' value, and for > > each element make sure that > > > > prevous_start + previous_count <= current_start > > > > And yes, that means we need to sort $id_maps twice, once by ct id, once > > by host id, and then iterate and do the above check. > > > > Should be much shorter (and faster). > > Yeah, good point, splitting $id_maps into separate uid/gid maps, and then > sorting+iterating twice (I'll call this the "sorting algorithm" below) does > sound more understandable than the current ad-hoc approach, and faster too. > > However, one small benefit of iterating over $id_maps in its original order > (instead of sorting) is that the error message always references the *first* > invalid map entry in the config, e.g. (omitting host uids for clarity) > > 1) u 1000 <...> 100 > 2) u 950 <...> 100 > 3) u 900 <...> 100 > 4) u 850 <...> 100 > > The sorting algorithm would error on entry 3, which might suggest to users > that entries 1-2 are okay (which they are not). The current algorithm errors > on line 2 already. Similar things would happen with interleaved uid/gid > mappings, I guess. > > But I'm not sure if this really matters to users. What do you think? Since it's about helping out users, even better would be to collect all the errors together and than die() with a message containing all of them. And then the order doesn't matter again ;-) ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs v2] conf: add support for a dark mode in the documentation
this commit adds support for a dark theme that behaves similarly to
that one used by the api viewer.
Signed-off-by: Stefan Sterz
---
asciidoc/pve-docs.css | 168 +
asciidoc/pve-html.conf | 4 +
2 files changed, 172 insertions(+)
create mode 100644 asciidoc/pve-docs.css
diff --git a/asciidoc/pve-docs.css b/asciidoc/pve-docs.css
new file mode 100644
index 000..45e2b11
--- /dev/null
+++ b/asciidoc/pve-docs.css
@@ -0,0 +1,168 @@
+:root {
+/* pre-defined colors */
+--pdt-grey-950: hsl(0deg, 0%, 95%);
+--pdt-grey-400: hsl(0deg, 0%, 40%);
+--pdt-grey-250: hsl(0deg, 0%, 25%);
+--pdt-grey-150: hsl(0deg, 0%, 15%);
+--pdt-grey-100: hsl(0deg, 0%, 10%);
+--pdt-primary-850: hsl(205deg, 100%, 85%);
+--pdt-primary-800: hsl(205deg, 100%, 80%);
+--pdt-primary-700: hsl(205deg, 100%, 70%);
+--pdt-secondary-850: hsl(250deg, 100%, 85%);
+}
+
+/* adjust admonition block spacing. this allows for a background on
+ * admonition blocks that doesn't make the elements look to tightly
+ * spaced.
+ */
+div.admonitionblock {
+border-radius: 3px;
+margin: 1.5em 0;
+padding: 0.5em 10% 0.5em 0.5em;
+}
+
+div.admonitionblock td.icon {
+padding-right: 0.5em;
+}
+
+div.admonitionblock td.icon > img {
+box-sizing: border-box;
+padding: 0.15em;
+}
+
+@media screen and (prefers-color-scheme: dark) {
+:root {
+color-scheme: dark;
+--pdt-body-background: var(--pdt-grey-150);
+--pdt-text: var(--pdt-grey-950);
+--pdt-headline: var(--pdt-primary-800);
+--pdt-link: var(--pdt-primary-700);
+--pdt-link-visited: var(--pdt-secondary-850);
+--pdt-highlighted-text: var(--pdt-primary-850);
+--pdt-background-sidebar: var(--pdt-grey-100);
+--pdt-background-listings: var(--pdt-grey-100);
+--pdt-border: var(--pdt-grey-400);
+--pdt-border-alt: var(--pdt-grey-250);
+--pdt-table-border: var(--pdt-grey-400);
+--pdt-background-admonition: var(--pdt-grey-250);
+}
+
+body {
+color: var(--pdt-text);
+background-color: var(--pdt-body-background);
+}
+
+a {
+color: var(--pdt-link);
+}
+
+a:visited {
+color: var(--pdt-link-visited);
+}
+
+/* style headlines, titles etc. */
+h1,
+h2,
+h3,
+h4,
+h5,
+h6,
+thead,
+#author,
+#toctitle,
+div.title,
+td.hdlist1,
+caption.title,
+p.tableblock.header {
+color: var(--pdt-headline);
+}
+
+h1,
+h2,
+h3,
+#footer {
+border-color: var(--pdt-border);
+}
+
+/* formatted colored text */
+dt,
+em,
+pre,
+code,
+strong,
+.monospaced {
+color: var(--pdt-highlighted-text);
+}
+
+/* style the table of contents sidebar */
+div #toc {
+color: var(--pdt-text);
+background-color: var(--pdt-background-sidebar);
+border-color: var(--pdt-border-alt);
+}
+
+div #toc a:link,
+div #toc a:visited {
+color: var(--pdt-text);
+}
+
+/* reduce the brigthness of images a bit and make it reversable
+ * through hovering over them.
+ */
+.image > img {
+filter: brightness(90%);
+}
+
+.image > img:hover {
+filter: none;
+}
+
+/* tables */
+th.tableblock,
+td.tableblock,
+table.tableblock {
+border-color: var(--pdt-table-border);
+}
+
+div.quoteblock,
+div.verseblock {
+color: var(--pdt-text);
+border-color: var(--pdt-border);
+}
+
+/* listings (e.g. code snippet blocks) */
+div.listingblock > div.content {
+background-color: var(--pdt-background-listings);
+border-color: var(--pdt-border-alt);
+}
+
+/* admonition blocks (e.g. notes, warnings etc.) */
+div.admonitionblock {
+color: var(--pdt-text);
+background-color: var(--pdt-background-admonition);
+}
+
+div.admonitionblock td.content {
+border-color: var(--pdt-border);
+}
+
+/* makes the admonition icons appear a bit more consistent, by
+ * adding a white background the shadows in the icons look
+ * "correct"
+ */
+div.admonitionblock td.icon > img {
+background-color: white;
+border-radius: 100%;
+filter: brightness(95%);
+}
+
+/* invert the logo */
+#header > h1 > .image > img {
+filter: invert(100%) hue-rotate(180deg) brightness(90%);
+}
+
+/* fixes the black text on unorderd lists */
+ul > li > * {
+color: var(--pdt-text);
+}
+}
diff --git a/asciidoc/pve-html.conf b/asciidoc/pve-html.conf
index 8a089d3..913169b 100644
--- a/asciidoc/pve-html.conf
+++ b/asciidoc/pve-html.conf
@@ -629,6 +629,10 @@ div .toclevel1 {
endif::toc2[]
+
+include1::{stylesdir=.}/pve-docs.css[]
+
+
ifndef::disable-javascript[]
ifdef::linkcss[]
--
2.30.2
___
