Re: [pve-devel] [PATCH v3 qemu-server] Fix ACPI-suspended VMs resuming after migration

2023-10-11 Thread Fiona Ebner
Am 10.10.23 um 16:19 schrieb Fiona Ebner:
>> Also, with that change we might have added a race for suspend-mode backups,
>> at least if VMs really can wake up without a QMP command (which I find 
>> likely).
>> I.e., between the time we checked and set vm_was_paused until we actually
>> suspend, because if the VM would wake up in between we might get inconsistent
>> stuff and skip things like fs-freeze.
> 
> That race is already there without the patch. QEMU does not transition
> from 'suspended' state to 'paused' state when QMP 'stop' is issued, i.e.
> what our 'qm suspend' or vm_suspend() actually does. So it doesn't
> matter if we call that during backup or not when the VM is already in
> 'suspended' state.
> 
> The window for the race is a bit larger though:
> now: VM wakes up between check if paused and 'backup' QMP
> before: VM wakes up after fsfreeze was skipped because guest agent was
> detected as not running
> 

What's written above actually applies to 'snapshot' mode backup, but not
to 'suspend' mode backup. 'suspend' mode backup is and was broken in
this regard already:

> root@pve8a1 ~ # vzdump 100 --storage pbs --mode suspend 
> INFO: starting new backup job: vzdump 100 --mode suspend --storage pbs
> INFO: Starting Backup of VM 100 (qemu)
> INFO: Backup started at 2023-10-11 09:25:53
> INFO: status = running
> INFO: backup mode: suspend
> INFO: ionice priority: 7
> INFO: VM Name: Copy-of-VM-apache
> INFO: include disk 'scsi0' 'rbd:vm-100-disk-0' 4618348134
> INFO: suspending guest
> INFO: snapshots found (not included into backup)
> INFO: creating Proxmox Backup Server archive 'vm/100/2023-10-11T07:25:53Z'
> INFO: skipping guest-agent 'fs-freeze', agent configured but not running?
> INFO: started backup task '923c95cf-6bb6-4476-825e-390c147a4e76'
> INFO: resuming VM again after 3 seconds
> INFO: scsi0: dirty-bitmap status: OK (8.0 MiB of 4.3 GiB dirty)
> INFO: using fast incremental mode (dirty-bitmap), 8.0 MiB dirty of 4.3 GiB 
> total
> INFO: 100% (8.0 MiB of 8.0 MiB) in 1s, read: 8.0 MiB/s, write: 8.0 MiB/s
> INFO: backup was done incrementally, reused 4.29 GiB (99%)
> INFO: transferred 8.00 MiB in 1 seconds (8.0 MiB/s)
> INFO: adding notes to backup
> INFO: resume vm
> INFO: Finished Backup of VM 100 (00:00:06)
> INFO: Backup finished at 2023-10-11 09:25:59
> INFO: Backup job finished successfully

We never got the fsfreeze in suspend mode, because we do it after the
QMP 'stop' when the vCPUs are paused, so the guest agent won't be
running. The backup just ends up with whatever state the filesystem was
in when QMP 'stop' is issued.

Time to finally deprecate 'suspend' mode backup for VMs for good? It
really has just disadvantages compared to 'snapshot' mode and our docs
state "This mode is provided for compatibility reason" for a long time now.

> We could either:
> 
> 1. (ironically) disallow 'suspend' mode backup when in 'suspended' runstate.
> 2. resume and pause the VM upon 'suspend' mode backup, but is also
> surprising
> 3. make the race window smaller again by doing the qga_check_running()
> and fs-freeze if true, if VM was in 'suspended' runstate.
> 
> I don't see how to avoid the possibility of a wake-up during an
> inconvenient time except with one of the first two options.
>


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



Re: [pve-devel] [PATCH v4 qemu-server 2/2] remote-migration: add target-cpu && target-reboot params

