Re: [pve-devel] [PATCH 1/2] assistant: keep prepared iso bootable on uefi with flash drives
On Tue Apr 30, 2024 at 10:54 AM CEST, Aaron Lauterer wrote:
> By mapping files into the ISO, the UUID for the partitions change as
> they depend on the timestamp. The result is, that grub cannot find its
> partition anymore and the user ends up on the grub shell.
>
> This only happens when booting from a blockdev in UEFI mode. E.g. a USB
> flash drive. Alternatively one can `dd` the ISO to a small (2GiB) VM
> disk and mark it as the first boot device.
>
> Booting in legacy mode or via CDROM (e.g. pass through via IPMI), it
> worked.
>
> Xorriso can report the commands needed to recreate the source ISO. The
> '-volume_date uuid' is the one needed to override the same UUIDs. We
> therefore read it first from the source iso and pass it as parameter
> whenever we inject a file into the iso.
>
> Signed-off-by: Aaron Lauterer
> ---
> proxmox-auto-install-assistant/src/main.rs | 44 --
> 1 file changed, 41 insertions(+), 3 deletions(-)
>
> diff --git a/proxmox-auto-install-assistant/src/main.rs
> b/proxmox-auto-install-assistant/src/main.rs
> index 0debd29..e9213f7 100644
> --- a/proxmox-auto-install-assistant/src/main.rs
> +++ b/proxmox-auto-install-assistant/src/main.rs
> @@ -276,6 +276,7 @@ fn show_system_info(_args: &CommandSystemInfo) ->
> Result<()> {
>
> fn prepare_iso(args: &CommandPrepareISO) -> Result<()> {
> check_prepare_requirements(args)?;
> +let uuid = get_iso_uuid(&args.input)?;
>
> if args.fetch_from == FetchAnswerFrom::Iso && args.answer_file.is_none()
> {
> bail!("Missing path to the answer file required for the fetch-from
> 'iso' mode.");
> @@ -331,10 +332,15 @@ fn prepare_iso(args: &CommandPrepareISO) -> Result<()> {
> instmode_file_tmp.push("auto-installer-mode.toml");
> fs::write(&instmode_file_tmp, toml::to_string_pretty(&config)?)?;
>
> -inject_file_to_iso(&tmp_iso, &instmode_file_tmp,
> "/auto-installer-mode.toml")?;
> +inject_file_to_iso(
> +&tmp_iso,
> +&instmode_file_tmp,
> +"/auto-installer-mode.toml",
> +&uuid,
> +)?;
>
> if let Some(answer_file) = &args.answer_file {
> -inject_file_to_iso(&tmp_iso, answer_file, "/answer.toml")?;
> +inject_file_to_iso(&tmp_iso, answer_file, "/answer.toml", &uuid)?;
> }
>
> println!("Moving prepared ISO to target location...");
> @@ -371,11 +377,14 @@ fn final_iso_location(args: &CommandPrepareISO) ->
> PathBuf {
> target.to_path_buf()
> }
>
> -fn inject_file_to_iso(iso: &PathBuf, file: &PathBuf, location: &str) ->
> Result<()> {
> +fn inject_file_to_iso(iso: &PathBuf, file: &PathBuf, location: &str, uuid:
> &String) -> Result<()> {
> let result = Command::new("xorriso")
> .arg("--boot_image")
> .arg("any")
> .arg("keep")
> +.arg("-volume_date")
> +.arg("uuid")
> +.arg(uuid)
> .arg("-dev")
> .arg(iso)
> .arg("-map")
> @@ -391,6 +400,35 @@ fn inject_file_to_iso(iso: &PathBuf, file: &PathBuf,
> location: &str) -> Result<(
> Ok(())
> }
>
> +fn get_iso_uuid(iso: &PathBuf) -> Result {
> +let result = Command::new("xorriso")
> +.arg("-dev")
> +.arg(iso)
> +.arg("-report_system_area")
> +.arg("cmd")
> +.output()?;
> +if !result.status.success() {
> +bail!(
> +"Error determining the UUID of the source ISO: {}",
> +String::from_utf8_lossy(&result.stderr)
> +);
> +}
> +let mut uuid = String::new();
> +for line in String::from_utf8(result.stdout)?.lines() {
> +if line.starts_with("-volume_date uuid") {
> +uuid = line
> +.split(' ')
> +.last()
> +.unwrap()
nit: while this probably won't happen, if xorriso ever starts returning
nothing to the above command, this unwrap may panic. it might be nice to
do a `ok_or_else(|| bail!("xorisso command behaved unexpectedly"))?` or
similar here.
> +.replace('\'', "")
> +.trim()
> +.into();
> +break;
> +}
> +}
> +Ok(uuid)
> +}
> +
> fn get_disks() -> Result>> {
> let unwantend_block_devs = vec![
> "ram[0-9]*",
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server v8 1/3] add C program to get hardware capabilities from CPUID
On Thu Apr 25, 2024 at 1:24 PM CEST, Markus Frank wrote:
> Implement a systemd service that runs a C program that extracts AMD
> SEV hardware information such as reduced-phys-bios and cbitpos from
> CPUID at boot time, looks if SEV, SEV-ES & SEV-SNP are enabled, and
> outputs these details as JSON to /run/qemu-server/hw-params.json.
>
> This programm can also be used to read and save other hardware
> information at boot time.
>
> Signed-off-by: Markus Frank
> Co-authored-by: Thomas Lamprecht
> ---
> v8:
> * renamed query-machine-params to query-machine-capabilities
>
> v7:
> * renamed amd-sev-support to query-machine-params
> * mv /run/amd-sev-params to /run/qemu-server/hw-params.json
> * add "mkdir /run/qemu-server" to ensure that the directory exists
> * moved json content to amd-sev property inside a bigger json
> so that other hardware parameters could also be read at boot time and
> included in this json file.
>
> Makefile | 1 +
> query-machine-capabilities/Makefile | 21 +++
> .../query-machine-capabilities.c | 55 +++
> .../query-machine-capabilities.service| 12
> 4 files changed, 89 insertions(+)
> create mode 100644 query-machine-capabilities/Makefile
> create mode 100644 query-machine-capabilities/query-machine-capabilities.c
> create mode 100644
> query-machine-capabilities/query-machine-capabilities.service
>
> diff --git a/Makefile b/Makefile
> index 133468d..ed67fe0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -65,6 +65,7 @@ install: $(PKGSOURCES)
> install -m 0644 -D bootsplash.jpg $(DESTDIR)/usr/share/$(PACKAGE)
> $(MAKE) -C PVE install
> $(MAKE) -C qmeventd install
> + $(MAKE) -C query-machine-capabilities install
> $(MAKE) -C qemu-configs install
> $(MAKE) -C vm-network-scripts install
> install -m 0755 qm $(DESTDIR)$(SBINDIR)
> diff --git a/query-machine-capabilities/Makefile
> b/query-machine-capabilities/Makefile
> new file mode 100644
> index 000..c5f6348
> --- /dev/null
> +++ b/query-machine-capabilities/Makefile
> @@ -0,0 +1,21 @@
> +DESTDIR=
> +PREFIX=/usr
> +SBINDIR=${PREFIX}/libexec/qemu-server
> +SERVICEDIR=/lib/systemd/system
> +
> +CC ?= gcc
> +CFLAGS += -O2 -fanalyzer -Werror -Wall -Wextra -Wpedantic -Wtype-limits
> -Wl,-z,relro -std=gnu11
> +
> +query-machine-capabilities: query-machine-capabilities.c
> + $(CC) $(CFLAGS) -o $@ $< $(LDFLAGS)
> +
> +.PHONY: install
> +install: query-machine-capabilities
> + install -d ${DESTDIR}/${SBINDIR}
> + install -d ${DESTDIR}${SERVICEDIR}
> + install -m 0644 query-machine-capabilities.service
> ${DESTDIR}${SERVICEDIR}
> + install -m 0755 query-machine-capabilities ${DESTDIR}${SBINDIR}
> +
> +.PHONY: clean
> +clean:
> + rm -f query-machine-capabilities
> diff --git a/query-machine-capabilities/query-machine-capabilities.c
> b/query-machine-capabilities/query-machine-capabilities.c
> new file mode 100644
> index 000..f4a9f9f
> --- /dev/null
> +++ b/query-machine-capabilities/query-machine-capabilities.c
> @@ -0,0 +1,55 @@
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +int main() {
> +uint32_t eax, ebx, ecx, edx;
> +
> +// query Encrypted Memory Capabilities, see:
> +//
> https://en.wikipedia.org/wiki/CPUID#EAX=801Fh:_Encrypted_Memory_Capabilities
> +uint32_t query_function = 0x801F;
> +asm volatile("cpuid"
> + : "=a"(eax), "=b"(ebx), "=c"(ecx), "=d"(edx)
> + : "0"(query_function)
> +);
> +
> +bool sev_support = (eax & (1<<1)) != 0;
> +bool sev_es_support = (eax & (1<<3)) != 0;
> +bool sev_snp_support = (eax & (1<<4)) != 0;
> +
> +uint8_t cbitpos = ebx & 0x3f;
> +uint8_t reduced_phys_bits = (ebx >> 6) & 0x3f;
> +
> +FILE *file;
> +char filename[] = "/run/qemu-server/host-hw-capabilities.json";
> +
> +mkdir("/run/qemu-server/", 0755);
> +
wouldn't it make sense to check whether this call succeeded too like you
do for the `fopen` below? also might be nice to use `strerror` and
handle `errno` in those cases too.
> +file = fopen(filename, "w");
> +if (file == NULL) {
> + perror("Error opening file");
> + return 1;
> +}
> +
> +fprintf(file,
> + "{"
> + " \"amd-sev\": {"
> + " \"cbitpos\": %u,"
> + " \"reduced-phys-bits\": %u,"
> + " \"sev-support\": %s,"
> + " \"sev-support-es\": %s,"
> + " \"sev-support-snp\": %s"
> + " }"
> + " }\n",
> + cbitpos,
> + reduced_phys_bits,
> + sev_support ? "true" : "false",
> + sev_es_support ? "true" : "false",
> + sev_snp_support ? "true" : "false"
> +);
> +
> +fclose(file);
> +return 0;
> +}
> diff --git a/query-machine-capabilities/query-machine-capabilities.service
> b/query-machine-capabilities/query-machine-capabilities.service
> new file mode 100644
> index 000..f926074
> --- /dev/null
> +++ b/query-machine-capabilities/query-machi
Re: [pve-devel] [PATCH manager] pve7to8: reword and fix typos in description
On Thu Apr 18, 2024 at 10:13 AM CEST, Thomas Lamprecht wrote: > Am 18/04/2024 um 10:03 schrieb Stefan Sterz: > >> + before, and during the upgrade of a Proxmox VE system.\n" >> $@.tmp > > > > i know this is pre-existing, but since you are touching this anyway: the > > comma here is odd, if this was supposed to be an oxford comma (or serial > > comma), please be aware that these only apply in lists of three or more > > items. here we have two lists of two items, so the oxford comma does not > > apply. > > yeah, that can be dropped – I often tend to go for the comma if I'm in > doubt, in a hurry, or the like... ^^ > > > > >> + printf "Any failures or warnings must be addressed prior to the > >> upgrade.\n" >> $@.tmp > >> + printf "If you think that a message is a false-positive, check this > >> carefully before proceeding.\n" >> $@.tmp > > > > again a matter of personal taste, but "check this carefully" sounds a > > bit clumsy to me. maybe "double-check that the tool is incorrect before > > proceeding". > > I'd not suggest the tool being incorrect, rather that one should ensure that > the warning does not apply to one's setup. alright, then just that: double-check that it does not apply to your setup. :) ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager] pveversion: show upgradable package version in verbose output
On Thu Apr 18, 2024 at 9:44 AM CEST, Alexander Zeidler wrote:
> when the state is "Installed", including not correctly installed, but
> not for (residual) "ConfigFiles".
>
> The information
> * can be inaccurate for offline nodes or when using POM.
> * is included in pveversion so it can also be used on public channels
> like the forum. The current System Report includes pveversion -v
>
> # pveversion -v
> proxmox-ve: 8.1.0 (running kernel: 6.5.13-5-pve)
> pve-manager: 8.1.6 (running version: 8.1.6/b7e8e914a1db70cc) [available:
> 8.1.10]
> ...
> vncterm: 1.8.0
> zfsutils-linux: 2.2.3-pve1 [available: 2.2.3-pve2]
>
> Signed-off-by: Alexander Zeidler
> ---
> bin/pveversion | 22 --
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/bin/pveversion b/bin/pveversion
> index 591f63e7..b98e1096 100755
> --- a/bin/pveversion
> +++ b/bin/pveversion
> @@ -21,19 +21,29 @@ sub print_status {
> print "$pkg: unknown package - internal error\n";
> return;
> }
> +
> +my $installed = $pkginfo->{CurrentState} eq 'Installed';
> +my $upgradable = $pkginfo->{OldVersion} && ($pkginfo->{OldVersion} ne
> $pkginfo->{Version});
> +
> my $version = "not correctly installed";
> -if ($pkginfo->{OldVersion} && $pkginfo->{CurrentState} eq 'Installed') {
> +if ($installed && $pkginfo->{OldVersion}) {
> $version = $pkginfo->{OldVersion};
> } elsif ($pkginfo->{CurrentState} eq 'ConfigFiles') {
> $version = 'residual config';
> }
>
> if ($pkginfo->{RunningKernel}) {
> - print "$pkg: $version (running kernel: $pkginfo->{RunningKernel})\n";
> + print "$pkg: $version (running kernel: $pkginfo->{RunningKernel})";
> } elsif ($pkginfo->{ManagerVersion}) {
> - print "$pkg: $version (running version: $pkginfo->{ManagerVersion})\n";
> + print "$pkg: $version (running version: $pkginfo->{ManagerVersion})";
> +} else {
> + print "$pkg: $version";
> +}
> +
> +if ($installed && $upgradable) {
> + print " [available: " . $pkginfo->{Version} . "]\n";
nit: i think we generally prefer string interpolation over
concatenation, as in the code above. so simply:
print " [available: $pkginfo->{Version} ]\n";
should work.
> } else {
> - print "$pkg: $version\n";
> + print "\n";
> }
> }
>
the changes below just remove superfluous whitespace, this might be
better handled separatelly as these changes have nothing to do with the
functionality added above.
> @@ -50,7 +60,7 @@ my $opt_verbose;
> if (!GetOptions ('verbose' => \$opt_verbose)) {
> print_usage ();
> exit (-1);
> -}
> +}
>
> if (scalar (@ARGV) != 0) {
> print_usage ();
> @@ -76,7 +86,7 @@ __END__
>
> =head1 NAME
>
> -pveversion - Proxmox VE version info
> +pveversion - Proxmox VE version info
>
> =head1 SYNOPSIS
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager] pve7to8: reword and fix typos in description
On Thu Apr 18, 2024 at 9:44 AM CEST, Alexander Zeidler wrote: > Signed-off-by: Alexander Zeidler > --- > bin/Makefile | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/bin/Makefile b/bin/Makefile > index 180a91b5..aa149c06 100644 > --- a/bin/Makefile > +++ b/bin/Makefile > @@ -66,10 +66,10 @@ pve6to7.1: > > pve7to8.1: > printf ".TH PVE7TO8 1\n.SH NAME\npve7to8 \- Proxmox VE upgrade checker > script for 7.4+ to current 8.x\n" > $@.tmp > - printf ".SH DESCRIPTION\nThis tool will help you to detect common > pitfalls and misconfguration\ > - before, and during the upgrade of a Proxmox VE system\n" >> $@.tmp > - printf "Any failure must be addressed before the upgrade, and any > waring must be addressed, \ > - or at least carefully evaluated, if a false-positive is suspected\n" > >> $@.tmp > + printf ".SH DESCRIPTION\nThis tool will help you to detect common > pitfalls and misconfiguration\ nit: the "to" in "to detect" is superfluous, "will help you detect" flows much nicer also, and this is a matter of personal style "misconfigurations" sounds better to me here because you stay in the plural, but that's really minor. > + before, and during the upgrade of a Proxmox VE system.\n" >> $@.tmp i know this is pre-existing, but since you are touching this anyway: the comma here is odd, if this was supposed to be an oxford comma (or serial comma), please be aware that these only apply in lists of three or more items. here we have two lists of two items, so the oxford comma does not apply. > + printf "Any failures or warnings must be addressed prior to the > upgrade.\n" >> $@.tmp > + printf "If you think that a message is a false-positive, check this > carefully before proceeding.\n" >> $@.tmp again a matter of personal taste, but "check this carefully" sounds a bit clumsy to me. maybe "double-check that the tool is incorrect before proceeding". > printf ".SH SYNOPSIS\npve7to8 [--full]\n" >> $@.tmp > mv $@.tmp $@ > ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH widget-toolkit] dark-mode: set intentionally black icons to `$icon-color`
On Mon Oct 16, 2023 at 6:28 PM CEST, Stefan Sterz wrote:
> some icons intentionally use black as their color in the light theme.
> this includes the little pencil and check mark icon in the acme
> overview. change their color to the regular dark-mode icon-color. for
> this to work the filter inversion needed for some other icons needs to
> be remove too.
>
> Signed-off-by: Stefan Sterz
> ---
> src/proxmox-dark/scss/other/_icons.scss | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/src/proxmox-dark/scss/other/_icons.scss
> b/src/proxmox-dark/scss/other/_icons.scss
> index d4dc316..c045cf4 100644
> --- a/src/proxmox-dark/scss/other/_icons.scss
> +++ b/src/proxmox-dark/scss/other/_icons.scss
> @@ -104,6 +104,9 @@
> }
>
> // pbs show task log in longest task list column
> +.fa.black,
> +.fa.black::after,
> +.fa.black::before,
> .x-action-col-icon.fa-chevron-right::before {
>filter: none;
> }
> @@ -222,6 +225,12 @@
>}
> }
>
> +// set icon color of intentional black icons (e.g.: pencil icon for
> +// quickly changing the ACME account)
> +.fa.black {
> + color: $icon-color;
> +}
> +
> // The usage icons dynamically displaying how full a storage is
> .usage-wrapper {
>border: 1px solid $icon-color;
> --
> 2.39.2
ping, this still applies for me :)
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-storage] esxi: add mapping for windows server 2016/2019
previously these were mapped to the linux 2.6 default Signed-off-by: Stefan Sterz --- src/PVE/Storage/ESXiPlugin.pm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/PVE/Storage/ESXiPlugin.pm b/src/PVE/Storage/ESXiPlugin.pm index 4212c36..e5082ea 100644 --- a/src/PVE/Storage/ESXiPlugin.pm +++ b/src/PVE/Storage/ESXiPlugin.pm @@ -878,6 +878,8 @@ my %guest_types_windows = ( winNetBusiness => 'w2k3', windows9=> 'win10', 'windows9-64' => 'win10', +windows9srv => 'win10', +'windows9srv-64'=> 'win10', 'windows11-64' => 'win11', 'windows12-64' => 'win11', # FIXME / win12? win2000AdvServ => 'w2k', -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH widget-toolkit v4] window: edit: avoid sharing custom config objects between subclasses
On Tue Apr 9, 2024 at 10:16 AM CEST, Friedrich Weber wrote:
> Currently, `Proxmox.window.Edit` initializes `extraRequestParams` and
> `submitOptions` to two objects that, if not overwritten, are shared
> between all instances of subclasses. This bears the danger of
> modifying the shared object in a subclass instead of overwriting it,
> which affects all edit windows of the current session and can cause
> hard-to-catch GUI bugs.
>
> One such bug is the following: Currently, the `PVE.pool.AddStorage`
> component inadvertently adds `poolid` to an `extraRequestParams`
> object that is shared between all instances of `Proxmox.window.Edit`.
> As a result, after adding a storage to a pool, opening any edit window
> will send a GET request with a superfluous `poolid` parameter and
> cause an error in the GUI:
>
> > Parameter verification failed. (400)
> > poolid: property is not defined in schema and the schema does not
> > allow additional properties
>
> This breaks all edit windows of the current session. A workaround is
> to reload the current browser session.
>
> To avoid this class of bugs in the future, implement a constructor
> that makes copies of `extraRequestParams` and `submitOptions`. This
> ensures that any subclass instance modifies only its own copies, and
> modifications do not leak to other subclass instances.
>
> Suggested-by: Stefan Sterz
> Suggested-by: Thomas Lamprecht
> Signed-off-by: Friedrich Weber
> ---
>
> Notes:
> @Thomas, I've added a Suggested-by, feel free to remove/keep as you
> prefer.
>
> Changes from v3:
> - Fix broken pool edit window (thx sterzy!) by passing all arguments
> to `callParent`. The `initConfig` call is obsolete as the constructor
> of `Ext.Component` [1] calls `initConfig` already.
>
> Changes from v1+v2:
> - As suggested by sterzy (thx!), avoid this class of bugs in a more
> generic fashion by introducing a `Proxmox.window.Edit` constructor
> that copies custom config objects
> - Added full error message to commit message for better searchability
>
> v3: https://lists.proxmox.com/pipermail/pve-devel/2024-April/062657.html
> v2: https://lists.proxmox.com/pipermail/pve-devel/2024-April/062561.html
> v1: https://lists.proxmox.com/pipermail/pve-devel/2024-March/062179.html
>
> [1]
> https://docs.sencha.com/extjs/7.0.0/classic/src/Component.js.html#line2203
>
> src/window/Edit.js | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/src/window/Edit.js b/src/window/Edit.js
> index d4a2b551..c55ff793 100644
> --- a/src/window/Edit.js
> +++ b/src/window/Edit.js
> @@ -69,6 +69,15 @@ Ext.define('Proxmox.window.Edit', {
> // onlineHelp of our first item, if set.
> onlineHelp: undefined,
>
> +constructor: function(conf) {
> + let me = this;
> + // make copies in order to prevent subclasses from accidentally writing
> + // to objects that are shared with other edit window subclasses
> + me.extraRequestParams = Object.assign({}, me.extraRequestParams);
> + me.submitOptions = Object.assign({}, me.submitOptions);
> + me.callParent(arguments);
> +},
> +
> isValid: function() {
> let me = this;
>
this looks good to me, i've also tested this here, so consider this:
Tested-by: Stefan Sterz
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH widget-toolkit v3] window: edit: avoid sharing custom config objects between subclasses
On Mon Apr 8, 2024 at 11:30 AM CEST, Friedrich Weber wrote:
> Currently, `Proxmox.window.Edit` initializes `extraRequestParams` and
> `submitOptions` to two objects that, if not overwritten, are shared
> between all instances of subclasses. This bears the danger of
> modifying the shared object in a subclass instead of overwriting it,
> which affects all edit windows of the current session and can cause
> hard-to-catch GUI bugs.
>
> One such bug is the following: Currently, the `PVE.pool.AddStorage`
> component inadvertently adds `poolid` to an `extraRequestParams`
> object that is shared between all instances of `Proxmox.window.Edit`.
> As a result, after adding a storage to a pool, opening any edit window
> will send a GET request with a superfluous `poolid` parameter and
> cause an error in the GUI:
>
> > Parameter verification failed. (400)
> > poolid: property is not defined in schema and the schema does not
> > allow additional properties
>
> This breaks all edit windows of the current session. A workaround is
> to reload the current browser session.
>
> To avoid this class of bugs in the future, implement a constructor
> that makes copies of `extraRequestParams` and `submitOptions`. This
> ensures that any subclass instance modifies only its own copies, and
> modifications do not leak to other subclass instances.
>
> Suggested-by: Stefan Sterz
> Suggested-by: Thomas Lamprecht
> Signed-off-by: Friedrich Weber
> ---
>
> Notes:
> @Thomas, I've added a Suggested-by, feel free to remove/keep as you
> prefer.
>
> Changes from v1+v2:
> - As suggested by sterzy (thx!), avoid this class of bugs in a more
> generic fashion by introducing a `Proxmox.window.Edit` constructor
> that copies custom config objects
> - Added full error message to commit message for better searchability
>
> v2: https://lists.proxmox.com/pipermail/pve-devel/2024-April/062561.html
> v1: https://lists.proxmox.com/pipermail/pve-devel/2024-March/062179.html
>
> src/window/Edit.js | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/src/window/Edit.js b/src/window/Edit.js
> index d4a2b551..d5163dd7 100644
> --- a/src/window/Edit.js
> +++ b/src/window/Edit.js
> @@ -69,6 +69,16 @@ Ext.define('Proxmox.window.Edit', {
> // onlineHelp of our first item, if set.
> onlineHelp: undefined,
>
> +constructor: function(conf) {
> + let me = this;
> + // make copies in order to prevent subclasses from accidentally writing
> + // to objects that are shared with other edit window subclasses
> + me.extraRequestParams = Object.assign({}, me.extraRequestParams);
> + me.submitOptions = Object.assign({}, me.submitOptions);
> + me.initConfig(conf);
> + me.callParent();
so, this seems like a fix bug a) creates bug b) type of situation...
this patch means that editing a pool allows changing the name suddenly,
but since we don't support that in the backend, that just creates a new
pool :/
this is due to the `editable` attribute depending on `isCreate`, which
in turn depends on the configs poolid being set. to fix this, the config
needs to also be passed to `callParent` so it can set the configurations
there too. so this line should be:
me.callParent([conf]);
sorry, could have noticed that earlier in my suggestion. also this needs
to be an arrray as `callParent` expects a list of arguments to pass to
parent's function and not the parameters themselves directly.
> +},
> +
> isValid: function() {
> let me = this;
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH widget-toolkit 3/3] window: edit: avoid shared object for extra request params
On Thu Apr 4, 2024 at 12:54 PM CEST, Stefan Sterz wrote:
> On Thu Apr 4, 2024 at 12:10 PM CEST, Friedrich Weber wrote:
> > On 04/04/2024 11:23, Stefan Sterz wrote:
> > > -- >8 snip 8< --
> > >>>
> > >>> i did a quick an dirty test and using a constructor like this seems to
> > >>> rule out this class of bug completelly:
> > >>>
> > >>> ```js
> > >>> constructor: function(conf) {
> > >>> let me = this;
> > >>> me.extraRequestParams = {};
> > >>> me.initConfig(conf);
> > >>> me.callParent();
> > >>> },
> > >>> ```
> > >>>
> > >>> basically it configures the edit window as usual, but overwrites the
> > >>> `extraRequestParams` object for each instance with a new empty object.
> > >>> so there are no more shared objects :) could you check whether that also
> > >>> fixes the other instances?
> > >>>
> > >>> [1]:
> > >>> https://docs-devel.sencha.com/extjs/7.0.0/classic/Ext.window.Window.html#method-constructor
> > >>
> > >> Nifty, didn't think about a constructor solution. Such a general
> > >> solution would be way more elegant, thanks for suggesting it!
> > >>
> > >> However, this particular constructor seems to break the pattern of
> > >> defining `extraRequestParams` in the subclass properties, as done by
> > >> `PVE.Pool.AddVM` [1]. With the constructor above, the API request done
> > >> by `AddVM` seems to be missing the `allow-move` parameter.
> > >>
> > >> Looks like once `PVE.Pool.AddVM` is instantiated and the constructor is
> > >> called, `extraRequestParams` with `allow-move` is only defined in
> > >> `me.__proto__`, so `me.extraRequestParams = {}` essentially shadows it
> > >> with an empty object, losing the `allow-move`.
> > >>
> > >
> > > not sure what you mean by that, if an `PVE.Pool.AddVM` is instantiated,
> > > the `extraRequestParams` is already set, so it isn't just in `__proto__`
> > > for me. but yeah, the problem is correct as `me.extraRequestParams = {}`
> > > overwrites the field.
> >
> > I agree it doesn't matter here, but just for completeness, I meant that
> > if I set a breakpoint before line 2, so before the overwrite:
> >
> > ```js
> > constructor: function(conf) {
> > let me = this;
> > =>me.extraRequestParams = {};
> > me.initConfig(conf);
> > me.callParent();
> > },
> > ```
> >
> > ... `extraRequestParams` is not a property of `me`, but inherited from
> > its prototype:
> >
> > ```
> > >> me.extraRequestParams
> > Object { "allow-move": 1 }
> > >> "extraRequestParams" in me
> > true
> > >> Object.hasOwn(me, "extraRequestParams")
> > false
> > ```
> >
> > Doesn't make a difference for the overwrite, though.
> >
>
> ah yeah, that makes sense, but yeah, it doesn't really matter here i
> suppose.
>
> > >> Do you have an idea how to fix this? Maybe making a copy of
> > >> `extraRequestParams` would work (I suppose the overhead of creating a
> > >> new object for all edit window (subclass) instances is negligible).
> > >>
> > >> [1]
> > >> https://git.proxmox.com/?p=pve-manager.git;a=blob;f=www/manager6/grid/PoolMembers.js;h=75f20cab;hb=4b06efb5#l9
> > >
> > > this worked for me, can you confirm that this also does what it should
> > > for you?
> > >
> > > ```js
> > > extraRequestParams: undefined,
> > >
> > > constructor: function(conf) {
> > > let me = this;
> > > if (!me.extraRequestParams) {
> > > me.extraRequestParams = {};
> > > }
> > > me.initConfig(conf);
> > > me.callParent();
> > > },
> > > ```
> >
> > It works in the sense that it fixes the bug mentioned in my patch 1/3,
> > and fixes the lost `allow-move` issue from the previous constructor. But
> > with this constructor, all instances of `AddVM` share the same
> > `extraRequestParams` (the body of the `if` never gets executed for
> > `AddVM` instances), which is the condition that my patch 2/3 tries to
> > avoid (even though it is curr
Re: [pve-devel] [PATCH widget-toolkit 3/3] window: edit: avoid shared object for extra request params
On Thu Apr 4, 2024 at 12:10 PM CEST, Friedrich Weber wrote:
> On 04/04/2024 11:23, Stefan Sterz wrote:
> > -- >8 snip 8< --
> >>>
> >>> i did a quick an dirty test and using a constructor like this seems to
> >>> rule out this class of bug completelly:
> >>>
> >>> ```js
> >>> constructor: function(conf) {
> >>> let me = this;
> >>> me.extraRequestParams = {};
> >>> me.initConfig(conf);
> >>> me.callParent();
> >>> },
> >>> ```
> >>>
> >>> basically it configures the edit window as usual, but overwrites the
> >>> `extraRequestParams` object for each instance with a new empty object.
> >>> so there are no more shared objects :) could you check whether that also
> >>> fixes the other instances?
> >>>
> >>> [1]:
> >>> https://docs-devel.sencha.com/extjs/7.0.0/classic/Ext.window.Window.html#method-constructor
> >>
> >> Nifty, didn't think about a constructor solution. Such a general
> >> solution would be way more elegant, thanks for suggesting it!
> >>
> >> However, this particular constructor seems to break the pattern of
> >> defining `extraRequestParams` in the subclass properties, as done by
> >> `PVE.Pool.AddVM` [1]. With the constructor above, the API request done
> >> by `AddVM` seems to be missing the `allow-move` parameter.
> >>
> >> Looks like once `PVE.Pool.AddVM` is instantiated and the constructor is
> >> called, `extraRequestParams` with `allow-move` is only defined in
> >> `me.__proto__`, so `me.extraRequestParams = {}` essentially shadows it
> >> with an empty object, losing the `allow-move`.
> >>
> >
> > not sure what you mean by that, if an `PVE.Pool.AddVM` is instantiated,
> > the `extraRequestParams` is already set, so it isn't just in `__proto__`
> > for me. but yeah, the problem is correct as `me.extraRequestParams = {}`
> > overwrites the field.
>
> I agree it doesn't matter here, but just for completeness, I meant that
> if I set a breakpoint before line 2, so before the overwrite:
>
> ```js
> constructor: function(conf) {
> let me = this;
> =>me.extraRequestParams = {};
> me.initConfig(conf);
> me.callParent();
> },
> ```
>
> ... `extraRequestParams` is not a property of `me`, but inherited from
> its prototype:
>
> ```
> >> me.extraRequestParams
> Object { "allow-move": 1 }
> >> "extraRequestParams" in me
> true
> >> Object.hasOwn(me, "extraRequestParams")
> false
> ```
>
> Doesn't make a difference for the overwrite, though.
>
ah yeah, that makes sense, but yeah, it doesn't really matter here i
suppose.
> >> Do you have an idea how to fix this? Maybe making a copy of
> >> `extraRequestParams` would work (I suppose the overhead of creating a
> >> new object for all edit window (subclass) instances is negligible).
> >>
> >> [1]
> >> https://git.proxmox.com/?p=pve-manager.git;a=blob;f=www/manager6/grid/PoolMembers.js;h=75f20cab;hb=4b06efb5#l9
> >
> > this worked for me, can you confirm that this also does what it should
> > for you?
> >
> > ```js
> > extraRequestParams: undefined,
> >
> > constructor: function(conf) {
> > let me = this;
> > if (!me.extraRequestParams) {
> > me.extraRequestParams = {};
> > }
> > me.initConfig(conf);
> > me.callParent();
> > },
> > ```
>
> It works in the sense that it fixes the bug mentioned in my patch 1/3,
> and fixes the lost `allow-move` issue from the previous constructor. But
> with this constructor, all instances of `AddVM` share the same
> `extraRequestParams` (the body of the `if` never gets executed for
> `AddVM` instances), which is the condition that my patch 2/3 tries to
> avoid (even though it is currently not buggy).
>
> Maybe we could do:
>
> ```js
> extraRequestParams: {},
>
> constructor: function(conf) {
> let me = this;
> me.extraRequestParams = Ext.clone(me.extraRequestParams);
> me.initConfig(conf);
> me.callParent();
> },
> ```
>
> ... which, if I'm not missing anything, *should* cover everything (with
> the cost of allocating unnecessary empty objects)?
yeah looks good to me. cloning shouldn't cost too much here. if we are
really worried we could check whether the object is empty, clone
it in that case and assign an empty object otherwise.
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH widget-toolkit 3/3] window: edit: avoid shared object for extra request params
-- >8 snip 8< --
> >
> > i did a quick an dirty test and using a constructor like this seems to
> > rule out this class of bug completelly:
> >
> > ```js
> > constructor: function(conf) {
> > let me = this;
> > me.extraRequestParams = {};
> > me.initConfig(conf);
> > me.callParent();
> > },
> > ```
> >
> > basically it configures the edit window as usual, but overwrites the
> > `extraRequestParams` object for each instance with a new empty object.
> > so there are no more shared objects :) could you check whether that also
> > fixes the other instances?
> >
> > [1]:
> > https://docs-devel.sencha.com/extjs/7.0.0/classic/Ext.window.Window.html#method-constructor
>
> Nifty, didn't think about a constructor solution. Such a general
> solution would be way more elegant, thanks for suggesting it!
>
> However, this particular constructor seems to break the pattern of
> defining `extraRequestParams` in the subclass properties, as done by
> `PVE.Pool.AddVM` [1]. With the constructor above, the API request done
> by `AddVM` seems to be missing the `allow-move` parameter.
>
> Looks like once `PVE.Pool.AddVM` is instantiated and the constructor is
> called, `extraRequestParams` with `allow-move` is only defined in
> `me.__proto__`, so `me.extraRequestParams = {}` essentially shadows it
> with an empty object, losing the `allow-move`.
>
not sure what you mean by that, if an `PVE.Pool.AddVM` is instantiated,
the `extraRequestParams` is already set, so it isn't just in `__proto__`
for me. but yeah, the problem is correct as `me.extraRequestParams = {}`
overwrites the field.
> Do you have an idea how to fix this? Maybe making a copy of
> `extraRequestParams` would work (I suppose the overhead of creating a
> new object for all edit window (subclass) instances is negligible).
>
> [1]
> https://git.proxmox.com/?p=pve-manager.git;a=blob;f=www/manager6/grid/PoolMembers.js;h=75f20cab;hb=4b06efb5#l9
this worked for me, can you confirm that this also does what it should
for you?
```js
extraRequestParams: undefined,
constructor: function(conf) {
let me = this;
if (!me.extraRequestParams) {
me.extraRequestParams = {};
}
me.initConfig(conf);
me.callParent();
},
```
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH widget-toolkit 3/3] window: edit: avoid shared object for extra request params
On Wed Apr 3, 2024 at 11:10 AM CEST, Friedrich Weber wrote:
> Currently, `Proxmox.window.Edit` initializes `extraRequestParams` to
> an object that, if not overwritten, is shared between all instances of
> subclasses. This bears the danger of modifying the shared object in a
> subclass instead of overwriting it, which affects all edit windows of
> the current session and can cause hard-to-catch UI bugs [1].
>
> To avoid such bugs in the future, initialize `extraRequestParams` to
> `undefined` instead, which forces subclasses to initialize their own
> objects.
>
> Note that bugs of the same kind can still happen if a subclass
> initializes `extraRequestParams` to an empty shared object and
> inadvertently modifies it, but at least they will be limited to that
> particular subclass.
>
> [1] https://lists.proxmox.com/pipermail/pve-devel/2024-March/062179.html
>
> Signed-off-by: Friedrich Weber
> ---
>
> Notes:
> With patch 2/3 applied, I think all occurrences of
> `extraRequestParams` in PVE/PBS create their own object (PMG does not
> seem to use `extraRequestParams`), so this patch should not break
> anything, and if it does, it should be quite noticeable.
>
> new in v2
>
> src/window/Edit.js | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/src/window/Edit.js b/src/window/Edit.js
> index d4a2b551..27cd8d01 100644
> --- a/src/window/Edit.js
> +++ b/src/window/Edit.js
> @@ -9,7 +9,7 @@ Ext.define('Proxmox.window.Edit', {
>
> // to submit extra params on load and submit, useful, e.g., if not all ID
> // parameters are included in the URL
> -extraRequestParams: {},
> +extraRequestParams: undefined,
>
> resizable: false,
>
> @@ -80,7 +80,9 @@ Ext.define('Proxmox.window.Edit', {
> let me = this;
>
> let values = {};
> - Ext.apply(values, me.extraRequestParams);
> + if (me.extraRequestParams) {
> + Ext.apply(values, me.extraRequestParams);
> + }
>
> let form = me.formPanel.getForm();
>
> @@ -209,7 +211,7 @@ Ext.define('Proxmox.window.Edit', {
> waitMsgTarget: me,
> }, options);
>
> - if (Object.keys(me.extraRequestParams).length > 0) {
> + if (me.extraRequestParams && Object.keys(me.extraRequestParams).length
> > 0) {
> let params = newopts.params || {};
> Ext.applyIf(params, me.extraRequestParams);
> newopts.params = params;
i did a quick an dirty test and using a constructor like this seems to
rule out this class of bug completelly:
```js
constructor: function(conf) {
let me = this;
me.extraRequestParams = {};
me.initConfig(conf);
me.callParent();
},
```
basically it configures the edit window as usual, but overwrites the
`extraRequestParams` object for each instance with a new empty object.
so there are no more shared objects :) could you check whether that also
fixes the other instances?
[1]:
https://docs-devel.sencha.com/extjs/7.0.0/classic/Ext.window.Window.html#method-constructor
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server 3/3] api: include not mapped resources for running vms in migrate preconditions
On Wed Mar 20, 2024 at 1:51 PM CET, Dominik Csapak wrote:
> so that we can show a proper warning in the migrate dialog and check it
> in the bulk migrate precondition check
>
> the unavailable_storages and allowed_nodes should be the same as before
>
> Signed-off-by: Dominik Csapak
> ---
> not super happy with this partial approach, we probably should just
> always return the 'allowed_nodes' and 'not_allowed_nodes' and change
> the gui to handle the running vs not running state?
>
> PVE/API2/Qemu.pm | 27 +++
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 8581a529..b0f155f7 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -4439,7 +4439,7 @@ __PACKAGE__->register_method({
> not_allowed_nodes => {
> type => 'object',
> optional => 1,
> - description => "List not allowed nodes with additional
> informations, only passed if VM is offline"
> + description => "List not allowed nodes with additional
> informations",
"information" has no plural, this should just be "additional
information".
> },
> local_disks => {
> type => 'array',
> @@ -4496,25 +4496,28 @@ __PACKAGE__->register_method({
>
> # if vm is not running, return target nodes where local storage/mapped
> devices are available
> # for offline migration
> + my $checked_nodes = {};
> + my $allowed_nodes = [];
> if (!$res->{running}) {
> - $res->{allowed_nodes} = [];
> - my $checked_nodes =
> PVE::QemuServer::check_local_storage_availability($vmconf, $storecfg);
> + $checked_nodes =
> PVE::QemuServer::check_local_storage_availability($vmconf, $storecfg);
> delete $checked_nodes->{$localnode};
> + }
>
> - foreach my $node (keys %$checked_nodes) {
> - my $missing_mappings = $missing_mappings_by_node->{$node};
> - if (scalar($missing_mappings->@*)) {
> - $checked_nodes->{$node}->{'unavailable-resources'} =
> $missing_mappings;
> - next;
> - }
> + foreach my $node ((keys $checked_nodes->%*, keys
> $missing_mappings_by_node->%*)) {
> + my $missing_mappings = $missing_mappings_by_node->{$node};
> + if (scalar($missing_mappings->@*)) {
> + $checked_nodes->{$node}->{'unavailable-resources'} =
> $missing_mappings;
> + next;
> + }
>
> + if (!$res->{running}) {
> if (!defined($checked_nodes->{$node}->{unavailable_storages})) {
> - push @{$res->{allowed_nodes}}, $node;
> + push $allowed_nodes->@*, $node;
> }
> -
> }
> - $res->{not_allowed_nodes} = $checked_nodes;
> }
> + $res->{not_allowed_nodes} = $checked_nodes if
> scalar(keys($checked_nodes->%*)) || !$res->{running};
> + $res->{allowed_nodes} = $allowed_nodes if scalar($allowed_nodes->@*) ||
> !$res->{running};
>
> my $local_disks = &$check_vm_disks_local($storecfg, $vmconf, $vmid);
> $res->{local_disks} = [ values %$local_disks ];;
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs 2/2] qm: add documentation on configuring multiqueue for windows guests
Signed-off-by: Stefan Sterz --- qm.adoc | 10 ++ 1 file changed, 10 insertions(+) diff --git a/qm.adoc b/qm.adoc index 8630419..711fa3f 100644 --- a/qm.adoc +++ b/qm.adoc @@ -780,6 +780,16 @@ ethtool command: where X is the number of the number of vCPUs of the VM. +To configure a Windows guest for Multiqueue install the +https://fedorapeople.org/groups/virt/virtio-win/direct-downloads/stable-virtio/virtio-win.iso[ +Redhat VirtIO Ethernet Adapter drivers], then adapt the NIC's configuration as +follows. Open the device manager, right click the NIC under "Network adapters", +and select "Properties". Then open the "Advanced" tab and select "Receive Side +Scaling" from the list on the left. Make sure it is set to "Enabled". Next, +navigate to "Maximum number of RSS Queues" in the list and set it to the number +of vCPUs of your VM. Once you verified that the settings are correct, click "OK" +to confirm them. + You should note that setting the Multiqueue parameter to a value greater than one will increase the CPU load on the host and guest systems as the traffic increases. We recommend to set this option only when the VM has to -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH docs 1/2] qm: multiqueue specify that it needs to be vCPUs not cores
total number of cores != vCPUs if there is more than one socket configured. according to the redhat docs it should be vCPUs not cores: > Multi-queue virtio-net provides the greatest performance benefit when: > [..] > - The number of queues is equal to the number of vCPUs. https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/virtualization_tuning_and_optimization_guide/sect-virtualization_tuning_optimization_guide-networking-techniques#sect-Virtualization_Tuning_Optimization_Guide-Networking-Multi-queue_virtio-net Signed-off-by: Stefan Sterz --- qm.adoc | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/qm.adoc b/qm.adoc index 1170dd1..8630419 100644 --- a/qm.adoc +++ b/qm.adoc @@ -770,14 +770,15 @@ vhost driver. With this option activated, it is possible to pass _multiple_ network queues to the host kernel for each NIC. //https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Virtualization_Tuning_and_Optimization_Guide/sect-Virtualization_Tuning_Optimization_Guide-Networking-Techniques.html#sect-Virtualization_Tuning_Optimization_Guide-Networking-Multi-queue_virtio-net -When using Multiqueue, it is recommended to set it to a value equal -to the number of Total Cores of your guest. You also need to set in -the VM the number of multi-purpose channels on each VirtIO NIC with the ethtool -command: +When using Multiqueue, it is recommended to set it to a value equal to the +number of vCPUs of your guest. Remember that the number of vCPUs is the number +of sockets times the number of cores configured for the VM. You also need to set +the number of multi-purpose channels on each VirtIO NIC in the VM with this +ethtool command: `ethtool -L ens1 combined X` -where X is the number of the number of vcpus of the VM. +where X is the number of the number of vCPUs of the VM. You should note that setting the Multiqueue parameter to a value greater than one will increase the CPU load on the host and guest systems as the -- 2.39.2 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH storage] esxi: detect correct os type in 'other' family
-->8 snip 8<-- > diff --git a/src/PVE/Storage/ESXiPlugin.pm.tdy > b/src/PVE/Storage/ESXiPlugin.pm.tdy > new file mode 100644 > index 000..2a08986 > --- /dev/null > +++ b/src/PVE/Storage/ESXiPlugin.pm.tdy > @@ -0,0 +1,1216 @@ > +package PVE::Storage::ESXiPlugin; > + talked off list with gabriel already this file was added accidentally. -->8 snip 8<-- ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH docs] system-requirements: mention that SSDs with PLP should be used
On Wed Mar 20, 2024 at 10:49 AM CET, Aaron Lauterer wrote: > > > On 2024-03-20 10:30, Fiona Ebner wrote: > > Am 20.03.24 um 09:56 schrieb Aaron Lauterer: > >> Signed-off-by: Aaron Lauterer > >> --- > >> pve-system-requirements.adoc | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/pve-system-requirements.adoc b/pve-system-requirements.adoc > >> index bc3689d..4db5358 100644 > >> --- a/pve-system-requirements.adoc > >> +++ b/pve-system-requirements.adoc > >> @@ -49,6 +49,8 @@ Recommended System Requirements > >> (BBU) or non-RAID for ZFS and Ceph. Neither ZFS nor Ceph are > >> compatible with a > >> hardware RAID controller. > >> ** Shared and distributed storage is possible. > >> +** SSDs with Power-Loss-Protection (PLP) are recommended for good > >> performance. > >> + Using consumer SSDs is discouraged. > >> > > > > Having PLP might correlate with having good performance, but it's not > > the reason for good performance and good performance is not the reason > > you want PLP. It's just that both things are present in many enterprise > > SSDs, I'd mention that explicitly to avoid potential confusion. > > When it comes to sync writes, it is definitely one reason for the good > performance ;) > But yeah, let's think about it, what about the following?: > > > Enterprise grade SSDs are recommended for good performance. Checking for > Power-Loss-Protection (PLP) is a good way to avoid consumer grade > SSDs. The use of consumer grade SSDs is discouraged. > > > Not too happy with that either, but phrasing it correctly and succinct > is an art in itself. > How about: Enterprise SSDs with good performance are recommended. Power-Loss-Protection (PLP) support can help identify such disks. Using consumer SSDs is discouraged > > > >> * Redundant (Multi-)Gbit NICs, with additional NICs depending on the > >> preferred > >> storage technology and cluster setup. > > > ___ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH docs] pveceph: document cluster shutdown
On Tue Mar 19, 2024 at 4:00 PM CET, Aaron Lauterer wrote:
> Signed-off-by: Aaron Lauterer
> ---
> pveceph.adoc | 50 ++
> 1 file changed, 50 insertions(+)
>
> diff --git a/pveceph.adoc b/pveceph.adoc
> index 089ac80..7b493c5 100644
> --- a/pveceph.adoc
> +++ b/pveceph.adoc
> @@ -1080,6 +1080,56 @@ scrubs footnote:[Ceph scrubbing
> {cephdocs-url}/rados/configuration/osd-config-re
> are executed.
>
>
> +[[pveceph_shutdown]]
> +Shutdown {pve} + Ceph HCI cluster
> +~
> +
> +To shut down the whole {pve} + Ceph cluster, first stop all Ceph clients.
> This
> +will mainly be VMs and containers. If you have additional clients that might
> +access a Ceph FS or an installed RADOS GW, stop these as well.
> +High available guests will switch their state to 'stopped' when powered down
I think this should be "Highly available" or "High availability".
> +via the {pve} tooling.
> +
> +Once all clients, VMs and containers are off or not accessing the Ceph
> cluster
> +anymore, verify that the Ceph cluster is in a healthy state. Either via the
> Web UI
> +or the CLI:
> +
> +
> +ceph -s
> +
> +
> +Then enable the following OSD flags in the Ceph -> OSD panel or the CLI:
> +
> +
> +ceph osd set noout
> +ceph osd set norecover
> +ceph osd set norebalance
> +ceph osd set nobackfill
> +ceph osd set nodown
> +ceph osd set pause
> +
> +
> +This will halt all self-healing actions for Ceph and the 'pause' will stop
> any client IO.
> +
> +Start powering down the nodes one node at a time. Power down nodes with a
> +Monitor (MON) last.
Might benefit from re-phrasing to avoid people only reading this while
already in the middle of shutting down:
Start powering down your nodes without a monitor (MON). After these
nodes are down, also shut down hosts with monitors.
> +
> +When powering on the cluster, start the nodes with Monitors (MONs) first.
> Once
> +all nodes are up and running, confirm that all Ceph services are up and
> running
> +before you unset the OSD flags:
> +
> +
> +ceph osd unset noout
> +ceph osd unset norecover
> +ceph osd unset norebalance
> +ceph osd unset nobackfill
> +ceph osd unset nodown
> +ceph osd unset pause
> +
> +
> +You can now start up the guests. High available guests will change their
> state
see above
> +to 'started' when they power on.
> +
> Ceph Monitoring and Troubleshooting
> ---
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager] ui: pool members: avoid setting request parameter for all edit windows
On Wed Mar 13, 2024 at 9:44 AM CET, Friedrich Weber wrote:
> Currently, after adding a storage to a pool, opening any edit window
> will send a GET request with a superfluous `poolid` parameter and
> cause a parameter verification error in the GUI. This breaks all edit
> windows of the current session. A workaround is to reload the current
> browser session.
>
> This happens because the `PVE.pool.AddStorage` component inadvertently
> adds `poolid` to an `extraRequestParams` object that is shared by all
> instances of `Proxmox.window.Edit`, affecting all edit windows in the
> current session. Fix this by instead creating a new object that is
> local to the component.
>
> Fixes: cd731902b7a724b1ab747276f9c6343734f1d8cb
> Signed-off-by: Friedrich Weber
> ---
>
> Notes:
> To check if we have this problem at other places, I did a quick search
> for `extraRequestParams` in PVE+PBS: Seems like for all other usages,
> the object is created fresh already.
>
> www/manager6/grid/PoolMembers.js | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/www/manager6/grid/PoolMembers.js
> b/www/manager6/grid/PoolMembers.js
> index 75f20cab..61e27dff 100644
> --- a/www/manager6/grid/PoolMembers.js
> +++ b/www/manager6/grid/PoolMembers.js
> @@ -123,7 +123,7 @@ Ext.define('PVE.pool.AddStorage', {
> me.isAdd = true;
> me.url = "/pools/";
> me.method = 'PUT';
> - me.extraRequestParams.poolid = me.pool;
> + me.extraRequestParams = { 'poolid': me.pool };
note that you don't need to quote `poolid` here as it doesn't contain
hyphens as such. quickly grepping over our codebase it seems that not
quoting keys is preferred if it isn't needed
>
> Ext.apply(me, {
> subject: gettext('Storage'),
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH pve-manager 1/2] fix #5198: ceph: mon: fix mon existence check in mon removal assertion
On Wed Mar 13, 2024 at 3:53 PM CET, Max Carrara wrote:
> The Ceph monitor removal assertion contains a condition that checks
> whether the given mon ID actually exists and thus may be removed.
>
> The first part of the condition checks whether the hash returned by
> `get_services_info` [0] contains the key "mon.$monid". However, the
> hash's keys are never prefixed with "mon.", which makes this check
> incorrect.
>
> This is fixed by just using "$monid" directly.
>
> The second part checks whether the mon hashes returned by
> Ceph contain the "name" key before comparing the key with the given
> mon ID. This key existence check is also incorrect; in particular:
> * If the lookup `$_->{name}` evaluates to e.g. "foo", the check
> passes, because "foo" is truthy. [1]
> * If the lookup `$_->{name}` evaluates to "0", the check fails,
> because "0" is falsy (due to it being equivalent to the number 0,
> according to Perl [1]).
>
> This is solved by using the inbuilt `exists()` instead of relying on
> Perl's definition of truthiness.
>
> [0]:
> https://git.proxmox.com/?p=pve-manager.git;a=blob;f=PVE/Ceph/Services.pm;h=e0f31e8eb6bc9b3777b3d0d548497276efaa5c41;hb=HEAD#l112
> [1]: https://perldoc.perl.org/perldata#Scalar-values
>
> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=5198
> Signed-off-by: Max Carrara
> ---
> PVE/API2/Ceph/MON.pm | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/PVE/API2/Ceph/MON.pm b/PVE/API2/Ceph/MON.pm
> index 1e959ef3..1737c294 100644
> --- a/PVE/API2/Ceph/MON.pm
> +++ b/PVE/API2/Ceph/MON.pm
> @@ -147,8 +147,8 @@ my $assert_mon_prerequisites = sub {
> my $assert_mon_can_remove = sub {
> my ($monhash, $monlist, $monid, $mondir) = @_;
>
> -if (!(defined($monhash->{"mon.$monid"}) ||
> - grep { $_->{name} && $_->{name} eq $monid } @$monlist))
not sure if splitting the fix and the code style clean up makes sense
here but otherwise this works as advertised. So:
Tested-by: Stefan Sterz
> +if (!(defined($monhash->{$monid}) ||
> + grep { exists($_->{name}) && $_->{name} eq $monid } @$monlist))
> {
> die "no such monitor id '$monid'\n"
> }
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH docs v2 1/5] package-repos: improve wording partly based on pmg-docs
On Mon Mar 11, 2024 at 1:29 PM CET, Christoph Heiss wrote:
> Some sentences are phrased better and more expansive in pmg-docs, so
> take them from there and adapt them as needed.
>
> Rephrases some redundant parts (e.g. enterprise repo introduction) and
> moves the email/changelog paragraph to the top, since that applies to
> all repos.
>
> Signed-off-by: Christoph Heiss
> ---
> Changes v1 -> v2:
> * deduplicate email/changelog paragraph, move to top
> * deduplicate enterprise repo introduction sentence
> * very slightly reword some phrases
>
> pve-package-repos.adoc | 26 ++
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> diff --git a/pve-package-repos.adoc b/pve-package-repos.adoc
> index b0c2a95..4586f70 100644
> --- a/pve-package-repos.adoc
> +++ b/pve-package-repos.adoc
> @@ -8,6 +8,10 @@ endif::wiki[]
> {pve} uses http://en.wikipedia.org/wiki/Advanced_Packaging_Tool[APT] as its
> package management tool like any other Debian-based system.
>
> +{pve} automatically checks for package updates on a daily basis. The
> `root@pam`
> +user is notified via email about available updates. From the GUI, The
"The" at the end of this line should be lower case.
> +'Changelog' button can be used to see more details about the selected update.
> +
> Repositories in {pve}
> ~
>
> @@ -57,21 +61,18 @@ deb http://security.debian.org/debian-security
> bookworm-security main contrib
> {pve} Enterprise Repository
> ~~~
>
> -This is the default, stable, and recommended repository, available for all
> {pve}
> -subscription users. It contains the most stable packages and is suitable for
> -production use. The `pve-enterprise` repository is enabled by default:
> +This is the recommended repository and available for all {pve} subscription
> +users. It contains the most stable packages and is suitable for production
> use.
> +The `pve-enterprise` repository is enabled by default:
>
> .File `/etc/apt/sources.list.d/pve-enterprise.list`
>
> deb https://enterprise.proxmox.com/debian/pve bookworm pve-enterprise
>
>
> -The `root@pam` user is notified via email about available updates. Click the
> -'Changelog' button in the GUI to see more details about the selected update.
> -
> -You need a valid subscription key to access the `pve-enterprise` repository.
> -Different support levels are available. Further details can be found at
> -{pricing-url}.
> +Please note that you need a valid subscription key to access the
> +`pve-enterprise` repository. We offer different support levels, which you can
> +find further details about at {pricing-url}.
>
> NOTE: You can disable this repository by commenting out the above line using
> a
> `#` (at the start of the line). This prevents error messages if your host
> does
> @@ -82,9 +83,10 @@ repository in that case.
> {pve} No-Subscription Repository
>
>
> -This is the recommended repository for testing and non-production use. Its
> -packages are not as heavily tested and validated. You don't need a
> subscription key
> -to access the `pve-no-subscription` repository.
> +As the name suggests, you do not need a subscription key to access
> +this repository. It can be used for testing and non-production
> +use. It's not recommended to use this on production servers, as these
> +packages are not always as heavily tested and validated.
>
> We recommend to configure this repository in `/etc/apt/sources.list`.
>
> --
> 2.43.1
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH docs] installation: reword `nomodeset` section a bit, add link to it
On Mon Feb 26, 2024 at 12:20 PM CET, Christoph Heiss wrote:
> The `nomodeset` section needs some massaging due to the text flow being
> broken a bit. While at it, link to it above at the 'Terminal UI'
> bootloader tip such that readers can find it more easily.
>
> Suggested-by: Alexander Zeidler
> Signed-off-by: Christoph Heiss
> ---
> pve-installation.adoc | 11 ++-
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/pve-installation.adoc b/pve-installation.adoc
> index 6b44fc0..c799570 100644
> --- a/pve-installation.adoc
> +++ b/pve-installation.adoc
> @@ -88,7 +88,8 @@ Both modes use the same code base for the actual
> installation process to
> benefit from more than a decade of bug fixes and ensure feature parity.
>
> TIP: The 'Terminal UI' option can be used in case the graphical installer
> does
> -not work correctly, due to e.g. driver issues.
> +not work correctly, due to e.g. driver issues. See also
> +<>.
>
> Advanced Options: Install {pve} (Graphical, Debug Mode)::
>
> @@ -300,14 +301,14 @@ following command:
> # zpool add log
>
>
> +[[nomodeset_kernel_param]]
> Adding the `nomodeset` Kernel Parameter
> ~
>
> Problems may arise on very old or very new hardware due to graphics drivers.
> If
> -the installation hangs during the boot. In that case, you can try adding the
> -`nomodeset` parameter. This prevents the Linux kernel from loading any
> -graphics drivers and forces it to continue using the BIOS/UEFI-provided
> -framebuffer.
> +the installation hangs during the boot, you can try adding the `nomodeset`
> +parameter. This prevents the Linux kernel from loading any graphics drivers
> and
> +forces it to continue using the BIOS/UEFI-provided framebuffer.
>
nit: "the installation hangs during the boot" doesn't require that
"the". imo "If the installation hangs during boot, you can..."
otherwise sounds good to me
> On the {pve} bootloader menu, navigate to 'Install {pve} (Terminal UI)' and
> press `e` to edit the entry. Using the arrow keys, navigate to the line
> starting
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager] sdn: add model based validation to dhcp ranges
this patch re-works the validation the in the subnit edit panel to
also validate whether the ip address version in a range match each
other.
Signed-off-by: Stefan Sterz
---
not super proud of this, but couldn't really find another way to
properly validate accross two columns and also have the panel still work
as intended.
www/manager6/sdn/SubnetEdit.js | 85 ++
1 file changed, 76 insertions(+), 9 deletions(-)
diff --git a/www/manager6/sdn/SubnetEdit.js b/www/manager6/sdn/SubnetEdit.js
index 8fc3f52b..cba80d8f 100644
--- a/www/manager6/sdn/SubnetEdit.js
+++ b/www/manager6/sdn/SubnetEdit.js
@@ -59,6 +59,49 @@ Ext.define('PVE.sdn.SubnetInputPanel', {
],
});
+Ext.define('PVE.sdn.model.DhcpRange', {
+extend: 'Ext.data.Model',
+fields: [
+{ name: 'startAddress' },
+{ name: 'endAddress' },
+ {
+ name: 'rangeErrors',
+ depends: ['startAddress', 'endAddress'],
+ calculate: (data) => {
+ let errors = [];
+ let sV4 = Proxmox.Utils.IP4_match.test(data.startAddress);
+ let sV6 = Proxmox.Utils.IP6_match.test(data.startAddress);
+ let eV4 = Proxmox.Utils.IP4_match.test(data.endAddress);
+ let eV6 = Proxmox.Utils.IP6_match.test(data.endAddress);
+
+ if (!((sV4 && eV4) || (sV6 && eV6))) {
+ errors.push("IP address versions do not match.");
+ }
+
+ return errors;
+ },
+ },
+ {
+ name: 'rangeErrorsClass',
+ depends: ['rangeErrors'],
+ calculate: (data) => {
+ if (data.rangeErrors.length !== 0) {
+ return ['x-form-trigger-wrap-default',
'x-form-trigger-wrap-invalid'];
+ }
+
+ return [];
+ }
+ }
+],
+
+validators: {
+startAddress: (value, record) =>
+ Ext.form.VTypes.IP64Address(value) || gettext("Not a valid IP
address."),
+ endAddress: (value, record) =>
+ Ext.form.VTypes.IP64Address(value) || gettext("Not a valid IP
address."),
+},
+});
+
Ext.define('PVE.sdn.SubnetDhcpRangePanel', {
extend: 'Ext.form.FieldContainer',
mixins: ['Ext.form.field.Field'],
@@ -86,8 +129,8 @@ Ext.define('PVE.sdn.SubnetDhcpRangePanel', {
// needs a deep copy otherwise we run in to ExtJS reference
// shenaningans
value.push({
- 'start-address': item.data['start-address'],
- 'end-address': item.data['end-address'],
+ 'start-address': item.data.startAddress,
+ 'end-address': item.data.endAddress,
});
});
@@ -121,8 +164,10 @@ Ext.define('PVE.sdn.SubnetDhcpRangePanel', {
// needs a deep copy otherwise we run in to ExtJS reference
// shenaningans
data.push({
- 'start-address': item['start-address'],
- 'end-address': item['end-address'],
+ startAddress: item['start-address'],
+ endAddress: item['end-address'],
+ rangeErrors: [],
+ rangeErrorsClass: [],
});
});
@@ -133,6 +178,18 @@ Ext.define('PVE.sdn.SubnetDhcpRangePanel', {
let me = this;
let errors = [];
+ let grid = me.lookup('grid');
+
+ grid.getStore().data.each((row, index) => {
+ if (!row.isValid()) {
+ errors.push(gettext("An entry in the DHCP Range is not a valid
IP address!"));
+ }
+
+ if (row.data.rangeErrors.length !== 0) {
+ row.data.rangeErrors.forEach((error) => errors.push(error));
+ }
+ });
+
return errors;
},
@@ -182,27 +239,37 @@ Ext.define('PVE.sdn.SubnetDhcpRangePanel', {
reference: 'grid',
scrollable: true,
store: {
- fields: ['start-address', 'end-address'],
+ model: 'PVE.sdn.model.DhcpRange',
+ },
+ itemConfig: {
+ viewModel: true,
},
+ modelValidation: true,
columns: [
{
text: gettext('Start Address'),
xtype: 'widgetcolumn',
- dataIndex: 'start-address',
flex: 1,
widget: {
xtype: 'textfield',
- vtype: '
[pve-devel] [PATCH installer] tui: bootdisk zfs config: add a maximum value to the `copies` option
according to `man zfsprops` the copies option can only be 1, 2, or 3.
limit the field to 3, as setting higher options can't work anyway.
Signed-off-by: Stefan Sterz
---
i would have added a `min_value` of 1 too, but `IntegerEditView` is
based on `NumericEditView` and that doesn't offer a minimal value.
proxmox-tui-installer/src/views/bootdisk.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs
b/proxmox-tui-installer/src/views/bootdisk.rs
index 00e6ade..7e13e91 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -592,7 +592,7 @@ impl ZfsBootdiskOptionsView {
.unwrap_or_default(),
),
)
-.child("copies", IntegerEditView::new().content(options.copies))
+.child("copies",
IntegerEditView::new().content(options.copies).max_value(3))
.child_conditional(
is_pve,
"ARC max size",
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH common] fix #5034 ldap attribute regex
On 15.11.23 15:49, Thomas Lamprecht wrote:
> Am 15/11/2023 um 14:28 schrieb Stefan Sterz:
>> On 15.11.23 13:23, Markus Frank wrote:
-- >8 snip 8< --
>>
>>> src/PVE/JSONSchema.pm | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
>>> index 49e0d7a..ef58b62 100644
>>> --- a/src/PVE/JSONSchema.pm
>>> +++ b/src/PVE/JSONSchema.pm
>>> @@ -408,7 +408,7 @@ PVE::JSONSchema::register_format('ldap-simple-attr',
>>> \&verify_ldap_simple_attr);
>>> sub verify_ldap_simple_attr {
>>> my ($attr, $noerr) = @_;
>>>
>>> -if ($attr =~ m/^[a-zA-Z0-9]+$/) {
>>> +if ($attr =~ m/^[a-zA-Z0-9\-]+$/) {
>>
>> if i'm not mistaken, this regex should try to filter an `AttributeValue`
>> [1]. in case we do stick with this regex approach here, you may want to
>> relax this even further, as per the standard:
>>
>>> If that UTF-8-encoded Unicode string does not have any of the
>>> following characters that need escaping, then that string can be used
>>> as the string representation
>>> of the value.
>>>
>>> - a space (' ' U+0020) or number sign ('#' U+0023) occurring at
>>>the beginning of the string;
>>>
>>> - a space (' ' U+0020) character occurring at the end of the
>>>string;
>>>
>>> - one of the characters '"', '+', ',', ';', '<', '>', or '\'
>>>(U+0022, U+002B, U+002C, U+003B, U+003C, U+003E, or U+005C,
>>>respectively);
>>>
>>> - the null (U+) character.
>>>
>
> Ack, so I was wrong, the format might still make sense albeit checking
> for above cases would then indeed better, something along the lines of:
>
> if ($attr !~ /(?:^(?:\s|#))|["+,;<>\0\\]|(?:\s$)/) {
> return $attr;
> }
>
> If we leave that regex out completely we should ensure that we don't get
> any tainting issues.
>
> The format could move to PVE::Auth::LDAP too, FWIW, but that's a different
> story.
just to through this out there, my last attempt at validating this [1]
looked something like this:
```
my $escaped = qr!\\(?:[ "#+,;<=>\\]|[0-9a-fA-F]{2})!;
my $start= qr!(?:${escaped}|[^"+,;<>\\\0 #])!;
my $middle = qr!(?:${escaped}|[^"+,;<>\\\0])!;
my $end = qr!(?:${escaped}|[^"+,;<>\\\0 ])!;
my $attr_val = qr!("[^"]+"|${start}(?:${middle}*${end})?)!;
```
since things can also be escaped or in quotes, which makes them valid
again. could probably be improved here, though.
[1]: https://lists.proxmox.com/pipermail/pve-devel/2023-May/056840.html
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH common] fix #5034 ldap attribute regex
On 15.11.23 14:28, Stefan Sterz wrote:
>> src/PVE/JSONSchema.pm | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
>> index 49e0d7a..ef58b62 100644
>> --- a/src/PVE/JSONSchema.pm
>> +++ b/src/PVE/JSONSchema.pm
>> @@ -408,7 +408,7 @@ PVE::JSONSchema::register_format('ldap-simple-attr',
>> \&verify_ldap_simple_attr);
>> sub verify_ldap_simple_attr {
>> my ($attr, $noerr) = @_;
>>
>> -if ($attr =~ m/^[a-zA-Z0-9]+$/) {
>> +if ($attr =~ m/^[a-zA-Z0-9\-]+$/) {
>
> if i'm not mistaken, this regex should try to filter an `AttributeValue`
> [1]. in case we do stick with this regex approach here, you may want to
> relax this even further, as per the standard:
>
sorry just noticed i forgot to add:
[1]: https://datatracker.ietf.org/doc/html/rfc4514#section-2.4
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH common] fix #5034 ldap attribute regex
On 15.11.23 13:23, Markus Frank wrote:
> Change regex from "m/^[a-zA-Z0-9]+$/" to "m/^[a-zA-Z0-9\-]+$/"
> to allow hyphen in ldap attribute names for pve & pmg.
>
> Signed-off-by: Markus Frank
> ---
> There does not seem to be a regex for LDAP attributes in pbs.
> Should a regex be added for this?
>
we recently moved away from using regex for validating a LDAP
configuration, for two reasons:
1. turns out finding a regex that validates all possible valid LDAP DNs
is pretty hard and fixing this often comes along with breaking older
setups
2. even a valid *looking* DN may not actual work against a real LDAP
server.
so instead we now actually try to query an LDAP server with the provided
config and see if that returns anything. thus, users are more likely to
get what they want, and we don't have to validate for every possible case.
i guess there could be an argument why a regex here makes sense.
however, i'd imagine it's a little odd for users that we are stricter
here than we are with DNs.
> src/PVE/JSONSchema.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
> index 49e0d7a..ef58b62 100644
> --- a/src/PVE/JSONSchema.pm
> +++ b/src/PVE/JSONSchema.pm
> @@ -408,7 +408,7 @@ PVE::JSONSchema::register_format('ldap-simple-attr',
> \&verify_ldap_simple_attr);
> sub verify_ldap_simple_attr {
> my ($attr, $noerr) = @_;
>
> -if ($attr =~ m/^[a-zA-Z0-9]+$/) {
> +if ($attr =~ m/^[a-zA-Z0-9\-]+$/) {
if i'm not mistaken, this regex should try to filter an `AttributeValue`
[1]. in case we do stick with this regex approach here, you may want to
relax this even further, as per the standard:
> If that UTF-8-encoded Unicode string does not have any of the
> following characters that need escaping, then that string can be used
> as the string representation
> of the value.
>
> - a space (' ' U+0020) or number sign ('#' U+0023) occurring at
>the beginning of the string;
>
> - a space (' ' U+0020) character occurring at the end of the
>string;
>
> - one of the characters '"', '+', ',', ';', '<', '>', or '\'
>(U+0022, U+002B, U+002C, U+003B, U+003C, U+003E, or U+005C,
>respectively);
>
> - the null (U+) character.
>
> Other characters may be escaped.
> return $attr;
> }
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-kernel] backport exposing FLUSHBYASID when running nested VMs on AMD CPUs
this exposes the FLUSHBYASID CPU flag to nested VMs when running on an
AMD CPU. also reverts a made up check that would advertise
FLUSHBYASID as not supported. this enable certain modern hypervisors
such as VMWare ESXi 7 and Workstation 17 to run nested VMs properly
again.
Signed-off-by: Stefan Sterz
---
...k-for-reserved-encodings-of-TLB_CONT.patch | 49 +++
...-Advertise-support-for-flush-by-ASID.patch | 39 +++
2 files changed, 88 insertions(+)
create mode 100644
patches/kernel/0014-Revert-nSVM-Check-for-reserved-encodings-of-TLB_CONT.patch
create mode 100644
patches/kernel/0015-KVM-nSVM-Advertise-support-for-flush-by-ASID.patch
diff --git
a/patches/kernel/0014-Revert-nSVM-Check-for-reserved-encodings-of-TLB_CONT.patch
b/patches/kernel/0014-Revert-nSVM-Check-for-reserved-encodings-of-TLB_CONT.patch
new file mode 100644
index 000..2c77272
--- /dev/null
+++
b/patches/kernel/0014-Revert-nSVM-Check-for-reserved-encodings-of-TLB_CONT.patch
@@ -0,0 +1,49 @@
+From 379ad2e0326c55682d0bb9391f16f1072fe400d2 Mon Sep 17 00:00:00 2001
+From: Stefan Sterz
+Date: Wed, 18 Oct 2023 10:45:45 +0200
+Subject: [PATCH 1/2] Revert "nSVM: Check for reserved encodings of TLB_CONTROL
+ in nested VMCB"
+
+This reverts commit 174a921b6975ef959dd82ee9e8844067a62e3ec1.
+
+Signed-off-by: Stefan Sterz
+---
+ arch/x86/kvm/svm/nested.c | 15 ---
+ 1 file changed, 15 deletions(-)
+
+diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
+index add65dd59756..61a6c0235519 100644
+--- a/arch/x86/kvm/svm/nested.c
b/arch/x86/kvm/svm/nested.c
+@@ -242,18 +242,6 @@ static bool nested_svm_check_bitmap_pa(struct kvm_vcpu
*vcpu, u64 pa, u32 size)
+ kvm_vcpu_is_legal_gpa(vcpu, addr + size - 1);
+ }
+
+-static bool nested_svm_check_tlb_ctl(struct kvm_vcpu *vcpu, u8 tlb_ctl)
+-{
+- /* Nested FLUSHBYASID is not supported yet. */
+- switch(tlb_ctl) {
+- case TLB_CONTROL_DO_NOTHING:
+- case TLB_CONTROL_FLUSH_ALL_ASID:
+- return true;
+- default:
+- return false;
+- }
+-}
+-
+ static bool __nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
+struct vmcb_ctrl_area_cached *control)
+ {
+@@ -273,9 +261,6 @@ static bool __nested_vmcb_check_controls(struct kvm_vcpu
*vcpu,
+ IOPM_SIZE)))
+ return false;
+
+- if (CC(!nested_svm_check_tlb_ctl(vcpu, control->tlb_ctl)))
+- return false;
+-
+ return true;
+ }
+
+--
+2.39.2
+
diff --git
a/patches/kernel/0015-KVM-nSVM-Advertise-support-for-flush-by-ASID.patch
b/patches/kernel/0015-KVM-nSVM-Advertise-support-for-flush-by-ASID.patch
new file mode 100644
index 000..611a90c
--- /dev/null
+++ b/patches/kernel/0015-KVM-nSVM-Advertise-support-for-flush-by-ASID.patch
@@ -0,0 +1,39 @@
+From 42af81abf0b96ab661591d024aed55c05dd85b91 Mon Sep 17 00:00:00 2001
+From: Sean Christopherson
+Date: Wed, 18 Oct 2023 12:41:04 -0700
+Subject: [PATCH 2/2] KVM: nSVM: Advertise support for flush-by-ASID
+
+Advertise support for FLUSHBYASID when nested SVM is enabled, as KVM can
+always emulate flushing TLB entries for a vmcb12 ASID, e.g. by running L2
+with a new, fresh ASID in vmcb02. Some modern hypervisors, e.g. VMWare
+Workstation 17, require FLUSHBYASID support and will refuse to run if it's
+not present.
+
+Punt on proper support, as "Honor L1's request to flush an ASID on nested
+VMRUN" is one of the TODO items in the (incomplete) list of issues that
+need to be addressed in order for KVM to NOT do a full TLB flush on every
+nested SVM transition (see nested_svm_transition_tlb_flush()).
+
+Reported-by: Stefan Sterz
+Closes:
https://lkml.kernel.org/r/b9915c9c-4cf6-051a-2d91-44cc6380f455%40proxmox.com
+Signed-off-by: Sean Christopherson
+Signed-off-by: Stefan Sterz
+---
+ arch/x86/kvm/svm/svm.c | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
+index 9a194aa1a75a..0fde9b0c464b 100644
+--- a/arch/x86/kvm/svm/svm.c
b/arch/x86/kvm/svm/svm.c
+@@ -4880,6 +4880,7 @@ static __init void svm_set_cpu_caps(void)
+ if (nested) {
+ kvm_cpu_cap_set(X86_FEATURE_SVM);
+ kvm_cpu_cap_set(X86_FEATURE_VMCBCLEAN);
++ kvm_cpu_cap_set(X86_FEATURE_FLUSHBYASID);
+
+ if (nrips)
+ kvm_cpu_cap_set(X86_FEATURE_NRIPS);
+--
+2.39.2
+
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH widget-toolkit] dark-mode: set intentionally black icons to `$icon-color`
some icons intentionally use black as their color in the light theme.
this includes the little pencil and check mark icon in the acme
overview. change their color to the regular dark-mode icon-color. for
this to work the filter inversion needed for some other icons needs to
be remove too.
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/other/_icons.scss | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/proxmox-dark/scss/other/_icons.scss
b/src/proxmox-dark/scss/other/_icons.scss
index d4dc316..c045cf4 100644
--- a/src/proxmox-dark/scss/other/_icons.scss
+++ b/src/proxmox-dark/scss/other/_icons.scss
@@ -104,6 +104,9 @@
}
// pbs show task log in longest task list column
+.fa.black,
+.fa.black::after,
+.fa.black::before,
.x-action-col-icon.fa-chevron-right::before {
filter: none;
}
@@ -222,6 +225,12 @@
}
}
+// set icon color of intentional black icons (e.g.: pencil icon for
+// quickly changing the ACME account)
+.fa.black {
+ color: $icon-color;
+}
+
// The usage icons dynamically displaying how full a storage is
.usage-wrapper {
border: 1px solid $icon-color;
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH proxmox-widget-toolkit v1 2/2] fix #4546: utils: save expiring date of user account for UI
On Fri Sep 22, 2023 at 4:36 PM CEST, Philipp Hufnagl wrote:
-- snip 8< --
> +$text-color-warning: hsl(48deg, 100%, 50%);
>
> // Borders
> $border-color: hsl(0deg, 0%, 40%);
> diff --git a/src/proxmox-dark/scss/extjs/_menu.scss
> b/src/proxmox-dark/scss/extjs/_menu.scss
> index 2983f60..aa51260 100644
> --- a/src/proxmox-dark/scss/extjs/_menu.scss
> +++ b/src/proxmox-dark/scss/extjs/_menu.scss
> @@ -33,6 +33,10 @@
>color: $icon-color;
> }
>
> +.x-menu-item-icon-default.warning {
> + color: $text-color-warning;
> +}
> +
same here, please don't define new colors unless you must. in this case
there is an argument to be made that this needs to be a new color as we
don't have a text color for warnings.
except, we kind of do: gauges use a brigther version of the warning and
invalid background colors. please factor these out into their own
variables and use those here and in `src/proxmox-dark/scss/_charts.scss`,
which is where we curretnly define the brigther colors.
-- snip 8< --
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH proxmox-widget-toolkit v1 1/2] fix #4546: css: Inform user administrator about user accounts expiring soon
On Fri Oct 6, 2023 at 3:16 PM CEST, Lukas Wagner wrote: -- snip 8< -- > > +$background-hint: hsl(233deg, 99%, 60%); > > That particular color tone looks pretty out of place to me in dark mode. > In light mode, you use the same hue as other interface elements, is > there a reason why you use a different color in dark mode? > > Playing around a bit, hsl(205, 100%, 40%) or hsl(205, 100%, 45%) would > look about right for me, that's the same hue as other elements, > while being a bit toned down (reduced lightness). > > @sterzy, maybe you can provide some feedback as well? > > Also, small bug: the hint-color seems to disappear when the 'expiry' > is clicked, only seems to affect dark mode. in general: please refrain from defining new colors. you are very likely not the first person that needs to highlight something in a row. in this case: why not use either `$primary-dark` if it needs to be blue. or in my opinion: use `$background-warning`. this would be semantically fitting as you want to warn an admin that this user is about to expire. for the light theme you could simply use the `.warning` class instead then you can simply overwrite that with a more specific selector in the dark theme again, like we already do in a couple of places. defining new colors each time you want to highlight something in the ui quikly leads to a visual mess, instead try to re-use colors that already have some kind of semantic meaning. this will make it easier to visually parse the ui quikly. -- snip 8< -- ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH pve-zsync] parse disks: improve error messages
On Tue Sep 12, 2023 at 11:31 AM CEST, Fiona Ebner wrote:
> The one with the backup flag was reported in the community forum:
> https://forum.proxmox.com/threads/77254/
>
> Signed-off-by: Fiona Ebner
> ---
> pve-zsync | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/pve-zsync b/pve-zsync
> index 98190b2..0e48865 100755
> --- a/pve-zsync
> +++ b/pve-zsync
> @@ -949,11 +949,12 @@ sub parse_disks {
> $num++;
>
> } else {
> - die "ERROR: in path\n";
> + die "unexpected path '$path'\n";
> }
> }
>
> -die "Vm include no disk on zfs.\n" if !$disks->{0};
> +die "guest does not include any ZFS volumes (or all excluded by the
> backup flag).\n"
small nit: "guest" should be capitalized if this is the beginning of a
sentence. and imo "all are excluded" sounds more proper.
> + if !$disks->{0};
> return $disks;
> }
>
> --
> 2.39.2
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH pve-eslint] switch to using `Command.opts()` to access options
this fixes an issue where the options where not properly passed to
eslint, which rendered them useless. uses the `opts()` function to
access them. see [1] for more on info on option parsing with
commander.
[1]: https://www.npmjs.com/package/commander#user-content-options
Signed-off-by: Stefan Sterz
---
not sure how we handle this, but might need a:
Reported-by: Max Carrara
src/bin/app.js | 29 +++--
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/src/bin/app.js b/src/bin/app.js
index 48ae043..7d0088f 100644
--- a/src/bin/app.js
+++ b/src/bin/app.js
@@ -27,13 +27,14 @@ program.on('--help', function() {
});
program.parse(process.argv);
+let options = program.opts();
-if (program.config && program.extend) {
+if (options.config && options.extend) {
console.error('Cannot use both, --config and --extend, at the same time!');
process.exit(1);
}
-if (program.args.length < 1 && !program.outputConfig) {
+if (program.args.length < 1 && !options.outputConfig) {
program.help();
}
@@ -44,8 +45,8 @@ if (!paths.length) {
}
let threadCount = 4;
-if (program.threads) {
-threadCount = program.threads;
+if (options.threads) {
+threadCount = options.threads;
}
const defaultConfig = {
@@ -274,16 +275,16 @@ let pathExpand = (p) => {
};
let config = defaultConfig;
-if (program.config) {
+if (options.config) {
config = {
- "extends": pathExpand(program.config),
+ "extends": pathExpand(options.config),
};
-} else if (program.extend) {
-config.extends = pathExpand(program.extend);
+} else if (options.extend) {
+config.extends = pathExpand(options.extend);
console.log(`Extend with path: ${config.extends}`);
}
-if (program.outputConfig) {
+if (options.outputConfig) {
let cfg = JSON.stringify(config, null, 2);
console.log(cfg);
process.exit(0);
@@ -292,7 +293,7 @@ if (program.outputConfig) {
const cliOptions = {
baseConfig: config,
useEslintrc: true,
-fix: !!program.fix,
+fix: !!options.fix,
cwd: process.cwd(),
};
@@ -322,7 +323,7 @@ results.forEach(function(result) {
let filename = path.relative(process.cwd(), result.filePath);
let msgs = result.messages;
let max_sev = 0;
-if (!!program.fix && result.output) {
+if (!!options.fix && result.output) {
fixes++;
}
if (msgs.length <= 0) {
@@ -337,7 +338,7 @@ results.forEach(function(result) {
let msg = `: line ${color.bold(e.line)} col ${color.bold(e.column)}:
${e.ruleId}`;
if (e.severity === 1) {
msg = color.yellow("WARN" + msg);
- if (exitcode < 1 && !!program.strict) {
+ if (exitcode < 1 && !!options.strict) {
exitcode = 1;
}
} else if (e.severity === 2) {
@@ -351,7 +352,7 @@ results.forEach(function(result) {
if (e.message) {
msg += ` - ${e.message}`;
}
- if (!program.fix && e.fix) {
+ if (!options.fix && e.fix) {
fixes++;
msg += ' (*)';
}
@@ -386,7 +387,7 @@ if (results.length > 1) {
}
console.log('');
-if (program.fix) {
+if (options.fix) {
if (fixes > 0) {
console.log(`Writing ${color.bold(fixes)} fixed files...`);
await eslint.ESLint.outputFixes(results);
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer v2] tui: persist disk selection for zfs and btrfs
previously the disk selection was reset if the advanced options
dialogue was re-opened. this commit adapts the behavior to restore
the previous selection.
Signed-off-by: Stefan Sterz
---
changes since v2 (thanks @Maximiliano Sandoval):
* added a comment that the updated `change_default` functions may
panic
* added a comment and renamed the `get_value` function to reflect the
new return type better (to `get_value_and_selection`).
proxmox-tui-installer/src/options.rs| 12 +-
proxmox-tui-installer/src/views/bootdisk.rs | 43 +++--
2 files changed, 42 insertions(+), 13 deletions(-)
diff --git a/proxmox-tui-installer/src/options.rs
b/proxmox-tui-installer/src/options.rs
index 5f0612e..62804c2 100644
--- a/proxmox-tui-installer/src/options.rs
+++ b/proxmox-tui-installer/src/options.rs
@@ -118,12 +118,16 @@ impl LvmBootdiskOptions {
#[derive(Clone, Debug)]
pub struct BtrfsBootdiskOptions {
pub disk_size: f64,
+pub selected_disks: Vec,
}
impl BtrfsBootdiskOptions {
-pub fn defaults_from(disk: &Disk) -> Self {
+/// This panics if the provided slice is empty.
+pub fn defaults_from(disks: &[Disk]) -> Self {
+let disk = &disks[0];
Self {
disk_size: disk.size,
+selected_disks: (0..disks.len()).collect(),
}
}
}
@@ -191,16 +195,20 @@ pub struct ZfsBootdiskOptions {
pub checksum: ZfsChecksumOption,
pub copies: usize,
pub disk_size: f64,
+pub selected_disks: Vec,
}
impl ZfsBootdiskOptions {
-pub fn defaults_from(disk: &Disk) -> Self {
+/// This panics if the provided slice is empty.
+pub fn defaults_from(disks: &[Disk]) -> Self {
+let disk = &disks[0];
Self {
ashift: 12,
compress: ZfsCompressOption::default(),
checksum: ZfsChecksumOption::default(),
copies: 1,
disk_size: disk.size,
+selected_disks: (0..disks.len()).collect(),
}
}
}
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs
b/proxmox-tui-installer/src/views/bootdisk.rs
index 75f70a1..222cab7 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -137,11 +137,11 @@ impl AdvancedBootdiskOptionsView {
)),
FsType::Zfs(_) =>
view.add_child(ZfsBootdiskOptionsView::new(
disks,
-&ZfsBootdiskOptions::defaults_from(&disks[0]),
+&ZfsBootdiskOptions::defaults_from(disks),
)),
FsType::Btrfs(_) =>
view.add_child(BtrfsBootdiskOptionsView::new(
disks,
-&BtrfsBootdiskOptions::defaults_from(&disks[0]),
+&BtrfsBootdiskOptions::defaults_from(disks),
)),
}
}
@@ -274,7 +274,7 @@ struct MultiDiskOptionsView {
}
impl MultiDiskOptionsView {
-fn new(avail_disks: &[Disk], options_view: T) -> Self {
+fn new(avail_disks: &[Disk], selected_disks: &Vec, options_view: T)
-> Self {
let mut selectable_disks = avail_disks
.iter()
.map(|d| (d.to_string(), Some(d.clone(
@@ -289,7 +289,7 @@ impl MultiDiskOptionsView {
SelectView::new()
.popup()
.with_all(selectable_disks.clone())
-.selected(i),
+.selected(selected_disks[i]),
);
}
@@ -323,7 +323,13 @@ impl MultiDiskOptionsView {
self
}
-fn get_disks(&mut self) -> Option> {
+///
+/// This function returns a tuple of vectors. The first vector contains
the currently selected
+/// disks in order of their selection slot. Empty slots are filtered out.
The second vector
+/// contains indices of each slot's selection, which enables us to restore
the selection even
+/// for empty slots.
+///
+fn get_disks_and_selection(&mut self) -> Option<(Vec, Vec)> {
let mut disks = vec![];
let view_top_index = usize::from(self.has_top_panel());
@@ -337,6 +343,8 @@ impl MultiDiskOptionsView {
.downcast_ref::>()?
.get_inner();
+let mut selected_disks = Vec::new();
+
for i in 0..disk_form.len() {
let disk = disk_form.get_value::>, _>(i)?;
@@ -344,9 +352,15 @@ impl MultiDiskOptionsView {
if let Some(disk) = disk {
disks.push(disk);
}
+
+selected_disks.push(
+disk_form
+.get_child::>>(i)?
+.selected_id()?,
+);
}
-Some(disks)
+Some((disks, selected_disks))
}
fn inner_mut(&mut self) ->
Re: [pve-devel] [PATCH installer] tui: persist disk selection for zfs and btrfs
On 27.06.23 15:57, Lukas Wagner wrote:
>
>
> On 6/27/23 15:34, Maximiliano Sandoval wrote:
>>> impl BtrfsBootdiskOptions {
>>> - pub fn defaults_from(disk: &Disk) -> Self {
>>> + pub fn defaults_from(disks: &[Disk]) -> Self {
>>> + let disk = &disks[0];
>>> Self {
>>> disk_size: disk.size,
>>> + selected_disks: (0..disks.len()).collect(),
>>
>> Any reason not to use Vec::with_capacity(disks.len()) here?
>>
>
> I haven't really examined the rest of the code, but wouldn't that change
> the behavior
> completely? E.g., if `disk.len()` is 3, then
> `(0..disks.len()).collect()` will give you a Vec [0, 1, 2], while
> `Vec::with_capacity(disks.len())` would give you an empty Vec with an
> initial capacity
> of at least 3.
>
>
yes. we've already discussed this off list. this is needed here because
otherwise you panic out in `MultiDiskOptionsView::new()` because
`selected_disk` would have a length of zero. the ascending numbers are
needed to have the same initial selection as we currently do.
i'll send a patch with the other nits resolved in a minute.
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer] tui: persist disk selection for zfs and btrfs
previously the disk selection was reset if the advanced options
dialogue was re-opened. this commit adapts the behavior to restore
the previous selection.
Signed-off-by: Stefan Sterz
---
proxmox-tui-installer/src/options.rs| 11 +--
proxmox-tui-installer/src/views/bootdisk.rs | 36 +++--
2 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/proxmox-tui-installer/src/options.rs
b/proxmox-tui-installer/src/options.rs
index 5f0612e..56150c4 100644
--- a/proxmox-tui-installer/src/options.rs
+++ b/proxmox-tui-installer/src/options.rs
@@ -118,12 +118,15 @@ impl LvmBootdiskOptions {
#[derive(Clone, Debug)]
pub struct BtrfsBootdiskOptions {
pub disk_size: f64,
+pub selected_disks: Vec,
}
impl BtrfsBootdiskOptions {
-pub fn defaults_from(disk: &Disk) -> Self {
+pub fn defaults_from(disks: &[Disk]) -> Self {
+let disk = &disks[0];
Self {
disk_size: disk.size,
+selected_disks: (0..disks.len()).collect(),
}
}
}
@@ -191,16 +194,20 @@ pub struct ZfsBootdiskOptions {
pub checksum: ZfsChecksumOption,
pub copies: usize,
pub disk_size: f64,
+pub selected_disks: Vec,
}
impl ZfsBootdiskOptions {
-pub fn defaults_from(disk: &Disk) -> Self {
+pub fn defaults_from(disks: &[Disk]) -> Self {
+let disk = &disks[0];
+
Self {
ashift: 12,
compress: ZfsCompressOption::default(),
checksum: ZfsChecksumOption::default(),
copies: 1,
disk_size: disk.size,
+selected_disks: (0..disks.len()).collect(),
}
}
}
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs
b/proxmox-tui-installer/src/views/bootdisk.rs
index 75f70a1..c3cf4b6 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -137,11 +137,11 @@ impl AdvancedBootdiskOptionsView {
)),
FsType::Zfs(_) =>
view.add_child(ZfsBootdiskOptionsView::new(
disks,
-&ZfsBootdiskOptions::defaults_from(&disks[0]),
+&ZfsBootdiskOptions::defaults_from(disks),
)),
FsType::Btrfs(_) =>
view.add_child(BtrfsBootdiskOptionsView::new(
disks,
-&BtrfsBootdiskOptions::defaults_from(&disks[0]),
+&BtrfsBootdiskOptions::defaults_from(disks),
)),
}
}
@@ -274,7 +274,7 @@ struct MultiDiskOptionsView {
}
impl MultiDiskOptionsView {
-fn new(avail_disks: &[Disk], options_view: T) -> Self {
+fn new(avail_disks: &[Disk], selected_disks: &Vec, options_view: T)
-> Self {
let mut selectable_disks = avail_disks
.iter()
.map(|d| (d.to_string(), Some(d.clone(
@@ -289,7 +289,7 @@ impl MultiDiskOptionsView {
SelectView::new()
.popup()
.with_all(selectable_disks.clone())
-.selected(i),
+.selected(selected_disks[i]),
);
}
@@ -323,7 +323,7 @@ impl MultiDiskOptionsView {
self
}
-fn get_disks(&mut self) -> Option> {
+fn get_disks(&mut self) -> Option<(Vec, Vec)> {
let mut disks = vec![];
let view_top_index = usize::from(self.has_top_panel());
@@ -337,6 +337,8 @@ impl MultiDiskOptionsView {
.downcast_ref::>()?
.get_inner();
+let mut selected_disks = Vec::new();
+
for i in 0..disk_form.len() {
let disk = disk_form.get_value::>, _>(i)?;
@@ -344,9 +346,15 @@ impl MultiDiskOptionsView {
if let Some(disk) = disk {
disks.push(disk);
}
+
+selected_disks.push(
+disk_form
+.get_child::>>(i)?
+.selected_id()?,
+);
}
-Some(disks)
+Some((disks, selected_disks))
}
fn inner_mut(&mut self) -> Option<&mut T> {
@@ -382,6 +390,7 @@ impl BtrfsBootdiskOptionsView {
fn new(disks: &[Disk], options: &BtrfsBootdiskOptions) -> Self {
let view = MultiDiskOptionsView::new(
disks,
+&options.selected_disks,
FormView::new().child("hdsize",
DiskSizeEditView::new().content(options.disk_size)),
)
.top_panel(TextView::new("Btrfs integration is a technology
preview!").center());
@@ -390,10 +399,16 @@ impl BtrfsBootdiskOptionsView {
}
fn get_values(&mut self) -> Option<(Vec, BtrfsBootdiskOptions)> {
-let disks = self.view.get_disks()?;
+let (disks, selected_disks)
[pve-devel] [PATCH installer] tui: switch to `f64` for disk sizes
previously the tui used `u64` internally to represent the disk size.
since the perl-based installer expects GiB as floats and that is also
what is displayed in the tui that meant a lot of converting back and
forth. it also lead to an error where the disk sizes that were set
seemed to not have been persisted, even though the sizes were
correctly set. this commit refactors the installer to convert the size
once in the beginning and then stick to `f64`.
Signed-off-by: Stefan Sterz
---
proxmox-tui-installer/src/options.rs | 26 --
proxmox-tui-installer/src/setup.rs | 16
proxmox-tui-installer/src/views/mod.rs | 18 +++---
3 files changed, 27 insertions(+), 33 deletions(-)
diff --git a/proxmox-tui-installer/src/options.rs
b/proxmox-tui-installer/src/options.rs
index 5f3d295..e1218df 100644
--- a/proxmox-tui-installer/src/options.rs
+++ b/proxmox-tui-installer/src/options.rs
@@ -86,11 +86,11 @@ pub const FS_TYPES: &[FsType] = {
#[derive(Clone, Debug)]
pub struct LvmBootdiskOptions {
-pub total_size: u64,
-pub swap_size: Option,
-pub max_root_size: Option,
-pub max_data_size: Option,
-pub min_lvm_free: Option,
+pub total_size: f64,
+pub swap_size: Option,
+pub max_root_size: Option,
+pub max_data_size: Option,
+pub min_lvm_free: Option,
}
impl LvmBootdiskOptions {
@@ -107,7 +107,7 @@ impl LvmBootdiskOptions {
#[derive(Clone, Debug)]
pub struct BtrfsBootdiskOptions {
-pub disk_size: u64,
+pub disk_size: f64,
}
impl BtrfsBootdiskOptions {
@@ -180,7 +180,7 @@ pub struct ZfsBootdiskOptions {
pub compress: ZfsCompressOption,
pub checksum: ZfsChecksumOption,
pub copies: usize,
-pub disk_size: u64,
+pub disk_size: f64,
}
impl ZfsBootdiskOptions {
@@ -202,12 +202,12 @@ pub enum AdvancedBootdiskOptions {
Btrfs(BtrfsBootdiskOptions),
}
-#[derive(Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Debug, PartialEq)]
pub struct Disk {
pub index: String,
pub path: String,
pub model: Option,
-pub size: u64,
+pub size: f64,
}
impl fmt::Display for Disk {
@@ -219,11 +219,7 @@ impl fmt::Display for Disk {
// FIXME: ellipsize too-long names?
write!(f, " ({model})")?;
}
-write!(
-f,
-" ({:.2} GiB)",
-(self.size as f64) / 1024. / 1024. / 1024.
-)
+write!(f, " ({:.2} GiB)", self.size)
}
}
@@ -233,6 +229,8 @@ impl From<&Disk> for String {
}
}
+impl cmp::Eq for Disk {}
+
impl cmp::PartialOrd for Disk {
fn partial_cmp(&self, other: &Self) -> Option {
self.index.partial_cmp(&other.index)
diff --git a/proxmox-tui-installer/src/setup.rs
b/proxmox-tui-installer/src/setup.rs
index 43e4b0d..1c5ff3e 100644
--- a/proxmox-tui-installer/src/setup.rs
+++ b/proxmox-tui-installer/src/setup.rs
@@ -109,15 +109,15 @@ pub struct InstallConfig {
#[serde(serialize_with = "serialize_fstype")]
filesys: FsType,
-hdsize: u64,
+hdsize: f64,
#[serde(skip_serializing_if = "Option::is_none")]
-swapsize: Option,
+swapsize: Option,
#[serde(skip_serializing_if = "Option::is_none")]
-maxroot: Option,
+maxroot: Option,
#[serde(skip_serializing_if = "Option::is_none")]
-minfree: Option,
+minfree: Option,
#[serde(skip_serializing_if = "Option::is_none")]
-maxvz: Option,
+maxvz: Option,
#[serde(skip_serializing_if = "Option::is_none")]
zfs_opts: Option,
@@ -153,7 +153,7 @@ impl From for InstallConfig {
autoreboot: options.autoreboot as usize,
filesys: options.bootdisk.fstype,
-hdsize: 0,
+hdsize: 0.,
swapsize: None,
maxroot: None,
minfree: None,
@@ -243,13 +243,13 @@ fn deserialize_disks_map<'de, D>(deserializer: D) ->
Result, D::Error>
where
D: Deserializer<'de>,
{
-let disks = >::deserialize(deserializer)?;
+let disks = >::deserialize(deserializer)?;
Ok(disks
.into_iter()
.map(
|(index, device, size_mb, model, logical_bsize, _syspath)| Disk {
index: index.to_string(),
-size: size_mb * logical_bsize,
+size: (size_mb * logical_bsize) / 1024. / 1024. / 1024.,
path: device,
model: (!model.is_empty()).then_some(model),
},
diff --git a/proxmox-tui-installer/src/views/mod.rs
b/proxmox-tui-installer/src/views/mod.rs
index ee96398..faa0052 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -189,17 +189,15 @@ impl DiskSizeEditView {
}
}
-pub fn content(mut self, content: u64) -> Self {
-let val = (content as f64) / 1024. /
Re: [pve-devel] [PATCH installer] tui: multiply the disk size back into bytes
On 21.06.23 16:36, Thomas Lamprecht wrote: > Am 21/06/2023 um 16:00 schrieb Stefan Sterz: >> previously the installer correctly divided the value when using them >> for the `FloatEditView`, but forgot to multiply the value again when >> retrieving it after editing. this commit fixes that >> >> Signed-off-by: Stefan Sterz >> --- >> tested this only locally and didn't build the installer completelly. >> i am not sure if the installer handles this value correctly once it >> is forwarded to the perl installer. if the perl installer expects >> bytes here, it should work just fine, though. > > no it doesn't it expects Gigabyte in floats, see: > https://git.proxmox.com/?p=pve-installer.git;a=commitdiff;h=9a2d64977f73cec225c407ff13765ef02e2ff9e9 > alright, thanks for that, i am not too familiar with this code base ^^'. should we then model these sizes as `f64` instead? i'd go ahead and prepare a patch with that, but it's a bit more churn so i want to make sure that's the way to go. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH installer] tui: multiply the disk size back into bytes
previously the installer correctly divided the value when using them
for the `FloatEditView`, but forgot to multiply the value again when
retrieving it after editing. this commit fixes that
Signed-off-by: Stefan Sterz
---
tested this only locally and didn't build the installer completelly.
i am not sure if the installer handles this value correctly once it
is forwarded to the perl installer. if the perl installer expects
bytes here, it should work just fine, though.
proxmox-tui-installer/src/views/mod.rs | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/proxmox-tui-installer/src/views/mod.rs
b/proxmox-tui-installer/src/views/mod.rs
index 94c3993..8503d82 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -234,7 +234,7 @@ impl DiskSizeEditView {
.flatten()
})
.flatten()
-.map(|val| val as u64)
+.map(|val| (val * 1024. * 1024. * 1024.) as u64)
}
}
--
2.39.2
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH access-control] ldap: fix ldap distinguished names regex
On 23.05.23 12:12, Christoph Heiss wrote: > On Tue, May 23, 2023 at 10:56:24AM +0200, Stefan Sterz wrote: >> On 23.05.23 08:58, Christoph Heiss wrote: >>> On Wed, May 17, 2023 at 03:39:31PM +0200, Stefan Sterz wrote: >>>> [..] >>> While reviewing that, I had a look at the `Net::LDAP` perl library >>> again, if it provides a way to _somehow_ validate DNs properly without >>> having to resort to very fragile regexes. >>> >>> => `Net::LDAP` provides a canonical_dn() function in `Net::LDAP::Util` >>>https://metacpan.org/pod/Net::LDAP::Util#canonical_dn >>> >> >> handling this via Net::LDAP is probably the better way to go. however, >> just to through this suggestion out there: do we maybe want to use a >> more lax regex in general instead of actually checking whether a dn >> *could* be valid? maybe this check should just be a sanity check instead >> of making the dn conform to the spec. > I guess so. In the end, it will get rejected by the LDAP server anyway > if it isn't valid. Offloading some of that logic might be indeed a good > idea. > > When saving, we also don't currently check whether the server actually > accepts it, is allowed to login, etc. So a sync might fail for other > reason too, so it could fail due to invalid DN syntax as well at this > stage IMO. > > Or - just a quick idea - maybe directly check the DN with the server on > changes via the API (given that there is a valid server configuration as > well)? > yeah that would probably be best, as it's also closer to what the user wants (a working ldap setup) than either what the regex or `Net::LDAP` can do (making sure that the dn conforms to spec). since, my knowledge about ldap is fairly shallow, im not sure how this would work in terms of timeouts etc. another point that comes to mind is that lukas reminded me that the same regex is used in pbs. i haven't yet looked at that, but we probably want to make sure that both implementations work as similarly as possible. >> >>> I quickly hacked up your test script to use canonical_dn() instead of >>> the regex. Works pretty much the same, except that it considers empty >>> strings and attributes with empty values as valid DNs and allows >>> whitespaces at both the beginning and the end of attribute values. But >>> these edge-cases can be much more easily checked than the whole thing >>> (esp. with escaping and such), and should be more robust. >> >> yes, but imo that might end up being somewhat confusing. especially if >> we end up using the canonical version of the dn returned by >> `canonical_dn()`. in my testing it also escaped already validly escaped >> values such as `CN=James \"Jim\" Smith\, III` and turned them into >> `CN=James \22Jim\22 Smith\2c III`. this may be confusing to some users. > If the entered DN is suddenly replaced with the canonicalized version, > definitely. But we could make that clear in the UI and documentation. > Although I would favor keeping it as-is, after thinking about it more. > me too. entering one value just to come back to this setting when something breaks to see another sounds extremely frustrating. especially considering most users probably can't tell that semantically the dns are the same. even if well documented. 8< --- snip --- >8 >> imo if we go this route we either need additional logic to handle spaces >> and empty values/dns before passing the dn to `canonical_dn()` for >> validation only, or we need to, at least, document the transformations >> that `canonical_dn()` does. otherwise, these substitutions may end up >> confusing users. > > I don't have a strong opinion on what overall approach to take here. > > To quickly summarize, I guess > - keep storing the DN as-is from user > - only do a lax sanity check ourselves > - let it fail later on sync if it is invalid > - maybe contact the server on changes via the API and check DN/login? > > would be a pretty good way and save us the trouble from tweaking the > regex every few weeks for all eternity. yeah i agree, we should probably still keep the tests for the lax sanity check, just in case. i'll take a look at the pbs side. if you want to take this over, feel free to, just give me a heads-up. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH access-control] ldap: fix ldap distinguished names regex
On 23.05.23 08:58, Christoph Heiss wrote:
> On Wed, May 17, 2023 at 03:39:31PM +0200, Stefan Sterz wrote:
>> [..]
>>
>> this commit also adds a test file that tests the regex against a
>> number of common pitfalls. including distinguished names that are
>> structurally similar to those reported as erroneously forbidden
>> earlier. these tests should help avoid regressions in the future.
> That's a really good idea :^)
>
>>
>> Signed-off-by: Stefan Sterz
>> ---
>> would really appreciate another pair of eyes here. given the recent
>> churn related to this regex. it's very likely i missed something too.
>>
>> src/PVE/Auth/LDAP.pm | 9 +--
>> src/test/Makefile | 1 +
>> src/test/dn-regex-test.pl | 54 +++
>> 3 files changed, 62 insertions(+), 2 deletions(-)
>> create mode 100755 src/test/dn-regex-test.pl
>>
>> diff --git a/src/PVE/Auth/LDAP.pm b/src/PVE/Auth/LDAP.pm
>> index fc82a17..ccc6864 100755
>> --- a/src/PVE/Auth/LDAP.pm
>> +++ b/src/PVE/Auth/LDAP.pm
>> @@ -10,8 +10,13 @@ use PVE::Tools;
>>
>> use base qw(PVE::Auth::Plugin);
>>
>> -my $dn_part_regex = qr!("[^"]+"|[^ ,+"/<>;=#][^,+"/<>;=]*[^ ,+"/<>;=]|[^
>> ,+"/<>;=#])!;
>> -our $dn_regex = qr!\w+=${dn_part_regex}(,\s*\w+=${dn_part_regex})*!;
>> +my $escaped = qr!\\(?:[ "#+,;<=>\\]|[0-9a-fA-F]{2})!;
>> +my $start= qr!(?:${escaped}|[^"+,;<>\\\0 #])!;
>> +my $middle = qr!(?:${escaped}|[^"+,;<>\\\0])!;
>> +my $end = qr!(?:${escaped}|[^"+,;<>\\\0 ])!;
>> +my $attr_val = qr!("[^"]+"|${start}(?:${middle}*${end})?)!;
>> +
>> +our $dn_regex = qr!\w+=${attr_val}([,\+]\s*\w+=${attr_val})*!;
> While reviewing that, I had a look at the `Net::LDAP` perl library
> again, if it provides a way to _somehow_ validate DNs properly without
> having to resort to very fragile regexes.
>
> => `Net::LDAP` provides a canonical_dn() function in `Net::LDAP::Util`
>https://metacpan.org/pod/Net::LDAP::Util#canonical_dn
>
handling this via Net::LDAP is probably the better way to go. however,
just to through this suggestion out there: do we maybe want to use a
more lax regex in general instead of actually checking whether a dn
*could* be valid? maybe this check should just be a sanity check instead
of making the dn conform to the spec.
> I quickly hacked up your test script to use canonical_dn() instead of
> the regex. Works pretty much the same, except that it considers empty
> strings and attributes with empty values as valid DNs and allows
> whitespaces at both the beginning and the end of attribute values. But
> these edge-cases can be much more easily checked than the whole thing
> (esp. with escaping and such), and should be more robust.
yes, but imo that might end up being somewhat confusing. especially if
we end up using the canonical version of the dn returned by
`canonical_dn()`. in my testing it also escaped already validly escaped
values such as `CN=James \"Jim\" Smith\, III` and turned them into
`CN=James \22Jim\22 Smith\2c III`. this may be confusing to some users.
it's handling of spaces is imo especially strange. according to the docs:
> Converts all leading and trailing spaces in values to be \20.
but `ou= test ,dc=net` yields `OU=test,DC=net` for me. so it doesn't
escape them, it just drops them (also note that capitalization)? if you
quote the value it does what it says though. so `ou=" test ",dc=net`
becomes `OU=\20test\20,DC=net` as expected, although somewhat
pointlessly so in this case as spaces in quoted values are in-spec.
imo if we go this route we either need additional logic to handle spaces
and empty values/dns before passing the dn to `canonical_dn()` for
validation only, or we need to, at least, document the transformations
that `canonical_dn()` does. otherwise, these substitutions may end up
confusing users.
> [ I've attached the diff for the test script below for reference. ]
>
> So IHMO that should be the way forward.
>> @Stefan: I'd be willing to properly rewrite the DN checking using
> canonical_dn() - or do you want to?
>
>> [..]
>>
>> ___
>> pve-devel mailing list
>> pve-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>
>
> diff --git a/src/test/dn-regex-test.pl b/src/test/dn-regex-test.pl
> index a2410af..9a48483 100755
> --- a/src/test/dn-regex-test.pl
> +++ b/src/test/dn-regex-test.pl
> @@ -3,6 +3,7 @@ use strict;
> use warnings;
>
> u
Re: [pve-devel] [PATCH access-control] ldap: fix ldap distinguished names regex
sorry just noticed i forgot: adding tests for this was
Suggested-by: Dominik Csapak
On 17.05.23 15:39, Stefan Sterz wrote:
> according to the current specification of the string representation of
> ldap distinguished names (DN) presented by RFC 4514 [1] the current
> regex checking ldap DNs still prevents users from entering valid
> DNs.
>
> for example we do not allow multi-valued RelativeDistinguishedNames as
> these are separated by a '+'. this was already reported as an issue in
> the subscription support.
>
> escaped sequences in unquoted AttributeValues are also not allowed.
> this includes commas (',') and quotes ('"'). for example, the
> following does not work even though the RFC explicitly mentions it as
> a valid example ([1], section 4):
>
> CN=James \"Jim\" Smith\, III,DC=example,DC=net
>
> while this may not be usable as a valid username, as argued
> previously [2], it seems arbitrary to allow this for quoted
> AttributeValues but not for unquoted ones.
>
> this commit also adds a test file that tests the regex against a
> number of common pitfalls. including distinguished names that are
> structurally similar to those reported as erroneously forbidden
> earlier. these tests should help avoid regressions in the future.
>
> [1]: https://datatracker.ietf.org/doc/html/rfc4514
> [2]: https://git.proxmox.com/?p=pve-access-control.git;h=1aa2355a
>
> Signed-off-by: Stefan Sterz
> ---
> would really appreciate another pair of eyes here. given the recent
> churn related to this regex. it's very likely i missed something too.
>
> src/PVE/Auth/LDAP.pm | 9 +--
> src/test/Makefile | 1 +
> src/test/dn-regex-test.pl | 54 +++
> 3 files changed, 62 insertions(+), 2 deletions(-)
> create mode 100755 src/test/dn-regex-test.pl
>
> diff --git a/src/PVE/Auth/LDAP.pm b/src/PVE/Auth/LDAP.pm
> index fc82a17..ccc6864 100755
> --- a/src/PVE/Auth/LDAP.pm
> +++ b/src/PVE/Auth/LDAP.pm
> @@ -10,8 +10,13 @@ use PVE::Tools;
>
> use base qw(PVE::Auth::Plugin);
>
> -my $dn_part_regex = qr!("[^"]+"|[^ ,+"/<>;=#][^,+"/<>;=]*[^ ,+"/<>;=]|[^
> ,+"/<>;=#])!;
> -our $dn_regex = qr!\w+=${dn_part_regex}(,\s*\w+=${dn_part_regex})*!;
> +my $escaped = qr!\\(?:[ "#+,;<=>\\]|[0-9a-fA-F]{2})!;
> +my $start= qr!(?:${escaped}|[^"+,;<>\\\0 #])!;
> +my $middle = qr!(?:${escaped}|[^"+,;<>\\\0])!;
> +my $end = qr!(?:${escaped}|[^"+,;<>\\\0 ])!;
> +my $attr_val = qr!("[^"]+"|${start}(?:${middle}*${end})?)!;
> +
> +our $dn_regex = qr!\w+=${attr_val}([,\+]\s*\w+=${attr_val})*!;
>
> sub type {
> return 'ldap';
> diff --git a/src/test/Makefile b/src/test/Makefile
> index 859a84b..b4c05eb 100644
> --- a/src/test/Makefile
> +++ b/src/test/Makefile
> @@ -13,3 +13,4 @@ check:
> perl -I.. perm-test7.pl
> perl -I.. perm-test8.pl
> perl -I.. realm_sync_test.pl
> + perl -I.. dn-regex-test.pl
> diff --git a/src/test/dn-regex-test.pl b/src/test/dn-regex-test.pl
> new file mode 100755
> index 000..a2410af
> --- /dev/null
> +++ b/src/test/dn-regex-test.pl
> @@ -0,0 +1,54 @@
> +
> +use strict;
> +use warnings;
> +
> +use PVE::Auth::LDAP;
> +
> +# this test file attempts to check whether the ldap distinguished name regex
> +# defined in `PVE::Auth::LDAP` complies with RFC 4514.
> +#
> +# see: https://datatracker.ietf.org/doc/html/rfc4514
> +
> +my @pass = (
> +"ou=a", # single AttributeTypeValue
> +"ou=orga,dc=com,cn=name",# multiple RelativeDistinguishedNames
> +"STREET=a,cn=a,C=c", # single character AttributeValues
> +"UID=tt,cn=\"#+,;<>\\ \"", # forbidden characters are allowed when
> quoted
> +"c=\\\"\\#\\+\\;\\<\\=\\>", # specific characters allowed when
> escaped
> +"a=",# escaped backslashes are allowed
> +"ST=a,cn=\"Test, User\"",# allow un-escaped commas in quoted
> AttributeValues
> +"o2u=bc,cn=Test\\, User",# allow escaped commas
> +"T2=a #b", # spaces (' ') and '#' are allowed in
> the middle of AttributeValues
> +"word4word=ab#", # allow '#' at the end of an AttributeValue
> +"ou=orga+sub=ab",# allow '+' as separators for
> multi-valued RelativeDistinguishedName
> +&quo
[pve-devel] [PATCH access-control] ldap: fix ldap distinguished names regex
according to the current specification of the string representation of
ldap distinguished names (DN) presented by RFC 4514 [1] the current
regex checking ldap DNs still prevents users from entering valid
DNs.
for example we do not allow multi-valued RelativeDistinguishedNames as
these are separated by a '+'. this was already reported as an issue in
the subscription support.
escaped sequences in unquoted AttributeValues are also not allowed.
this includes commas (',') and quotes ('"'). for example, the
following does not work even though the RFC explicitly mentions it as
a valid example ([1], section 4):
CN=James \"Jim\" Smith\, III,DC=example,DC=net
while this may not be usable as a valid username, as argued
previously [2], it seems arbitrary to allow this for quoted
AttributeValues but not for unquoted ones.
this commit also adds a test file that tests the regex against a
number of common pitfalls. including distinguished names that are
structurally similar to those reported as erroneously forbidden
earlier. these tests should help avoid regressions in the future.
[1]: https://datatracker.ietf.org/doc/html/rfc4514
[2]: https://git.proxmox.com/?p=pve-access-control.git;h=1aa2355a
Signed-off-by: Stefan Sterz
---
would really appreciate another pair of eyes here. given the recent
churn related to this regex. it's very likely i missed something too.
src/PVE/Auth/LDAP.pm | 9 +--
src/test/Makefile | 1 +
src/test/dn-regex-test.pl | 54 +++
3 files changed, 62 insertions(+), 2 deletions(-)
create mode 100755 src/test/dn-regex-test.pl
diff --git a/src/PVE/Auth/LDAP.pm b/src/PVE/Auth/LDAP.pm
index fc82a17..ccc6864 100755
--- a/src/PVE/Auth/LDAP.pm
+++ b/src/PVE/Auth/LDAP.pm
@@ -10,8 +10,13 @@ use PVE::Tools;
use base qw(PVE::Auth::Plugin);
-my $dn_part_regex = qr!("[^"]+"|[^ ,+"/<>;=#][^,+"/<>;=]*[^ ,+"/<>;=]|[^
,+"/<>;=#])!;
-our $dn_regex = qr!\w+=${dn_part_regex}(,\s*\w+=${dn_part_regex})*!;
+my $escaped = qr!\\(?:[ "#+,;<=>\\]|[0-9a-fA-F]{2})!;
+my $start= qr!(?:${escaped}|[^"+,;<>\\\0 #])!;
+my $middle = qr!(?:${escaped}|[^"+,;<>\\\0])!;
+my $end = qr!(?:${escaped}|[^"+,;<>\\\0 ])!;
+my $attr_val = qr!("[^"]+"|${start}(?:${middle}*${end})?)!;
+
+our $dn_regex = qr!\w+=${attr_val}([,\+]\s*\w+=${attr_val})*!;
sub type {
return 'ldap';
diff --git a/src/test/Makefile b/src/test/Makefile
index 859a84b..b4c05eb 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -13,3 +13,4 @@ check:
perl -I.. perm-test7.pl
perl -I.. perm-test8.pl
perl -I.. realm_sync_test.pl
+ perl -I.. dn-regex-test.pl
diff --git a/src/test/dn-regex-test.pl b/src/test/dn-regex-test.pl
new file mode 100755
index 000..a2410af
--- /dev/null
+++ b/src/test/dn-regex-test.pl
@@ -0,0 +1,54 @@
+
+use strict;
+use warnings;
+
+use PVE::Auth::LDAP;
+
+# this test file attempts to check whether the ldap distinguished name regex
+# defined in `PVE::Auth::LDAP` complies with RFC 4514.
+#
+# see: https://datatracker.ietf.org/doc/html/rfc4514
+
+my @pass = (
+"ou=a",# single AttributeTypeValue
+"ou=orga,dc=com,cn=name", # multiple RelativeDistinguishedNames
+"STREET=a,cn=a,C=c", # single character AttributeValues
+"UID=tt,cn=\"#+,;<>\\ \"", # forbidden characters are allowed when quoted
+"c=\\\"\\#\\+\\;\\<\\=\\>",# specific characters allowed when
escaped
+"a=", # escaped backslashes are allowed
+"ST=a,cn=\"Test, User\"", # allow un-escaped commas in quoted
AttributeValues
+"o2u=bc,cn=Test\\, User", # allow escaped commas
+"T2=a #b", # spaces (' ') and '#' are allowed in the
middle of AttributeValues
+"word4word=ab#", # allow '#' at the end of an AttributeValue
+"ou=orga+sub=ab", # allow '+' as separators for multi-valued
RelativeDistinguishedName
+"dc=\\f0\\Ac\\93", # allow escaping hex values in unquoted
AttributeValues
+
+# regression tests
+"ou=adf-bd,dc=abcd+efOuId=BL:BL:sldkf:704004,dc=or,dc=com",
+
"gvGid=DE:8A:wordCaps,ou=Service,dc=alsdkj+abOuId=UK:A8:137100,dc=edu,dc=de",
+);
+
+my @fail = (
+"",# no empty distinguished name
+"ou=a,", # no empty AttributeTypeAndValue
+"ou=a+", # no multi-valued RelativeDistinguishedName
with empty second part
+"ou", # missing separator and AttributeValue
+"ou=",
[pve-devel] [PATCH widget-toolkit 2/2] fix #4618: dark-mode: lighten critical/warning charts/gauges colors
by increasing the lightness of these colors to to make them have the
same amount of lightness as the primary color.
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/extjs/_progress.scss | 4 ++--
src/proxmox-dark/scss/other/_charts.scss | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/proxmox-dark/scss/extjs/_progress.scss
b/src/proxmox-dark/scss/extjs/_progress.scss
index 4f2bb49..5d5941c 100644
--- a/src/proxmox-dark/scss/extjs/_progress.scss
+++ b/src/proxmox-dark/scss/extjs/_progress.scss
@@ -11,9 +11,9 @@
}
.x-progress.warning .x-progress-bar {
- background-color: $background-warning;
+ background-color: var(--pwt-gauge-warn);
}
.x-progress.critical .x-progress-bar {
- background-color: $background-invalid;
+ background-color: var(--pwt-gauge-crit);
}
diff --git a/src/proxmox-dark/scss/other/_charts.scss
b/src/proxmox-dark/scss/other/_charts.scss
index 5c67282..26b0104 100644
--- a/src/proxmox-dark/scss/other/_charts.scss
+++ b/src/proxmox-dark/scss/other/_charts.scss
@@ -7,8 +7,8 @@
--pwt-text-color: #{$text-color};
--pwt-gauge-default: #{$primary-color};
--pwt-gauge-back: #{$background-dark};
- --pwt-gauge-warn: #{$background-warning};
- --pwt-gauge-crit: #{$background-invalid};
+ --pwt-gauge-warn: #{adjust-color($background-warning, $lightness:
lightness($primary-color))};
+ --pwt-gauge-crit: #{adjust-color($background-invalid, $lightness:
lightness($primary-color))};
--pwt-chart-primary: #{$primary-color};
--pwt-chart-grid-stroke: #{$content-background-selected};
}
--
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 widget-toolkit 0/2] Proxmox Dark Theme Fix-ups - Round 6
this patch series adds a couple more tweaks to the dark theme. first it improves the contrast ratios on panel header tool icons. then the chart and gauge colors are brightened to match the primary color. for the backup server first the path to the dark theme css in the api viewer is fixed. then the theme switcher menu item is renamed to "color theme" to match promox ve. the final patch also renames this menu item for the mail gateway. Stefan Sterz (2) @ widget-toolkit: dark-mode: adjust panel header tool icons fix #4618: dark-mode: lighten critical/warning charts/gauges colors src/proxmox-dark/scss/extjs/_panel.scss| 37 +++--- src/proxmox-dark/scss/extjs/_progress.scss | 4 +-- src/proxmox-dark/scss/other/_charts.scss | 4 +-- 3 files changed, 36 insertions(+), 9 deletions(-) Stefan Sterz (2) @ proxmox-backup: docs: fix api viewer dark theme path ui: main view: rename "Theme" selector to "Color Theme" docs/api-viewer/index.html | 2 +- www/MainView.js| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) Stefan Sterz (1) @ pmg-gui: main view/quarantine: rename "Theme" selector to "Color Theme" js/MainView.js | 2 +- js/QuarantineView.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 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 widget-toolkit 1/2] dark-mode: adjust panel header tool icons
by brigthenening the icons on a more individual basis some darker ones
can now feature higher contrasts while others won't be too bright.
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/extjs/_panel.scss | 37 +
1 file changed, 32 insertions(+), 5 deletions(-)
diff --git a/src/proxmox-dark/scss/extjs/_panel.scss
b/src/proxmox-dark/scss/extjs/_panel.scss
index 217d3f8..a3c9682 100644
--- a/src/proxmox-dark/scss/extjs/_panel.scss
+++ b/src/proxmox-dark/scss/extjs/_panel.scss
@@ -2,15 +2,42 @@
background-color: $content-background-color;
border: none;
- // The small navigation elements in the panel header bar e.g. to
- // collapse a panel
.x-tool-tool-el {
background-color: transparent;
-filter: brightness(120%);
+ }
+}
+
+// The small navigation elements in the panel header bar e.g. to
+// collapse a panel
+.x-tool-img {
+ filter: brightness(175%);
+
+ // these are brighter per default, so they don't need to be
+ // brigthened as much
+ &.x-tool-expand,
+ &.x-tool-collapse,
+ &.x-tool-refresh {
+filter: brightness(125%);
+ }
+
+ // this icon uses multiple tones, to have them behave appropriatelly
+ // invert them before brightening them
+ &.x-tool-print {
+filter: invert(100%) hue-rotate(180deg) brightness(125%);
+ }
+
+ .x-tool-over & {
+filter: brightness(200%);
+ }
+
+ .x-tool-over &.x-tool-expand,
+ .x-tool-over &.x-tool-collapse,
+ .x-tool-over &.x-tool-refresh {
+filter: brightness(150%);
}
- .x-tool-over .x-tool-tool-el {
-filter: brightness(140%);
+ .x-tool-over &.x-tool-print {
+filter: invert(100%) hue-rotate(180deg) brightness(150%);
}
}
--
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 v2 widget-toolkit] fix #1454: InfoWidget for Memory
On 28.03.23 14:49, Matthias Heiserer wrote:
> Signed-off-by: Matthias Heiserer
> ---
>
> I'm a bit unsure about the color. If clashes a bit with the red when the
> RAM is near full in light mode. Open for better suggestions, but
> should work for now.
>
> changes from v1:
> ignore arcsize when not set
> separate progress/memory widget:
> override rendertpl and dont inject second bar
>
> src/Makefile | 1 +
> src/css/ext6-pmx.css | 6
> src/panel/NodeMemoryWidget.js | 54 +++
> src/panel/StatusView.js | 2 +-
> 4 files changed, 62 insertions(+), 1 deletion(-)
> create mode 100644 src/panel/NodeMemoryWidget.js
>
> diff --git a/src/Makefile b/src/Makefile
> index 30e8fd5..a10f019 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -55,6 +55,7 @@ JSSRC= \
> panel/EOLNotice.js \
> panel/InputPanel.js \
> panel/InfoWidget.js \
> + panel/NodeMemoryWidget.js \
> panel/LogView.js\
> panel/NodeInfoRepoStatus.js \
> panel/JournalView.js\
> diff --git a/src/css/ext6-pmx.css b/src/css/ext6-pmx.css
> index 2ffd2a8..8d2c3ef 100644
> --- a/src/css/ext6-pmx.css
> +++ b/src/css/ext6-pmx.css
> @@ -348,3 +348,9 @@ div.right-aligned {
> color: #555;
> }
> /* action column fix end */
> +
> +.zfs-arc {
> + background-color: #c976b7;
> + color: #c976b7;
thanks for this, it will surely help user's better understand the ram
usage chart. i'd have two suggestions:
1) can we start using hsl for color going forward? the dark theme
already does that, and it helps us stay more consistent (e.g., if you
want a dark/lighter version of a color, that is much easier in hsl, in
rgb it's very easy to accidentally get a different hue etc.)
2) i think it would also be good to start using css variables in
general. if we ever do need to change these colors in a theme, it's much
easier to change a variable, then to have to override each css style.
in regard to the general color choice: yeah this purple is not ideal.
maybe you could use something that is closer to the blue in hue.
currently, the two colors used have these two closest hsl equivalents,
blue: hsl(206deg, 65%, 85%) red: hsl(360deg, 100%, 77%). so, potentially
hsl(280deg, 82.5%, 60%) would work. but im not to sure either.
purple in other contexts is used as a more intense version of red (e.g.,
heat maps) so perhaps we should think of a different hue altogether.
however, putting this all together would yield:
:root {
--pwt-zfs-arc: hsl(280deg, 82.5%, 60%);
}
.zfs-arc {
background-color: var(--pwt-zfs-arc);
color: var(--pwt-zfs-arc);
}
then in the dark theme we could simply do (this color is imo more
consistent with the dark theme chart/gauge colors):
:root {
--pwt-zfs-arc: hsl(280deg, 100%, 40.5%);
}
> + height: 100%;
> +}
> diff --git a/src/panel/NodeMemoryWidget.js b/src/panel/NodeMemoryWidget.js
> new file mode 100644
> index 000..e7619fd
> --- /dev/null
> +++ b/src/panel/NodeMemoryWidget.js
> @@ -0,0 +1,54 @@
> +Ext.define('Proxmox.panel.ArcProgress', {
> +extend: 'Ext.ProgressBar',
> +alias: 'widget.pmxArcProgress',
> +
> + childEls: [
> + 'arcbar',
> + ],
> +
> +// modified from
> https://docs.sencha.com/extjs/7.0.0/classic/src/ProgressBar.js.html
> +renderTpl: [
> + ` + style='margin-right: 100%; width: auto;'
> + class='zfs-arc'>
> + `,
> + '',
> + '',
> + '',
> + '{text}',
> + '',
> + '',
> + '',
> +],
> +
> +updateArc: function(width) {
> + this.arcbar.setStyle('margin-right', `${width}%`);
> +},
> +
> +initComponent: function() {
> + this.callParent();
> +},
> +});
> +
> +
> +Ext.define('Proxmox.widget.NodeMemory', {
> +extend: 'Proxmox.widget.Info',
> +alias: 'widget.pmxNodeMemoryWidget',
> +
> +updateValue: function(text, usage, mem) {
> + let me = this;
> +
> + if (mem) {
> + usage = (mem.used - (mem.arcsize || 0)) / mem.total;
> + me.getComponent("progress").updateArc((mem.free / mem.total) * 100);
> + me.callParent([text, usage]);
> + }
> +},
> +
> +initComponent: function() {
> + let me = this;
> + me.items.filter(i => i.xtype === 'progressbar')
> + .forEach(i => { i.xtype = 'pmxArcProgress'; });
> + me.callParent();
> +},
> +});
> diff --git a/src/panel/StatusView.js b/src/panel/StatusView.js
> index e2e81e2..7258f36 100644
> --- a/src/panel/StatusView.js
> +++ b/src/panel/StatusView.js
> @@ -72,7 +72,7 @@ Ext.define('Proxmox.panel.StatusView', {
> if (Ext.isFunction(field.calculate)) {
> calculate = field.calculate;
> }
> - field.updateValue(renderer.call(field, used, max), calculate(used,
> max));
> + field.
[pve-devel] [PATCH widget-toolkit 4/4] dark-mode: improve apt repo group header contrast ratios
make the group headers darker, so that they have some contrast with
the surrounding rows. also add back the border at the bottom. both
changes improve the legibility of the table.
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/proxmox/_nodes.scss | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/proxmox-dark/scss/proxmox/_nodes.scss
b/src/proxmox-dark/scss/proxmox/_nodes.scss
index 02b15f9..84f9127 100644
--- a/src/proxmox-dark/scss/proxmox/_nodes.scss
+++ b/src/proxmox-dark/scss/proxmox/_nodes.scss
@@ -1,7 +1,7 @@
// Table headers under Node > "Updates" > "Repositories"
.proxmox-apt-repos .x-grid-group-hd {
- background-color: $background-darker;
- border-bottom-width: 0;
+ background-color: $background-darkest;
+ border-color: $border-color-alt;
}
.proxmox-apt-repos .x-grid-group-title {
--
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 widget-toolkit 3/4] dark-mode: style the icon for the maintenance mode in pbs
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/other/_icons.scss | 14 ++
1 file changed, 14 insertions(+)
diff --git a/src/proxmox-dark/scss/other/_icons.scss
b/src/proxmox-dark/scss/other/_icons.scss
index df81969..d4dc316 100644
--- a/src/proxmox-dark/scss/other/_icons.scss
+++ b/src/proxmox-dark/scss/other/_icons.scss
@@ -156,6 +156,20 @@
}
}
+.pmx-tree-icon-custom {
+ &::after {
+text-shadow: -1px 0 1px $background-darker;
+ }
+
+ &.maintenance::before {
+color: $icon-color-alt;
+ }
+
+ &.maintenance::after {
+color: $icon-color;
+ }
+}
+
// icons for templates in the storages view
.x-treelist-item-icon {
&.lxc::after,
--
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 widget-toolkit 2/4] dark-mode: fix the focused state for background image grid icons
some icons in grids are background images for the whole grid element.
so we need to filter the entire element, which also means that any
background or inner border color would get filtered too. this inverts
the focused border on inner elements and the focused background so
that it looks correct when inverted again.
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/other/_icons.scss | 16 ++--
.../scss/proxmox/_loadingindicator.scss | 8
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/src/proxmox-dark/scss/other/_icons.scss
b/src/proxmox-dark/scss/other/_icons.scss
index 691ea75..df81969 100644
--- a/src/proxmox-dark/scss/other/_icons.scss
+++ b/src/proxmox-dark/scss/other/_icons.scss
@@ -90,12 +90,16 @@
color: black;
}
- .x-grid-cell-inner::before {
-// this is a somewhat hacky work-around for the focus borders on
-// these elements. since we use the invert filter to fix the icon
-// color we need to also invert the border color first too, not
-// just the text. add "!important" to properly override.
-border-color: invert($primary-color, $weight: 90%) !important;
+ // this is a somewhat hacky work-around for the focus borders and
+ // background on these elements. since we use the invert filter to
+ // fix the icon color we need to also invert the border color first
+ // too, not just the text.
+ .x-keyboard-mode &.x-grid-item-focused {
+background-color: invert($selection-background-color, $weight: 90%);
+
+.x-grid-cell-inner::before {
+ border-color: invert($primary-color, $weight: 90%);
+}
}
}
diff --git a/src/proxmox-dark/scss/proxmox/_loadingindicator.scss
b/src/proxmox-dark/scss/proxmox/_loadingindicator.scss
index 3b58917..69fd962 100644
--- a/src/proxmox-dark/scss/proxmox/_loadingindicator.scss
+++ b/src/proxmox-dark/scss/proxmox/_loadingindicator.scss
@@ -8,6 +8,14 @@
color: black;
}
+.x-keyboard-mode .x-grid-item-focused.x-grid-row-loading {
+ background-color: invert($selection-background-color, $weight: 90%);
+
+ .x-grid-cell-inner::before {
+border-color: invert($primary-color, $weight: 90%) !important;
+ }
+}
+
.x-mask-msg {
background-color: $form-field-body-color;
border: solid 1px $border-color-alt;
--
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 widget-toolkit 1/4] dark-mode: fix focus and focus-over states for tabs
previously the focus and focus-over states weren't styled so the crisp
styling was used, which made them appear too brightly.
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/extjs/_tabbar.scss | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/proxmox-dark/scss/extjs/_tabbar.scss
b/src/proxmox-dark/scss/extjs/_tabbar.scss
index e1b5f66..6fe945e 100644
--- a/src/proxmox-dark/scss/extjs/_tabbar.scss
+++ b/src/proxmox-dark/scss/extjs/_tabbar.scss
@@ -26,6 +26,8 @@
color: $text-color;
}
+ .x-keyboard-mode &.x-tab-focus,
+ .x-keyboard-mode &.x-tab-focus.x-tab-over,
.x-keyboard-mode &.x-tab-focus.x-tab-active {
background-color: $primary-color;
border-color: $primary-color;
--
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 widget-toolkit 0/4] Proxmox Dark Theme Fix-ups - Round 5
this series fixes some more icons (e.g., the maintenance mode icon in pbs), sets appropriates styles for tabs in focus and focus-over states, fixes focused states for grid elements and improves contrats for the apt group headers. Stefan Sterz (4): dark-mode: fix focus and focus-over states for tabs dark-mode: fix the focused state for background image grid icons dark-mode: style the icon for the maintenance mode in pbs dark-mode: improve apt repo group header contrast ratios src/proxmox-dark/scss/extjs/_tabbar.scss | 2 ++ src/proxmox-dark/scss/other/_icons.scss | 30 +++ .../scss/proxmox/_loadingindicator.scss | 8 + src/proxmox-dark/scss/proxmox/_nodes.scss | 4 +-- 4 files changed, 36 insertions(+), 8 deletions(-) -- 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 widget-toolkit 5/5] fix #4610: add a small white padding to the totp qr code
some qr code readers need a white "quiet zone" around the main qr
code. otherwise, they won't be able to scan it at all which made it
impossible to scan the totp qr code on certain devices.
Signed-off-by: Stefan Sterz
---
src/window/AddTotp.js | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/window/AddTotp.js b/src/window/AddTotp.js
index 080b361..53fdaad 100644
--- a/src/window/AddTotp.js
+++ b/src/window/AddTotp.js
@@ -224,11 +224,11 @@ Ext.define('Proxmox.window.AddTotp', {
visible: '{!secretEmpty}',
},
style: {
- 'margin-left': 'auto',
- 'margin-right': 'auto',
+ margin: '5px auto',
padding: '5px',
width: '266px',
height: '266px',
+ 'background-color': 'white',
},
},
{
--
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 widget-toolkit 0/5] Proxmox Dark Theme Fixups - Round 4
this round of fixups improves the contrast ratios on several icons and the mask message. it also fixes the background of boundlists which appeared to brightly in certain contexts and fixes #4610. #4610 is fixed by adding a white padding to the totp qr code so most (hopefully, all) qr code scanners can scan it again. Stefan Sterz (5): dark-mode: improve contrast on split buttons dark-mode: color the custom grid and tree icons dark-mode: set boundlist background dark-mode: improve contrast ratios on the mask message fix #4610: add a small white padding to the totp qr code src/proxmox-dark/scss/extjs/_presentation.scss | 3 +-- src/proxmox-dark/scss/extjs/form/_button.scss| 12 ++-- src/proxmox-dark/scss/extjs/form/_combobox.scss | 1 + src/proxmox-dark/scss/other/_icons.scss | 2 ++ src/proxmox-dark/scss/proxmox/_loadingindicator.scss | 2 +- src/window/AddTotp.js| 4 ++-- 6 files changed, 13 insertions(+), 11 deletions(-) -- 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 widget-toolkit 1/5] dark-mode: improve contrast on split buttons
this improves the contrast of the little triangle in split buttons,
making it stand out more especially when focused.
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/extjs/form/_button.scss | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/proxmox-dark/scss/extjs/form/_button.scss
b/src/proxmox-dark/scss/extjs/form/_button.scss
index bf02135..e48eccc 100644
--- a/src/proxmox-dark/scss/extjs/form/_button.scss
+++ b/src/proxmox-dark/scss/extjs/form/_button.scss
@@ -32,16 +32,16 @@
color: $neutral-button-icon-color;
}
+ // the little arrow in certain toolbar buttons with dropdowns
+ .x-btn-wrap-default-toolbar-small.x-btn-arrow-right::after,
+ .x-btn-wrap-default-toolbar-small.x-btn-split-right::after {
+filter: brightness(150%);
+ }
+
&.x-btn-over,
.x-keyboard-mode &.x-btn-focus {
background-color: $neutral-button-color-alt;
border-color: $neutral-button-color-alt;
-
-// the little arrow in certain toolbar buttons with dropdowns
-.x-btn-wrap-default-toolbar-small.x-btn-arrow-right::after,
-.x-btn-wrap-default-toolbar-small.x-btn-split-right::after {
- filter: invert($icon-brightness);
-}
}
&.x-btn.x-btn-disabled {
--
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 widget-toolkit 2/5] dark-mode: color the custom grid and tree icons
this wasn't noticed before because usually vms would either be running
or stopped/offline/unknown etc. and there the colors are set
separately. however, in e.g., the backup view's missing backups
window these weren't colored properly. so this commit sets a default
color.
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/other/_icons.scss | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/proxmox-dark/scss/other/_icons.scss
b/src/proxmox-dark/scss/other/_icons.scss
index 164cd35..9aef511 100644
--- a/src/proxmox-dark/scss/other/_icons.scss
+++ b/src/proxmox-dark/scss/other/_icons.scss
@@ -117,6 +117,8 @@
.x-tree-icon-custom,
.x-grid-icon-custom {
+ color: $icon-color;
+
&::after {
color: $icon-color;
text-shadow: -1px 0 1px $background-darker;
--
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 widget-toolkit 4/5] dark-mode: improve contrast ratios on the mask message
this removes the transparent background of the masks message box. this
should help improve the contrast between the background and text.
also sets an appropriate border color to make it stand out more.
Signed-off-by: Stefan Sterz
---
interestingly, sassc removes the alpha value from something like
`rgba(38, 38, 38, 0.5)` but keeps it in this case. so
`rgba($background-darker, 0.5)` works, but `rgba(38, 38, 38, 0.5)`
does not.
src/proxmox-dark/scss/extjs/_presentation.scss | 3 +--
src/proxmox-dark/scss/proxmox/_loadingindicator.scss | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/proxmox-dark/scss/extjs/_presentation.scss
b/src/proxmox-dark/scss/extjs/_presentation.scss
index 6be322a..6bf1ab5 100644
--- a/src/proxmox-dark/scss/extjs/_presentation.scss
+++ b/src/proxmox-dark/scss/extjs/_presentation.scss
@@ -1,8 +1,7 @@
// The mask that is applied when the window is unaccessible (Login
// screen, Loading, ...)
.x-mask {
- background-color: $background-darker;
- opacity: 0.5;
+ background-color: rgba($background-darker, 0.5);
}
// Shadows of floating windows like window modals, form selectors and
diff --git a/src/proxmox-dark/scss/proxmox/_loadingindicator.scss
b/src/proxmox-dark/scss/proxmox/_loadingindicator.scss
index 5c320b3..3b58917 100644
--- a/src/proxmox-dark/scss/proxmox/_loadingindicator.scss
+++ b/src/proxmox-dark/scss/proxmox/_loadingindicator.scss
@@ -10,5 +10,5 @@
.x-mask-msg {
background-color: $form-field-body-color;
- border: solid 1px mix(black, $form-field-body-color, 25%);
+ border: solid 1px $border-color-alt;
}
--
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 widget-toolkit 3/5] dark-mode: set boundlist background
this is only visible when no boundlist items are present, which only
occurs when loading elements for the boundlist (e.g., when adding a
nfs storage)
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/extjs/form/_combobox.scss | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/proxmox-dark/scss/extjs/form/_combobox.scss
b/src/proxmox-dark/scss/extjs/form/_combobox.scss
index a6405a1..d6119db 100644
--- a/src/proxmox-dark/scss/extjs/form/_combobox.scss
+++ b/src/proxmox-dark/scss/extjs/form/_combobox.scss
@@ -4,6 +4,7 @@
}
.x-boundlist {
+ background-color: $form-field-body-color;
border-color: $form-field-body-color;
}
--
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 widget-toolkit] dark-mode: style locked guest icons properly
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/other/_icons.scss | 4
1 file changed, 4 insertions(+)
diff --git a/src/proxmox-dark/scss/other/_icons.scss
b/src/proxmox-dark/scss/other/_icons.scss
index d492e2e..164cd35 100644
--- a/src/proxmox-dark/scss/other/_icons.scss
+++ b/src/proxmox-dark/scss/other/_icons.scss
@@ -131,6 +131,10 @@
color: $icon-color-alt;
}
+ &.locked::after {
+color: $icon-color;
+ }
+
&.lxc::after,
&.qemu::after {
background-color: $background-darker;
--
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 widget-toolkit 3/3] dark-mode: style checkboxes that don't use blueish active states
e.g., in the backup job creation window the filter column checkmark
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/other/_icons.scss | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/proxmox-dark/scss/other/_icons.scss
b/src/proxmox-dark/scss/other/_icons.scss
index aa36b54..d492e2e 100644
--- a/src/proxmox-dark/scss/other/_icons.scss
+++ b/src/proxmox-dark/scss/other/_icons.scss
@@ -108,7 +108,10 @@
.x-form-checkbox-default,
.x-form-radio-default,
.x-column-header-checkbox .x-column-header-checkbox::after,
-.x-grid-checkcolumn::after {
+.x-grid-checkcolumn::after,
+// checkboxes without the extra "blueish" active states
+.x-menu-item-checked .x-menu-item-icon-default.x-menu-item-checkbox,
+.x-menu-item-unchecked .x-menu-item-icon-default.x-menu-item-checkbox {
filter: invert($icon-brightness) hue-rotate(180deg) brightness(125%);
}
--
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 widget-toolkit 1/3] dark-mode: fix highlighting of active elements in drop down menus
e.g.,: the filter menu item in the backup job creation window
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/extjs/_menu.scss | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/proxmox-dark/scss/extjs/_menu.scss
b/src/proxmox-dark/scss/extjs/_menu.scss
index 74be901..2983f60 100644
--- a/src/proxmox-dark/scss/extjs/_menu.scss
+++ b/src/proxmox-dark/scss/extjs/_menu.scss
@@ -19,7 +19,8 @@
}
// When hovering over a menu item
- &.x-menu-item-focus {
+ &.x-menu-item-focus,
+ &.x-menu-item-active {
@include selection;
}
}
--
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 widget-toolkit 2/3] dark-mode: set the icon color of filtered column headers properly
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/other/_icons.scss | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/proxmox-dark/scss/other/_icons.scss
b/src/proxmox-dark/scss/other/_icons.scss
index b4443ee..aa36b54 100644
--- a/src/proxmox-dark/scss/other/_icons.scss
+++ b/src/proxmox-dark/scss/other/_icons.scss
@@ -28,7 +28,8 @@
// e.g. permission tree view in pve
.x-tree-icon-leaf:not(.x-tree-icon-custom)::before,
.x-tree-icon-parent:not(.x-tree-icon-custom)::before,
-.x-tree-icon-parent-expanded:not(.x-tree-icon-custom)::before {
+.x-tree-icon-parent-expanded:not(.x-tree-icon-custom)::before,
+.x-grid-filters-filtered-column .x-column-header-text::after {
color: $icon-color;
}
--
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 widget-toolkit] dark-mode: set the background mask to `background-darker` again
this removes an issue where the mask would look awkward and
inconsistent (e.g., in the quarantine view, the retention tab of a
zfs storage etc). also makes the shadow a big bigger to be closer to
crisp and also to improve contrast ratios
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/extjs/_presentation.scss | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/proxmox-dark/scss/extjs/_presentation.scss
b/src/proxmox-dark/scss/extjs/_presentation.scss
index 6e463da..6be322a 100644
--- a/src/proxmox-dark/scss/extjs/_presentation.scss
+++ b/src/proxmox-dark/scss/extjs/_presentation.scss
@@ -1,7 +1,7 @@
// The mask that is applied when the window is unaccessible (Login
// screen, Loading, ...)
.x-mask {
- background-color: black;
+ background-color: $background-darker;
opacity: 0.5;
}
@@ -10,5 +10,5 @@
.x-css-shadow {
// the additional styling from the pve css overwrites the setting on
// the element with "!important", that's why we need it here.
- box-shadow: black 0 0 10px !important;
+ box-shadow: black 0 -1px 15px 5px !important;
}
--
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] applied: [PATCH 1/3] Revert "dark-theme: let the background "shine through" mask more"
On 3/21/23 14:05, Thomas Lamprecht wrote:
> Am 21/03/2023 um 11:04 schrieb Wolfgang Bumiller:
>> This looks horrible.
>>
>> A *much* *much* better way to improve readability is to
>> simply set the opacity down to 0.5.
>>
>> This reverts commit 2c837f5766b48629a835c62d4b7af6c3ae4dc1c0.
>> ---
>> I can't stand this.
>> Seriously.
>>
>> src/proxmox-dark/scss/extjs/_presentation.scss | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>>
>
> applied all three patches, they actually fulfil my request to be able to still
> read the stuff in the bg even a bit better and look fine enough otherwise,
> thanks!
>
> FWIW, just from a aesthetic POV the best way to make modal windows stand out
> would
> be to blur the background, i.e.:
>
> .x-mask {
> background-color: rgba(0,0,0,0.5);
> backdrop-filter: blur(2px);
> }
>
> (backdrop-filter + opacity doesn't work, so one needs to use rgba instead)
>
> But I prefer usability/accessibility over aesthetics here.
>
well rgba values sadly don't work properly with sassc. so that will
become a solid black background.
also as discussed off-list, setting the background of the mask to black
looks awkward/less consistent with crisp in several situation (e.g., the
retention tab of a zfs storage or the quarantine mail preview panel).
however @Wolfgang and i agreed on the following:
.x-mask {
background-color: $background-darker;
opacity: 0.5;
}
.x-css-shadow {
box-shadow: black 0 -1px 15px 5px !important;
}
that should hopefully fullfill all three requirements (see-through,
contrast to background, not looking strange in the given situations).
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH widget-toolkit 3/4] dark-theme: dim warning and invalid colors more
this brings them more in-line with the appearance of crisp. Signed-off-by: Stefan Sterz --- src/proxmox-dark/scss/abstracts/_variables.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/proxmox-dark/scss/abstracts/_variables.scss b/src/proxmox-dark/scss/abstracts/_variables.scss index 29c9020..cac51eb 100644 --- a/src/proxmox-dark/scss/abstracts/_variables.scss +++ b/src/proxmox-dark/scss/abstracts/_variables.scss @@ -24,8 +24,8 @@ $content-background-selected: hsl(0deg, 0%, 30%); $background-dark: hsl(0deg, 0%, 20%); $background-darker: hsl(0deg, 0%, 15%); $background-darkest: hsl(0deg, 0%, 10%); -$background-invalid: hsl(360deg, 60%, 30%); -$background-warning: hsl(40deg, 100%, 30%); +$background-invalid: hsl(360deg, 60%, 20%); +$background-warning: hsl(40deg, 100%, 20%); // Buttons $neutral-button-color: hsl(0deg, 0%, 25%); -- 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 widget-toolkit 1/4] dark-theme: improve help button contrast ratios in focused state
also improves the hovered/focused state and makes it more consistent
with other buttons by making it brighter than the default state.
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/abstracts/_variables.scss | 2 +-
src/proxmox-dark/scss/proxmox/_helpbutton.scss | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/proxmox-dark/scss/abstracts/_variables.scss
b/src/proxmox-dark/scss/abstracts/_variables.scss
index cf61ad2..29c9020 100644
--- a/src/proxmox-dark/scss/abstracts/_variables.scss
+++ b/src/proxmox-dark/scss/abstracts/_variables.scss
@@ -35,7 +35,7 @@ $neutral-button-icon-color: $neutral-button-text-color;
// Help Buttons
$help-button-color: hsl(0deg, 0%, 65%);
-$help-button-color-alt: hsl(0deg, 0%, 55%);
+$help-button-color-alt: hsl(0deg, 0%, 75%);
$help-button-text-color: hsl(0deg, 0%, 10%);
$help-button-icon-color: $help-button-text-color;
diff --git a/src/proxmox-dark/scss/proxmox/_helpbutton.scss
b/src/proxmox-dark/scss/proxmox/_helpbutton.scss
index 817d6a1..5032cc1 100644
--- a/src/proxmox-dark/scss/proxmox/_helpbutton.scss
+++ b/src/proxmox-dark/scss/proxmox/_helpbutton.scss
@@ -5,7 +5,8 @@
color: $help-button-text-color;
&.x-btn-over,
- &.x-btn.x-btn-pressed.x-btn-default-toolbar-small {
+ &.x-btn.x-btn-pressed.x-btn-default-toolbar-small,
+ .x-keyboard-mode &.x-btn-default-toolbar-small.x-btn-focus {
background-color: $help-button-color-alt;
border-color: $help-button-color-alt;
}
--
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 widget-toolkit 2/4] dark-theme: make "sorted-by" header highlight more subtle
by making the highlight more subtle, the theme is more consistent with
the look of crisp.
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/extjs/_grid.scss | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/proxmox-dark/scss/extjs/_grid.scss
b/src/proxmox-dark/scss/extjs/_grid.scss
index 3719fdc..beef492 100644
--- a/src/proxmox-dark/scss/extjs/_grid.scss
+++ b/src/proxmox-dark/scss/extjs/_grid.scss
@@ -88,7 +88,7 @@
// header element that the grid is currently sorted by
.x-column-header-sort-ASC,
.x-column-header-sort-DESC {
- background-color: $primary-dark;
+ background-color: mix($background-darker, $primary-color, 70%);
}
// summary rows (e.g. ceph pools last row)
--
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 widget-toolkit 0/4] Dark Theme Fix-ups Round 3
this series adds a few more fix-ups for the dark theme: * fix the contrast ratio of the focus state of help buttons * make help buttons hovered/focused state brigther to be more consistent with the rest of the theme * dim warning and invalid colors further to be more consistent with crisp * dim the color of "sorted-by" headers * make the background mask more see-through while also letting windows stand out more Stefan Sterz (4): dark-theme: improve help button contrast ratios in focused state dark-theme: make "sorted-by" header highlight more subtle dark-theme: dim warning and invalid colors more dark-theme: let the background "shine through" mask more src/proxmox-dark/scss/abstracts/_variables.scss | 6 +++--- src/proxmox-dark/scss/extjs/_grid.scss | 2 +- src/proxmox-dark/scss/extjs/_presentation.scss | 6 +++--- src/proxmox-dark/scss/proxmox/_helpbutton.scss | 3 ++- 4 files changed, 9 insertions(+), 8 deletions(-) -- 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 widget-toolkit 4/4] dark-theme: let the background "shine through" mask more
this makes the background mask a bit brighter and more see-through to
make it possible to read values from behind the mask, if needed. it
also adds a more visible shadow to windows so that the stand out more
at the same time.
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/extjs/_presentation.scss | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/proxmox-dark/scss/extjs/_presentation.scss
b/src/proxmox-dark/scss/extjs/_presentation.scss
index 2503368..5fd35a9 100644
--- a/src/proxmox-dark/scss/extjs/_presentation.scss
+++ b/src/proxmox-dark/scss/extjs/_presentation.scss
@@ -1,8 +1,8 @@
// The mask that is applied when the window is unaccessible (Login
// screen, Loading, ...)
.x-mask {
- background-color: black;
- opacity: 0.85;
+ background-color: $background-darker;
+ opacity: 0.75;
}
// Shadows of floating windows like window modals, form selectors and
@@ -10,5 +10,5 @@
.x-css-shadow {
// the additional styling from the pve css overwrites the setting on
// the element with "!important", that's why we need it here.
- box-shadow: $background-darkest 0 0 5px !important;
+ box-shadow: $background-dark 0 -1px 15px 7px !important;
}
--
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 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::disabl
[pve-devel] [PATCH docs] html 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-html.conf | 4
1 file changed, 4 insertions(+)
diff --git a/asciidoc/pve-html.conf b/asciidoc/pve-html.conf
index 8a089d3..99b767d 100644
--- a/asciidoc/pve-html.conf
+++ b/asciidoc/pve-html.conf
@@ -629,6 +629,10 @@ div .toclevel1 {
endif::toc2[]
+
+include1::{stylesdir=/usr/share/javascript/proxmox-widget-toolkit-dev}/theme-proxmox-dark-adocs.css[]
+
+
ifndef::disable-javascript[]
ifdef::linkcss[]
--
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 widget-toolkit 1/1] dark-theme: add a dark theme for the asciidoc-based documentation
this commit adds a css file to the proxmox-widget-toolkit-dev package
that makes it possible for pve and pmg docs to use a dark theme
Signed-off-by: Stefan Sterz
---
debian/proxmox-widget-toolkit-dev.install| 1 +
src/proxmox-dark/Makefile| 12 +-
src/proxmox-dark/scss/ProxmoxDarkADocs.scss | 9 ++
src/proxmox-dark/scss/adocs/_admonition.scss | 18 +++
src/proxmox-dark/scss/adocs/_general.scss| 120 +++
5 files changed, 159 insertions(+), 1 deletion(-)
create mode 100644 src/proxmox-dark/scss/ProxmoxDarkADocs.scss
create mode 100644 src/proxmox-dark/scss/adocs/_admonition.scss
create mode 100644 src/proxmox-dark/scss/adocs/_general.scss
diff --git a/debian/proxmox-widget-toolkit-dev.install
b/debian/proxmox-widget-toolkit-dev.install
index 620d69f..b0cf14b 100644
--- a/debian/proxmox-widget-toolkit-dev.install
+++ b/debian/proxmox-widget-toolkit-dev.install
@@ -1,2 +1,3 @@
Toolkit.js /usr/share/javascript/proxmox-widget-toolkit-dev/
api-viewer/APIViewer.js /usr/share/javascript/proxmox-widget-toolkit-dev/
+proxmox-dark/theme-proxmox-dark-adocs.css
/usr/share/javascript/proxmox-widget-toolkit-dev/
diff --git a/src/proxmox-dark/Makefile b/src/proxmox-dark/Makefile
index 2ac6f22..1777e36 100644
--- a/src/proxmox-dark/Makefile
+++ b/src/proxmox-dark/Makefile
@@ -30,8 +30,14 @@ SCSSSRC=scss/ProxmoxDark.scss\
scss/proxmox/_tags.scss \
scss/proxmox/_datepicker.scss
+DOCSSCSSRC=scss/ProxmoxDarkADocs.scss \
+ scss/abstracts/_mixins.scss \
+ scss/abstracts/_variables.scss \
+ scss/adocs/_general.scss \
+ scss/adocs/_admonition.scss
+
.PHONY: all
-all: theme-proxmox-dark.css
+all: theme-proxmox-dark.css theme-proxmox-dark-adocs.css
.PHONY: install
install: theme-proxmox-dark.css
@@ -42,6 +48,10 @@ theme-proxmox-dark.css: ${SCSSSRC}
sassc -t compressed $< $@.tmp
mv $@.tmp $@
+theme-proxmox-dark-adocs.css: ${DOCSSCSSRC}
+ sassc -t expanded $< $@.tmp
+ mv $@.tmp $@
+
.PHONY: clean
clean:
rm -rf theme-proxmox-dark.css
diff --git a/src/proxmox-dark/scss/ProxmoxDarkADocs.scss
b/src/proxmox-dark/scss/ProxmoxDarkADocs.scss
new file mode 100644
index 000..1efc3fa
--- /dev/null
+++ b/src/proxmox-dark/scss/ProxmoxDarkADocs.scss
@@ -0,0 +1,9 @@
+@charset "utf-8";
+
+// Abstracts
+@import "abstracts/mixins";
+@import "abstracts/variables";
+
+// ascii doc styles
+@import "adocs/general";
+@import "adocs/admonition"
diff --git a/src/proxmox-dark/scss/adocs/_admonition.scss
b/src/proxmox-dark/scss/adocs/_admonition.scss
new file mode 100644
index 000..46a47a5
--- /dev/null
+++ b/src/proxmox-dark/scss/adocs/_admonition.scss
@@ -0,0 +1,18 @@
+/* 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;
+
+ td.icon {
+padding-right: 0.5em;
+ }
+
+ td.icon > img {
+padding: 0.15em;
+box-sizing: border-box;
+ }
+}
diff --git a/src/proxmox-dark/scss/adocs/_general.scss
b/src/proxmox-dark/scss/adocs/_general.scss
new file mode 100644
index 000..d411ed1
--- /dev/null
+++ b/src/proxmox-dark/scss/adocs/_general.scss
@@ -0,0 +1,120 @@
+@media (prefers-color-scheme: dark) {
+ :root {
+color-scheme: dark;
+ }
+
+ body {
+background-color: $background-darker;
+color: $text-color;
+ }
+
+ a {
+color: $highlighted-text;
+
+&:visited {
+ color: $highlighted-text-alt;
+}
+ }
+
+ // style headlines, titles etc.
+ h1,
+ h2,
+ h3,
+ h4,
+ h5,
+ h6,
+ thead,
+ #author,
+ #toctitle,
+ div.title,
+ td.hdlist1,
+ caption.title,
+ p.tableblock.header {
+color: $highlighted-text-alt;
+ }
+
+ h1,
+ h2,
+ h3,
+ #footer {
+border-color: $border-color;
+ }
+
+ // formatted colored text
+ dt,
+ em,
+ pre,
+ code,
+ strong,
+ .monospaced {
+color: $highlighted-text;
+ }
+
+ // style the table of contents sidebar
+ div #toc {
+color: $text-color;
+background-color: $background-darkest;
+border-color: $border-color-alt;
+ }
+
+ div #toc a:link,
+ div #toc a:visited {
+color: $text-color;
+ }
+
+ // reduce the brigthness of images a bit and make it reversable
+ // through hovering over them.
+ .image > img {
+filter: brightness(90%);
+
+&:hover {
+ filter: none;
+}
+ }
+
+ // tables
+ table.tableblock {
+border-color: $primary-light;
+ }
+
+ div.quoteblock,
+ div.verseblock {
+color: $text-color;
+border-color: $border-color;
+ }
+
+ // listings (e.g. code snippet blocks)
+ div.listingblock > div.content {
+background-color: $background-darkest;
+border-color: $border-color-alt;
+
[pve-devel] [PATCH widget-toolkit 0/1] Proxmox Dark Theme - AsciiDoc
this series add a dark theme for the asciidoc-based documentation for pve and pmg. first it adds a new css dependency to the proxmox-widget-toolkit-dev package. then the following two commits, one for pve and for pmg each, uses that css file to add a dark theme to the respective documentation. it behaves like the api viewer or the gui's 'auto' theme (that is to say, the dark theme is used based on a user's preference). Stefan Sterz (1): dark-theme: add a dark theme for the asciidoc-based documentation debian/proxmox-widget-toolkit-dev.install| 1 + src/proxmox-dark/Makefile| 12 +- src/proxmox-dark/scss/ProxmoxDarkADocs.scss | 9 ++ src/proxmox-dark/scss/adocs/_admonition.scss | 18 +++ src/proxmox-dark/scss/adocs/_general.scss| 120 +++ 5 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 src/proxmox-dark/scss/ProxmoxDarkADocs.scss create mode 100644 src/proxmox-dark/scss/adocs/_admonition.scss create mode 100644 src/proxmox-dark/scss/adocs/_general.scss -- 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 widget-toolkit 0/2] Proxmox Dark Theme Fix-ups Round 2
this theme switches the default theme to the 'auto' theme for all three producs (pve, pmg, pbs) in addition to two minor fix-ups * for the widget toolkit a bit of brightness is added to the check-box icons * in the pmg-gui the main logo in the mobile quarantine is styled properly note that the patches for pbs and pmg are meant as fix-ups for the yet to be applied patches from the initial dark theme series. if this is too inconvenient ill re-submit these patches once they are applied. if they are applied, squashing these into the previous commits might make sense, though. Stefan Sterz (2): utils: move to using the auto theme per default dark-theme: increase contrast on check-boxes src/Utils.js| 4 ++-- src/proxmox-dark/scss/other/_icons.scss | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) -- 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] pveproxy/template: switch to using the "auto" theme per default
the auto theme uses media queries to detect a users preferred theme,
switch to using it per default instead of the light theme.
Signed-off-by: Stefan Sterz
---
PVE/Service/pveproxy.pm | 5 -
www/index.html.tpl | 8
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/PVE/Service/pveproxy.pm b/PVE/Service/pveproxy.pm
index 3fd76128..7b305f76 100755
--- a/PVE/Service/pveproxy.pm
+++ b/PVE/Service/pveproxy.pm
@@ -215,6 +215,10 @@ sub get_index {
}
}
+if ($theme eq "") {
+ $theme = "auto"
+}
+
if (!$lang) {
my $dc_conf = PVE::Cluster::cfs_read_file('datacenter.cfg');
$lang = $dc_conf->{language} // 'en';
@@ -264,7 +268,6 @@ sub get_index {
version => "$version",
wtversion => $wtversion,
theme => $theme,
- auto => $theme == "auto",
};
# by default, load the normal index
diff --git a/www/index.html.tpl b/www/index.html.tpl
index be31dd40..b07ce5f1 100644
--- a/www/index.html.tpl
+++ b/www/index.html.tpl
@@ -12,11 +12,11 @@
-[%- IF theme %]
- [%- IF theme == 'auto' %]
-
- [%- ELSE %]
+[%- IF theme != 'crisp' %]
+ [%- IF theme != 'auto' %]
+ [%- ELSE %]
+
[%- END -%]
[%- END -%]
--
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 widget-toolkit 1/2] utils: move to using the auto theme per default
make the new default theme the "auto" theme that uses media queries to
detect a users preferred theme.
Signed-off-by: Stefan Sterz
---
src/Utils.js | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/Utils.js b/src/Utils.js
index 2ab1d0a..c9c00a9 100644
--- a/src/Utils.js
+++ b/src/Utils.js
@@ -110,13 +110,13 @@ utilities: {
},
theme_map: {
- auto: 'auto',
+ crisp: 'Light theme',
"proxmox-dark": 'Proxmox Dark',
},
render_theme: function(value) {
if (!value || value === '__default__') {
- return Proxmox.Utils.defaultText + ' (Light theme)';
+ return Proxmox.Utils.defaultText + ' (auto)';
}
let text = Proxmox.Utils.theme_map[value];
if (text) {
--
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 widget-toolkit 2/2] dark-theme: increase contrast on check-boxes
by adding a bit of brightness to the icons they stand out a bit more,
especially when selected but not active (grey check-mark)
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/other/_icons.scss | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/proxmox-dark/scss/other/_icons.scss
b/src/proxmox-dark/scss/other/_icons.scss
index f132dae..b4443ee 100644
--- a/src/proxmox-dark/scss/other/_icons.scss
+++ b/src/proxmox-dark/scss/other/_icons.scss
@@ -108,7 +108,7 @@
.x-form-radio-default,
.x-column-header-checkbox .x-column-header-checkbox::after,
.x-grid-checkcolumn::after {
- filter: invert($icon-brightness) hue-rotate(180deg);
+ filter: invert($icon-brightness) hue-rotate(180deg) brightness(125%);
}
.x-tree-icon-custom,
--
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 widget-toolkit 1/6] dark-theme: fix summary row background
previously an "!important" was missing from the `background-color`
property. this meant that the background color wasn't properly
overridden. the "!important" is necessary as it is also used in the
light theme.
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/extjs/_grid.scss | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/proxmox-dark/scss/extjs/_grid.scss
b/src/proxmox-dark/scss/extjs/_grid.scss
index 77872b0..3719fdc 100644
--- a/src/proxmox-dark/scss/extjs/_grid.scss
+++ b/src/proxmox-dark/scss/extjs/_grid.scss
@@ -96,8 +96,10 @@
.x-grid-cell,
.x-grid-rowwrap,
.x-grid-cell-rowbody {
+// the "!important" is needed here, because crisp also sets this
+// as important
+background-color: $background-darker !important;
border-color: $border-color-alt;
-background-color: $background-darker;
}
}
--
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 widget-toolkit 0/6] Proxmox Dark Theme Fix-ups Round 1
this series adds a couple of fix-ups to the recently introduced dark-theme. it first fixes summary rows again (they wore broken in a refactoring step), then darkens the background mask further, dims buttons more, removes thicker borders that are inconsistent with crisp and removes a border around the pve resource tree. the last commit concerns the pve-manager. it removes a class from the "tree settings" button that made it appear like a help button. this doesn't change its appearance in crisp, but makes it stand out more in the dark theme. Stefan Sterz (5): dark-theme: fix summary row background dark-theme: make windows stand out more against the background mask dark-theme: re-work buttons colors to appear dimmer dark-theme: remove thicker borders around content dark-theme: visually remove the border around the pve resource tree src/proxmox-dark/scss/abstracts/_mixins.scss| 7 --- src/proxmox-dark/scss/abstracts/_variables.scss | 10 +- src/proxmox-dark/scss/extjs/_grid.scss | 4 +++- src/proxmox-dark/scss/extjs/_panel.scss | 6 ++ src/proxmox-dark/scss/extjs/_presentation.scss | 2 +- src/proxmox-dark/scss/extjs/_toolbar.scss | 2 +- src/proxmox-dark/scss/extjs/form/_button.scss | 1 + src/proxmox-dark/scss/proxmox/_general.scss | 13 - src/proxmox-dark/scss/proxmox/_helpbutton.scss | 1 + 9 files changed, 18 insertions(+), 28 deletions(-) -- 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 widget-toolkit 5/6] dark-theme: visually remove the border around the pve resource tree
by setting the color of the border of the resource tree to the panel
background color, it doesn't appear visually anymore while keeping
alignments in place.
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/extjs/_panel.scss | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/proxmox-dark/scss/extjs/_panel.scss
b/src/proxmox-dark/scss/extjs/_panel.scss
index 5344c8f..217d3f8 100644
--- a/src/proxmox-dark/scss/extjs/_panel.scss
+++ b/src/proxmox-dark/scss/extjs/_panel.scss
@@ -23,3 +23,9 @@
border-color: $border-color-alt;
color: $text-color;
}
+
+// override the border around the pve-resource-tree specifically to be
+// more consistent with crisp, while keep allignments "correct"
+div[id^="pveResourceTree-"][id$="-body"] {
+ border-color: $background-darker;
+}
--
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 widget-toolkit 4/6] dark-theme: remove thicker borders around content
previously the dark theme used thicker borders in certain places to
space out the content a bit more. this removes them again to make the
appearance more consistent with "crisp".
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/abstracts/_mixins.scss | 7 ---
src/proxmox-dark/scss/extjs/_toolbar.scss| 2 +-
src/proxmox-dark/scss/proxmox/_general.scss | 13 -
3 files changed, 1 insertion(+), 21 deletions(-)
diff --git a/src/proxmox-dark/scss/abstracts/_mixins.scss
b/src/proxmox-dark/scss/abstracts/_mixins.scss
index 570a783..4f226f2 100644
--- a/src/proxmox-dark/scss/abstracts/_mixins.scss
+++ b/src/proxmox-dark/scss/abstracts/_mixins.scss
@@ -1,10 +1,3 @@
-// A border to the left and on top of the content panels for the
-// selected resource
-@mixin content-border {
- border-top: solid 3px $background-darkest;
- border-left: solid 3px $background-darkest;
-}
-
// selected items in dropdown etc
@mixin selection {
background-color: $selection-background-color;
diff --git a/src/proxmox-dark/scss/extjs/_toolbar.scss
b/src/proxmox-dark/scss/extjs/_toolbar.scss
index 2ea8527..aa6ffc5 100644
--- a/src/proxmox-dark/scss/extjs/_toolbar.scss
+++ b/src/proxmox-dark/scss/extjs/_toolbar.scss
@@ -4,7 +4,7 @@
.x-toolbar-default {
background-color: $background-darker;
- border: solid 3px $background-darkest;
+ border-color: $border-color-alt;
&.x-docked-top {
border-width: 1px;
diff --git a/src/proxmox-dark/scss/proxmox/_general.scss
b/src/proxmox-dark/scss/proxmox/_general.scss
index 805a187..c319f6d 100644
--- a/src/proxmox-dark/scss/proxmox/_general.scss
+++ b/src/proxmox-dark/scss/proxmox/_general.scss
@@ -9,19 +9,6 @@ div[id^="versioninfo-"] + div[id^="panel-"] >
div[id^="panel-"][id$="-bodyWrap"]
border-color: transparent;
}
-// border around the main datacenter view
-div[id^="PVE-dc-Config-"][id$="-body"],
-// border around the main pool view
-div[id^="pvePoolConfig-"][id$="-body"],
-// Container content config views
-div[id^="pveLXCConfig-"][id$="-body"],
-// VM content config views
-div[id^="PVE-qemu-Config-"][id$="-body"],
-div[id^="PVE-storage-Browser-"][id$="-body"],
-div[id^="PVE-node-Config-"][id$="-body"] {
- @include content-border;
-}
-
// Section header in the "My Settings" page
.x-fieldset-header-default > .x-fieldset-header-text {
color: $text-color;
--
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 widget-toolkit 3/6] dark-theme: re-work buttons colors to appear dimmer
this dims buttons further by removing pure white text color and
adjusting backgrounds and border accordingly. it also keeps the help
buttons brighter than other buttons to draw (possibly confused) users
to them.
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/abstracts/_variables.scss | 10 +-
src/proxmox-dark/scss/extjs/form/_button.scss | 1 +
src/proxmox-dark/scss/proxmox/_helpbutton.scss | 1 +
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/proxmox-dark/scss/abstracts/_variables.scss
b/src/proxmox-dark/scss/abstracts/_variables.scss
index 96581df..cf61ad2 100644
--- a/src/proxmox-dark/scss/abstracts/_variables.scss
+++ b/src/proxmox-dark/scss/abstracts/_variables.scss
@@ -28,14 +28,14 @@ $background-invalid: hsl(360deg, 60%, 30%);
$background-warning: hsl(40deg, 100%, 30%);
// Buttons
-$neutral-button-color: hsl(0deg, 0%, 35%);
-$neutral-button-color-alt: hsl(0deg, 0%, 55%);
-$neutral-button-text-color: hsl(0deg, 0%, 100%);
+$neutral-button-color: hsl(0deg, 0%, 25%);
+$neutral-button-color-alt: hsl(0deg, 0%, 35%);
+$neutral-button-text-color: hsl(0deg, 0%, 95%);
$neutral-button-icon-color: $neutral-button-text-color;
// Help Buttons
-$help-button-color: hsl(0deg, 0%, 70%);
-$help-button-color-alt: hsl(0deg, 0%, 60%);
+$help-button-color: hsl(0deg, 0%, 65%);
+$help-button-color-alt: hsl(0deg, 0%, 55%);
$help-button-text-color: hsl(0deg, 0%, 10%);
$help-button-icon-color: $help-button-text-color;
diff --git a/src/proxmox-dark/scss/extjs/form/_button.scss
b/src/proxmox-dark/scss/extjs/form/_button.scss
index 0aa1475..bf02135 100644
--- a/src/proxmox-dark/scss/extjs/form/_button.scss
+++ b/src/proxmox-dark/scss/extjs/form/_button.scss
@@ -35,6 +35,7 @@
&.x-btn-over,
.x-keyboard-mode &.x-btn-focus {
background-color: $neutral-button-color-alt;
+border-color: $neutral-button-color-alt;
// the little arrow in certain toolbar buttons with dropdowns
.x-btn-wrap-default-toolbar-small.x-btn-arrow-right::after,
diff --git a/src/proxmox-dark/scss/proxmox/_helpbutton.scss
b/src/proxmox-dark/scss/proxmox/_helpbutton.scss
index aad92e2..817d6a1 100644
--- a/src/proxmox-dark/scss/proxmox/_helpbutton.scss
+++ b/src/proxmox-dark/scss/proxmox/_helpbutton.scss
@@ -1,6 +1,7 @@
// help buttons
.proxmox-inline-button {
background-color: $help-button-color;
+ border-color: $help-button-color-alt;
color: $help-button-text-color;
&.x-btn-over,
--
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 widget-toolkit 2/6] dark-theme: make windows stand out more against the background mask
makes the background mask darker so windows stand out a bit more
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/extjs/_presentation.scss | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/proxmox-dark/scss/extjs/_presentation.scss
b/src/proxmox-dark/scss/extjs/_presentation.scss
index c7d3c8f..2503368 100644
--- a/src/proxmox-dark/scss/extjs/_presentation.scss
+++ b/src/proxmox-dark/scss/extjs/_presentation.scss
@@ -1,7 +1,7 @@
// The mask that is applied when the window is unaccessible (Login
// screen, Loading, ...)
.x-mask {
- background-color: $background-darker;
+ background-color: black;
opacity: 0.85;
}
--
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 6/6] ui: make tree settings button like regular buttons
the "proxmox-inline-button" class is redundant in the crisp theme as
it only sets the buttons text to black. we mainly use that class for
"help" buttons. this is useful in the dark theme, because we want help
buttons to stand out a bit so (possibly confused) users are drawn to
them more easily. removing the class here doesn't change anything for
"crisp", but makes the dark theme appear more consistent. also fixes
up an unnecessary space.
Signed-off-by: Stefan Sterz
---
www/manager6/Workspace.js | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/www/manager6/Workspace.js b/www/manager6/Workspace.js
index 78ab37b6..8cee3638 100644
--- a/www/manager6/Workspace.js
+++ b/www/manager6/Workspace.js
@@ -469,8 +469,8 @@ Ext.define('PVE.StdWorkspace', {
selview,
{
xtype: 'button',
- cls: 'x-btn-default-toolbar-small
proxmox-inline-button',
- iconCls: 'fa fa-fw fa-gear
x-btn-icon-el-default-toolbar-small ',
+ cls: 'x-btn-default-toolbar-small',
+ iconCls: 'fa fa-fw fa-gear
x-btn-icon-el-default-toolbar-small',
handler: () => {
Ext.create('PVE.window.TreeSettingsEdit', {
autoShow: true,
--
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] applied-series: [PATCH manager v1 1/4] gui: create user info menu intro for selecting the theme
On 3/9/23 09:16, Thomas Lamprecht wrote: > Am 09/03/2023 um 09:07 schrieb Stefan Sterz: >> On 3/8/23 18:05, Thomas Lamprecht wrote: >>> we might want to make auto default rather quickly ;-) >> >> yes that might make sense. my intention was to not "surprise" existing >> users with a potentially unwanted overhaul of the gui. however, im not >> sure how relevant that concern is, as it is fairly easy to switch back > > > IMO not a concern as it only changes for those whose browser already tells our > web UI to prefer a dark-color scheme, so while it might come as a "surprise", > I think it's safe to say that it'll be a welcomed one - as it'd be odd if they > configured their OS and/or Browser to prefer the dark mode, if they don't. > sure, ill send a follow-up then :) > > ps. something funky happened with your subject (pasted by mistake?) > yeah ok i probably messed something up over here o.O not sure why this happened i didn't mess with the subject line at all. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] Shell command and Emacs Lisp code injection in emacsclient-mail.desktop
On 3/8/23 18:05, Thomas Lamprecht wrote: > Am 08/03/2023 um 17:40 schrieb Stefan Sterz: >> From: Daniel Tschlatscher >> >> this requires a bump of the widget toolkit so the version includes the >> necessary widgets. >> >> Signed-off-by: Daniel Tschlatscher >> Signed-off-by: Stefan Sterz >> --- >> www/manager6/Workspace.js | 8 >> 1 file changed, 8 insertions(+) >> >> > > applied series, huge thanks to you and Daniel again! > > we might want to make auto default rather quickly ;-) yes that might make sense. my intention was to not "surprise" existing users with a potentially unwanted overhaul of the gui. however, im not sure how relevant that concern is, as it is fairly easy to switch back ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] [PATCH manager v1 2/4] pveproxy/template: add support for switching themes
load the dark theme only if requested through a cookie, also adds
support for the "auto" theme that uses the dark theme based on a
media query.
this requires a bump of the widget toolkit so the dark-theme css file
is available.
Signed-off-by: Stefan Sterz
---
PVE/Service/pveproxy.pm | 13 +
www/index.html.tpl | 8
2 files changed, 21 insertions(+)
diff --git a/PVE/Service/pveproxy.pm b/PVE/Service/pveproxy.pm
index f73fdd6f..3fd76128 100755
--- a/PVE/Service/pveproxy.pm
+++ b/PVE/Service/pveproxy.pm
@@ -88,6 +88,7 @@ sub init {
add_dirs($dirs, '/xtermjs/' => "$basedirs->{xtermjs}/");
add_dirs($dirs, '/pwt/images/' => "$basedirs->{widgettoolkit}/images/");
add_dirs($dirs, '/pwt/css/' => "$basedirs->{widgettoolkit}/css/");
+add_dirs($dirs, '/pwt/themes/' => "$basedirs->{widgettoolkit}/themes/");
$self->{server_config} = {
title => 'Proxmox VE API',
@@ -191,6 +192,7 @@ sub get_index {
my $lang;
my $username;
my $token = 'null';
+my $theme = "";
if (my $cookie = $r->header('Cookie')) {
if (my $newlang = ($cookie =~ /(?:^|\s)PVELangCookie=([^;]*)/)[0]) {
@@ -198,6 +200,15 @@ sub get_index {
$lang = $newlang;
}
}
+
+ if (my $newtheme = ($cookie =~ /(?:^|\s)PVEThemeCookie=([^;]*)/)[0]) {
+ # theme names need to be kebab case, with each segment a maximum of
10 characters long
+ # and at most 6 segments
+ if ($newtheme =~ m/^[a-z]{1,10}(-[a-z]{1,10}){0,5}$/) {
+ $theme = $newtheme;
+ }
+ }
+
my $ticket = PVE::APIServer::Formatter::extract_auth_value($cookie,
$server->{cookie_name});
if (($username = PVE::AccessControl::verify_ticket($ticket, 1))) {
$token =
PVE::AccessControl::assemble_csrf_prevention_token($username);
@@ -252,6 +263,8 @@ sub get_index {
debug => $debug,
version => "$version",
wtversion => $wtversion,
+ theme => $theme,
+ auto => $theme == "auto",
};
# by default, load the normal index
diff --git a/www/index.html.tpl b/www/index.html.tpl
index 0c819977..be31dd40 100644
--- a/www/index.html.tpl
+++ b/www/index.html.tpl
@@ -12,6 +12,14 @@
+[%- IF theme %]
+ [%- IF theme == 'auto' %]
+
+ [%- ELSE %]
+
+ [%- END -%]
+[%- END -%]
+
[% IF langfile %]
[%- ELSE %]
--
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 v1 3/4] subscription/summary/backup: stop setting the background color
setting the background color in js code adds that property as a style
attribute to the element. that makes it hard to alter later via css
and makes it hard to dynamically change the color e.g. if we want to
add different themes. the background color for these elements are
white already anyway, so just remove them here.
Signed-off-by: Stefan Sterz
---
www/manager6/node/Subscription.js | 1 -
www/manager6/node/Summary.js| 1 -
www/manager6/window/BackupConfig.js | 1 -
3 files changed, 3 deletions(-)
diff --git a/www/manager6/node/Subscription.js
b/www/manager6/node/Subscription.js
index 3b53c7e2..5ca36d42 100644
--- a/www/manager6/node/Subscription.js
+++ b/www/manager6/node/Subscription.js
@@ -40,7 +40,6 @@ Ext.define('PVE.node.Subscription', {
itemId: 'system-report-view',
scrollable: true,
style: {
- 'background-color': 'white',
'white-space': 'pre',
'font-family': 'monospace',
padding: '5px',
diff --git a/www/manager6/node/Summary.js b/www/manager6/node/Summary.js
index 320f2247..03512d70 100644
--- a/www/manager6/node/Summary.js
+++ b/www/manager6/node/Summary.js
@@ -18,7 +18,6 @@ Ext.define('PVE.node.Summary', {
id: 'pkgversions',
padding: 5,
style: {
- 'background-color': 'white',
'white-space': 'pre',
'font-family': 'monospace',
},
diff --git a/www/manager6/window/BackupConfig.js
b/www/manager6/window/BackupConfig.js
index ca61b1e4..48649efc 100644
--- a/www/manager6/window/BackupConfig.js
+++ b/www/manager6/window/BackupConfig.js
@@ -10,7 +10,6 @@ Ext.define('PVE.window.BackupConfig', {
itemId: 'configtext',
autoScroll: true,
style: {
- 'background-color': 'white',
'white-space': 'pre',
'font-family': 'monospace',
padding: '5px',
--
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 docs v1] docs: add dark mode support to the api viewer
adds dark mode support to the api viewer that is activated depending on the users theme preference. similar to the main gui's "auto" theme. this requires a bumps of the widget toolkit so the loaded css file is present Signed-off-by: Stefan Sterz --- api-viewer/index.html | 1 + 1 file changed, 1 insertion(+) diff --git a/api-viewer/index.html b/api-viewer/index.html index 8528420..006e64b 100644 --- a/api-viewer/index.html +++ b/api-viewer/index.html @@ -6,6 +6,7 @@ Proxmox VE API Documentation + -- 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 v1 4/4] ui: make ceph charts change color more dynamically
add support for setting the background and text color via css. also
allows for dynamically switching the color when a theme change is
detected.
Signed-off-by: Stefan Sterz
---
www/manager6/ceph/StatusDetail.js | 35 +-
www/manager6/panel/RunningChart.js | 29 +
2 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/www/manager6/ceph/StatusDetail.js
b/www/manager6/ceph/StatusDetail.js
index 88b1775c..d6c0763b 100644
--- a/www/manager6/ceph/StatusDetail.js
+++ b/www/manager6/ceph/StatusDetail.js
@@ -199,6 +199,18 @@ Ext.define('PVE.ceph.StatusDetail', {
},
],
+checkThemeColors: function() {
+ let me = this;
+ let rootStyle = getComputedStyle(document.documentElement);
+
+ // get color
+ let background =
rootStyle.getPropertyValue("--pwt-panel-background").trim() || "#ff";
+
+ // set the colors
+ me.chart.setBackground(background);
+ me.chart.redraw();
+},
+
updateAll: function(metadata, status) {
let me = this;
me.suspendLayout = true;
@@ -257,7 +269,7 @@ Ext.define('PVE.ceph.StatusDetail', {
me.statecategories[result].states.push(state);
});
- me.getComponent('pgchart').getStore().setData(me.statecategories);
+ me.chart.getStore().setData(me.statecategories);
me.getComponent('pgs').update({ states: pgs_by_state });
let health = status.health || {};
@@ -303,5 +315,26 @@ Ext.define('PVE.ceph.StatusDetail', {
me.suspendLayout = false;
me.updateLayout();
},
+
+ initComponent: function() {
+ var me = this;
+ me.callParent();
+
+ me.chart = me.getComponent('pgchart');
+ me.checkThemeColors();
+
+ // switch colors on media query changes
+ me.mediaQueryList = window.matchMedia("(prefers-color-scheme: dark)");
+ me.themeListener = (e) => { me.checkThemeColors(); };
+ me.mediaQueryList.addEventListener("change", me.themeListener);
+},
+
+doDestroy: function() {
+ let me = this;
+
+ me.mediaQueryList.removeEventListener("change", me.themeListener);
+
+ me.callParent();
+},
});
diff --git a/www/manager6/panel/RunningChart.js
b/www/manager6/panel/RunningChart.js
index 19db8b50..d89a6ecb 100644
--- a/www/manager6/panel/RunningChart.js
+++ b/www/manager6/panel/RunningChart.js
@@ -98,6 +98,20 @@ Ext.define('PVE.widget.RunningChart', {
// show the last x seconds default is 5 minutes
timeFrame: 5*60,
+checkThemeColors: function() {
+ let me = this;
+ let rootStyle = getComputedStyle(document.documentElement);
+
+ // get color
+ let background =
rootStyle.getPropertyValue("--pwt-panel-background").trim() || "#ff";
+ let text = rootStyle.getPropertyValue("--pwt-text-color").trim() ||
"#00";
+
+ // set the colors
+ me.chart.setBackground(background);
+ me.chart.valuesprite.setAttributes({ fillStyle: text }, true);
+ me.chart.redraw();
+},
+
addDataPoint: function(value, time) {
let view = this.chart;
let panel = view.up();
@@ -156,5 +170,20 @@ Ext.define('PVE.widget.RunningChart', {
stroke: me.color,
});
}
+
+ me.checkThemeColors();
+
+ // switch colors on media query changes
+ me.mediaQueryList = window.matchMedia("(prefers-color-scheme: dark)");
+ me.themeListener = (e) => { me.checkThemeColors(); };
+ me.mediaQueryList.addEventListener("change", me.themeListener);
+},
+
+doDestroy: function() {
+ let me = this;
+
+ me.mediaQueryList.removeEventListener("change", me.themeListener);
+
+ me.callParent();
},
});
--
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 v1 1/4] gui: create user info menu intro for selecting the theme
From: Daniel Tschlatscher
this requires a bump of the widget toolkit so the version includes the
necessary widgets.
Signed-off-by: Daniel Tschlatscher
Signed-off-by: Stefan Sterz
---
www/manager6/Workspace.js | 8
1 file changed, 8 insertions(+)
diff --git a/www/manager6/Workspace.js b/www/manager6/Workspace.js
index 0c8869a7..78ab37b6 100644
--- a/www/manager6/Workspace.js
+++ b/www/manager6/Workspace.js
@@ -395,6 +395,14 @@ Ext.define('PVE.StdWorkspace', {
me.selectById('root');
},
},
+ {
+ iconCls: 'fa fa-paint-brush',
+ text: gettext('Theme'),
+ handler: function() {
+
Ext.create('Proxmox.window.ThemeEditWindow')
+ .show();
+ },
+ },
{
iconCls: 'fa fa-language',
text: gettext('Language'),
--
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 widget-toolkit v1 6/6] dark-theme: add support for the pmg quarantine theme toggle
allows using the theme toggle in the pmg quarantine properly. adds a
filter over the iframes in the quarantine to make them appear properly
in a dark environment.
Signed-off-by: Stefan Sterz
---
src/proxmox-dark/scss/proxmox/_quarantine.scss | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/proxmox-dark/scss/proxmox/_quarantine.scss
b/src/proxmox-dark/scss/proxmox/_quarantine.scss
index bdda69a..9e467df 100644
--- a/src/proxmox-dark/scss/proxmox/_quarantine.scss
+++ b/src/proxmox-dark/scss/proxmox/_quarantine.scss
@@ -45,3 +45,9 @@ tr.bounced,
.x-keyboard-mode tr.bounced .x-grid-item-focused {
background-color: $background-warning;
}
+
+.pmg-mail-preview-themed div > iframe {
+ // by reducing the brightness first, pure blacks won't get inverted
+ // to pure white.
+ filter: brightness(95%) invert(95%);
+}
--
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 widget-toolkit v1 5/6] util/window/form: add a theme selector
From: Daniel Tschlatscher
add a widget that implements a theme selector and sets a cookie to
load the appropriate theme.
Co-authored-by: Daniel Tschlatscher
Co-authored-by: Stefan Sterz
Signed-off-by: Daniel Tschlatscher
Signed-off-by: Stefan Sterz
---
src/Makefile | 2 ++
src/Utils.js | 25
src/form/ThemeSelector.js | 6 +
src/window/ThemeEdit.js | 49 +++
4 files changed, 82 insertions(+)
create mode 100644 src/form/ThemeSelector.js
create mode 100644 src/window/ThemeEdit.js
diff --git a/src/Makefile b/src/Makefile
index 54727f6..11790a0 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -44,6 +44,7 @@ JSSRC=\
form/TaskTypeSelector.js\
form/ACME.js\
form/UserSelector.js\
+ form/ThemeSelector.js \
button/Button.js\
button/AltText.js \
button/HelpButton.js\
@@ -90,6 +91,7 @@ JSSRC=\
window/AddYubico.js \
window/TfaEdit.js \
window/NotesEdit.js \
+ window/ThemeEdit.js \
node/APT.js \
node/APTRepositories.js \
node/NetworkEdit.js \
diff --git a/src/Utils.js b/src/Utils.js
index 8a97487..2ab1d0a 100644
--- a/src/Utils.js
+++ b/src/Utils.js
@@ -109,6 +109,31 @@ utilities: {
return data;
},
+theme_map: {
+ auto: 'auto',
+ "proxmox-dark": 'Proxmox Dark',
+},
+
+render_theme: function(value) {
+ if (!value || value === '__default__') {
+ return Proxmox.Utils.defaultText + ' (Light theme)';
+ }
+ let text = Proxmox.Utils.theme_map[value];
+ if (text) {
+ return text;
+ }
+ return value;
+},
+
+theme_array: function() {
+ let data = [['__default__', Proxmox.Utils.render_theme('')]];
+ Ext.Object.each(Proxmox.Utils.theme_map, function(key, value) {
+ data.push([key, Proxmox.Utils.render_theme(value)]);
+ });
+
+ return data;
+},
+
bond_mode_gettext_map: {
'802.3ad': 'LACP (802.3ad)',
'lacp-balance-slb': 'LACP (balance-slb)',
diff --git a/src/form/ThemeSelector.js b/src/form/ThemeSelector.js
new file mode 100644
index 000..fa5ddb7
--- /dev/null
+++ b/src/form/ThemeSelector.js
@@ -0,0 +1,6 @@
+Ext.define('Proxmox.form.ThemeSelector', {
+extend: 'Proxmox.form.KVComboBox',
+xtype: 'proxmoxThemeSelector',
+
+comboItems: Proxmox.Utils.theme_array(),
+});
diff --git a/src/window/ThemeEdit.js b/src/window/ThemeEdit.js
new file mode 100644
index 000..aec7082
--- /dev/null
+++ b/src/window/ThemeEdit.js
@@ -0,0 +1,49 @@
+Ext.define('Proxmox.window.ThemeEditWindow', {
+extend: 'Ext.window.Window',
+alias: 'widget.pmxThemeEditWindow',
+
+viewModel: {
+ parent: null,
+ data: {
+ language: '__default__',
+ },
+},
+controller: {
+ xclass: 'Ext.app.ViewController',
+ init: function(view) {
+ let theme = Ext.util.Cookies.get(view.cookieName) || '__default__';
+ this.getViewModel().set('theme', theme);
+ },
+ applyTheme: function(button) {
+ let view = this.getView();
+ let vm = this.getViewModel();
+
+ let expire = Ext.Date.add(new Date(), Ext.Date.YEAR, 10);
+ Ext.util.Cookies.set(view.cookieName, vm.get('theme'), expire);
+ view.mask(gettext('Please wait...'), 'x-mask-loading');
+ window.location.reload();
+ },
+},
+
+cookieName: 'PVEThemeCookie',
+
+title: gettext('Theme'),
+modal: true,
+bodyPadding: 10,
+resizable: false,
+items: [
+ {
+ xtype: 'proxmoxThemeSelector',
+ fieldLabel: gettext('Theme'),
+ bind: {
+ value: '{theme}',
+ },
+ },
+],
+buttons: [
+ {
+ text: gettext('Apply'),
+ handler: 'applyTheme',
+ },
+],
+});
--
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 widget-toolkit v1 3/6] gauge widget: add support for a dark theme and dynamic theme switching
the gauges in the data center overview should use a dark style if the
relevant css variables are set. this also makes it possible to switch
the colors dynamically by adding an event listener
Signed-off-by: Stefan Sterz
---
src/panel/GaugeWidget.js | 45
1 file changed, 45 insertions(+)
diff --git a/src/panel/GaugeWidget.js b/src/panel/GaugeWidget.js
index 0cc2079..00567a2 100644
--- a/src/panel/GaugeWidget.js
+++ b/src/panel/GaugeWidget.js
@@ -61,6 +61,36 @@ Ext.define('Proxmox.panel.GaugeWidget', {
initialValue: 0,
+checkThemeColors: function() {
+ let me = this;
+ let rootStyle = getComputedStyle(document.documentElement);
+
+ // get colors
+ let panelBg =
rootStyle.getPropertyValue("--pwt-panel-background").trim() || "#ff";
+ let textColor = rootStyle.getPropertyValue("--pwt-text-color").trim()
|| "#00";
+ me.defaultColor =
rootStyle.getPropertyValue("--pwt-gauge-default").trim() || '#c2ddf2';
+ me.criticalColor =
rootStyle.getPropertyValue("--pwt-gauge-crit").trim() || '#ff6c59';
+ me.warningColor = rootStyle.getPropertyValue("--pwt-gauge-warn").trim()
|| '#fc0';
+ me.backgroundColor =
rootStyle.getPropertyValue("--pwt-gauge-back").trim() || '#f5f5f5';
+
+ // set gauge colors
+ let value = me.chart.series[0].getValue() / 100;
+
+ let color = me.defaultColor;
+
+ if (value >= me.criticalThreshold) {
+ color = me.criticalColor;
+ } else if (value >= me.warningThreshold) {
+ color = me.warningColor;
+ }
+
+ me.chart.series[0].setColors([color, me.backgroundColor]);
+
+ // set text and background colors
+ me.chart.setBackground(panelBg);
+ me.valueSprite.setAttributes({ fillStyle: textColor }, true);
+ me.chart.redraw();
+},
updateValue: function(value, text) {
let me = this;
@@ -100,5 +130,20 @@ Ext.define('Proxmox.panel.GaugeWidget', {
me.text = me.getComponent('text');
me.chart = me.getComponent('chart');
me.valueSprite = me.chart.getSurface('chart').get('valueSprite');
+
+ me.checkThemeColors();
+
+ // switch colors on media query changes
+ me.mediaQueryList = window.matchMedia("(prefers-color-scheme: dark)");
+ me.themeListener = (e) => { me.checkThemeColors(); };
+ me.mediaQueryList.addEventListener("change", me.themeListener);
+},
+
+doDestroy: function() {
+ let me = this;
+
+ me.mediaQueryList.removeEventListener("change", me.themeListener);
+
+ me.callParent();
},
});
--
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 widget-toolkit v1 2/6] subscription/summary/backup: stop setting the background color
setting the background color in js code adds that property as a style
attribute to the element. that makes it hard to alter later via css
and makes it hard to dynamically change the color e.g., if we want to
add different themes. the background color for these elements are
white already anyway, so just remove them here.
Signed-off-by: Stefan Sterz
---
src/node/APT.js | 1 -
src/window/AddTotp.js | 1 -
src/window/DiskSmart.js | 1 -
src/window/PackageVersions.js | 1 -
4 files changed, 4 deletions(-)
diff --git a/src/node/APT.js b/src/node/APT.js
index 2e5a776..739aaf3 100644
--- a/src/node/APT.js
+++ b/src/node/APT.js
@@ -116,7 +116,6 @@ Ext.define('Proxmox.node.APT', {
let view = Ext.createWidget('component', {
autoScroll: true,
style: {
- 'background-color': 'white',
'white-space': 'pre',
'font-family': 'monospace',
padding: '5px',
diff --git a/src/window/AddTotp.js b/src/window/AddTotp.js
index bdb4826..080b361 100644
--- a/src/window/AddTotp.js
+++ b/src/window/AddTotp.js
@@ -224,7 +224,6 @@ Ext.define('Proxmox.window.AddTotp', {
visible: '{!secretEmpty}',
},
style: {
- 'background-color': 'white',
'margin-left': 'auto',
'margin-right': 'auto',
padding: '5px',
diff --git a/src/window/DiskSmart.js b/src/window/DiskSmart.js
index b538ea1..1cae512 100644
--- a/src/window/DiskSmart.js
+++ b/src/window/DiskSmart.js
@@ -74,7 +74,6 @@ Ext.define('Proxmox.window.DiskSmart', {
autoScroll: true,
padding: 5,
style: {
- 'background-color': 'white',
'white-space': 'pre',
'font-family': 'monospace',
},
diff --git a/src/window/PackageVersions.js b/src/window/PackageVersions.js
index e79d29f..aaa1372 100644
--- a/src/window/PackageVersions.js
+++ b/src/window/PackageVersions.js
@@ -45,7 +45,6 @@ Ext.define('Proxmox.window.PackageVersions', {
html: '{packageList}',
},
style: {
- 'background-color': 'white',
'white-space': 'pre',
'font-family': 'monospace',
},
--
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 widget-toolkit v1 4/6] rrd chart: add support for the dark theme and dynamic theme switching
by integrating the theme switching logic into the chart panel itself,
themes can be switched more responsively based on css variables.
Signed-off-by: Stefan Sterz
---
src/panel/RRDChart.js | 37 +
1 file changed, 37 insertions(+)
diff --git a/src/panel/RRDChart.js b/src/panel/RRDChart.js
index 263070a..dc5940c 100644
--- a/src/panel/RRDChart.js
+++ b/src/panel/RRDChart.js
@@ -183,6 +183,27 @@ Ext.define('Proxmox.widget.RRDChart', {
me.callParent([config]);
},
+checkThemeColors: function() {
+ let me = this;
+ let rootStyle = getComputedStyle(document.documentElement);
+
+ // get colors
+ let background =
rootStyle.getPropertyValue("--pwt-panel-background").trim() || "#ff";
+ let text = rootStyle.getPropertyValue("--pwt-text-color").trim() ||
"#00";
+ let primary = rootStyle.getPropertyValue("--pwt-chart-primary").trim()
|| "#00";
+ let gridStroke =
rootStyle.getPropertyValue("--pwt-chart-grid-stroke").trim() || "#dd";
+
+ // set the colors
+ me.setBackground(background);
+ me.axes.forEach((axis) => {
+ axis.setLabel({ color: text });
+ axis.setTitle({ color: text });
+ axis.setStyle({ strokeStyle: primary });
+ axis.setGrid({ stroke: gridStroke });
+ });
+ me.redraw();
+},
+
initComponent: function() {
let me = this;
@@ -275,5 +296,21 @@ Ext.define('Proxmox.widget.RRDChart', {
easing: 'easeIn',
});
}, this, { single: true });
+
+
+ me.checkThemeColors();
+
+ // switch colors on media query changes
+ me.mediaQueryList = window.matchMedia("(prefers-color-scheme: dark)");
+ me.themeListener = (e) => { me.checkThemeColors(); };
+ me.mediaQueryList.addEventListener("change", me.themeListener);
+},
+
+doDestroy: function() {
+ let me = this;
+
+ me.mediaQueryList.removeEventListener("change", me.themeListener);
+
+ me.callParent();
},
});
--
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 widget-toolkit v1 1/6] dark-theme: add initial version of the proxmox-dark theme
From: Daniel Tschlatscher
adds an initial version of a dark theme for all proxmox products. the
theme works by simply loading an additional css file that adjust the
colors of the original theme "crisp" to be more suitable for a dark
theme.
the theme itself is written in scss, so we need to add sassc as a
build dependency. while sassc is deprecated, it is still maintained in
the debian repositories and, thus, needs no additional packaging on
our end.
this version adds the following on-top of Daniel Tschlatscher's
original draft:
* removes checked-in build artifacts and other stuff that shouldn't be
tracked
* code clean-up and removal of redundant code
* refactors:
* icon styling
* color handling for charts (moved to css variables)
* color variables, consolidates them and makes the "functional"
* color values, improves contrast and makes the theme appear more
consistent
* using the "theme-" prefix
* adds:
* markdown note styles
* combo-box trigger styles
* even more icon styles (e.g., template icons, check boxes etc.)
* loading spinners styles
* number field up and down arrow styles
* an additional auto theme that switches between light and dark
theme dynamically
* widget toolkit hints
* ceph install mask
* grid group headers
* color to toggled buttons
* date picker styles
* drag and drop proxy styles
* fixes:
* contrast on control elements for "scrollable" sidebars
* make the general appearance closer to the light theme ("crisp")
* buttons (when hovered, toggled etc)
* background masking (e.g., when showing the log-in form)
* grid header separator (adds an outline)
* separator lines in some menus
* makes the custom unknown icon more discernible
* makes headers more readable
* color adjustments to several components for consistency
* reduces brightness of dividers in toolbars
* border color on chart legend elements
* removes a black border from docked toolbars (e.g., tag edit)
* dims the "invalid" color to appear less aggressive
* add hover effects in grids and make them consistent with "crisp"
* summary rows
* selected and hovered elements in boundlists
* row numberers in grids
* contrast of links in hints
* ceph overview border colors (e.g., OSD in/out/up/down grid)
* bottom splitter contrast in certain situations
* tag visibility
* pbs compatibility (help buttons stylings, icons, tabs)
* pmg compatibility:
* remove border around the spanning element in the header
* style spam score grid
* style tracking center rows
* add appropriate colors to buttons in the quarantine
* style mail-info element in the quarantine
Co-authored-by: Daniel Tschlatscher
Co-authored-by: Stefan Sterz
Signed-off-by: Daniel Tschlatscher
Signed-off-by: Stefan Sterz
---
debian/control| 1 +
src/Makefile | 2 +-
src/defines.mk| 1 +
src/proxmox-dark/Makefile | 47
src/proxmox-dark/scss/ProxmoxDark.scss| 37
src/proxmox-dark/scss/abstracts/_mixins.scss | 12 ++
.../scss/abstracts/_variables.scss| 67 ++
src/proxmox-dark/scss/extjs/_body.scss| 23 ++
src/proxmox-dark/scss/extjs/_grid.scss| 146 +
src/proxmox-dark/scss/extjs/_menu.scss| 39
src/proxmox-dark/scss/extjs/_panel.scss | 25 +++
.../scss/extjs/_presentation.scss | 14 ++
src/proxmox-dark/scss/extjs/_progress.scss| 19 ++
src/proxmox-dark/scss/extjs/_splitter.scss| 18 ++
src/proxmox-dark/scss/extjs/_tabbar.scss | 43
src/proxmox-dark/scss/extjs/_tip.scss | 18 ++
src/proxmox-dark/scss/extjs/_toolbar.scss | 21 ++
src/proxmox-dark/scss/extjs/_treepanel.scss | 24 +++
src/proxmox-dark/scss/extjs/_window.scss | 39
src/proxmox-dark/scss/extjs/form/_button.scss | 57 +
.../scss/extjs/form/_combobox.scss| 23 ++
.../scss/extjs/form/_formfield.scss | 40
src/proxmox-dark/scss/other/_charts.scss | 39
src/proxmox-dark/scss/other/_icons.scss | 200 ++
.../scss/proxmox/_datepicker.scss | 61 ++
src/proxmox-dark/scss/proxmox/_general.scss | 56 +
.../scss/proxmox/_helpbutton.scss | 16 ++
.../scss/proxmox/_loadingindicator.scss | 14 ++
src/proxmox-dark/scss/proxmox/_markdown.scss | 31 +++
src/proxmox-dark/scss/proxmox/_nodes.scss | 9 +
.../scss/proxmox/_quarantine.scss | 47
src/proxmox-dark/scss/proxmox/_storages.scss | 19 ++
src/proxmox-dark/scss/proxmox/_tags.scss | 14 ++
33 files changed, 1221 insertions(+), 1 deletion(-)
create mode 100644 src/proxmox-dark/
[pve-devel] [PATCH widget-toolkit v1] Proxmox Dark Theme
this patch series aims to add support for a dark theme to all current proxmox products. including: * proxmox virtual environment web gui & api viewer * proxmox backup server gui & api viewer * proxmox mail gateway gui, api viewer & (mobile) quarantine this patch series is split into several parts. the first concerns the widget toolkit and adds the necessary files and adaptions needed accross all three products. the other three parts integrate the new dark theme into pve, pbs and pmg and also adds changes needed by each product. part 1: widget toolkit the first commit adds styles that give the main gui a "dark" appearance. this includes adjusting the icons and other images as well as all backgrounds text and link colors. the second commit removes certain hard-coded background colors that are set via js. these don't behave properly in the dark theme and the gui behaves properly without them, so they are removed. the third and fourth commit make it possible for charts and gauges to switch their colors based on the current theme. the fifth commit adds a theme switcher widget that allows switching the current theme and then sets a cookie accordingly. the last commit is only relevant for the proxmox mail gateway and enables the proposed "theme switcher" function. this allows users to enable or disable a filter over the email preview in the quarantine. if the filter is enabled the email will have it's brightness reduced and the colors will be inverted. thus, plain text emails shouldn't appear too bright part 2: proxmox virtual environment the first commit for pve adds the theme switcher to the gui, the second adjusts the pveproxy template to properly handle the cookie set by the theme switcher. the third removes unnecessary hard-coded background colors and the forth add support for the charts used in pve only. the fifth commit concern pve-docs and adds support for the dark theme in the api viewer. part 3: proxmox backup server the commits for proxmox backup server first add the theme switcher, then removes hard-coded backgrounds and finally add dark theme support to the api viewer. part 4: proxmox mail gateway the first commit concerns the api and let's it handle cookie properly. the next five commits adjust gui to first add a theme switcher, then style the spam info grid, remove hard-coded white backgrounds, style the hourly mail chart, and enable the dark mode in the mobile quarantine. the second-to-last commit enables the theme switching mechanism for the quarantine mail preview. this part could be dropped in favor of extending the preview endpoint to add some styles to the preview that handle a dark theme. this would have the added benefit of also working in the mobile quarantine. the last commit for the mail gateway enables the dark theme in the api viewer. Daniel Tschlatscher (2): dark-theme: add initial version of the proxmox-dark theme util/window/form: add a theme selector Stefan Sterz (4): subscription/summary/backup: stop setting the background color gauge widget: add support for a dark theme and dynamic theme switching rrd chart: add support for the dark theme and dynamic theme switching dark-theme: add support for the pmg quarantine theme toggle debian/control| 1 + src/Makefile | 4 +- src/Utils.js | 25 +++ src/defines.mk| 1 + src/form/ThemeSelector.js | 6 + src/node/APT.js | 1 - src/panel/GaugeWidget.js | 45 src/panel/RRDChart.js | 37 src/proxmox-dark/Makefile | 47 src/proxmox-dark/scss/ProxmoxDark.scss| 37 src/proxmox-dark/scss/abstracts/_mixins.scss | 12 ++ .../scss/abstracts/_variables.scss| 67 ++ src/proxmox-dark/scss/extjs/_body.scss| 23 ++ src/proxmox-dark/scss/extjs/_grid.scss| 146 + src/proxmox-dark/scss/extjs/_menu.scss| 39 src/proxmox-dark/scss/extjs/_panel.scss | 25 +++ .../scss/extjs/_presentation.scss | 14 ++ src/proxmox-dark/scss/extjs/_progress.scss| 19 ++ src/proxmox-dark/scss/extjs/_splitter.scss| 18 ++ src/proxmox-dark/scss/extjs/_tabbar.scss | 43 src/proxmox-dark/scss/extjs/_tip.scss | 18 ++ src/proxmox-dark/scss/extjs/_toolbar.scss | 21 ++ src/proxmox-dark/scss/extjs/_treepanel.scss | 24 +++ src/proxmox-dark/scss/extjs/_window.scss | 39 src/proxmox-dark/scss/extjs/form/_button.scss | 57 + .../scss/extjs/form/_combobox.scss| 23 ++ .../scss/extjs/form/_formfield.scss | 40 src/proxmox-dark/scss/other/_charts.scss | 39 src/proxmox-dark/scss/other/_icons.scss | 200 ++ .../scss/proxmox/_datepicker.
[pve-devel] [PATCH widget-toolkit v1] Proxmox Dark Theme
this patch series aims to add support for a dark theme to all current proxmox products. including: * proxmox virtual environment web gui & api viewer * proxmox backup server gui & api viewer * proxmox mail gateway gui, api viewer & (mobile) quarantine this patch series is split into several parts. the first concerns the widget toolkit and adds the necessary files and adaptions needed accross all three products. the other three parts integrate the new dark theme into pve, pbs and pmg and also adds changes needed by each product. part 1: widget toolkit the first commit adds styles that give the main gui a "dark" appearance. this includes adjusting the icons and other images as well as all backgrounds text and link colors. the second commit removes certain hard-coded background colors that are set via js. these don't behave properly in the dark theme and the gui behaves properly without them, so they are removed. the third and fourth commit make it possible for charts and gauges to switch their colors based on the current theme. the fifth commit adds a theme switcher widget that allows switching the current theme and then sets a cookie accordingly. the last commit is only relevant for the proxmox mail gateway and enables the proposed "theme switcher" function. this allows users to enable or disable a filter over the email preview in the quarantine. if the filter is enabled the email will have it's brightness reduced and the colors will be inverted. thus, plain text emails shouldn't appear too bright part 2: proxmox virtual environment the first commit for pve adds the theme switcher to the gui, the second adjusts the pveproxy template to properly handle the cookie set by the theme switcher. the third removes unnecessary hard-coded background colors and the forth add support for the charts used in pve only. the fifth commit concern pve-docs and adds support for the dark theme in the api viewer. part 3: proxmox backup server the commits for proxmox backup server first add the theme switcher, then removes hard-coded backgrounds and finally add dark theme support to the api viewer. part 4: proxmox mail gateway the first commit concerns the api and let's it handle cookie properly. the next five commits adjust gui to first add a theme switcher, then style the spam info grid, remove hard-coded white backgrounds, style the hourly mail chart, and enable the dark mode in the mobile quarantine. the second-to-last commit enables the theme switching mechanism for the quarantine mail preview. this part could be dropped in favor of extending the preview endpoint to add some styles to the preview that handle a dark theme. this would have the added benefit of also working in the mobile quarantine. the last commit for the mail gateway enables the dark theme in the api viewer. Daniel Tschlatscher (2): dark-theme: add initial version of the proxmox-dark theme util/window/form: add a theme selector Stefan Sterz (4): subscription/summary/backup: stop setting the background color gauge widget: add support for a dark theme and dynamic theme switching rrd chart: add support for the dark theme and dynamic theme switching dark-theme: add support for the pmg quarantine theme toggle debian/control| 1 + src/Makefile | 4 +- src/Utils.js | 25 +++ src/defines.mk| 1 + src/form/ThemeSelector.js | 6 + src/node/APT.js | 1 - src/panel/GaugeWidget.js | 45 src/panel/RRDChart.js | 37 src/proxmox-dark/Makefile | 47 src/proxmox-dark/scss/ProxmoxDark.scss| 37 src/proxmox-dark/scss/abstracts/_mixins.scss | 12 ++ .../scss/abstracts/_variables.scss| 67 ++ src/proxmox-dark/scss/extjs/_body.scss| 23 ++ src/proxmox-dark/scss/extjs/_grid.scss| 146 + src/proxmox-dark/scss/extjs/_menu.scss| 39 src/proxmox-dark/scss/extjs/_panel.scss | 25 +++ .../scss/extjs/_presentation.scss | 14 ++ src/proxmox-dark/scss/extjs/_progress.scss| 19 ++ src/proxmox-dark/scss/extjs/_splitter.scss| 18 ++ src/proxmox-dark/scss/extjs/_tabbar.scss | 43 src/proxmox-dark/scss/extjs/_tip.scss | 18 ++ src/proxmox-dark/scss/extjs/_toolbar.scss | 21 ++ src/proxmox-dark/scss/extjs/_treepanel.scss | 24 +++ src/proxmox-dark/scss/extjs/_window.scss | 39 src/proxmox-dark/scss/extjs/form/_button.scss | 57 + .../scss/extjs/form/_combobox.scss| 23 ++ .../scss/extjs/form/_formfield.scss | 40 src/proxmox-dark/scss/other/_charts.scss | 39 src/proxmox-dark/scss/other/_icons.scss | 200 ++ .../scss/proxmox/_datepicker.
