[pve-devel] applied-series: [PATCH stable-7 container 0/2] setup: cherry-pick ubuntu 24.04 support

2024-06-04 Thread Thomas Lamprecht
Am 04/06/2024 um 10:34 schrieb Christoph Heiss:
> This backports the series to support Ubuntu 24.04 "Noble" CTs [0].
> 
> They could be cherry-picked cleanly. Tested on a up-to-date PVE 7.4
> system (running on `pvetest`), template for Ubuntu 24.04
> (`ubuntu-24.04-standard_24.04-2_amd64.tar.zst`) was copied from a 8.2
> host for testing.
> 
> Tested creating new CT from that template, using either a static IP or
> DHCP. Then working around with the system a bit, everything seems to
> work just fine.
> 
> [0] https://lists.proxmox.com/pipermail/pve-devel/2024-April/063766.html
> 
> Fiona Ebner (2):
>   setup: support Ubuntu 24.04 Noble
>   setup: unlink default netplan configuration even with Ubuntu >= 23.04
> 
>  src/PVE/LXC/Setup/Ubuntu.pm | 3 +++
>  1 file changed, 3 insertions(+)
> 


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 1/2] migration: avoid crash with heavy IO on local VM disk

2024-05-23 Thread Thomas Lamprecht
Am 23/05/2024 um 11:08 schrieb Fabian Grünbichler:
>> +my $kvm_version = PVE::QemuServer::kvm_user_version();
> wouldn't this need to check the *running* qemu binary for the migrated
> VM, not the *installed* qemu binary on the system?

Yes, this would need to check the running QEMU version via QMP,
as otherwise, the new command will be also get issued to VMs
started before the updated, which then will fail.


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


[pve-devel] applied-series: [PATCH zfsonlinux v2 0/2] Update to ZFS 2.2.4

2024-05-21 Thread Thomas Lamprecht
Am 07/05/2024 um 17:02 schrieb Stoiko Ivanov:
> v1->v2:
> Patch 2/2 (adaptation of arc_summary/arcstat patch) modified:
> * right after sending the v1 I saw a report where pinning kernel 6.2 (thus
>   ZFS 2.1) leads to a similar traceback - which I seem to have overlooked
>   when packaging 2.2.0 ...
>   adapted the patch by booting a VM with kernel 6.2 and the current
>   userspace and running arc_summary /arcstat -a until no traceback was
>   displayed with a single-disk pool.
> 
> original cover-letter for v1:
> This patchset updates ZFS to the recently released 2.2.4
> 
> We had about half of the patches already in 2.2.3-2, due to the needed
> support for kernel 6.8.
> 
> Compared to the last 2.2 point releases this one compares quite a few
> potential performance improvments:
> * for ZVOL workloads (relevant for qemu guests) multiple taskq were
>   introduced [1] - this change is active by default (can be put back to
>   the old behavior with explicitly setting `zvol_num_taskqs=1`
> * the interface for ZFS submitting operations to the kernel's block layer
>   was augmented to better deal with split-pages [2] - which should also
>   improve performance, and prevent unaligned writes which are rejected by
>   e.g. the SCSI subsystem. - The default remains with the current code
>   (`zfs_vdev_disk_classic=0` turns on the 'new' behavior...)
> * Speculative prefetching was improved [3], which introduced not kstats,
>   which are reported by`arc_summary` and `arcstat`, as before with the
>   MRU/MFU additions there was not guard for running the new user-space
>   with an old kernel resulting in Python exceptions of both tools.
>   I adapted the patch where Thomas fixed that back in the 2.1 release
>   times. - sending as separate patch for easier review - and I hope it's
>   ok that I dropped the S-o-b tag (as it's changed code) - glad to resend
>   it, if this should be adapted.
> 
> Minimally tested on 2 VMs (the arcstat/arc_summary changes by running with
> an old kernel and new user-space)
> 
> 
> [0] https://github.com/openzfs/zfs/releases/tag/zfs-2.2.4
> [1] https://github.com/openzfs/zfs/pull/15992
> [2] https://github.com/openzfs/zfs/pull/15588
> [3] https://github.com/openzfs/zfs/pull/16022
> 
> Stoiko Ivanov (2):
>   update zfs submodule to 2.2.4 and refresh patches
>   update arc_summary arcstat patch with new introduced values
> 
>  ...md-unit-for-importing-specific-pools.patch |   4 +-
>  ...-move-manpage-arcstat-1-to-arcstat-8.patch |   2 +-
>  ...-guard-access-to-freshly-introduced-.patch | 438 
>  ...-guard-access-to-l2arc-MFU-MRU-stats.patch | 113 ---
>  ...hten-bounds-for-noalloc-stat-availab.patch |   4 +-
>  ...rectly-handle-partition-16-and-later.patch |  52 --
>  ...-use-splice_copy_file_range-for-fall.patch | 135 
>  .../0014-linux-5.4-compat-page_size.patch | 121 
>  .../patches/0015-abd-add-page-iterator.patch  | 334 -
>  ...-existing-functions-to-vdev_classic_.patch | 349 -
>  ...v_disk-reorganise-vdev_disk_io_start.patch | 111 ---
>  ...-read-write-IO-function-configurable.patch |  69 --
>  ...e-BIO-filling-machinery-to-avoid-spl.patch | 671 --
>  ...dule-parameter-to-select-BIO-submiss.patch | 104 ---
>  ...se-bio_chain-to-submit-multiple-BIOs.patch | 363 --
>  ...on-t-use-compound-heads-on-Linux-4.5.patch |  96 ---
>  ...ault-to-classic-submission-for-2.2.x.patch |  90 ---
>  ...ion-caused-by-mmap-flushing-problems.patch | 104 ---
>  ...touch-vbio-after-its-handed-off-to-t.patch |  57 --
>  debian/patches/series |  16 +-
>  upstream  |   2 +-
>  21 files changed, 445 insertions(+), 2790 deletions(-)
>  create mode 100644 
> debian/patches/0009-arc-stat-summary-guard-access-to-freshly-introduced-.patch
>  delete mode 100644 
> debian/patches/0009-arc-stat-summary-guard-access-to-l2arc-MFU-MRU-stats.patch
>  delete mode 100644 
> debian/patches/0012-udev-correctly-handle-partition-16-and-later.patch
>  delete mode 100644 
> debian/patches/0013-Linux-6.8-compat-use-splice_copy_file_range-for-fall.patch
>  delete mode 100644 debian/patches/0014-linux-5.4-compat-page_size.patch
>  delete mode 100644 debian/patches/0015-abd-add-page-iterator.patch
>  delete mode 100644 
> debian/patches/0016-vdev_disk-rename-existing-functions-to-vdev_classic_.patch
>  delete mode 100644 
> debian/patches/0017-vdev_disk-reorganise-vdev_disk_io_start.patch
>  delete mode 100644 
> debian/patches/0018-vdev_disk-make-read-write-IO-function-configurable.patch
>  delete mode 100644 
> debian/patches/0019-vdev_disk-rewrite-BIO-filling-machinery-to-avoid-spl.patch
>  delete mode 100644 
> debian/patches/0020-vdev_disk-add-module-parameter-to-select-BIO-submiss.patch
>  delete mode 100644 
> debian/patches/0021-vdev_disk-use-bio_chain-to-submit-multiple-BIOs.patch
>  delete mode 100644 
> debian/patches/0022-abd_iter_page-don-t-use-compound-heads-on-Linux-4.5.patch
>  delete mode 100644 

[pve-devel] applied: [PATCH docs] network: override device names: suggest running update-initramfs

2024-05-21 Thread Thomas Lamprecht
Am 21/05/2024 um 14:55 schrieb Friedrich Weber:
> The initramfs-tools hook /usr/share/initramfs-tools/hooks/udev copies
> link files from /etc/systemd/network to the initramfs, where they take
> effect in early userspace. If the link files in the initramfs diverge
> from the link files in the rootfs, this can lead to confusing
> behavior, as reported in enterprise support. For instance:
> 
> - If an interface matches link files both in the initramfs and the
>   rootfs, it will be renamed twice during boot.
> - A leftover link file in the initramfs renaming an interface A to a
>   new name X may prevent a link file in the rootfs from renaming a
>   different interface B to the same name X (it will fail with "File
>   exists").
> 
> To avoid this confusion, mention the link files are copied to the
> initramfs, and suggest updating the initramfs after making changes to
> the link files.
> 
> Suggested-by: Hannes Laimer 
> Signed-off-by: Friedrich Weber 
> ---
>  pve-network.adoc | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH v2 manager] api: add proxmox-firewall to versions pkg list

2024-05-21 Thread Thomas Lamprecht
Am 24/04/2024 um 13:35 schrieb Mira Limbeck:
> Signed-off-by: Mira Limbeck 
> ---
> v2:
>  - add `api: ` prefix to commit msg
> 
>  PVE/API2/APT.pm | 1 +
>  1 file changed, 1 insertion(+)
> 
>

applied, thanks!


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



[pve-devel] applied-series: [PATCH proxmox-firewall 1/2] firewall: improve handling of ARP traffic for guests

2024-05-21 Thread Thomas Lamprecht
Am 15/05/2024 um 15:37 schrieb Stefan Hanreich:
> In order to be able to send outgoing ARP packets when the default
> policy is set to drop or reject, we need to explicitly allow ARP
> traffic in the outgoing chain of guests. We need to do this in the
> guest chain itself in order to be able to filter spoofed packets via
> the MAC filter.
> 
> Contrary to the out direction we can simply accept all incoming ARP
> traffic, since we do not do any MAC filtering for incoming traffic.
> Since we create fdb entries for every NIC, guests should only see ARP
> traffic for their MAC addresses anyway.
> 
> Signed-off-by: Stefan Hanreich 
> Originally-by: Laurent Guerby 
> ---
>  proxmox-firewall/resources/proxmox-firewall.nft   | 1 +
>  proxmox-firewall/src/firewall.rs  | 8 
>  .../tests/snapshots/integration_tests__firewall.snap  | 4 ++--
>  3 files changed, 7 insertions(+), 6 deletions(-)
> 
>

applied both patches, thanks!

I reworded the subject here too and re-ordered the git trailers, as they
should have a causal order where possible. I.e., if someone else made a
patch, or helped you to do so, their co-authored-by or originally-by is
normally before your signed-off-by, as your "signature" shows that all
above it is (to your best knowledge) correct w.r.t patch ownership and
description, and like on "real" documents that signature goes at the
bottom.


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



[pve-devel] applied: [PATCH proxmox-firewall v2 1/1] firewall: properly reject ipv6 traffic

2024-05-21 Thread Thomas Lamprecht
Am 13/05/2024 um 14:14 schrieb Stefan Hanreich:
> ICMPv6 has different message types for rejecting traffic. With ICMP we
> used host-prohibited as rejection type, which doesn't exist in ICMPv6.
> Add an additional rule for IPv6, so it uses admin-prohibited.
> 
> Additionally, add a terminal drop statement in order to prevent any
> traffic that does not get matched from bypassing the reject chain.
> 
> Signed-off-by: Stefan Hanreich 
> ---
> Changes from v1 -> v2:
> * add a terminal drop statement to prevent any unmatched traffic from
>   bypassing the reject chain
> * properly match ICMPv6 traffic via l4proto
> 
>  proxmox-firewall/resources/proxmox-firewall.nft | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
>

applied, with an updated commit subject (as per our guideline[0], using the
"firewall" tag inside a repo that has "firewall" already in the name is not
really adding much), thanks!

[0]: 
https://pve.proxmox.com/wiki/Developer_Documentation#Commits_and_Commit_Messages


___
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 v10 1/4] add C program to get hardware capabilities from CPUID

2024-05-21 Thread Thomas Lamprecht
Am 17/05/2024 um 13:21 schrieb Dominik Csapak:
> one small nit inline:
> 
> On 5/10/24 13:47, Markus Frank wrote:
>> 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
>> +
> 
> PREFIX is only used once here, so it's probably better inlining the value

No, having the PREFIX variable separate is a common pattern
that allows customizing installation.
Even if we do not need that ourselves, it's still not costing us really
anything to keep following that here and could make comparing changes
between two packages with binaries installed in different paths easier.
So while I wouldn't go through all our build systems and introduce this
variable if missing, I'd also not recommend developers to drop it, as it
is not better to do so.


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



[pve-devel] applied: [PATCH qemu] fixes for QEMU 9.0

2024-05-17 Thread Thomas Lamprecht
Am 17/05/2024 um 10:44 schrieb Fiona Ebner:
> Most importantly, fix forwards and backwards migration with VirtIO-GPU
> display.
> 
> Other fixes are for a regression in pflash device (introduced in 8.2)
> and some fixes for x86(_64) TCG emulation. One of the patches needed
> to be adapted, because it removed a helper that is still in use in
> 9.0.0.
> 
> There also is a revert for a fix in VirtIO PCI devices that turned out
> to cause some issues, see the revert itself for more details.
> 
> Lastly, there is a change to move compatibility flags for a new
> VirtIO-net feature to the correct machine type. The feature was
> introduced in QEMU 8.2, but the compatibility flags got added to
> machine version 8.0 instead of 8.1. This breaks backwards migration
> with machine version 8.1 from a 8.2/9.0 binary to an 8.1 binary, in
> cases where the guest kernel enables the feature (e.g. Ubuntu 23.10).
> While that breaks migration with machine version 8.1 from an unpatched
> to a patched binary, Proxmox VE only ever had 8.2 on the test
> repository and 9.0 not yet in any public repository. An upstream
> developer suggested it is the proper fix [0]. Upstream submission [1].
> 
> [0]: 
> https://lore.kernel.org/qemu-devel/cacgkmetzrjuhof+hugvrvllqe+8nqe5xmshpt0naq1epnqf...@mail.gmail.com/T/#u
> [1]: 
> https://lore.kernel.org/qemu-devel/20240517075336.104091-1-f.eb...@proxmox.com/T/#u
> 
> Signed-off-by: Fiona Ebner 
> ---
>  .../0006-virtio-gpu-fix-v2-migration.patch| 98 +++
>  ...0007-hw-pflash-fix-block-write-start.patch | 59 +++
>  ...operand-size-for-DATA16-REX.W-POPCNT.patch | 51 ++
>  ...ru-wrpkru-are-no-prefix-instructions.patch | 40 
>  ...6-fix-feature-dependency-for-WAITPKG.patch | 33 +++
>  ...tio-pci-fix-use-of-a-released-vector.patch | 87 
>  ...move-compatibility-flags-for-VirtIO-.patch | 57 +++
>  ...sed-balloon-qemu-4-0-config-size-fal.patch |  4 +-
>  debian/patches/series |  7 ++
>  9 files changed, 434 insertions(+), 2 deletions(-)
>  create mode 100644 
> debian/patches/extra/0006-virtio-gpu-fix-v2-migration.patch
>  create mode 100644 
> debian/patches/extra/0007-hw-pflash-fix-block-write-start.patch
>  create mode 100644 
> debian/patches/extra/0008-target-i386-fix-operand-size-for-DATA16-REX.W-POPCNT.patch
>  create mode 100644 
> debian/patches/extra/0009-target-i386-rdpkru-wrpkru-are-no-prefix-instructions.patch
>  create mode 100644 
> debian/patches/extra/0010-target-i386-fix-feature-dependency-for-WAITPKG.patch
>  create mode 100644 
> debian/patches/extra/0011-Revert-virtio-pci-fix-use-of-a-released-vector.patch
>  create mode 100644 
> debian/patches/extra/0012-hw-core-machine-move-compatibility-flags-for-VirtIO-.patch
> 
>

applied, thanks!


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



Re: [pve-devel] [RFC qemu] savevm-async: improve check for blockers

2024-05-17 Thread Thomas Lamprecht
subject might be improved by being less general/ambiguous, something like:

savevm-async: improve coverage by also checking for migration blockers

or 

savevm-async: block snapshot also if migration would fail

or

savevm-async: reuse migration blocker check for snapshots

Would have helped me to have a better initial context for reading this commit
(message).

Am 17/05/2024 um 13:39 schrieb Fiona Ebner:
> Same rationale as with upstream QEMU commit 5aaac46793 ("migration:
> savevm: consult migration blockers"), migration and (async) snapshot
> are essentially the same operation and thus snapshot also needs to
> check for migration blockers. For example, this catches passed-through
> PCI devices, where the driver does not support migration.
> 
> However, the commit notes:
> 
>> There is really no difference between live migration and savevm, except
>> that savevm does not require bdrv_invalidate_cache to be implemented
>> by all disks.  However, it is unlikely that savevm is used with anything
>> except qcow2 disks, so the penalty is small and worth the improvement
>> in catching bad usage of savevm.
> 
> and for Proxmox VE, suspend-to-disk with VMDK does use savevm-async
> and would be broken by simply using migration_is_blocked(). To keep
> this working, introduce a new helper that filters blockers with the
> prefix used by the VMDK migration blocker.
> 
> The function qemu_savevm_state_blocked() is called as part of
> migration_is_blocked_allow_prefix() so no check is lost with this
> patch.
> 
> Signed-off-by: Fiona Ebner 
> ---
> 
> An alternative would be to mark the VMDK blocker as a
> "live-migration-only" blocker and extending migration_is_blocked() or
> using separate helpers to check for migration and snapshot blockers
> differently. But that requires touching more machinery and probably
> needs more adaptation going forward than the approach here.
> 
>  migration/migration.c| 22 ++
>  migration/migration.h|  1 +
>  migration/savevm-async.c |  7 ++-
>  3 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index b8d7e471a4..6235309a00 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1897,6 +1897,28 @@ void qmp_migrate_pause(Error **errp)
> "during postcopy-active or postcopy-recover state");
>  }
>  
> +/*
> + * HACK to allow hibernation in Proxmox VE even when VMDK image is present.
> + */
> +bool migration_is_blocked_allow_prefix(Error **errp, const char *prefix)
> +{
> +GSList *blockers = migration_blockers[migrate_mode()];
> +
> +if (qemu_savevm_state_blocked(errp)) {
> +return true;
> +}
> +
> +while (blockers) {
> +if (!g_str_has_prefix(error_get_pretty(blockers->data), prefix)) {
> +error_propagate(errp, error_copy(blockers->data));
> +return true;
> +}
> +blockers = g_slist_next(blockers);
> +}
> +
> +return false;
> +}
> +
>  bool migration_is_blocked(Error **errp)
>  {
>  GSList *blockers = migration_blockers[migrate_mode()];
> diff --git a/migration/migration.h b/migration/migration.h
> index 8045e39c26..575805a8e2 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -484,6 +484,7 @@ int migration_call_notifiers(MigrationState *s, 
> MigrationEventType type,
>   Error **errp);
>  
>  int migrate_init(MigrationState *s, Error **errp);
> +bool migration_is_blocked_allow_prefix(Error **errp, const char *prefix);
>  bool migration_is_blocked(Error **errp);
>  /* True if outgoing migration has entered postcopy phase */
>  bool migration_in_postcopy(void);
> diff --git a/migration/savevm-async.c b/migration/savevm-async.c
> index bf36fc06d2..33085446e1 100644
> --- a/migration/savevm-async.c
> +++ b/migration/savevm-async.c
> @@ -363,7 +363,12 @@ void qmp_savevm_start(const char *statefile, Error 
> **errp)
>  return;
>  }
>  
> -if (qemu_savevm_state_blocked(errp)) {
> +/*
> + * The limitation for VMDK images only applies to live-migration, not
> + * snapshots, see commit 5aaac46793 ("migration: savevm: consult 
> migration
> + * blockers").
> + */
> +if (migration_is_blocked_allow_prefix(errp, "The vmdk format used by 
> node")) {

meh, not a big fan of matching strings here, especially as that is not
stable ABI, I mean I did not check, but I would be surprised if that's
the case – maybe we could factor out that string here and when its added
as blocker into a common constant so that we'd notice if it changes.

And if we only uses this here then why add a generic "ignore one specific
blocker" helper, might be better to at least contain that detail in a
"qemu_savevm_async_state_blocked" one that takes only the `errp` as
parameter, as hacks should IMO always be quite specific to avoid the
spread of them (I know you would check in detail before doing so, but
not everybody does).

>  

[pve-devel] applied: [PATCH qemu-server] suspend: continue cleanup even if savevm-end QMP command fails

2024-05-15 Thread Thomas Lamprecht
Am 14/05/2024 um 16:11 schrieb Fiona Ebner:
> The savevm-end command also fails when no snapshot operation was
> started before. In particular, this is the case when savevm-start
> failed early, because of unmigratable devices.
> 
> Avoid potentially leaving an orphaned volume and snasphot-related
> configuration keys around by continuing with cleanup instead.
> 
> Signed-off-by: Fiona Ebner 
> ---
>  PVE/QemuServer.pm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
>

applied, thanks!


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



Re: [pve-devel] [RFC PATCH manager] ui: make label for mediated device types more clear

2024-05-08 Thread Thomas Lamprecht
On 08/05/2024 14:54, Dominik Csapak wrote:
> 'MDev' could be interpreted as either 'Mediated Device' or 'Mapped
> Device', which can confuse users.
> 
> At least one user was confused:
> https://forum.proxmox.com/threads/146586/#post-662091
> 
> Fix that by writing out 'Mediated Device'.
> 
> Signed-off-by: Dominik Csapak 
> ---
> sending as RFC because the text is now too long for one line, which is
> bad IMHO, but I could not come up with a better text that fits, maybe
> someone else has a better idea (or we leave it as is)
> 
> 
>  www/manager6/qemu/PCIEdit.js | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/www/manager6/qemu/PCIEdit.js b/www/manager6/qemu/PCIEdit.js
> index 8cef1b10..940e017c 100644
> --- a/www/manager6/qemu/PCIEdit.js
> +++ b/www/manager6/qemu/PCIEdit.js
> @@ -262,7 +262,7 @@ Ext.define('PVE.qemu.PCIInputPanel', {
>   name: 'mdev',
>   reference: 'mdev',
>   disabled: true,
> - fieldLabel: gettext('MDev Type'),
> + fieldLabel: gettext('Mediated Device Type'),

Alternatively you could add just a tooltip here, not a perfect solution
either but better than nothing for most users.

While it does not solve the wrapping, could we drop the "type" part, as
is this really a type of something or a selection a specific mediated
device?

One idea would be a bigger re-layout, i.e.:

move the radio groups beside each other so that they and their selection
combobox take up the first two rows, in the row below you could then place
the two checkboxes "all functions" and "primary GPU", and as bbar (above
the advanced section) you could have the mdev field with a wider label,
at the bottom it might not look that bad even if a bit asymmetric to the
other fields.

One would have to try and play around to see if that is an improvement,
if not, having a label wrap over is probably the lesser evil compared to
bad UX due to the label being a tad cryptic.


>   nodename: me.nodename,
>   listeners: {
>   change: function(field, value) {



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



Re: [pve-devel] [PATCH storage 1/1] esxi: improve error handling for fuse mount tool

2024-05-08 Thread Thomas Lamprecht
On 08/05/2024 14:41, Dominik Csapak wrote:
> if the fuse tool encounters an error early, it prints it like:
>Error: some error message
> on stderr.
> 
> We can capture that here by redirecting STDERR to $wr and die'ing with

using just a variable name like $wr without context in a commit message
is hardly telling or useful, maybe replace above and the paragraph below
with something like:

"Redirect the STDERR of the child process that mounts the ESXi instance to
the pipe of the parent (API) process, so that it can pass a hopefully more
meaningful message to the user than just an erroneous return code."


> the error message.
> 
> With this we die with the original error message instead of only
> with the return code which is telling the user nothing and does not
> help us debug.
> 
> Signed-off-by: Dominik Csapak 
> ---
>  src/PVE/Storage/ESXiPlugin.pm | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Storage/ESXiPlugin.pm b/src/PVE/Storage/ESXiPlugin.pm
> index b8bce0e..8dc33fc 100644
> --- a/src/PVE/Storage/ESXiPlugin.pm
> +++ b/src/PVE/Storage/ESXiPlugin.pm
> @@ -222,6 +222,10 @@ sub esxi_mount : prototype($$$;$) {
>   // die "failed to get file descriptor flags: $!\n";
>   fcntl($wr, F_SETFD, $flags & ~FD_CLOEXEC)
>   // die "failed to remove CLOEXEC flag from fd: $!\n";
> +
> + # capture errors from stderr

nit: you capture all that gets printed to stderr, not just errors, and no
hard feelings, but the comment feels a tiny bit superfluous, at least with
the error message.

> + open(STDERR, ">&", \*$wr) or die "unable to redirect STDERR: $!\n";

Don't the \ reference operator and the * dereference operator here cancel
each other out?

> +
>   # FIXME: use the user/group options!
>   exec {$ESXI_FUSE_TOOL}
>   $ESXI_FUSE_TOOL,
> @@ -245,7 +249,7 @@ sub esxi_mount : prototype($$$;$) {
>  undef $wr;
>  
>  my $result = do { local $/ = undef; <$rd> };
> -if ($result =~ /^ERROR: (.*)$/) {
> +if ($result =~ /^ERROR: (.*)$/i) {
>   die "$1\n";
>  }
>  



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



Re: [pve-devel] [PATCH pve-esxi-import-tools 1/1] improve error handling before mounting

2024-05-08 Thread Thomas Lamprecht
On 08/05/2024 14:41, Dominik Csapak wrote:
> when we fail early in the mount process, we did not log any error to the
> syslog, but only the top most one to stderr.
> 
> sadly we were not able to see them anywhere, so improve the log by
> * log the complete error chain with log::error (so we also can see the
>   causes)
> * add more context hints in main_do
> 
> This can help debug issues where we failed early and could not see any
> error output otherwise, e.g. this thread in the forum:
> 
> https://forum.proxmox.com/threads/146248/
> 
> Signed-off-by: Dominik Csapak 
> ---
>  src/main.rs | 36 +++-
>  1 file changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/src/main.rs b/src/main.rs
> index 281ec3d..a8b2bb6 100644
> --- a/src/main.rs
> +++ b/src/main.rs
> @@ -1,4 +1,5 @@
>  use std::ffi::{CString, OsStr, OsString};
> +use std::fmt::Write;
>  use std::io;
>  use std::os::fd::RawFd;
>  use std::os::unix::ffi::OsStrExt;
> @@ -221,7 +222,12 @@ fn main() {
>  });
>  
>  if let Err(err) = runtime.block_on(main_do()) {
> -eprintln!("Error: {}", err);
> +let mut err_chain = String::new();
> +for err in err.chain() {
> +let _ = writeln!(err_chain, " {err}");

is the extra whitespace intended? At least the first line would then have two
consecutive whitespaces, one from here and one from the prefix below on log and
println.

> +}
> +println!("Error: {err}");
> +log::error!("Error: {err_chain}");
>  std::process::exit(-1);
>  }
>  }
> @@ -236,15 +242,15 @@ async fn main_do() -> Result<(), Error> {
>  usage(, std::io::stderr(), 1);
>  }
>  };
> -parse_manifest()?;
> +parse_manifest().context("failed to parse manifest")?;
>  
>  let change_uid = match args.change_user.as_deref() {
> -Some(user) => Some(get_uid(user)?),
> +Some(user) => Some(get_uid(user).context("failed to get uid")?),
>  None => None,
>  };
>  
>  let change_gid = match args.change_group.as_deref() {
> -Some(group) => Some(get_gid(group)?),
> +Some(group) => Some(get_gid(group).context("failed to get gid")?),
>  None => None,
>  };
>  
> @@ -255,10 +261,12 @@ async fn main_do() -> Result<(), Error> {
>  .enable_readdirplus();
>  
>  for opt in args.mount_options {
> -fuse = fuse.options_os()?;
> +fuse = fuse
> +.options_os()
> +.context("failed to get fuse options")?;
>  }
>  
> -unmount_if_mounted(_path)?;
> +unmount_if_mounted(_path).context("failed to unmount")?;
>  
>  let mut fuse = fuse
>  .build()
> @@ -302,9 +310,19 @@ async fn main_do() -> Result<(), Error> {
>  let fs_datastore = fs_datacenter.create_datastore(datastore);
>  
>  log::debug!("loading {datacenter:?}/{datastore:?}/{path:?}");
> -let config =
> -vmx::VmConfig::parse(client.open_file(datacenter, datastore, 
> path).await?, path)
> -.await?;
> +let config = vmx::VmConfig::parse(
> +client
> +.open_file(datacenter, datastore, path)
> +.await
> +.with_context(|| {
> +format!("error opening file for: {datacenter} 
> {datastore} {path}")

should that context be added unconditionally in open_file?

Also drop the `for: `, rather something like "error when opening file '{...}'"

And FWIW, in above debug log the variable triple is printed as path,
and it's also accessible that way on FS level, so might make sense to
keep it that way.

> +})?,

can we move this to a separate `let config =` line before calling parse on is,
this gets rather convoluted and with rust one can nicely override the same
variable in an explicit way, so this here IMO better expressed that way.

> +path,
> +)
> +.await
> +.with_context(|| {
> +format!("failed to parse vm config for {datacenter} 
> {datastore} {path}")
> +})?;

Similar here, isn't adding that context belonging in the parse fn itself? Might
hold true for the static context too, as long as the context does not include
call-site specific info, but only fn implementation-site one, they might be 
better
added at the site of implementation to avoid potential repetition.

also s/vm/VM/

>  log::debug!("{config:#?}");
>  for disk in config.disks.values() {
>  let other_fs_datastore;



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



[pve-devel] applied-series: [PATCH installer v3 0/2] assistant: keep prepared iso bootable on uefi with flash drives

2024-05-08 Thread Thomas Lamprecht
On 30/04/2024 12:46, Aaron Lauterer wrote:
> booting a prepared iso in UEFI mode from a blockdev (e.g. usb flash
> drive) did not work as grub could not find the partition.
> 
> we now read the uuid / volume_date from the source iso and always set it
> explictly to the same value when injecting files.
> 
> more details in the actual commit message
> 
> the second patch is a style patch
> 
> this version should now include everything. sorry for the noise :)
> 
> changes since:
> v2:
> * add import of format_err that was missed in v2
> v1:
> * improve error handling in case xorriso does return empty output
> 
> Aaron Lauterer (2):
>   assistant: keep prepared iso bootable on uefi with flash drives
>   assistant: use single dash for xorriso parameter
> 
>  proxmox-auto-install-assistant/src/main.rs | 48 +++---
>  1 file changed, 43 insertions(+), 5 deletions(-)
> 


applied series with Stoiko's R-b and T-b, thanks!


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



[pve-devel] applied: [PATCH qemu-server] schema: fix description of migrate_downtime parameter

2024-05-03 Thread Thomas Lamprecht
On 03/05/2024 14:01, Fiona Ebner wrote:
> Since commit 865ef132 ("implement dynamic migration_downtime") the
> migration downtime will be automatically increased when migration
> cannot converge at the very end. Update the description to reflect
> reality.
> 
> Signed-off-by: Fiona Ebner 
> ---
>  PVE/QemuServer.pm | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
>

applied, thanks! we might deprecate this option then, as a user cannot
force a minimal downtime anyway anymore through it, and not sure how
much use it has to set the initial value.

Deprecating would for now mostly mean adding that word to the description
though, so no biggie.


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



[pve-devel] applied-series: [PATCH v2 container 1/2] setup: support Ubuntu 24.04 Noble

2024-05-02 Thread Thomas Lamprecht
On 30/04/2024 16:42, Fiona Ebner wrote:
> Minimally tested, that an upgrade from an existing 23.04 container
> works, there still is network and no obviously bad messages in the
> container's journal.
> 
> Reported in the community forum:
> https://forum.proxmox.com/threads/145848/
> 
> Signed-off-by: Fiona Ebner 
> ---
> 
> No changes in v2.
> 
>  src/PVE/LXC/Setup/Ubuntu.pm | 1 +
>  1 file changed, 1 insertion(+)
> 
>

applied both patches, thanks!


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



Re: [pve-devel] [PATCH container] setup: support Ubuntu 24.04 Noble

2024-04-30 Thread Thomas Lamprecht
On 30/04/2024 10:43, Fiona Ebner wrote:
> So this is not new (already present for Ubuntu 23.10) and stems from the
> fact that these images from linuxcontainers.org contain:
> 
>> root@CT113:~# cat /etc/netplan/10-lxc.yaml 
>> network:
>>   version: 2
>>   ethernets:
>> eth0:
>>   dhcp4: true
>>   dhcp-identifier: mac
> 
> and that generates a configuration that will be ordered before
> ours/preferred by systemd-networkd:
> 
>> root@CT113:~# networkctl status eth0
>> ● 2: eth0
>>   
>>  Link File: n/a
>>   Network File: /run/systemd/network/10-netplan-eth0.network
> 
> Should we still change something in the setup code? I suppose our
> template will not have the netplan configuration file and in a way it'd
> just be a race to the bottom of being ordered first.

Why should there be a incentive for a race to the bottom?

If we have users running into this then yes, we should do something
about it, we do not have a hard requirement of the Ubuntu templates
being build through DAB and especially as we use the LXC template
builder (or well its artefacts) for other non-Debian images, I'd
see why users take it as a source.

If the change in ordering is the correct solution I cannot say without
looking into all deeper – but I'm sure you can evaluate that.
One possibility might be disabling netplan on CT creation, if PVE wants
to control network in another way itself.


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


[pve-devel] applied-series: [PATCH storage 1/2] don't bail on whitespaces in backing devices

2024-04-30 Thread Thomas Lamprecht
On 30/04/2024 09:53, Wolfgang Bumiller wrote:
> This prevents importing from vmdks with whitespaces in file names.
> Further, some operations that include file sizes (like listing disks)
> would potentially fail entirely if a custom disk with a badly name
> backing device exists in a VM images directory since they don't expect
> this. Specifically, since we don't necessarily know the actual naming
> scheme of the current storage in the plain Plugin.pm version, we don't
> check the full name anyway, so why bother with whitespaces...
> 
> See-also: 
> https://forum.proxmox.com/threads/new-import-wizard-available-for-migrating-vmware-esxi-based-virtual-machines.144023/page-16#post-658697

FYI, forum links can be de-SEO'd like:

https://forum.proxmox.com/threads/144023/page-16#post-658697

> Signed-off-by: Wolfgang Bumiller 
> ---
>  src/PVE/Storage/Plugin.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied both patches, thanks!


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



[pve-devel] applied-series: [PATCH v3 qemu 1/2] fix #5409: backup: fix copy-before-write timeout

2024-04-30 Thread Thomas Lamprecht
On 29/04/2024 17:20, Fiona Ebner wrote:
> The type for the copy-before-write timeout in nanoseconds was wrong.
> By being just uint32_t, a maximum of slightly over 4 seconds was
> possible. Larger values would overflow and thus the 45 seconds set by
> Proxmox's backup with fleecing, resulted in effectively 2 seconds
> timeout for copy-before-write operations.
> 
> Reported-by: Friedrich Weber 
> Signed-off-by: Fiona Ebner 
> ---
> 
> Changes in v3:
> * rebase on master
> 
> Upstream submission of this patch:
> https://lore.kernel.org/qemu-devel/20240429141934.442154-1-f.eb...@proxmox.com/T/#u
> 
>  ...e-write-use-uint64_t-for-timeout-in-.patch | 35 +++
>  ...ock-copy-before-write-fix-permission.patch |  2 +-
>  ...e-write-support-unligned-snapshot-di.patch |  2 +-
>  ...e-write-create-block_copy-bitmap-in-.patch |  2 +-
>  ...-backup-add-discard-source-parameter.patch |  4 +--
>  ...e-allow-specifying-minimum-cluster-s.patch |  2 +-
>  ...um-cluster-size-to-performance-optio.patch |  2 +-
>  debian/patches/series |  1 +
>  8 files changed, 43 insertions(+), 7 deletions(-)
>  create mode 100644 
> debian/patches/extra/0005-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
> 
>

applied series, thanks!


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



[pve-devel] applied: [PATCH v2 qemu 1/2] fix #5409: backup: fix copy-before-write timeout

2024-04-29 Thread Thomas Lamprecht
Am 29/04/2024 um 16:27 schrieb Fiona Ebner:
> The type for the copy-before-write timeout in nanoseconds was wrong.
> By being just uint32_t, a maximum of slightly over 4 seconds was
> possible. Larger values would overflow and thus the 45 seconds set by
> Proxmox's backup with fleecing, resulted in effectively 2 seconds
> timeout for copy-before-write operations.
> 
> Reported-by: Friedrich Weber 
> Signed-off-by: Fiona Ebner 
> ---
> 
> New in v2.
> 
> Upstream submission of this patch:
> https://lore.kernel.org/qemu-devel/20240429141934.442154-1-f.eb...@proxmox.com/T/#u
> 
>  ...e-write-use-uint64_t-for-timeout-in-.patch | 35 +++
>  ...ock-copy-before-write-fix-permission.patch |  2 +-
>  ...e-write-support-unligned-snapshot-di.patch |  2 +-
>  ...e-write-create-block_copy-bitmap-in-.patch |  2 +-
>  ...-backup-add-discard-source-parameter.patch |  4 +--
>  ...e-allow-specifying-minimum-cluster-s.patch |  2 +-
>  ...um-cluster-size-to-performance-optio.patch |  2 +-
>  debian/patches/series |  1 +
>  8 files changed, 43 insertions(+), 7 deletions(-)
>  create mode 100644 
> debian/patches/extra/0014-block-copy-before-write-use-uint64_t-for-timeout-in-.patch
> 
>

applied this one to a new stable-8 branch that was split of the last 8.1.5-5
release (8 in the branch name is meaning the PVE major release here though,
can then be merged with master before that switches over to target PVE 9).

Does not apply anymore on master, which is QEMU 9.0 now, so please send a
rebase for that.



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



[pve-devel] applied: [PATCH docs v2] fix #5429: network: override device names: include Type=ether

2024-04-29 Thread Thomas Lamprecht
Am 29/04/2024 um 09:49 schrieb Friedrich Weber:
> Mention that the systemd link file should contain `Type=ether` in most
> setup, to make sure it only applies to Ethernet devices and does not
> ever apply to e.g. bridges or bonds which inherit the MAC address of
> the Ethernet device. Mention that some setups may require other
> options.
> 
> Reported in the forum [0] and in #5429 [1].
> 
> [0] https://forum.proxmox.com/threads/144557/post-656188
> [1] https://bugzilla.proxmox.com/show_bug.cgi?id=5429
> 
> Fixes: 96c0261 ("fix #4847: network: extend section on interface naming 
> scheme")
> Signed-off-by: Friedrich Weber 
> ---
> 
> Notes:
> Changes v1 -> v2:
> 
> - link #5429 which was opened in the meantime
> - expand on why Type=ether is recommended
> - mention that some setups may require other choices (thx Thomas)
> 
> v1: https://lists.proxmox.com/pipermail/pve-devel/2024-April/063659.html
> 
>  pve-network.adoc | 19 ++-
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH-SERIES qemu] update to QEMU 9.0.0

2024-04-29 Thread Thomas Lamprecht
Am 25/04/2024 um 17:21 schrieb Fiona Ebner:
> QEMU 8.2.2 required many changes, in particular to the alloc-track
> block driver. It should be the same as [0] just with backup fleecing
> patches added in. See the patch for details.
> 
> The only bigger change in QEMU 9.0.0 is that the AioContext locking
> was removed, and it just required dropping the calls to acquire and
> release the lock. See the patch for details.
> 
> Did not see any outstanding important fixes on the qemu-stable mailing
> list currently, so none picked up.
> 
> [0]: https://lists.proxmox.com/pipermail/pve-devel/2024-March/062422.html
> 
> 
> Fiona Ebner (4):
>   makefile: adapt firmware blob removal to changes for QEMU 8.2
>   makefile: also filter 64-bit hppa ROM for QEMU 8.2
>   update submodule and patches to QEMU 8.2.2
>   update submodule and patches to QEMU 9.0.0
> 

applied series, thanks!

I did a separate upload of 8.2.2 and imported that to pvetest so that we
have it in the repos for quick (regression) testing/comparison, even if
we skip directly to 9.0.


___
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: Remove pveACMEPluginView in favor of pmxACMEPluginView

2024-04-29 Thread Thomas Lamprecht
subject:

ui: acme: switch plugin view over to the one from widget-toolkit


(having internal xtypes in the subject already is not really that
useful)

Am 29/08/2023 um 13:00 schrieb Filip Schauer:
> Remove pveACMEPluginView and use the ACMEPluginView from the
> proxmox-widget-toolkit instead. This leaves pveACMEPluginEditor unused,
> so remove it as well.

when got this moved, would be good to have some references here, which would
it also make it easier to decide if we need a new bump of the verisoned
dependency in d/control.




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



Re: [pve-devel] [PATCH container] setup: support Ubuntu 24.04 Noble

2024-04-29 Thread Thomas Lamprecht
Am 29/04/2024 um 11:56 schrieb Fiona Ebner:
> Am 29.04.24 um 11:36 schrieb Fiona Ebner:
>> Am 29.04.24 um 11:23 schrieb Fiona Ebner:
>>> Reported in the community forum:
>>> https://forum.proxmox.com/threads/145848/#post-658694
>>>
>>> Signed-off-by: Fiona Ebner 
>>> ---
>>>
>>> Minimally tested, that an upgrade from an existing 23.04 container
>>> works, there still is network and no obviously bad messages in the
>>> container's journal.
>>>
>> Hmm, while the upgrade did work, starting from an Ubuntu 24.04 template
>> and setting a static IP does not seem to work, like described here:
>> https://forum.proxmox.com/threads/145848/post-658058
>
> Seems like the ordering of the configuration files is the issue. The
> following would fix it, but probably needs to be special-cased for new
> Ubuntu (or new systemd, would still need to check where the change came
> in exactly) not to mess up existing containers, right?

Yes, at least that would reduce regression potential of unknown issues.


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



[pve-devel] applied: [PATCH proxmox-firewall] config: macros: add SPICEproxy macro

2024-04-25 Thread Thomas Lamprecht
Am 25/04/2024 um 19:16 schrieb Stefan Hanreich:
> Signed-off-by: Stefan Hanreich 
> ---
>  proxmox-ve-config/resources/macros.json | 9 +
>  1 file changed, 9 insertions(+)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH proxmox-firewall] firewall: improve error handling of firewall

2024-04-25 Thread Thomas Lamprecht
Am 25/04/2024 um 19:23 schrieb Stefan Hanreich:
> Error handling of the firewall binary should now be much more robust
> on configuration errors. Instead of panicking in some cases it should
> now log an error.
> 
> Signed-off-by: Stefan Hanreich 
> ---
>  proxmox-firewall/src/bin/proxmox-firewall.rs |   7 +-
>  proxmox-firewall/src/config.rs   | 239 +--
>  proxmox-firewall/src/firewall.rs |   7 +-
>  proxmox-firewall/tests/integration_tests.rs  |  51 ++--
>  4 files changed, 155 insertions(+), 149 deletions(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH proxmox-firewall] config: nftables: add support for icmp-type any

2024-04-25 Thread Thomas Lamprecht
Am 25/04/2024 um 19:16 schrieb Stefan Hanreich:
> We support any as wildcard for matching all icmp types. Implement
> parsing logic for parsing the any value and support converting the any
> value into an nftables expression.
> 
> Signed-off-by: Stefan Hanreich 
> ---
>  proxmox-nftables/src/expression.rs |  2 ++
>  proxmox-ve-config/src/firewall/types/rule_match.rs | 12 
>  2 files changed, 14 insertions(+)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH container] fix #5414: use proper percentages in `pct df`

2024-04-25 Thread Thomas Lamprecht
Am 25/04/2024 um 09:40 schrieb Fabian Grünbichler:
> while some people write percentages as 0.XX , putting a % next to that is just
> confusing. also, combined with the format modifier this would be rather lossy,
> and also not match regular `df` output..
> 

Fixes: c6b5965 ("added 'pct df'")

(but I now forgot to amend it too...)

> Signed-off-by: Fabian Grünbichler 
> ---
>  src/PVE/CLI/pct.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!


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


[pve-devel] applied-series: [PATCH installer] install module: getters: correctly use plural in error messages

2024-04-25 Thread Thomas Lamprecht
Am 25/04/2024 um 10:40 schrieb Alexander Zeidler:
> Signed-off-by: Alexander Zeidler 
> ---
>  Proxmox/Install.pm | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
>

applied both patches, thanks!


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



Re: [pve-devel] [PATCH docs] network: override device names: include Type=ether in link file

2024-04-24 Thread Thomas Lamprecht
Am 24/04/2024 um 18:55 schrieb Friedrich Weber:
> Mention that the systemd link file should contain `Type=ether`, to
> make sure it only applies to Ethernet devices and does not ever apply
> to e.g. bridges or bonds which inherit the MAC address of the Ethernet
> device. Reported in the forum [0].
> 
> [0] https://forum.proxmox.com/threads/144557/post-656188
> 
> Fixes: 96c0261 ("fix #4847: network: extend section on interface naming 
> scheme")
> Signed-off-by: Friedrich Weber 
> ---
>  pve-network.adoc | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/pve-network.adoc b/pve-network.adoc
> index ef586ec..8e5fa1c 100644
> --- a/pve-network.adoc
> +++ b/pve-network.adoc
> @@ -170,6 +170,9 @@ identifier. A link file has two sections: `[Match]` 
> determines which interfaces
>  the file will apply to; `[Link]` determines how these interfaces should be
>  configured, including their naming.
>  
> +The `[Match]` section should contain `Type=ether`, to make sure it only 
> matches
> +Ethernet devices.

With have some users with different uplinks though, and while that is rather 
rare,
I'd still mention this here in passing, so that users know about the existence 
of
wlan or wwan, maybe even just in a footnote with a reference to:

https://www.freedesktop.org/software/systemd/man/latest/systemd.link.html#Type=

> +
>  To assign a name to a particular network device, you need a way to uniquely 
> and
>  permanently identify that device in the `[Match]` section. One possibility is
>  to match the device's MAC address using the `MACAddress` option, as it is
> @@ -183,6 +186,7 @@ the following contents:
>  
>  [Match]
>  MACAddress=aa:bb:cc:dd:ee:ff
> +Type=ether
>  
>  [Link]
>  Name=enwan0



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



[pve-devel] applied: [PATCH proxmox-firewall] fix #5410: config: fix naming scheme for names in firewall config

2024-04-24 Thread Thomas Lamprecht
Am 24/04/2024 um 18:15 schrieb Stefan Hanreich:
> This should bring the allowed names on par with the pve-firewall
> naming scheme [1].
> 
> [1] 
> https://git.proxmox.com/?p=pve-firewall.git;a=blob;f=src/PVE/Firewall.pm;h=0abfeccffc94cec940760e69a894e392dc33f151;hb=29b48c381d14bf425232dc65c9c0d18f95c8f222#l51
> 
> Signed-off-by: Stefan Hanreich 
> ---
>  proxmox-ve-config/src/firewall/parse.rs   |  8 +++-
>  proxmox-ve-config/src/firewall/types/alias.rs | 14 ++
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH] auto install assistant: fix newline before ':'

2024-04-24 Thread Thomas Lamprecht
Am 24/04/2024 um 11:02 schrieb Dominik Csapak:
> this belongs after the ':' otherwise the output looks weird:
> 
>   [..] can be
>   : * integrated into [..]
>   * needs to be [..]
> 
> Signed-off-by: Dominik Csapak 
> ---
>  proxmox-auto-install-assistant/src/main.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH proxmox-i18n] update German translations

2024-04-24 Thread Thomas Lamprecht
Am 24/04/2024 um 11:14 schrieb Max Carrara:
> Signed-off-by: Max Carrara 
> ---
>  de.po | 220 --
>  1 file changed, 107 insertions(+), 113 deletions(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH pve-installer] answer: perform basic input validation for keyboard

2024-04-24 Thread Thomas Lamprecht
Am 24/04/2024 um 10:48 schrieb Christian Ebner:
> Currently it is possible to validate and create an iso with an
> invalid keyboad layout, only failing later during installation.
> 
> Add a basic check for correct keyboard layout by defining an enum
> with allowed variants.
> 
> Signed-off-by: Christian Ebner 
> ---
>  proxmox-auto-installer/src/answer.rs | 39 +++-
>  proxmox-auto-installer/src/utils.rs  |  8 --
>  2 files changed, 44 insertions(+), 3 deletions(-)
> 
>

applied, with a follow-up to use serde_plain::derive_display_from_serialize
to derive the Display impl, thanks!

btw. if we probably want to use the same perl code that generates the valid
list for the installer environment to output that at build time in such a
way that we can source it from rust on build.


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



[pve-devel] partially-applied-series: [PATCH many v2 00/20] notifications: move template strings to template files; PBS preparations

2024-04-23 Thread Thomas Lamprecht
Am 19/04/2024 um 16:17 schrieb Lukas Wagner:
> proxmox:
> 
> Lukas Wagner (12):
>   notify: switch to file-based templating system
>   notify: make api methods take config struct ownership
>   notify: convert Option> -> Vec in config structs
>   notify: don't make tests require pve-context
>   notify: make the `mail-forwarder` feature depend on proxmox-sys
>   notify: cargo.toml: add spaces before curly braces
>   notify: give each notification a unique ID
>   notify: api: add get_targets
>   notify: derive `api` for Deleteable*Property
>   notify: derive Deserialize/Serialize for Notification struct
>   notify: pbs context: include nodename in default sendmail author
>   notify: renderer: add relative-percentage helper from PBS
> 
>  proxmox-notify/Cargo.toml   |   7 +-
>  proxmox-notify/examples/render.rs   |  63 --
>  proxmox-notify/src/api/gotify.rs|  48 ++---
>  proxmox-notify/src/api/matcher.rs   |  59 +++--
>  proxmox-notify/src/api/mod.rs   | 113 --
>  proxmox-notify/src/api/sendmail.rs  |  60 +++---
>  proxmox-notify/src/api/smtp.rs  | 122 +--
>  proxmox-notify/src/context/mod.rs   |  10 +-
>  proxmox-notify/src/context/pbs.rs   |  18 +-
>  proxmox-notify/src/context/pve.rs   |  15 ++
>  proxmox-notify/src/context/test.rs  |   9 +
>  proxmox-notify/src/endpoints/common/mail.rs |  20 +-
>  proxmox-notify/src/endpoints/gotify.rs  |  12 +-
>  proxmox-notify/src/endpoints/sendmail.rs|  34 +--
>  proxmox-notify/src/endpoints/smtp.rs|  38 ++--
>  proxmox-notify/src/lib.rs   |  59 ++---
>  proxmox-notify/src/matcher.rs   |  71 +++---
>  proxmox-notify/src/renderer/html.rs |  14 --
>  proxmox-notify/src/renderer/mod.rs  | 226 
>  proxmox-notify/src/renderer/plaintext.rs|  39 
>  20 files changed, 506 insertions(+), 531 deletions(-)
>  delete mode 100644 proxmox-notify/examples/render.rs
> 
> 

applied above, i.e., the proxmox/proxmox-notify ones. 

As talked off-list, for the PVE side I'd like to wait out the next one or two
weeks until the dust of the release settles. Until then, the libpve-rs should
stay on the proxmox-notify 0.3 release, while a hot-fix would be slightly more
work, it's still doable – so I see no real practical issue in having this
divergence between PVE and PBS (where this is all completely new anyway).


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


[pve-devel] applied: [PATCH proxmox-firewall] firewall: properly handle REJECT rules

2024-04-23 Thread Thomas Lamprecht
Am 23/04/2024 um 18:02 schrieb Stefan Hanreich:
> Currently we generated DROP statements for all rules involving REJECT.
> We only need to generate DROP when in the postrouting chain of tables
> with type bridge, since REJECT is disallowed there. Otherwise we jump
> into the do-reject chain which properly handles rejects for different
> protocol types.
> 
> Signed-off-by: Stefan Hanreich 
> ---
> Seems like the proper handling for this got lost somewhere during my
> big refactoring :/
> 
>  .../resources/proxmox-firewall.nft|   7 +-
>  proxmox-firewall/src/firewall.rs  |   9 +-
>  proxmox-firewall/src/rule.rs  |  22 ++-
>  proxmox-firewall/tests/input/100.fw   |   2 +
>  proxmox-firewall/tests/input/host.fw  |   2 +
>  .../integration_tests__firewall.snap  | 158 +-
>  proxmox-nftables/src/statement.rs |   6 +-
>  7 files changed, 197 insertions(+), 9 deletions(-)
> 
>

applied, with the Reported-by from Sterz amended in, thanks!


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



[pve-devel] applied: [PATCH installer] assistant: prepare iso: s/direct/included to match current naming

2024-04-23 Thread Thomas Lamprecht
Am 23/04/2024 um 11:00 schrieb Alexander Zeidler:
> Signed-off-by: Alexander Zeidler 
> ---
>  proxmox-auto-install-assistant/src/main.rs | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH installer] assistant: error out on set network config for dhcp

2024-04-23 Thread Thomas Lamprecht
Am 23/04/2024 um 17:18 schrieb Stoiko Ivanov:
> potentially will not happen too often in practice if the sample files
> always contain the right source. Still having settings in an answer
> file that get ignored does not seem right.
> 
> tested with `validate-answer` on a file without `source` in the
> network section (which initially caused confusion for me)
> 
> Signed-off-by: Stoiko Ivanov 
> ---
>  proxmox-auto-installer/src/answer.rs | 13 +
>  1 file changed, 13 insertions(+)
> 
>

applied, thanks!

I'm wonder if we should allow filtering by management NIC for the DHCP case
too though (in the long term).


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



[pve-devel] applied: [PATCH installer] auto-installer: move ssh keys setup to low-level installer

2024-04-23 Thread Thomas Lamprecht
Am 23/04/2024 um 16:44 schrieb Christoph Heiss:
> .. thereby, also fixing a accidental shell injection.
> 
> Since run_cmd{,s}() is nowhere else used anymore, they can be removed
> too.
> 
> Also mostly reverts commit
> 
>   5878dc4ae "auto-installer: handle auto-reboot info messages directly"
> 

would have preferred a bit more reasoning and possibly having this split
in two patches, but fine enough I guess.

> in the process too.
> 
> Reported-by: Friedrich Weber 
> Signed-off-by: Christoph Heiss 
> ---
>  Proxmox/Install.pm|  7 ++
>  Proxmox/Install/Config.pm |  4 ++
>  .../src/bin/proxmox-auto-installer.rs | 34 +
>  proxmox-auto-installer/src/utils.rs   | 70 ++-
>  .../resources/parse_answer/disk_match.json|  2 +-
>  .../parse_answer/disk_match_all.json  |  2 +-
>  .../parse_answer/disk_match_any.json  |  2 +-
>  .../tests/resources/parse_answer/minimal.json |  2 +-
>  .../resources/parse_answer/nic_matching.json  |  2 +-
>  .../resources/parse_answer/specific_nic.json  |  2 +-
>  .../tests/resources/parse_answer/zfs.json |  2 +-
>  proxmox-installer-common/src/setup.rs |  2 +
>  proxmox-tui-installer/src/setup.rs|  1 +
>  13 files changed, 27 insertions(+), 105 deletions(-)
> 
>

applied, thanks!


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



[pve-devel] applied-series: [PATCH manager v2] fix #5251: login: set autocomplete on password and user

2024-04-23 Thread Thomas Lamprecht
Am 19/02/2024 um 11:37 schrieb Maximiliano Sandoval:
> By default they have 'autocomplete=off'. From [1]:
> 
>  > In most modern browsers, setting autocomplete to "off" will not
>  > prevent a password manager from asking the user if they would like to
>  > save username and password information, or from automatically filling
>  > in those values in a site's login form. See the autocomplete
>  > attribute and login fields [2].
> 
> [1] https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/autocomplete
> [2] 
> https://developer.mozilla.org/en-US/docs/Web/Security/Securing_your_site/Turning_off_form_autocompletion#the_autocomplete_attribute_and_login_fields
> 
> Signed-off-by: Maximiliano Sandoval 
> ---
> Differences from v1:
>  - Learn how to send emails
>  - Fix 'fix #5251' prefix in commit titles
> 
>  www/manager6/window/LoginWindow.js | 2 ++
>  1 file changed, 2 insertions(+)
> 
>

applied series, thanks!


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



[pve-devel] applied: [PATCH manager 1/3] ui: user edit: protect user's TFA settings again

2024-04-23 Thread Thomas Lamprecht
Am 09/02/2024 um 14:08 schrieb Fiona Ebner:
> Same rationale as in 5b25580d ("Protect the user's tfa key setting."):
> it should not be possible to change the value when it's not an actual
> secret but a reference to what TFA method is used or, in case of 'x',
> whether TFA is used.
> 
> Signed-off-by: Fiona Ebner 
> ---
>  www/manager6/dc/UserEdit.js | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
>

applied this one for now, thanks!


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



[pve-devel] applied: [PATCH manager] vzdump: also warn when hook script fails for backup-abort or log-end phase

2024-04-23 Thread Thomas Lamprecht
Am 22/01/2024 um 10:55 schrieb Fiona Ebner:
> to make it more visible, also in task logs.
> 
> Signed-off-by: Fiona Ebner 
> ---
>  PVE/VZDump.pm | 2 ++
>  1 file changed, 2 insertions(+)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH qemu-server] qmeventd: also treat 'prelaunch' and 'suspended' states as active

2024-04-23 Thread Thomas Lamprecht
Am 10/10/2023 um 10:57 schrieb Fiona Ebner:
> Otherwise, a VM in those states would be terminated after a backup
> in handle_qmp_return() with QMP 'quit', which is pretty bad in case
> of the 'suspended' state.
> 
> Does not change the fact that a VM started in prelaunch mode for
> backup is terminated later (that is handled by the Perl code).
> 
> Signed-off-by: Fiona Ebner 
> ---
>  qmeventd/qmeventd.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [RFC PATCH pve-manager 1/2] ui: mobile: fix totp login

2024-04-23 Thread Thomas Lamprecht
Am 14/12/2023 um 10:55 schrieb Dominik Csapak:
> logging in with totp on mobile was broken with these two commits:
> 
> pve-manager:
> 509d7a20 ("mobile ui: implement dummy message box and scrip loader")
> and
> pve-access-control:
> cb64967 ("api: drop old verify_tfa api call")
> 
> the pve-manager one overwrote the Ext.MessageBox and Ext.Msg classes and
> thus removed the Ext.MessageBox.OKCANCEL constant that represented the
> buttons of popup messages (without those no buttons on message boxes
> where shown).
> 
> This override did not work as intended, as we still  showed the message
> box by accident, because at that point the Ext.MessageBox was already
> initialized (so it was overwritten), but Ext.Msg was not (this happens
> later).
> 
> and the pve-access-control removed the old tfa verify api (which is now
> done via the /access/ticket api)
> 
> so to fix that, we have to adapt to the api changes and restore the
> stock Ext.MessageBox and Ext.Msg classes by removing the overrides
> (i couldn't find where we would need those)
> 
> we still cannot handle u2f or recovery methods though
> 
> Signed-off-by: Dominik Csapak 
> ---
>  www/mobile/Login.js   | 8 ++--
>  www/mobile/Toolkit.js | 5 -
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 
>

applied both patches, thanks!


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



[pve-devel] applied: [PATCH proxmox-firewall] firewall: properly cleanup tables when firewall is inactive

2024-04-23 Thread Thomas Lamprecht
Am 23/04/2024 um 11:21 schrieb Stefan Hanreich:
> When executing multiple nft commands they are transactional, either
> all get applied or none. When only the host or guest firewall is
> active, only one table exists and this causes the delete commands to
> fail. To fix this we need to send the delete commands separately.
> 
> It might make sense to support running multiple separate batches in
> the NftClient in the future in order to avoid having to call nft
> twice.
> 
> Signed-off-by: Stefan Hanreich 
> ---
>  proxmox-firewall/src/bin/proxmox-firewall.rs |  9 +
>  proxmox-firewall/src/firewall.rs | 10 +-
>  2 files changed, 10 insertions(+), 9 deletions(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH common] interfaces: support stanzas without types/methods

2024-04-23 Thread Thomas Lamprecht
Am 23/04/2024 um 15:03 schrieb Wolfgang Bumiller:
> This is allowed in ifupdown2 and previously interfaces named
> 'vmbr\d+' were recognized as bridges even if they used this mode.
> With commit e68ebda4f109 this is no longer the case.
> 
> Fixes: e68ebda4f109 ("fix #545: interfaces: allow arbitrary bridge names in 
> network config")
> Signed-off-by: Wolfgang Bumiller 
> ---
> The `__interface_to_string portion` is much better viewied with `-w`
> 
>  src/PVE/INotify.pm| 97 +--
>  .../t.ifupdown2-typeless.pl   | 47 +
>  2 files changed, 117 insertions(+), 27 deletions(-)
>  create mode 100644 test/etc_network_interfaces/t.ifupdown2-typeless.pl
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH installer] auto-installer: support UTC as timezone

2024-04-23 Thread Thomas Lamprecht
Am 23/04/2024 um 13:29 schrieb Christoph Heiss:
> Reported-by: Fiona Ebner 
> Signed-off-by: Christoph Heiss 
> ---
>  proxmox-auto-installer/src/utils.rs | 3 +++
>  1 file changed, 3 insertions(+)
> 
>

applied, thanks!


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



[pve-devel] applied-series: [PATCH installer 1/2] move secure boot state to RunEnv

2024-04-23 Thread Thomas Lamprecht
Am 23/04/2024 um 14:27 schrieb Fabian Grünbichler:
> as preparation for using it in more than one place.
> 
> Signed-off-by: Fabian Grünbichler 
> ---
>  Proxmox/Install.pm| 18 +-
>  Proxmox/Install/RunEnv.pm | 12 +++-
>  2 files changed, 16 insertions(+), 14 deletions(-)
> 
>

applied both patches, thanks!


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


[pve-devel] applied: [PATCH manager 1/2] ui: backup jobs: fix fleecing parameters for 'run now' button

2024-04-23 Thread Thomas Lamprecht
Am 23/04/2024 um 14:14 schrieb Dominik Csapak:
> we have to 'printPropertyString' the fleecing parameters, otherwise
> we'll get api parameter errors for that
> 
> Signed-off-by: Dominik Csapak 
> ---
>  www/manager6/dc/Backup.js | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied both patches, thanks!


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



[pve-devel] applied: [PATCH proxmox-i18n] es: update translation

2024-04-23 Thread Thomas Lamprecht
Am 23/04/2024 um 10:13 schrieb Maximiliano Sandoval:
> Signed-off-by: Maximiliano Sandoval 
> ---
>  es.po | 211 +++---
>  1 file changed, 98 insertions(+), 113 deletions(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH manager] ui: fix reset behavior of backup job editor

2024-04-23 Thread Thomas Lamprecht
Am 23/04/2024 um 09:30 schrieb Dominik Csapak:
> when we `bind` we also have to set the initial value correctly,
> otherwise the form dirty tracking is off (the initial bind set does not
> reset the `originalValue`)
> 
> also the bandwidth selector auto transformed the value `null` to `0`
> when there was no initial transformation. Since this is not a valid
> value anyway, skip that.
> 
> Signed-off-by: Dominik Csapak 
> ---
>  www/manager6/dc/Backup.js   | 1 +
>  www/manager6/form/BandwidthSelector.js  | 2 +-
>  www/manager6/panel/BackupAdvancedOptions.js | 1 +
>  3 files changed, 3 insertions(+), 1 deletion(-)
> 
>

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 v7 2/3] config: QEMU AMD SEV enable

2024-04-23 Thread Thomas Lamprecht
Am 22/04/2024 um 14:16 schrieb Markus Frank:
> This patch is for enabling AMD SEV (Secure Encrypted
> Virtualization) support in QEMU

try to keep a somewhat unified line length over the whole commit message,
most editors support re-flowing (parts of the) text to e.g. the for
commit messages commonly used 70 or 72 text width.

> 
> VM-Config-Examples:
> amd_sev: type=std,nodbg=1,noks=1
> amd_sev: es,nodbg=1,kernel-hashes=1
> 
> Node-Config-Example (gets generated automatically):
> amd_sev: cbitpos=47,reduced-phys-bios=1
> 
> kernel-hashes, reduced-phys-bios & cbitpos correspond to the varibles

typo: variables

> with the same name in qemu.
> 
> kernel-hashes=1 adds kernel-hashes to enable measured linux kernel
> launch since it is per default off for backward compatibility.
> 
> reduced-phys-bios and cbitpos are system specific and are read out by
> the amd-sev-support.service on boot and saved to the /run/amd-sev-params
> file. This file is parsed and than used by qemu-server to correctly
> start a AMD SEV VM.
> 
> type=std stands for standard sev to differentiate it from sev-es (es)
> or sev-snp (snp) when support is upstream.
> 
> QEMU's sev-guest policy gets calculated with the parameters nodbg & noks
> These parameters correspond to policy-bits 0 & 1.
> If type is 'es' than policy-bit 2 gets set to 1 to activate SEV-ES.
> Policy bit 3 (nosend) is always set to 1, because migration
> features for sev are not upstream yet and are attackable.
> 
> SEV-ES is highly experimental since it could not be tested.
> 
> see coherent doc patch
> 
> Signed-off-by: Markus Frank 
> ---
> v7:
> * adjustments for the changes made in the query-machine-params C program
> 
> v6:
> * rebase on master
> * removed unused $sev_node_fmt object
> 
> v5:
> * parse /run/amd-sev-params for hardware parameters
> * removed NodeConfig dependency
> * only disallow live-migration and snapshots with vmstate
>   -> allow offline migration and snapshots without vmstate
> 
> v4:
> * reduced lines of code
> * added text that SEV-ES is experimental
> 
>  PVE/API2/Qemu.pm   | 10 ++
>  PVE/QemuMigrate.pm |  4 +++
>  PVE/QemuServer.pm  | 83 ++
>  3 files changed, 97 insertions(+)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 2a349c8..2e8d654 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -4512,6 +4512,10 @@ __PACKAGE__->register_method({
>   push $local_resources->@*, "clipboard=vnc";
>   }
>  > +  if ($res->{running} && $vmconf->{amd_sev}) {

a comment might be good here

> + push $local_resources->@*, "amd_sev";
> + }
> +
>   # if vm is not running, return target nodes where local storage/mapped 
> devices are available
>   # for offline migration
>   if (!$res->{running}) {
> @@ -5192,6 +5196,12 @@ __PACKAGE__->register_method({
>   die "unable to use snapshot name 'pending' (reserved name)\n"
>   if lc($snapname) eq 'pending';
>  
> + my $conf = PVE::QemuConfig->load_config($vmid);
> + if ($param->{vmstate} && $conf->{amd_sev}) {
> + die "Snapshots that include memory are not supported while memory"
> + ." is encrypted by AMD SEV.\n"
> + }
> +
>   my $realcmd = sub {
>   PVE::Cluster::log_msg('info', $authuser, "snapshot VM $vmid: 
> $snapname");
>   PVE::QemuConfig->snapshot_create($vmid, $snapname, 
> $param->{vmstate},
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index 8d9b35a..7db18b2 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -260,6 +260,10 @@ sub prepare {
>   die "VMs with 'clipboard' set to 'vnc' are not live migratable!\n";
>  }
>  
> +if ($running && $conf->{'amd_sev'}) {
> + die "VMs with AMD SEV are not live migratable!\n";

cannot live-migrate VM when AMD SEV is enabled.

> +}
> +
>  my $vollist = PVE::QemuServer::get_vm_volumes($conf);
>  
>  my $storages = {};
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 28e630d..b03f1b4 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -177,6 +177,40 @@ my $agent_fmt = {
>  },
>  };
>  
> +my $sev_fmt = {
> +type => {
> + description => "Enable standard SEV with type='std' or enable 
> experimental SEV-ES"
> + ." with the 'es' option.",
> + type => 'string',
> + default_key => 1,
> + format_description => "qemu-sev-type",
> + enum => ['std', 'es'],
> + maxLength => 3,
> +},
> +nodbg => {

'no-debug' would be more telling

> + description => "Sets policy bit 0 to 1 to disallow debugging of guest",
> + type => 'boolean',
> + format_description => "qemu-sev-nodbg",

do we need a format description for a boolean

> + default => 0,
> + optional => 1,
> +},
> +noks => {

'no-key-sharing' would be also more telling

> + description => "Sets policy bit 1 to 1 to disallow key sharing with 
> other guests",
> + type => 'boolean',
> + 

[pve-devel] applied: [PATCH proxmox-i18n] update Italian translations

2024-04-23 Thread Thomas Lamprecht
Am 23/04/2024 um 08:29 schrieb Christian Ebner:
> Signed-off-by: Christian Ebner 
> ---
>  it.po | 118 +++---
>  1 file changed, 39 insertions(+), 79 deletions(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH pve-docs v4 5/5] firewall: add documentation for proxmox-firewall

2024-04-23 Thread Thomas Lamprecht
Am 19/04/2024 um 11:42 schrieb Stefan Hanreich:
> Add a section that explains how to use the new nftables-based
> proxmox-firewall.
> 
> Signed-off-by: Stefan Hanreich 
> ---
>  pve-firewall.adoc | 181 ++
>  1 file changed, 181 insertions(+)
> 
>

applied this one too now, thanks!


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



Re: [pve-devel] [PATCH manager v10 1/2] ui: machine: add viommu ComboBox

2024-04-22 Thread Thomas Lamprecht
Am 22/04/2024 um 15:24 schrieb Fiona Ebner:
> Am 22.04.24 um 15:16 schrieb Dominik Csapak:
>> On 4/22/24 15:11, Fiona Ebner wrote:
>>> Should we display some hint that Intel can/should also be used even if
>>> you have an AMD? Maybe even just in the text we display, like "Intel
>>> (also used for AMD)" but hope somebody can come up with something better.
>>
>> mhh.. i mean it is a virtual device, so should we also add this info
>> for e.g. e1000 devices?

Intel e1000 is a network card where no real relation to similar models from
AMD exist, nor is the network market a duopoly like the (x86_64) CPU market;
so this comparison does not really work IMO.

> Personally, I'd go in assuming the "Intel" setting is wrong with my AMD
> CPU and so might many users.
> 
>> since it's in the advanced section and it is documented in pve-docs,
>> i'd leave it out here (we can still add a notice later if users
>> are confused, but most users won't use/need it anyway)
> 
> Yes, we could also wait. But if the confusion can be avoided/reduced
> without much effort, I think it's worth doing up-front.

Yeah, definitively agreed, this is guaranteed to be a source of confusion
otherwise. I changed the display value to "Intel (AMD Compatible)".
Albeit now I'm thinking that it might have been slightly better to
lowercase "compatible", oh well..

https://git.proxmox.com/?p=pve-manager.git;a=commit;h=216398458d4be8781155f7d64835a38971258793


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



[pve-devel] applied: [PATCH network v2 0/3] Advertise MTU via DHCP / RA

2024-04-22 Thread Thomas Lamprecht
Am 22/04/2024 um 15:15 schrieb Stefan Hanreich:
> Changes from v1 -> v2:
> * rebased branch, everything else unchanged
> 
> pve-network:
> 
> Stefan Hanreich (3):
>   dhcp: fix function signatures in abstract class
>   zones: add method for getting MTU
>   dhcp: dnsmasq: send mtu option via dhcp
> 
>  src/PVE/Network/SDN/Dhcp.pm   |  2 +-
>  src/PVE/Network/SDN/Dhcp/Dnsmasq.pm   |  7 ++-
>  src/PVE/Network/SDN/Dhcp/Plugin.pm| 12 ++--
>  src/PVE/Network/SDN/Zones.pm  |  8 
>  src/PVE/Network/SDN/Zones/Plugin.pm   |  7 +++
>  src/PVE/Network/SDN/Zones/SimplePlugin.pm |  8 +++-
>  6 files changed, 35 insertions(+), 9 deletions(-)
> 
> 
> Summary over all repositories:
>   6 files changed, 35 insertions(+), 9 deletions(-)
> 


applied series, thanks!


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



Re: [pve-devel] [PATCH manager v10 2/2] ui: machine: add link to documentation of the system settings

2024-04-22 Thread Thomas Lamprecht
Am 22/04/2024 um 15:18 schrieb Dominik Csapak:
> On 4/22/24 15:13, Fiona Ebner wrote:
>> Why not the more accurate 'qm_machine_type' (was introduced in pve-docs = 
>> 8.1.0)?
> 
> you're right, would be even better (did not realize that existed), i/you can 
> send/push a follow up?
> 

both of you can push to this repo, for this little change I think doing so 
directly
without patch is fine


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



[pve-devel] applied: [PATCH pve-network] fix #5343 : isis: fix ipv6 && custom router config

2024-04-22 Thread Thomas Lamprecht
Am 16/04/2024 um 18:52 schrieb Alexandre Derumier:
> Signed-off-by: Alexandre Derumier 
> ---
>  src/PVE/Network/SDN/Controllers/IsisPlugin.pm| 3 ++-
>  src/test/zones/evpn/isis/expected_controller_config  | 2 ++
>  src/test/zones/evpn/isis_loopback/expected_controller_config | 2 ++
>  3 files changed, 6 insertions(+), 1 deletion(-)
> 
>

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 pve-network 0/3] Advertise MTU via DHCP / RA

2024-04-22 Thread Thomas Lamprecht
Am 14/12/2023 um 17:46 schrieb Stefan Hanreich:
> Stefan Hanreich (3):
>   dhcp: fix function signatures in abstract class
>   zones: add method for getting MTU
>   dhcp: dnsmasq: send mtu option via dhcp
> 
>  src/PVE/Network/SDN/Dhcp.pm   |  2 +-
>  src/PVE/Network/SDN/Dhcp/Dnsmasq.pm   |  7 ++-
>  src/PVE/Network/SDN/Dhcp/Plugin.pm| 12 ++--
>  src/PVE/Network/SDN/Zones.pm  |  8 
>  src/PVE/Network/SDN/Zones/Plugin.pm   |  7 +++
>  src/PVE/Network/SDN/Zones/SimplePlugin.pm |  8 +++-
>  6 files changed, 35 insertions(+), 9 deletions(-)
> 

seems OK from a high-level glance, would need a rebase now though


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



[pve-devel] applied: [PATCH v2 pve-network] fix #5364: bgp|evpn: derivated router-id from mac address for ipv6 underlay

2024-04-22 Thread Thomas Lamprecht
Am 12/04/2024 um 14:57 schrieb Alexandre Derumier:
> for ipv4, we use the iface ipv4 router-id as router-id need to 32bit.
> 
> That's doesn't work for pure ipv6 underlay network.
> 
> since https://www.rfc-editor.org/rfc/rfc6286, we can use any 32bit id,
> it's just need to be unique in the ASN.
> 
> Simply use the last 4 bytes of iface mac address as unique id
> 
> 
> changelog V2: add missing test
> 
> Signed-off-by: Alexandre Derumier 
> ---
>  src/PVE/Network/SDN/Controllers/BgpPlugin.pm  |  3 +-
>  src/PVE/Network/SDN/Controllers/EvpnPlugin.pm |  7 ++-
>  src/PVE/Network/SDN/Controllers/Plugin.pm | 21 +
>  src/test/run_test_zones.pl|  8 
>  .../ipv6underlay/expected_controller_config   | 45 +++
>  .../evpn/ipv6underlay/expected_sdn_interfaces | 42 +
>  src/test/zones/evpn/ipv6underlay/interfaces   |  7 +++
>  src/test/zones/evpn/ipv6underlay/sdn_config   | 27 +++
>  8 files changed, 157 insertions(+), 3 deletions(-)
>  create mode 100644 
> src/test/zones/evpn/ipv6underlay/expected_controller_config
>  create mode 100644 src/test/zones/evpn/ipv6underlay/expected_sdn_interfaces
>  create mode 100644 src/test/zones/evpn/ipv6underlay/interfaces
>  create mode 100644 src/test/zones/evpn/ipv6underlay/sdn_config
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH pve-network] fix #5344: isis: add isis networkid parser

2024-04-22 Thread Thomas Lamprecht
Am 16/04/2024 um 18:24 schrieb Alexandre Derumier:
> Signed-off-by: Alexandre Derumier 
> ---
>  src/PVE/Network/SDN/Controllers/IsisPlugin.pm | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
>

applied, thanks!

this had some slight conflict with the other patches that git could auto-merge
though, just mentioning for the case that something is off (I did not check it
out very closely).


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



[pve-devel] applied: [PATCH pve-network] fix #5319: frr.local: add support for bgp-community

2024-04-22 Thread Thomas Lamprecht
Am 16/04/2024 um 18:25 schrieb Alexandre Derumier:
> Need to be inserted after ip prefix-list and before route map
> 
> Signed-off-by: Alexandre Derumier 
> ---
>  src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH pve-network 1/1] api: sdn: fix missing types for 'pending' fields.

2024-04-22 Thread Thomas Lamprecht
Am 18/04/2024 um 18:44 schrieb Johannes Cornelis Draaijer:
> Signed-off-by: Johannes Cornelis Draaijer 
> ---
>  src/PVE/API2/Network/SDN/Controllers.pm | 2 +-
>  src/PVE/API2/Network/SDN/Zones.pm   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH container/docs/firewall/manager/qemu-server v4 0/5] proxmox firewall nftables

2024-04-22 Thread Thomas Lamprecht
Am 19/04/2024 um 11:42 schrieb Stefan Hanreich:
> This patch series contains the remaining patches that are necessary for
> proxmox-firewall to work. It adds documentation as well as changes how
> firewall-bridges are created when proxmox-firewall is activated. It also 
> patches
> pve-firewall to not generate rules when proxmox-firewall is active.
> 
> Dependencies:
> * qemu-server, pve-container & pve-manager depend on a bump of pve-firewall
> 
> Changes from v3 -> v4:
> * additionally check for the existence of proxmox-firewall bin
> * extracted checks into helper functions
> * update docs to reflect the changes in behavior
> 
> (omitted description & changes only relevant for the firewall itself)
> 
> qemu-server:
> 
> Stefan Hanreich (1):
>   firewall: add handling for new nft firewall
> 
>  vm-network-scripts/pve-bridge | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> 
> pve-container:
> 
> Stefan Hanreich (1):
>   firewall: add handling for new nft firewall
> 
>  src/PVE/LXC.pm | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> 
> pve-firewall:
> 
> Stefan Hanreich (1):
>   add configuration option for new nftables firewall
> 
>  src/PVE/Firewall.pm | 41 -
>  1 file changed, 32 insertions(+), 9 deletions(-)
> 
> 
> pve-manager:
> 
> Stefan Hanreich (1):
>   firewall: expose configuration option for new nftables firewall
> 
>  www/manager6/grid/FirewallOptions.js | 1 +
>  1 file changed, 1 insertion(+)
> 
> 
> pve-docs:
> 
> Stefan Hanreich (1):
>   firewall: add documentation for proxmox-firewall
> 
>  pve-firewall.adoc | 181 ++
>  1 file changed, 181 insertions(+)
> 
> 
> Summary over all repositories:
>   5 files changed, 224 insertions(+), 13 deletions(-)
> 


applied, thanks!


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



[pve-devel] applied: [PATCH manager] acme: ui: handle missing meta field in directory response

2024-04-22 Thread Thomas Lamprecht
Am 22/04/2024 um 11:01 schrieb Folke Gleumes:
> When none of the meta fields is set by the directory, the whole
> dictionary is missing from the response, leading to an exception
> when testing for fields inside it.
> 
> Signed-off-by: Folke Gleumes 
> ---
>  www/manager6/node/ACME.js | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
>

applied, thanks!

fwiw and just to have it noted, as this is pre-existing and so not the fault
of your EAB patches: this component could do well with some style rework,
manual IDs, split between viewModel and custom window.down('') querys +
change/disable/.. logic and quite a few intermediate variables that are only
used once.


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



Re: [pve-devel] [PATCH manager 1/2] ui: form: add DescriptionFieldContainer

2024-04-22 Thread Thomas Lamprecht
Am 22/04/2024 um 09:43 schrieb Dominik Csapak:
> this is a field container, showing a field on the left column and a
> description on the right one, with a (default) flex ratio of 1:2
> 
> this is helpful when wanting a longer description on the right column
> but still have the fields aligned.
> 
> Signed-off-by: Dominik Csapak 
> ---
> alternatively, we could make a more generic '2column' container, where
> we give a 'leftFieldConfig' and a 'rightFieldConfig', and just
> let the user put in a displayfield on the right?
> 
> would be the more generic solution that could also work when we don't
> want to have a description in the right column
> 

just for the record, the more generic solution is the one that got in,
making this series obsolete:

https://git.proxmox.com/?p=pve-manager.git;a=commit;h=9540e48c0fae5356c3eebabf30f2a708d8d7c939


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



[pve-devel] applied: [PATCH manager v2 1/2] ui: form: add TwoColumnContainer

2024-04-22 Thread Thomas Lamprecht
Am 22/04/2024 um 10:16 schrieb Dominik Csapak:
> this is a container, showing a widget on the left column and another one
> on the right one, with a (default) flex ratio of 1:2
> 
> this is helpful when wanting fields to align vertically in an input
> panel that have different height (e.g. because of text wrapping)
> 
> Signed-off-by: Dominik Csapak 
> ---
> changes from v1:
> * make the container more generic as a simple container that holds two
>   widgets, so it can be useful in more situations
> 
>  www/manager6/Makefile   |  1 +
>  www/manager6/form/TwoColumnContainer.js | 53 +
>  2 files changed, 54 insertions(+)
>  create mode 100644 www/manager6/form/TwoColumnContainer.js
> 
>

applied both patches with some rework, thanks!

Reworked was:
- startWidget -> startColumn and same for the end one

- expose both start and end flex, and name it flex again as it's then not
  guaranteed to be a 1/x ratio anymore, and flex is more widely used.

- exposed the columnPadding, can be nice to make it narrower for some UIs

- fixed the eslint warnings of the second patch and integrated changes from
  the first one also in there.


___
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 v6 1/3] add C program to get AMD SEV hardware parameters from CPUID

2024-04-22 Thread Thomas Lamprecht
Am 19/04/2024 um 12:59 schrieb Markus Frank
> diff --git a/amd-sev-support/amd-sev-support.c 
> b/amd-sev-support/amd-sev-support.c
> new file mode 100644
> index 000..73a7bd8
> --- /dev/null
> +++ b/amd-sev-support/amd-sev-support.c
> @@ -0,0 +1,48 @@
> +#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/amd-sev-params";
> +
> +file = fopen(filename, "w");
> +if (file == NULL) {
> + perror("Error opening file");
> + return 1;
> +}
> +
> +fprintf(file, "{"

oh, and as per my last mail it might also make sense to move this inside an
"amd-sev" object, so that extending it in the future to get other machine
capabilities can be done without potential clashes.

> + " \"cbitpos\": %u,"
> + " \"reduced-phys-bits\": %u,"
> + " \"sev\": %s,"
> + " \"sev-es\": %s,"
> + " \"sev-snp\": %s"

With above comment the three "sev" prefix options might be better off if changed
to use "sev-support" as prefix instead.

> + " }\n",
> + cbitpos,
> + reduced_phys_bits,
> + sev_support ? "true" : "false",
> + sev_es_support ? "true" : "false",
> + sev_snp_support ? "true" : "false"
> +);
> +
> +fclose(file);
> +return 0;
> +}


___
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 v6 1/3] add C program to get AMD SEV hardware parameters from CPUID

2024-04-22 Thread Thomas Lamprecht
Am 19/04/2024 um 12:59 schrieb Markus Frank:
> Implement a systemd service that runs a C program that extracts AMD SEV
> hardware parameters 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/amd-sev-params.

general comments:

I'd not call this SEV support, we will probably re-use this to get other
features from both Intel and AMD in the future, so I'd name it something
slightly more general like "query-machine-capabilities"

The output file should not pollute top-level /run, we already have
/run/qemu-server in use, so that could be used here too.

> 

misses a co-authored or originally-by me trailer ;-)

> Signed-off-by: Markus Frank 
> ---
>  Makefile|  1 +
>  amd-sev-support/Makefile| 21 +++
>  amd-sev-support/amd-sev-support.c   | 48 +
>  amd-sev-support/amd-sev-support.service | 12 +++
>  4 files changed, 82 insertions(+)
>  create mode 100644 amd-sev-support/Makefile
>  create mode 100644 amd-sev-support/amd-sev-support.c
>  create mode 100644 amd-sev-support/amd-sev-support.service
> 
> diff --git a/Makefile b/Makefile
> index 133468d..ccd12a1 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 amd-sev-support install
>   $(MAKE) -C qemu-configs install
>   $(MAKE) -C vm-network-scripts install
>   install -m 0755 qm $(DESTDIR)$(SBINDIR)
> diff --git a/amd-sev-support/Makefile b/amd-sev-support/Makefile
> new file mode 100644
> index 000..022ed94
> --- /dev/null
> +++ b/amd-sev-support/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
> +
> +amd-sev-support: amd-sev-support.c
> + $(CC) $(CFLAGS) -o $@ $< $(LDFLAGS)
> +
> +.PHONY: install
> +install: amd-sev-support
> + install -d ${DESTDIR}/${SBINDIR}
> + install -d ${DESTDIR}${SERVICEDIR}
> + install -m 0644 amd-sev-support.service ${DESTDIR}${SERVICEDIR}
> + install -m 0755 amd-sev-support ${DESTDIR}${SBINDIR}
> +
> +.PHONY: clean
> +clean:
> + rm -f amd-sev-support
> diff --git a/amd-sev-support/amd-sev-support.c 
> b/amd-sev-support/amd-sev-support.c
> new file mode 100644
> index 000..73a7bd8
> --- /dev/null
> +++ b/amd-sev-support/amd-sev-support.c
> @@ -0,0 +1,48 @@
> +#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/amd-sev-params";

besides saving this in the /run/qemu-server directory it'd be additionally
nice to add a ".json" file type suffix to clarify what format this
is in.

> +
> +file = fopen(filename, "w");
> +if (file == NULL) {
> + perror("Error opening file");
> + return 1;
> +}
> +
> +fprintf(file, "{"
> + " \"cbitpos\": %u,"
> + " \"reduced-phys-bits\": %u,"
> + " \"sev\": %s,"
> + " \"sev-es\": %s,"
> + " \"sev-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/amd-sev-support/amd-sev-support.service 
> b/amd-sev-support/amd-sev-support.service
> new file mode 100644
> index 000..466dd0a
> --- /dev/null
> +++ b/amd-sev-support/amd-sev-support.service
> @@ -0,0 +1,12 @@
> +[Unit]
> +Description=Read AMD SEV Parameters
> +RequiresMountsFor=/run
> +Before=pve-ha-lrm.service
> +Before=pve-guests.service
> +
> +[Service]
> +ExecStart=/usr/libexec/qemu-server/amd-sev-support
> +Type=forking

why is this "forking", you do not even fork once?! (nor should you)
I'd use the "oneshot" type that is designed for such use cases.
 

Korrigieren

Schließen

Grammatik

This sentence does not start with an uppercase letter.

MissesIgnorieren



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


[pve-devel] applied-series: [PATCH common/docs/firewall/manager/proxmox-widget-toolkit v4 0/6] drop vmbr prefix for bridges

2024-04-21 Thread Thomas Lamprecht
Am 12/04/2024 um 10:07 schrieb Stefan Hanreich:
> Original patch series by Jillian Morgan 
> 
> I've refrained from adding arbitrary bond names in this patch series, since
> that would require a bigger amount of changes in the firewall simulator. I'll
> look into adding that in a future patch series.
> 
> Dependencies:
>   * pve-manager requires proxmox-widget-toolkit & pve-common
>   * pve-firewall requires pve-common
> 
> Changes from v3 -> v4:
>   * Improved wording of the documentation (thanks @Fabian!)
>   * added information about dependencies
> 
> Changes from v2 -> v3:
>   * limit bridge names to 10 characters in Web UI
>   * introduce common variable for bridge names in firewall simulator
>   * update docs to reflect changes
>   * add warning for interfaces named vmbrX that are not bridges
> 
> Changes from v1 -> v2:
>   * limit name to 15 characters
>   * properly validate bridge names in vlan/qinq zones
>   * properly handle OVS bridges
>   * handle new bridge names in firewall simulator
> 
> pve-common:
> 
> Stefan Hanreich (1):
>   fix #545: interfaces: allow arbitrary bridge names in network config
> 
>  src/PVE/INotify.pm | 28 +++-
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> 
> pve-docs:
> 
> Stefan Hanreich (1):
>   network: update specification for bridge names
> 
>  pve-network.adoc | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> 
> pve-manager:
> 
> Stefan Hanreich (2):
>   sdn: qinq: vlan: properly validate bridge name
>   sdn: vlan: fix indentation in vlan edit dialogue
> 
>  www/manager6/sdn/zones/QinQEdit.js |  3 +++
>  www/manager6/sdn/zones/VlanEdit.js | 11 +++
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> 
> pve-firewall:
> 
> Stefan Hanreich (1):
>   simulator: use new bridge naming scheme
> 
>  src/PVE/FirewallSimulator.pm| 29 +++--
>  src/PVE/Service/pve_firewall.pm |  5 +++--
>  2 files changed, 22 insertions(+), 12 deletions(-)
> 
> 
> proxmox-widget-toolkit:
> 
> Stefan Hanreich (1):
>   network: allow bridges to have any valid interface name
> 
>  src/Toolkit.js  | 4 ++--
>  src/node/NetworkEdit.js | 4 +++-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> 
> Summary over all repositories:
>   8 files changed, 60 insertions(+), 38 deletions(-)
> 


applied series, thanks!


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



[pve-devel] partially-applied: [PATCH manager v2 1/2] ui: dc: backup: improve UX for the different 'notification-mode's

2024-04-21 Thread Thomas Lamprecht
Am 15/04/2024 um 11:51 schrieb Lukas Wagner:
>   - Switch order of 'mailto' and 'mailnotification' field
>   - When mode is 'auto', disable 'mailtnotification' field
>   - When mode is 'auto' and 'mailto' is empty, show
> hint that the notification system will be used

If one starts making lists about different things a commit does, it might
be good to evaluate if it should be split into multiple commits..

I dropped the hint part, this is IMO really not better than nothing in
its current state, as besides being quite flashy it doesn't really
explains what's going on.

Now with the advanced tab we could move the extra fields in there, add a
extended explanation potentially even providing a link to the local docs'
notification section (or wherever this is explained in detail in the
docs).

As compromise for now I'd accept that text without the pmx-hint class and
a link to the docs added in the text (could also be just a question mark
icon at the end, i.e. the help button without a text label).


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



Re: [pve-devel] [PATCH manager v3 5/5] fix #4474: ui: guest stop: offer to overrule active shutdown tasks

2024-04-21 Thread Thomas Lamprecht
Am 19/04/2024 um 12:17 schrieb Dominik Csapak:
> nit: imho a short high level description why we extend the messagebox instead
> of e.g. our edit/safedestroy window would be nice
> 
> also maybe we could rewrite this a bit more generic so that the safedestroy
> window users could this instead? (not necessary for now though)
> 

Yeah, I'd do the following:
- create a new proxmoxPrompt / Proxmox.window.Prompt that is a generic variant
  of the SafeDestroy button, allowing to override the confirm button text, API
  call HTTP method, adding extra items both inside the message container and
  below that, providing basic plumbing to easily integrate extra validation
  to allow confirmation and infra to show (various) hints. As both are things
  that are quite common in prompts (at least should be) it's IMO worth it to
  have native support for that functionality directly in such a general widget
  even if a widget that extends from this could add it themselves.
 

- move over SafeDestroy to this new widget as base, it probably should be one
  or two dozens lines of (mostly declarative) code then.

FWIW, I started the first part here, but it's still pretty bare bones, and I'm
not sure if I get around to finish it, especially with good polishing, in the
near future so just come to me if you, or someone else, want's to continue on
this. IMO this would be something quite worth doing, as our prompts are pretty
rough, often lacking even conveying the basic implications of (destructive)
actions, and having a good component that makes it easy to show hints or add
extra (non-intrusive) confirmation validation could help here.


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



[pve-devel] applied: [PATCH manager v3 5/5] fix #4474: ui: guest stop: offer to overrule active shutdown tasks

2024-04-20 Thread Thomas Lamprecht
Am 12/04/2024 um 16:15 schrieb Friedrich Weber:
> Implement a new "guest stop" confirmation message box which first
> checks if there is an active shutdown task for the same guest that is
> visible to the logged-in user. If there is at least one, the dialog
> displays an additional default-on checkbox for overruling active
> shutdown tasks. If the user confirms and the checkbox is checked, the
> UI sends a guest stop API request with the `overrule-shutdown`
> parameter set to 1. If there are no active shutdown tasks, or the
> checkbox is unchecked, the UI sends a guest stop API request without
> `overrule-shutdown`.
> 
> To avoid an additional API request for querying active shutdown tasks,
> check the UI's current view of cluster tasks instead, which is fetched
> from the `pve-cluster-tasks` store.
> 
> As the UI might hold an outdated task list, there are some
> opportunities for races, e.g., the UI may miss a new shutdown task or
> consider a shutdown task active even though it has already terminated.
> These races either result in a surviving shutdown task that the user
> still needs to abort manually, or a superfluous `override-shutdown=1`
> parameter that does not actually abort any tasks. Since "stop
> overrules shutdown" is merely a convenience feature, both outcomes
> seem bearable.
> 
> The confirmation message box is now always marked as dangerous (with a
> warning sign icon), whereas previously it was only marked dangerous if
> the stop issued from the guest panel, but not when issued from the
> resource tree command menu.
> 
> Signed-off-by: Friedrich Weber 
> ---
> 
> Notes:
> changes v2 -> v3:
> - aligned permission checks with changed backend logic
> - replace access of `this.vm` with `me.vm`
> 
> changes v1 -> v2:
> - instead of a second modal dialog that offers to overrule shutdown
>   tasks, display an additional checkbox in the "guest stop"
>   confirmation dialog if there is a running matching shutdown task
> - spin out pve-cluster-tasks store fix into its own patch
> 
>  www/manager6/Makefile|  1 +
>  www/manager6/lxc/CmdMenu.js  |  8 +++-
>  www/manager6/lxc/Config.js   |  8 ++--
>  www/manager6/qemu/CmdMenu.js |  8 +++-
>  www/manager6/qemu/Config.js  |  8 ++--
>  www/manager6/window/GuestStop.js | 79 
>  6 files changed, 104 insertions(+), 8 deletions(-)
>  create mode 100644 www/manager6/window/GuestStop.js
> 
>

applied, with some smaller style nits squashed in directly and a follow-up
that changed this to be shown more often, mostly because the UI task state
can be dated (slightly more rationale in the commit message), thanks!


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



[pve-devel] applied-series: [PATCH proxmox-firewall 1/2] firewall: wait for nft process

2024-04-19 Thread Thomas Lamprecht
Am 19/04/2024 um 15:00 schrieb Stefan Hanreich:
> NftClient never waits for the child process to terminate leading to
> defunct leftover processes.
> 
> Signed-off-by: Stefan Hanreich 
> ---
>  proxmox-nftables/src/client.rs | 38 --
>  1 file changed, 9 insertions(+), 29 deletions(-)
> 
>

applied both patches, thanks!


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



[pve-devel] applied: [PATCH manager] pveversion: fix whitespaces

2024-04-19 Thread Thomas Lamprecht
Am 19/04/2024 um 18:33 schrieb Alexander Zeidler:
> Signed-off-by: Alexander Zeidler 
> ---
>  bin/pveversion | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
>

applied, thanks!


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



[pve-devel] applied-series: [PATCH cluster/manager/storage/docs 0/9] fix #4886: improve SSH handling

2024-04-19 Thread Thomas Lamprecht
Am 11/01/2024 um 11:51 schrieb Fabian Grünbichler:
> this series replaces the old mechanism that used a cluster-wide merged known
> hosts file with distributing of each node's host key via pmxcfs, and pinning
> the distributed key explicitly for internal SSH connections.
> 
> the main changes in pve-cluster somewhat break the old manager and
> storage versions, but only when such a partial upgrade is mixed with a
> host key rotation of some sort.
> 
> pve-storage uses a newly introduced helper, so needs a versioned
> dependency accordingly.
> 
> the last pve-docs patch has a placeholder for the actual version shipping the
> changes which needs to be replaced when applying.
> 
> there's still some potential for follow-ups:
> - 'pvecm ssh' wrapper to debug and/or re-use the host key pinning (and other
>   future changes)
> - also add non-RSA host keys
> - key (and thus authorized keys) and/or sshd disentangling (this
>   potentially also affects external access, so might be done on a major
>   release to give more heads up)
> 
> cluster:
> 
> Fabian Grünbichler (4):
>   fix #4886: write node SSH hostkey to pmxcfs
>   fix #4886: SSH: pin node's host key if available
>   ssh: expose SSH options on their own
>   pvecm: stop merging SSH known hosts by default
> 
>  src/PVE/CLI/pvecm.pm | 10 --
>  src/PVE/Cluster/Setup.pm | 24 +---
>  src/PVE/SSHInfo.pm   | 31 +++
>  3 files changed, 56 insertions(+), 9 deletions(-)
> 
> docs:
> 
> Fabian Grünbichler (2):
>   ssh: make pitfalls a regular section instead of block
>   ssh: document PVE-specific setup
> 
>  pvecm.adoc | 26 +-
>  1 file changed, 21 insertions(+), 5 deletions(-)
> 
> manager:
> 
> Fabian Grünbichler (2):
>   vnc: use SSH command helper
>   pvesh: use SSH command helper
> 
>  PVE/API2/Nodes.pm | 3 ++-
>  PVE/CLI/pvesh.pm  | 4 ++--
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> storage:
> 
> Fabian Grünbichler (1):
>   upload: use SSH helper to get ssh/scp options
> 
>  src/PVE/API2/Storage/Status.pm | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 


applied series, thanks!


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


[pve-devel] applied-series: [PATCH qemu-server/container/docs 0/4] overrule-shutdown: documentation fixes

2024-04-18 Thread Thomas Lamprecht
Am 18/04/2024 um 15:46 schrieb Friedrich Weber:
> All patches are optional:
> 
> - 1/4 fixes spacing and punctuation in the qmshutdown/qmstop descriptions
> - 2/4 rewords the overrule-shutdown description for VMs
> - 3/4 is the same change for containers
> - 4/4 adds a usage example for qm stop -overrule-shutdown to the docs
> 
> qemu-server:
> 
> Friedrich Weber (2):
>   api: fix spacing and punctuation in shutdown and stop descriptions
>   api: stop: reword overrule-shutdown parameter description
> 
>  PVE/API2/Qemu.pm | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> 
> container:
> 
> Friedrich Weber (1):
>   api: stop: reword overrule-shutdown parameter description
> 
>  src/PVE/API2/LXC/Status.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> docs:
> 
> Friedrich Weber (1):
>   qm: add overrule-shutdown to CLI usage examples
> 
>  qm.adoc | 7 +++
>  1 file changed, 7 insertions(+)
> 
> 
> Summary over all repositories:
>   3 files changed, 14 insertions(+), 0 deletions(-)
> 


applied series, 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 v3 35/39] firewall: add handling for new nft firewall

2024-04-18 Thread Thomas Lamprecht
Am 18/04/2024 um 18:14 schrieb Stefan Hanreich:
> When the nftables firewall is enabled, we do not need to create
> firewall bridges.
> 
> Signed-off-by: Stefan Hanreich 
> ---
>  vm-network-scripts/pve-bridge | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/vm-network-scripts/pve-bridge b/vm-network-scripts/pve-bridge
> index 85997a0..ac2eb3b 100755
> --- a/vm-network-scripts/pve-bridge
> +++ b/vm-network-scripts/pve-bridge
> @@ -6,6 +6,7 @@ use warnings;
>  use PVE::QemuServer;
>  use PVE::Tools qw(run_command);
>  use PVE::Network;
> +use PVE::Firewall;
>  
>  my $have_sdn;
>  eval {
> @@ -44,13 +45,17 @@ die "unable to get network config '$netid'\n"
>  my $net = PVE::QemuServer::parse_net($netconf);
>  die "unable to parse network config '$netid'\n" if !$net;
>  
> +my $cluster_fw_conf = PVE::Firewall::load_clusterfw_conf();
> +my $host_fw_conf = PVE::Firewall::load_hostfw_conf($cluster_fw_conf);
> +my $firewall = $net->{firewall} && !($host_fw_conf->{options}->{nftables} // 
> 0);

we could add a helper for this in PVE::Firewall to make this and the container
one a bit shorter, while it's not much we'd have to bump firewall anyway, so
not a high cost to do.

> +
>  if ($have_sdn) {
>  PVE::Network::SDN::Vnets::add_dhcp_mapping($net->{bridge}, 
> $net->{macaddr}, $vmid, $conf->{name});
>  PVE::Network::SDN::Zones::tap_create($iface, $net->{bridge});
> -PVE::Network::SDN::Zones::tap_plug($iface, $net->{bridge}, $net->{tag}, 
> $net->{firewall}, $net->{trunks}, $net->{rate});
> +PVE::Network::SDN::Zones::tap_plug($iface, $net->{bridge}, $net->{tag}, 
> $firewall, $net->{trunks}, $net->{rate});
>  } else {
>  PVE::Network::tap_create($iface, $net->{bridge});
> -PVE::Network::tap_plug($iface, $net->{bridge}, $net->{tag}, 
> $net->{firewall}, $net->{trunks}, $net->{rate});
> +PVE::Network::tap_plug($iface, $net->{bridge}, $net->{tag}, $firewall, 
> $net->{trunks}, $net->{rate});
>  }
>  
>  exit 0;



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



Re: [pve-devel] [PATCH pve-firewall v2 37/39] add configuration option for new nftables firewall

2024-04-18 Thread Thomas Lamprecht
Am 17/04/2024 um 15:54 schrieb Stefan Hanreich:
> Introduces new nftables configuration option that en/disables the new
> nftables firewall.
> 
> pve-firewall reads this option and only generates iptables rules when
> nftables is set to `0`. Conversely proxmox-firewall only generates
> nftables rules when the option is set to `1`.

this is missing from your v3, but I assume it staid the same.

> 
> Signed-off-by: Stefan Hanreich 
> ---
>  src/PVE/Firewall.pm | 20 
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index 81a8798..b39843d 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -1408,6 +1408,12 @@ our $host_option_properties = {
>   default => 0,
>   optional => 1
>  },
> +nftables => {
> + description => "Enable nftables based firewall",
> + type => 'boolean',
> + default => 0,
> + optional => 1,
> +},
>  };
>  
>  our $vm_option_properties = {
> @@ -2929,7 +2935,7 @@ sub parse_hostfw_option {
>  
>  my $loglevels = "emerg|alert|crit|err|warning|notice|info|debug|nolog";
>  
> -if ($line =~ 
> m/^(enable|nosmurfs|tcpflags|ndp|log_nf_conntrack|nf_conntrack_allow_invalid|protection_synflood):\s*(0|1)\s*$/i)
>  {
> +if ($line =~ 
> m/^(enable|nosmurfs|tcpflags|ndp|log_nf_conntrack|nf_conntrack_allow_invalid|protection_synflood|nftables):\s*(0|1)\s*$/i)
>  {
>   $opt = lc($1);
>   $value = int($2);
>  } elsif ($line =~ 
> m/^(log_level_in|log_level_out|tcp_flags_log_level|smurf_log_level):\s*(($loglevels)\s*)?$/i)
>  {
> @@ -4676,7 +4682,11 @@ sub remove_pvefw_chains_ebtables {
>  sub init {
>  my $cluster_conf = load_clusterfw_conf();
>  my $cluster_options = $cluster_conf->{options};
> -my $enable = $cluster_options->{enable};
> +
> +my $host_conf = load_hostfw_conf($cluster_conf);
> +my $host_options = $host_conf->{options};
> +
> +my $enable = $cluster_options->{enable} && !$host_options->{nftables};

should we also check for -x '/usr/libexec/proxmox/proxmox-firewall' to consider
the nftables option enabled, as otherwise one might tick the check box while not
having the proxmox-firewall package installed (we probably make that only a
Recommends for now), which would then mean no firewall at all..

>  
>  return if !$enable;
>  
> @@ -4689,12 +4699,14 @@ sub update {
>   my $cluster_conf = load_clusterfw_conf();
>   my $cluster_options = $cluster_conf->{options};
>  
> - if (!$cluster_options->{enable}) {
> + my $hostfw_conf = load_hostfw_conf($cluster_conf);
> + my $host_options = $hostfw_conf->{options};
> +
> + if (!$cluster_options->{enable} || $host_options->{nftables}) {

same here, maybe add this in a helper

>   PVE::Firewall::remove_pvefw_chains();
>   return;
>   }
>  
> - my $hostfw_conf = load_hostfw_conf($cluster_conf);
>  
>   my ($ruleset, $ipset_ruleset, $rulesetv6, $ebtables_ruleset) = 
> compile($cluster_conf, $hostfw_conf);
>  



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



[pve-devel] applied: [PATCH manager 3/7] report: add `apt-cache policy` to list recognized APT sources

2024-04-18 Thread Thomas Lamprecht
Am 18/04/2024 um 11:16 schrieb Alexander Zeidler:
> with their details as well as pinned packages. Omit the "origin"
> lines, as their value is already visible in the URLs.
> 
>  # apt-cache policy ...
>  Package files:
>   100 /var/lib/dpkg/status
>   release a=now
>   500 https://enterprise.proxmox.com/debian/pve bookworm/pve-enterprise amd64 
> Packages
>   release o=Proxmox,a=stable,n=bookworm,l=Proxmox VE Enterprise Debian 
> Repository,c=pve-enterprise,b=amd64
>  ...
>  Pinned packages:
>   intel-microcode -> 3.20231114.1~deb12u1 with priority 1234
> 
> Signed-off-by: Alexander Zeidler 
> ---
> Expects applied:
> report: fix regex of config filenames
> https://lists.proxmox.com/pipermail/pve-devel/2024-April/063254.html
> 
> 
> v2:
> * no changes
> 
> v1: https://lists.proxmox.com/pipermail/pve-devel/2024-March/062344.html
> 
> 
>  PVE/Report.pm | 1 +
>  1 file changed, 1 insertion(+)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH manager] report: fix regex of config filenames

2024-04-18 Thread Thomas Lamprecht
Am 18/04/2024 um 09:44 schrieb Alexander Zeidler:
> to only match those that are correct/accepted by their software
> 
> Signed-off-by: Alexander Zeidler 
> ---
>  PVE/Report.pm | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH manager 4/7] report: list held back packages

2024-04-18 Thread Thomas Lamprecht
Am 18/04/2024 um 11:16 schrieb Alexander Zeidler:
> Signed-off-by: Alexander Zeidler 
> ---
> v2:
> * newly added
> 
> 
>  PVE/Report.pm | 1 +
>  1 file changed, 1 insertion(+)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH manager 1/7] report: add kernel command line from current boot

2024-04-18 Thread Thomas Lamprecht
Am 18/04/2024 um 11:16 schrieb Alexander Zeidler:
> to get a first clue for debugging passthrough and similar issues, when
> no dmesg output has been provided yet.
> 
> Signed-off-by: Alexander Zeidler 
> ---
> v2:
> * move away from dmesg base
> * only print kernel command line (boot times can be added by another patch)
> 
> v1: https://lists.proxmox.com/pipermail/pve-devel/2024-March/062342.html
> 
> 
>  PVE/Report.pm | 1 +
>  1 file changed, 1 insertion(+)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH manager 2/7] report: create "jobs" section, add `jobs.cfg`

2024-04-18 Thread Thomas Lamprecht
Am 18/04/2024 um 11:16 schrieb Alexander Zeidler:
> to recognize temporal correlations with network/load/backup/etc issues
> 
> Suggested-by: Friedrich Weber 
> Signed-off-by: Alexander Zeidler 
> ---
> v2:
> * move away from "general" section
> 
> v1: https://lists.proxmox.com/pipermail/pve-devel/2024-March/062346.html
> 
> 
>  PVE/Report.pm | 6 ++
>  1 file changed, 6 insertions(+)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH manager] ui: acl: group selector: allow search

2024-04-18 Thread Thomas Lamprecht
Am 12/04/2024 um 11:16 schrieb Fiona Ebner:
> Makes it consistent with the user selector and token selector.
> 
> Requested in the community forum:
> https://forum.proxmox.com/threads/144978/
> 
> Signed-off-by: Fiona Ebner 
> ---
>  www/manager6/form/GroupSelector.js | 4 
>  1 file changed, 4 insertions(+)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH qemu] Makefile: drop -j option from dpkg-buildpackage

2024-04-18 Thread Thomas Lamprecht
Am 12/04/2024 um 14:26 schrieb Fiona Ebner:
> From man dpkg-buildpackage:
> 
>> -j, --jobs[=jobs|auto]
>> Specifies the number of jobs allowed to be run simultaneously (since
>> dpkg 1.14.7, long option since dpkg 1.18.8). The number of jobs
>> matching the number of online processors if auto is specified (since
>> dpkg 1.17.10), or unlimited number if jobs is not specified. The
>> default behavior is auto (since dpkg 1.18.11) in non-forced mode
>> (since dpkg 1.21.10), and as such it is always safer to use with any
>> package including those that are not parallel-build safe.
> 
> The option was added in the Makefile by commit 4ba321f ("build qemu
> multithreaded") which states:
> 
>> same as in pve-kernel where we have --jobs=auto
> 
> But according to the man page, -j without an argument is not the same
> and means unlimited. Using the number of online cores seems more
> sensible and was the original intention. Again, according to the man
> page, the default is auto since dpkg 1.18.11 (or Debian Stretch), so
> just drop the option.
> 
> The motivation to look into this was that after the recent upstream
> commit d1ce2cc95b ("Makefile: preserve --jobserver-auth argument when
> calling ninja") having -j as the make flag would be broken as it was
> mistakenly passed to ninja (for which the argument for -j is not
> optional). Should get fixed soon [0].
> 
> [0]: 
> https://lore.kernel.org/qemu-devel/20240412100401.20047-2-pbonz...@redhat.com/T/#u
> 
> Signed-off-by: Fiona Ebner 
> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH manager] api: apt versions: track optional amd64/intel-microcode packages

2024-04-18 Thread Thomas Lamprecht
Am 18/04/2024 um 09:44 schrieb Alexander Zeidler:
> Signed-off-by: Alexander Zeidler 
> ---
>  PVE/API2/APT.pm | 2 ++
>  1 file changed, 2 insertions(+)
> 
>

applied, thanks!


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



[pve-devel] applied: [PATCH docs] secure boot: mention proxmox-secure-boot-support metapackage

2024-04-18 Thread Thomas Lamprecht
Am 18/04/2024 um 16:24 schrieb Stoiko Ivanov:
> Signed-off-by: Stoiko Ivanov 
> ---
> Just had the opportunity to try this on a testsystem - it worked flawlessly :)
> 
> I did consider dropping the explicit list of packages and replace it by the
> metapackage only, but think that the additional explanation of how they
> interact is worth keeping.
> 
>  system-booting.adoc | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
>

applied, with some slight rewording in a follow-up, thanks!


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



[pve-devel] partially-applied-series: [PATCH container/docs/firewall/manager/proxmox-firewall/qemu-server v3 00/39] proxmox firewall nftables implementation

2024-04-18 Thread Thomas Lamprecht
Am 18/04/2024 um 18:13 schrieb Stefan Hanreich:
> proxmox-firewall:
> 
> Stefan Hanreich (34):
>   config: add proxmox-ve-config crate
>   config: firewall: add types for ip addresses
>   config: firewall: add types for ports
>   config: firewall: add types for log level and rate limit
>   config: firewall: add types for aliases
>   config: host: add helpers for host network configuration
>   config: guest: add helpers for parsing guest network config
>   config: firewall: add types for ipsets
>   config: firewall: add types for rules
>   config: firewall: add types for security groups
>   config: firewall: add generic parser for firewall configs
>   config: firewall: add cluster-specific config + option types
>   config: firewall: add host specific config + option types
>   config: firewall: add guest-specific config + option types
>   config: firewall: add firewall macros
>   config: firewall: add conntrack helper types
>   nftables: add crate for libnftables bindings
>   nftables: add helpers
>   nftables: expression: add types
>   nftables: expression: implement conversion traits for firewall config
>   nftables: statement: add types
>   nftables: statement: add conversion traits for config types
>   nftables: commands: add types
>   nftables: types: add conversion traits
>   nftables: add nft client
>   firewall: add firewall crate
>   firewall: add base ruleset
>   firewall: add config loader
>   firewall: add rule generation logic
>   firewall: add object generation logic
>   firewall: add ruleset generation logic
>   firewall: add proxmox-firewall binary and move existing code into lib
>   firewall: add files for debian packaging
>   firewall: add integration test
> 

applied above proxmox-firewall patches, thanks!

I squashed in some fixes into the packaging change and rebased the whole thing
to fix the git trailers order (that one should grow only downward, so the R-b
one go below your S-o-b as they came in later)

Also created public repos and uploaded a build to our internal repo.


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



[pve-devel] applied: [PATCH v3 container] fix #4846: Avoid the outdated noacl mount option on ext4

2024-04-18 Thread Thomas Lamprecht
Am 17/04/2024 um 16:35 schrieb Filip Schauer:
> Do not use the 'noacl' mount option when mounting a container disk with
> an ext4 file system. The option was removed from the kernel in commit
> 2d544ec923db
> 
> Signed-off-by: Filip Schauer 
> ---
> Changes since v3:
> * Simplify ext4 detection
> * Do not add noacl if $acl is undefined
> 
>  src/PVE/LXC.pm | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
>

applied, with some clean ups on top, making this shorter and covering
Fabians suggestions, thanks!


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



Re: [pve-devel] [PATCH installer v6 36/36] autoinst-helper: add prepare-iso subcommand

2024-04-18 Thread Thomas Lamprecht
Am 18/04/2024 um 10:48 schrieb Christoph Heiss:
> Do we really need _yet another_ crate dependency for that? Below is a
> check / bail! anyway when running the command proper.
> 
> And if we really want a explicit check beforehand, I'd just do something
> like
> 
>   fn which(name: ) -> Result<()> {
>   match Command::new(name).output() {
>   Ok(_) => Ok(()),
>   Err(err) => Err(err.into()),
>   }
>   }


yeah, +1. I'd just catch the error and execute and wrap it as nicely as
possibly to convey to the user that xorriso is required somewhere in $PATH.


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



[pve-devel] applied: [PATCH http-server v3] http: support Content-Encoding=deflate

2024-04-18 Thread Thomas Lamprecht
Am 18/04/2024 um 11:16 schrieb Maximiliano Sandoval:
> Add support for compressing the body of responses with
> `Content-Encoding: deflate` following [RFC9110]. Note that in this
> context `deflate` is actually a "zlib" data format as defined in
> [RFC1950].
> 
> To preserve the current behavior we prefer `Content-Encoding: gzip`
> whenever `gzip` is listed as one of the encodings in the
> `Accept-Encoding` header and the data should be compressed.
> 
> [RFC9110] https://www.rfc-editor.org/rfc/rfc9110#name-deflate-coding
> [RFC1950] https://www.rfc-editor.org/rfc/rfc1950
> 
> Suggested-by: Lukas Wagner 
> Tested-by: Folke Gleumes 
> Signed-off-by: Maximiliano Sandoval 
> ---
> 
> Differences from v2:
>  - Remove redundant post-ifs
>  - Add Tested-by: Folke Gleumes trailer

thanks, but please add new trailers always to the bottom of the existing
trailer list.

> 
> Differences from v1:
>  - The commit documents the behavior better
>  - Add Suggested-by
>  - We set both gzip and deflate in the Accept-Encoding header if available
> 
>  src/PVE/APIServer/AnyEvent.pm | 28 ++--
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
>

applied, thanks!


While I applied this, I still got some questions:
- this uses the default compression level [0], was this chosen by default,
  did you test throughput of the various different levels, e.g. with some
  bigger API call to see if it would be worth it to go for a lower or higher
  level – doing a rough evaluation of this might still be good.

- you use the compression interface, not the deflate one, the former talks
  about RFC 1950 data streams, which often are using the deflate algorithm
  but, as per that RFC, do not necessarily have to.
  Why not use the deflate interface to ensure it's always using the correct
  algorithm?

[0]: https://metacpan.org/pod/Compress::Zlib#COMPRESS/UNCOMPRESS


___
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 v4 1/2] config: QEMU AMD SEV enable

2024-04-18 Thread Thomas Lamprecht
Am 18/04/2024 um 10:25 schrieb Markus Frank:
> This patch is for enabling AMD SEV (Secure Encrypted
> Virtualization) support in QEMU.
> 
> VM-Config-Examples:
> amd_sev: type=std,nodbg=1,noks=1
> amd_sev: es,nodbg=1,kernel-hashes=1
> 
> Node-Config-Example (gets generated automatically):
> amd_sev: cbitpos=47,reduced-phys-bios=1
> 
> kernel-hashes, reduced-phys-bios & cbitpos correspond to the varibles
> with the same name in qemu.
> 
> kernel-hashes=1 adds kernel-hashes to enable measured linux kernel
> launch since it is per default off for backward compatibility.
> 
> reduced-phys-bios and cbitpos are system specific and can be read out
> with QMP. If not set by the user, a dummy-vm gets started to read QMP
> for these variables out and save them to the node config.
> Afterwards the dummy-vm gets stopped.
> 
> type=std stands for standard sev to differentiate it from sev-es (es)
> or sev-snp (snp) when support is upstream.
> 
> QEMU's sev-guest policy gets calculated with the parameters nodbg & noks
> These parameters correspond to policy-bits 0 & 1.
> If type is 'es' than policy-bit 2 gets set to 1 to activate SEV-ES.
> Policy bit 3 (nosend) is always set to 1, because migration
> features for sev are not upstream yet and are attackable.
> 
> SEV-ES is very experimental since it could not be tested.
> 
> see coherent doc patch
> 
> Signed-off-by: Markus Frank 
> ---
> v4: 
> * reduced lines of code
> * added text that SEV-ES is experimental
> 
>  PVE/API2/Qemu.pm  |  10 
>  PVE/QemuServer.pm | 117 ++
>  2 files changed, 127 insertions(+)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 497987f..23d3cd7 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -4616,6 +4616,10 @@ __PACKAGE__->register_method({
>   # test if VM exists
>   my $conf = PVE::QemuConfig->load_config($vmid);
>  
> + if ($conf->{amd_sev}) {
> + die "AMD SEV does not support migration\n";
> + }
> +
>   # try to detect errors early
>  
>   PVE::QemuConfig->check_lock($conf);
> @@ -5170,6 +5174,12 @@ __PACKAGE__->register_method({
>   die "unable to use snapshot name 'pending' (reserved name)\n"
>   if lc($snapname) eq 'pending';
>  
> + my $conf = PVE::QemuConfig->load_config($vmid);
> +
> + if ($conf->{amd_sev}) {
> + die "AMD SEV does not support snapshots\n"
> + }
> +
>   my $realcmd = sub {
>   PVE::Cluster::log_msg('info', $authuser, "snapshot VM $vmid: 
> $snapname");
>   PVE::QemuConfig->snapshot_create($vmid, $snapname, 
> $param->{vmstate},
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 6e2c805..ca26fc5 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -61,6 +61,7 @@ use PVE::QemuServer::Monitor qw(mon_cmd);
>  use PVE::QemuServer::PCI qw(print_pci_addr print_pcie_addr 
> print_pcie_root_port parse_hostpci);
>  use PVE::QemuServer::QMPHelpers qw(qemu_deviceadd qemu_devicedel 
> qemu_objectadd qemu_objectdel);
>  use PVE::QemuServer::USB;
> +use PVE::NodeConfig;

this is from pve-manager and adds a cyclic dependency

>  
>  my $have_sdn;
>  eval {
> @@ -185,6 +186,59 @@ my $agent_fmt = {
>  },
>  };
>  
> +my $sev_fmt = {
> +type => {
> + description => "Enable standard SEV with type='std' or enable 
> experimental SEV-ES"
> + ." with the 'es' option.",
> + type => 'string',
> + default_key => 1,
> + format_description => "qemu-sev-type",
> + enum => ['std', 'es'],
> + maxLength => 3,
> +},
> +nodbg => {
> + description => "Sets policy bit 0 to 1 to disallow debugging of guest",
> + type => 'boolean',
> + format_description => "qemu-sev-nodbg",
> + default => 0,
> + optional => 1,
> +},
> +noks => {
> + description => "Sets policy bit 1 to 1 to disallow key sharing with 
> other guests",
> + type => 'boolean',
> + format_description => "qemu-sev-noks",
> + default => 0,
> + optional => 1,
> +},
> +"kernel-hashes" => {
> + description => "Add kernel hashes to guest firmware for measured linux 
> kernel launch",
> + type => 'boolean',
> + format_description => "qemu-sev-kernel-hashes",
> + default => 0,
> + optional => 1,
> +},
> +};
> +PVE::JSONSchema::register_format('pve-qemu-sev-fmt', $sev_fmt);
> +
> +my $sev_node_fmt = {
> +cbitpos => {
> + description => "C-bit: marks if a memory page is protected. System 
> dependent",
> + type => 'integer',
> + default => 47,
> + optional => 1,
> + minimum => 0,
> + maximum => 100,
> +},
> +'reduced-phys-bits' => {
> + description => "Number of bits the physical address space is reduced 
> by. System dependent",
> + type => 'integer',
> + default => 1,
> + optional => 1,
> + minimum => 0,
> + maximum => 100,
> +},
> +};

even if it was OK to use PVE::NodeConfig here, which currently it isn't due to
the cyclic dependency 

Re: [pve-devel] [PATCH v2 container] fix #4846: Avoid the outdated noacl mount option on ext4

2024-04-18 Thread Thomas Lamprecht
Am 18/04/2024 um 10:48 schrieb Fabian Grünbichler:
> On April 18, 2024 10:17 am, Thomas Lamprecht wrote:
>> Am 11/04/2024 um 15:44 schrieb Fabian Grünbichler:
>>> if $storage && $format eq 'raw' => no noacl ?
>>
>> shouldn't this branch be taken if the format is _not_ raw, as only in that
>> case it might not use ext4?
>>
> 
> if the format is raw (presumed ext4), don't set 'noacl'
> - I think that is correct? we want to avoid 'noacl' for ext4..
> 
> note the double negation (no noacl) ;)
yeah, OK, the double negation got me confused here and in
Filip's v3, thanks for clearing this up.


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


[pve-devel] applied: [PATCH guest-common] replication: snapshot cleanup: only attempt to remove snapshots that exist

2024-04-18 Thread Thomas Lamprecht
Am 18/04/2024 um 09:06 schrieb Fiona Ebner:
> Since commit a6f5b35 ("replication: prepare: include volumes without
> snapshots in the result"), attempts would be made to remove previous
> replication snapshots from volumes on which they didn't exist. This
> was noticed by Thomas since the output of a replication test in
> pve-manager changed.
> 
> The issue is not completely new, i.e. there was no check that the
> (previous) replication snapshot acutally exists before attempting
> removal during the cleanup phase. Fix the issue by adding such a
> check.
> 
> The $replicate_snapshots hash is only used for this, so the change
> there is fine.
> 
> Fixes: a6f5b35 ("replication: prepare: include volumes without snapshots in 
> the result")
> Reported-by: Thomas Lamprecht 
> Signed-off-by: Fiona Ebner 
> ---
>  src/PVE/Replication.pm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
>

applied both patches, thanks!


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



Re: [pve-devel] [PATCH v2 container] fix #4846: Avoid the outdated noacl mount option on ext4

2024-04-18 Thread Thomas Lamprecht
Am 11/04/2024 um 15:44 schrieb Fabian Grünbichler:
> if $storage && $format eq 'raw' => no noacl ?


shouldn't this branch be taken if the format is _not_ raw, as only in that
case it might not use ext4?


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


Re: [pve-devel] [PATCH storage] plugin: move definition for 'port' option to base plugin

2024-04-18 Thread Thomas Lamprecht
Am 18/04/2024 um 09:38 schrieb Fiona Ebner:
> I was thinking, users might stumble upon this e.g. with "man pvesm", and
> then try it for storages like NFS and wonder why it doesn't work. With
> the "options" option we also explicitly mention NFS/CIFS. I'll send a v2
> without mentioning PBS/ESXi if you still want me to after reading my
> rationale (should remember to also mention such seemingly little things
> in the commit message).

Yeah, some users might indeed be confused about NFS or the like..
how about adding this info at the end and as example, to avoid conveying
that this is a definitive list, e.g.: "... (for example, PBS or ESXi)"?


___
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

2024-04-18 Thread Thomas Lamprecht
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.


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


[pve-devel] applied: [PATCH manager v2 0/2] fix #5093 add custom directory and eab to ui

2024-04-17 Thread Thomas Lamprecht
Am 17/04/2024 um 17:55 schrieb Folke Gleumes:
> This patch series adds the option to set a custom directory for ACME and
> enables the user to use external account binding, which is required by
> some providers.
> 
> manager:
> 
> Folke Gleumes (2):
>   fix #5093: webui: acme: custom directory option
>   webui: acme: add eab fields
> 
>  www/manager6/node/ACME.js | 167 ++
>  1 file changed, 135 insertions(+), 32 deletions(-)
> 
> 
> Summary over all repositories:
>   1 files changed, 135 insertions(+), 32 deletions(-)
> 


applied series, thanks!


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



  1   2   3   4   5   6   7   8   9   10   >