2023-10-11 Thread Fiona Ebner
Am 10.10.23 um 18:29 schrieb DERUMIER, Alexandre:
>>
>> I think that a true offline migration (without starting any
>> source/target vm) can be done with qemu-storage-daemon running nbd
>> server on target &&  qemu-img on source.
>>
>> This could be also used for online migration with unused/detached
>> disks.
>>
> 
>>> Yes, we could, but would require additional logic. So the question is
>>> if
>>> there are enough advantages to do it rather than via a full VM.
>>> Starting
>>> a VM of course requires more resources.
> 
> I don't known if it's possible to use the ndb server in the targetvm,
> if the target disk is not attached ? (I really don't known)

Yes, I think need to attach it as a blockdev first. But for 'restart'
migration, it should be fine to start out with NBD and migrate
non-attached disks with our current method. If we (later) want to add
logic for migrating non-attached disks via storage-daemon we can always
do that too and switch storage_migrate() to use that (maybe not
unconditionally, ZFS -> ZFS still makes more sense to do with send/recv
for example).


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH manager v2 1/1] pve7to8: check for proper grub meta-package for bootmode

2023-10-11 Thread Friedrich Weber
On 09/10/2023 14:52, Stoiko Ivanov wrote:
> +} elsif ( ! -f "/usr/share/doc/grub-efi-amd64/changelog.Debian.gz" ) {
> + log_warn(
> + "System booted in uefi mode but grub-efi-amd64 meta-package not 
> installed"
> + . " new grub versions will not be installed to /boot/efi!"
> + . " Install grub-efi-amd64."
>   );

I do like the exclamation mark, but I still think some punctuation (if
not newline) between "[...] not installed" and "new grub versions [...]"
would be good. Currently, the message reads like this:

WARN: System booted in uefi mode but grub-efi-amd64 meta-package
notinstalled new grub versions will not be installed to /boot/efi! Install
grub-efi-amd64.

which is a bit hard to parse -- the following seems easier to parse
(note the extra comma):

WARN: System booted in uefi mode but grub-efi-amd64 meta-package not
installed, new grub versions will not be installed to /boot/efi! Install
grub-efi-amd64.

Sorry for obsessing over the punctuation here, but I suspect there are a
lot of UEFI-booted PVE 7 installs with LVM root, so it would be good to
reduce the potential for confusion as much as possible.

The exact phrasing aside, consider this:

Tested-by: Friedrich Weber 

Can confirm that with this patch,

* pve7to8 prints the warning on UEFI-booted system with root on LVM and
grub-pc installed
* pve7to8 does *not* print the warning on
** the same system when grub-efi-amd64 is installed
** UEFI-booted system with root on ZFS (using systemd-boot)
** legacy-booted system with root on LVM or ZFS


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



Re: [pve-devel] [PATCH kernel-helper v2 2/2] proxmox-boot-tool: check if correct grub metapackage is installed

2023-10-11 Thread Friedrich Weber
Tested-by: Friedrich Weber 

Can confirm that with this patch,

* the warning appears after installing a new kernel on a UEFI-booted
system with root on LVM
* the warning does *not* appear after installing a new kernel on
** a UEFI-booted system with root on ZFS (using systemd-boot)
** a legacy-booted system with root on LVM or ZFS

On 09/10/2023 14:52, Stoiko Ivanov wrote:
> this part of the hook applies only to systems not using pbt for
> bootmangement.
> 
> Currently our ISO installs grub-pc unconditionally - and never the
> conflicting grub-efi-amd64. Both packages are responsible for
> running grub-install (for the appropriate disks) upon an upgrade of
> grub.
> 
> This results in grub currently not getting updated on uefi-booted
> systems (which do not use proxmox-boot-tool).
> 
> The patch causes a warning to be printed to notify the user.
> 
> Also considered putting the check+warning in d/postinst - but this way
> it will get triggered more often (upon every
> kernel-upgrade/update-initramfs, instead of only on
> proxmox-kernel-helper updates, which are less often), increasing the
> chances of being noticed.
> 
> checking for the changelog-presence was chosen, over `dpkg-query` for
> the status, for consistency with the similar patch for pve7to8 (and
> potentially a small speed-gain).
> 
> Suggested-by: Thomas Lamprecht 
> Signed-off-by: Stoiko Ivanov 
> ---
>  src/proxmox-boot/zz-proxmox-boot | 19 +++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/src/proxmox-boot/zz-proxmox-boot 
> b/src/proxmox-boot/zz-proxmox-boot
> index 1adc1b1..4dfa765 100755
> --- a/src/proxmox-boot/zz-proxmox-boot
> +++ b/src/proxmox-boot/zz-proxmox-boot
> @@ -215,6 +215,23 @@ disable_systemd_boot_hook() {
>  
>  }
>  
> +check_grub_efi_package() {
> +
> + if [ -f "${ESP_LIST}" ]; then
> + return
> + fi
> +
> + if [ ! -d /sys/firmware/efi ]; then
> + return
> + fi
> +
> + if [ -f /usr/share/doc/grub-efi-amd64/changelog.Debian.gz ]; then
> + return
> + fi
> + warn "uefi-booted system, without grub-efi-amd64 package - /boot/efi 
> will not be updated"
> +
> +}
> +
>  set -- $DEB_MAINT_PARAMS
>  mode="${1#\'}"
>  mode="${mode%\'}"
> @@ -228,6 +245,7 @@ case $0:$mode in
>   BOOT_KVERS="$(boot_kernel_list "$@")"
>   update_esps
>   disable_systemd_boot_hook
> + check_grub_efi_package
>   ;;
>*/postrm.d/*:|*/postrm.d/*:remove)
>   reexec_in_mountns "$@"
> @@ -235,6 +253,7 @@ case $0:$mode in
>   BOOT_KVERS="$(boot_kernel_list)"
>   update_esps
>   disable_systemd_boot_hook
> + check_grub_efi_package
>   ;;
>  esac
>  


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



Re: [pve-devel] [PATCH installer 1/1] fix #4869: Show state in management interface ComboBox

2023-10-11 Thread Filip Schauer

The green circle is not displayed correctly by the PVE installer as it
does not have the corresponding emoji font package. However, the
suggested circles for DOWN are rendered correctly.

Alternatively we could use an arrow pointing either ⬆ (UP) or ⬇ (DOWN).

https://unicode-explorer.com/c/2B06
https://unicode-explorer.com/c/2B07

These arrows are also displayed correctly by the PVE installer.


On 10/10/2023 15:13, Thomas Lamprecht wrote:

Am 10/10/2023 um 14:56 schrieb Christoph Heiss:

On Tue, Oct 10, 2023 at 01:55:43PM +0200, Thomas Lamprecht wrote:

UP   -> 🟢 https://unicode-explorer.com/c/1F7E2

A green circle would be problematic/non-optimal for people deuteranopia
(red-green color blindness). So maybe some other character/symbol
distinctly recognizable as "being connected, up" - but purely
color-coding things is not something that should be done IMO.

but those already get filled/not-filled as hint, i.e., this isn't just
purely color coding, that's why I explicitly used a *filled* green circle
for UP and for DOWN an *empty* one as preferred one due to vision
impairments like color blindness, like hinted ;-)

But sure, if you have a better idea for this just say so.
IMO the proposed one should be clear enough, as not only color (useful
for the majority of people) but also form is distinct, and I saw such
circles being used to indicate on/off or plugged/unplugged already in
a few UIs (such things are not easy to search for, so sorry, no concrete
example), but that doesn't mean there might be something better that is
still practical for both TUI and GUI.


DOWN -> ◯ (preferred, easier to differ for vision impaired people) or ⬤
 https://unicode-explorer.com/c/25EF or 
https://unicode-explorer.com/c/2B24

Do we really need to explicitly mark non-UP interfaces? I'd think the
fact of it being not marked UP should provide enough context in this
situation.

Yes, to give clear distinction to the UP state and make them more
distinct to each other, also to avoid text-alignment (visual symmetry)
problems (if the symbol prefixes the iface name, which I guesstimate
will look better).



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH kernel-helper v2 2/2] proxmox-boot-tool: check if correct grub metapackage is installed

2023-10-11 Thread Friedrich Weber
On 11/10/2023 11:39, Friedrich Weber wrote:
> Can confirm that with this patch,
> 
> * the warning appears after installing a new kernel on a UEFI-booted
> system with root on LVM

Just to be clear: This is if grub-pc is installed. If i install
grub-efi-amd64 instead, the warning does not appear anymore.



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH pve-manager] ui: makefile: readd compression selector form

2023-10-11 Thread Christian Ebner
Commit 65704cc2a88729479fb15ec2a5b3df683b8f2aac apparently removed by
misstake the form/CompressionSelector.js from the Makefile.

It is however required by the backup view to select the compression
method, so readd it.

Signed-off-by: Christian Ebner 
---
 www/manager6/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index 5bb8fa06..79c079ab 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -30,6 +30,7 @@ JSSRC=
\
form/CephPoolSelector.js\
form/CephFSSelector.js  \
form/ComboBoxSetStoreNode.js\
+   form/CompressionSelector.js \
form/ContentTypeSelector.js \
form/ControllerSelector.js  \
form/DayOfWeekSelector.js   \
-- 
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 installer 1/1] fix #4869: Show state in management interface ComboBox

2023-10-11 Thread Thomas Lamprecht
Am 11/10/2023 um 11:54 schrieb Filip Schauer:
> The green circle is not displayed correctly by the PVE installer as it
> does not have the corresponding emoji font package. However, the
> suggested circles for DOWN are rendered correctly.

The options I see (on top of that):

- we could just install a font package that ships it, e.g., the
  fonts-noto-color-emoji one, I mean 10.8 MB isn't negligible, but
  neither _that_ big..

- Use ◯ for down and ⬤ for up, and color them via CSS, or
  whatever is the easiest here for GTK combobox entries.

> Alternatively we could use an arrow pointing either ⬆ (UP) or ⬇ (DOWN).
> 
> https://unicode-explorer.com/c/2B06
> https://unicode-explorer.com/c/2B07
> 
> These arrows are also displayed correctly by the PVE installer.

I cannot 100% pin it down, but I do not really like arrows for
conveying that information even though the map to up/down directly,
but arrows are normally used for rather different things in UI
context (e.g., sorting, or resize handles), so IMO to overloaded
already.


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


[pve-devel] applied: [PATCH pve-manager] ui: makefile: readd compression selector form

2023-10-11 Thread Fiona Ebner
Am 11.10.23 um 12:29 schrieb Christian Ebner:
> Commit 65704cc2a88729479fb15ec2a5b3df683b8f2aac apparently removed by
> misstake the form/CompressionSelector.js from the Makefile.
> 
> It is however required by the backup view to select the compression
> method, so readd it.
> 
> Signed-off-by: Christian Ebner 

applied, thanks!


___
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] Fix: cpu hotplug feature can't be changed online

2023-10-11 Thread Fiona Ebner
Am 10.10.23 um 17:37 schrieb Alexandre Derumier:
> The cpus are passed as devices with specific id only when cpu hotplug is 
> enable
> at start.
> We can't enable/disable it online or vcpu hotplug api will thrown errors
> not finding core id.

When removing cores after enabling the option this is true, but I can
1. start a VM without CPU hotplug and fewer than max vCPUs
2. enable CPU hotplug
3. add more cores

And doing the disable of the option online also doesn't seem problematic
at a first glance. So I thought this would technically be a breaking change.

But actually, the change is completely justified, because of migration.
Because the QEMU commandline changes based on the hotplug setting, so
the source and target VM will not agree and loading the state on the
target will get confused and crash:

> Oct 11 14:08:23 pve8a2 QEMU[160882]: kvm: get_pci_config_device: Bad config 
> data: i=0x9a read: 8 device: 3 cmask: ff wmask: 0 w1cmask:0
> Oct 11 14:08:23 pve8a2 QEMU[160882]: kvm: Failed to load PCIDevice:config
> Oct 11 14:08:23 pve8a2 QEMU[160882]: kvm: Failed to load virtio-scsi:virtio
> Oct 11 14:08:23 pve8a2 QEMU[160882]: kvm: error while loading state for 
> instance 0x0 of device ':00:05.0/virtio-scsi'
> Oct 11 14:08:23 pve8a2 QEMU[160882]: kvm: load of migration failed: Invalid 
> argument

Therefore,
Reviewed-by: Fiona Ebner 


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



[pve-devel] [PATCH kernel-helper v3 2/2] proxmox-boot-tool: check if correct grub metapackage is installed

2023-10-11 Thread Stoiko Ivanov
this part of the hook applies only to systems not using pbt for
bootmangement.

Currently our ISO installs grub-pc unconditionally - and never the
conflicting grub-efi-amd64. Both packages are responsible for
running grub-install (for the appropriate disks) upon an upgrade of
grub.

This results in grub currently not getting updated on uefi-booted
systems (which do not use proxmox-boot-tool).

The patch causes a warning to be printed to notify the user.

Also considered putting the check+warning in d/postinst - but this way
it will get triggered more often (upon every
kernel-upgrade/update-initramfs, instead of only on
proxmox-kernel-helper updates, which are less often), increasing the
chances of being noticed.

checking for the changelog-presence was chosen, over `dpkg-query` for
the status, for consistency with the similar patch for pve7to8 (and
potentially a small speed-gain).

Suggested-by: Thomas Lamprecht 
Signed-off-by: Stoiko Ivanov 
---
 src/proxmox-boot/zz-proxmox-boot | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/src/proxmox-boot/zz-proxmox-boot b/src/proxmox-boot/zz-proxmox-boot
index 1adc1b1..4dfa765 100755
--- a/src/proxmox-boot/zz-proxmox-boot
+++ b/src/proxmox-boot/zz-proxmox-boot
@@ -215,6 +215,23 @@ disable_systemd_boot_hook() {
 
 }
 
+check_grub_efi_package() {
+
+   if [ -f "${ESP_LIST}" ]; then
+   return
+   fi
+
+   if [ ! -d /sys/firmware/efi ]; then
+   return
+   fi
+
+   if [ -f /usr/share/doc/grub-efi-amd64/changelog.Debian.gz ]; then
+   return
+   fi
+   warn "uefi-booted system, without grub-efi-amd64 package - /boot/efi 
will not be updated"
+
+}
+
 set -- $DEB_MAINT_PARAMS
 mode="${1#\'}"
 mode="${mode%\'}"
@@ -228,6 +245,7 @@ case $0:$mode in
BOOT_KVERS="$(boot_kernel_list "$@")"
update_esps
disable_systemd_boot_hook
+   check_grub_efi_package
;;
 */postrm.d/*:|*/postrm.d/*:remove)
reexec_in_mountns "$@"
@@ -235,6 +253,7 @@ case $0:$mode in
BOOT_KVERS="$(boot_kernel_list)"
update_esps
disable_systemd_boot_hook
+   check_grub_efi_package
;;
 esac
 
-- 
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 kernel-helper v3 1/2] proxmox-boot-tool: do not exit early in kernel-hook

2023-10-11 Thread Stoiko Ivanov
update_esps is called first in the actual execution below - exiting
early does not work for systems that don't use proxmox-boot-tool if a
check added later needs to work there too.

Signed-off-by: Stoiko Ivanov 
---
 src/proxmox-boot/zz-proxmox-boot | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/proxmox-boot/zz-proxmox-boot b/src/proxmox-boot/zz-proxmox-boot
index 793882b..1adc1b1 100755
--- a/src/proxmox-boot/zz-proxmox-boot
+++ b/src/proxmox-boot/zz-proxmox-boot
@@ -44,7 +44,7 @@ fi
 update_esps() {
if [ ! -f "${ESP_LIST}" ]; then
warn "No ${ESP_LIST} found, skipping ESP sync."
-   exit 0
+   return
fi
if [ -f /etc/kernel/cmdline ]; then
# we can have cmdline files with multiple or no new line at 
all, handle both!
-- 
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 manager v3 1/1] pve7to8: check for proper grub meta-package for bootmode

2023-10-11 Thread Stoiko Ivanov
This should catch installations from our ISO on non-ZFS in uefi mode,
which won't get the updated grub efi binary installed upon upgrade,
because grub-pc is installed instead of grub-efi-amd64.

Adding this to pve7to8 should make this even more visible, than the
corresponding patch for promxox-kernel-helper (warnings printed during
regular package upgrades might be overlooked more easily than
a yellow line in the major upgrade checkscript)

The if/else order was chosen to limit the nesting level of the long
messages.

Signed-off-by: Stoiko Ivanov 
---
 PVE/CLI/pve7to8.pm | 39 +++
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/PVE/CLI/pve7to8.pm b/PVE/CLI/pve7to8.pm
index d1a71eff..b34c8362 100644
--- a/PVE/CLI/pve7to8.pm
+++ b/PVE/CLI/pve7to8.pm
@@ -1302,29 +1302,36 @@ sub check_time_sync {
 
 sub check_bootloader {
 log_info("Checking bootloader configuration...");
-if (!$upgraded) {
-   log_skip("not yet upgraded, no need to check the presence of 
systemd-boot");
-   return;
-}
 
-if (! -f "/etc/kernel/proxmox-boot-uuids") {
-   log_skip("proxmox-boot-tool not used for bootloader configuration");
+if (! -d '/sys/firmware/efi') {
+   log_skip("System booted in legacy-mode - no need for additional 
packages");
return;
 }
 
-if (! -d "/sys/firmware/efi") {
-   log_skip("System booted in legacy-mode - no need for systemd-boot");
-   return;
-}
-
-if ( -f "/usr/share/doc/systemd-boot/changelog.Debian.gz") {
-   log_pass("systemd-boot is installed");
-} else {
+if ( -f "/etc/kernel/proxmox-boot-uuids") {
+   if (!$upgraded) {
+   log_skip("not yet upgraded, no need to check the presence of 
systemd-boot");
+   return;
+   }
+   if ( -f "/usr/share/doc/systemd-boot/changelog.Debian.gz") {
+   log_pass("bootloader packages installed correctly");
+   return;
+   }
log_warn(
"proxmox-boot-tool is used for bootloader configuration in uefi 
mode"
-   . "but the separate systemd-boot package, existing in Debian 
Bookworm  is not installed"
-   . "initializing new ESPs will not work until the package is 
installed"
+   . " but the separate systemd-boot package is not installed,"
+   . " initializing new ESPs will not work until the package is 
installed"
+   );
+   return;
+} elsif ( ! -f "/usr/share/doc/grub-efi-amd64/changelog.Debian.gz" ) {
+   log_warn(
+   "System booted in uefi mode but grub-efi-amd64 meta-package not 
installed,"
+   . " new grub versions will not be installed to /boot/efi!"
+   . " Install grub-efi-amd64."
);
+   return;
+} else {
+   log_pass("bootloader packages installed correctly");
 }
 }
 
-- 
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 kernel-helper/manager v3] check for fitting grub-meta package on uefi systems

2023-10-11 Thread Stoiko Ivanov
v2->v3:
* adapted Friedrich's feedback (huge thanks for the patience and attention
  to semantically important details!!) - so that the pve7to8 warning is
  actually understandable

v1->v2:
* adapted Friedrich's feedback (huge thanks!)
** fixed the wrongly negated check for installed grub-efi-amd64 in the
   boot-tool hook.
** Rephrased the error-message in pve7to8 to 2 sentences. I tried adding a
   newline as well, however this results in the message not being printed
   in the warning color anymore (most likely due to [0]) - and I felt this
   to be more important than having it on a separate line.

[0] https://perldoc.perl.org/Term::ANSIColor#RESTRICTIONS

original cover-letter for v1:
The following patchset is a followup to the one for the installer:
https://lists.proxmox.com/pipermail/pve-devel/2023-September/059270.html

As suggested by Thomas - adding the check to proxmox-kernel-helper seems
like a good idea. While adding it to d/postinst I thought that this might
not be the best place - and that getting the warning upon every
kernel-upgrade would be better vs. upon every upgrade of
proxmox-kernel-helper (which are far less often).
(Can gladly send the version with d/postinst as well)

If the pve-manager patch gets applied - I'd push the equivalent change to
pmg and provide one for pbs.

Tested on legacy and uefi VMs installed with pve-8.0 iso and
grub-efi-amd64 (and systemd-boot) removed vs. installed.

proxmox-kernel-helper
Stoiko Ivanov (2):
  proxmox-boot-tool: do not exit early in kernel-hook
  proxmox-boot-tool: check if correct grub metapackage is installed

 src/proxmox-boot/zz-proxmox-boot | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

pve-manager:
Stoiko Ivanov (1):
  pve7to8: check for proper grub meta-package for bootmode

 PVE/CLI/pve7to8.pm | 39 +++
 1 file changed, 23 insertions(+), 16 deletions(-)

-- 
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 kernel-helper/manager v3] check for fitting grub-meta package on uefi systems

2023-10-11 Thread Friedrich Weber
Looks good to me, thanks a lot!

Tested-by: Friedrich Weber 
Reviewed-by: Friedrich Weber 


___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



Re: [pve-devel] [PATCH font-logos/manager v2 0/5] fix #2435: lxc: show distro and privileged status in summary

2023-10-11 Thread Christoph Heiss


Ping - still applies cleanly on current master of both repositories.

On Wed, Jul 05, 2023 at 01:12:44PM +0200, Christoph Heiss wrote:
>
> This implements #2435 [0]. Show the unprivileged status in the summary
> panel, the distro logo and name in the title of the summary panel.
>
> Patch 1 & 2 fix two small typos in the `fonts-font-logos` package.
> Patch 3 then prepares the pveproxy to serve the required CSS and font
> files for the icon font we use [1] (packaged here [2]), and finally
> patch 4 & 5 finally wire everything up into the UI.
>
> N.B.: The `fonts-font-logos` package is not yet available through the
> repos yet (as far as I could see), so it must be built & installed
> locally for testing.
>
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=2435
> [1] https://github.com/Lukas-W/font-logos
> [2] https://git.proxmox.com/?p=fonts-font-logos.git;a=summary
>
> fonts-font-logos:
>
> Christoph Heiss (2):
>   d/install: fix typo in css install path
>   css: fix missing `@` for font-face rule
>
>  debian/install | 2 +-
>  src/font-logos.css | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> pve-manager:
>
> Christoph Heiss (3):
>   pveproxy, ui, d/control: add font-logos
>   ui: GuestStatusView: show privileged status as new row
>   ui: GuestStatusView: show distro logo and name in summary header
>
>  PVE/Service/pveproxy.pm   |  2 ++
>  debian/control|  1 +
>  www/index.html.tpl|  1 +
>  www/manager6/panel/GuestStatusView.js | 51 ++-
>  4 files changed, 54 insertions(+), 1 deletion(-)
>
> --
> 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



Re: [pve-devel] [PATCH installer 1/1] fix #4869: Show state in management interface ComboBox

2023-10-11 Thread Filip Schauer


On 11/10/2023 12:58, Thomas Lamprecht wrote:

Am 11/10/2023 um 11:54 schrieb Filip Schauer:

The green circle is not displayed correctly by the PVE installer as it
does not have the corresponding emoji font package. However, the
suggested circles for DOWN are rendered correctly.

The options I see (on top of that):

- we could just install a font package that ships it, e.g., the
   fonts-noto-color-emoji one, I mean 10.8 MB isn't negligible, but
   neither _that_ big..

- Use ◯ for down and ⬤ for up, and color them via CSS, or
   whatever is the easiest here for GTK combobox entries.



Installing the font package for a single symbol seems a bit excessive.
However, changing the color of individual characters turns out to be
impossible with combo boxes. At least I haven't found a way to do this
with Gtk's limited CSS functionality, nor with override_color or a
custom cell renderer.

That leaves us with the font package.





Alternatively we could use an arrow pointing either ⬆ (UP) or ⬇ (DOWN).

https://unicode-explorer.com/c/2B06
https://unicode-explorer.com/c/2B07

These arrows are also displayed correctly by the PVE installer.

I cannot 100% pin it down, but I do not really like arrows for
conveying that information even though the map to up/down directly,
but arrows are normally used for rather different things in UI
context (e.g., sorting, or resize handles), so IMO to overloaded
already.



___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel