Re: [pve-devel] applied: [PATCH http-server v3 1/2] proxy request: forward json content type and parameters

2023-06-07 Thread Thomas Lamprecht
Am 07/06/2023 um 13:49 schrieb Dominik Csapak:
> 
> 
> On 6/7/23 13:47, Thomas Lamprecht wrote:
>> Am 07/06/2023 um 13:23 schrieb Wolfgang Bumiller:
>>> applied both & bumped dependency for pve-common
>>>
>>
>> Isn't this breaking older common too?
>>
> 
> no it just forwards the parameters to either the daemon or the other nodes as 
> json if the proxy received it as json, should not have
> any other effects...

not this one 2/2, just replied here as it was the applied patch


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



Re: [pve-devel] [PATCH manager 3/3] ui: GuestStatusView: add 'privileged' and 'ostype' fields

2023-06-07 Thread Thomas Lamprecht
Am 03/05/2023 um 11:50 schrieb Christoph Heiss:
> The privileged status is displayed by adding a new row to the status
> panel, while the distribution logo and name are displayed on the right
> side of the title bar of the status panel. The latter fits neatly there,
> is rather unintrusive and yet still visible at first sight.
> 
> This also solves the problem of having to create a bigger row, so that
> the icon is still easily recognisable. At the default font-size of
> 13pt, this really wasn't the case.
> 
> I verified that each supported distro is present in the font and the
> name matches up and tested through all supported distros (including
> 'unmanaged').
> 
> Signed-off-by: Christoph Heiss 
> ---
>  www/manager6/panel/GuestStatusView.js | 51 ++-
>  1 file changed, 50 insertions(+), 1 deletion(-)
> 

please split this patch up, this is adding two relatively unrelated things.



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



Re: [pve-devel] applied: [PATCH http-server v3 1/2] proxy request: forward json content type and parameters

2023-06-07 Thread Thomas Lamprecht
Am 07/06/2023 um 13:23 schrieb Wolfgang Bumiller:
> applied both & bumped dependency for pve-common
> 

Isn't this breaking older common too?



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



Re: [pve-devel] [PATCH manager 2/3] ui: add 'font-logos' stylesheet and font files

2023-06-07 Thread Thomas Lamprecht
Am 03/05/2023 um 11:50 schrieb Christoph Heiss:
> From [0]. As they are licensed under the "Unlicence" license, including
> them here and re-distributing them should bear no problems [1].
> 
> [0] https://github.com/Lukas-W/font-logos
> [1] https://choosealicense.com/licenses/unlicense/
> 
> Signed-off-by: Christoph Heiss 
> ---
> Formatted the patch with `--no-binary` for now to ease reviewing, as it
> otherwise would be pretty big for the mailing list, due to the binary
> files. For testing, either download the latest release [2] and copy
> `assets/font-logs.{ttf,woff,woff2}` from the extraced folder to
> `www/css/fonts/`, or drop me a line and I can send the full patch
> off-list.
> 
> There's also the possibility to build a customized version of the font
> from source, to reduce it down to just the logos needed for the ten
> distributions currently supported/recogized. The file sizes are reduced
> to about 10-20% of their original files, so might be worth to do that -
> for example, the .woff goes from 73kB to 9.2kB.
> 
> It's fairly easy, basically just throwing out all unneeded entries in
> the `icons.tsv` file in the repo. If that would be preferred, I happily
> do that; adding some instructions too on how to replicate/rebuild the
> font as needed.
> 
> [2] 
> https://github.com/lukas-w/font-logos/releases/download/v1.0.1/font-logos-1.0.1.zip
> 
>  www/css/Makefile   |   3 +-
>  www/css/font-logos.css | 224 +
>  www/css/fonts/Makefile |  14 +++
>  www/css/fonts/README.md|   5 +
>  www/css/fonts/font-logos.ttf   | Bin 0 -> 29116 bytes
>  www/css/fonts/font-logos.woff  | Bin 0 -> 74576 bytes
>  www/css/fonts/font-logos.woff2 | Bin 0 -> 15724 bytes
>  www/index.html.tpl |   1 +
>  8 files changed, 246 insertions(+), 1 deletion(-)
>  create mode 100644 www/css/font-logos.css
>  create mode 100644 www/css/fonts/Makefile
>  create mode 100644 www/css/fonts/README.md
>  create mode 100644 www/css/fonts/font-logos.ttf
>  create mode 100644 www/css/fonts/font-logos.woff
>  create mode 100644 www/css/fonts/font-logos.woff2


Please, let's not repeat the mistake again and pull in some binary artefacts in
manager even if they don't are directly related to it.

Lets set up a separate package, that way we can also give proper attribution
(even if the license used wouldn't necessarily require it).

I created:
https://git.proxmox.com/?p=fonts-font-logos.git;a=summary

It contains a trivial packaging, the build fonts and the CSS file, but with the
web-font path adopted to use ../fonts/ to load them, just like Font Awesome 
does.

That way we can add it to pveproxy in a similar manner, please test if that 
works
out for you.


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



Re: [pve-devel] [RFC storage] content-dirs: check that all content dirs are pairwise inequal

2023-06-07 Thread Thomas Lamprecht
Am 07/06/2023 um 12:18 schrieb Friedrich Weber:
> On 07/06/2023 12:01, Fiona Ebner wrote:
>> Should we also check in the create/update API calls for syntactic
>> duplicates and fail the call? E.g. I can successfully issue:
>> pvesh set /storage/foo --content-dirs backup=bar,iso=bar
>> and only get the error later during activation.

Definitively a good idea (tbh. I thought it acted already that way there too) 

> Not allowing users to configure `content-dirs` with syntactic duplicates
> sounds good to me (it wouldn't catch situations involving symlinks, but
> those require quite some manual hacking from the user's side anyway).
> 
> As this would be mostly a convenience feature, I'd send a patch after
> next week if that's okay.
> 

Yeah that's fine.



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



Re: [pve-devel] applied: [PATCH edk2-firmware] add patch to work around older guest kernel bug

2023-06-07 Thread Thomas Lamprecht
Am 07/06/2023 um 12:12 schrieb Fiona Ebner:
>> Hmm, it's correct in my repository and in the formatted patch, but not
>> here in the mail anymore. Do I need to specify something special with
>> git send-email?
>>
> I asked Stoiko and sending the mail to him or myself directly works as
> intended, so it might be an issue with mailman.
> 
> If anybody wants to send an edk2-firmware patch until there is time to
> look at the issue more closely, he said that the
> --transfer-encoding=base64 option for git send-email might be worth a shot.


And possibly the `--keep-cr` option of `git am`


___
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: clean up remnants of in-tree font-awesome files

2023-06-07 Thread Thomas Lamprecht
Am 03/05/2023 um 11:50 schrieb Christoph Heiss:
> Commit e97c2601 ("change to debian font-awesome") removed the usage of
> the in-tree font-awesome files, replacing them with the Debian package.
> Thus clear out these leftovers out, as they are completely usused.
> 
> Signed-off-by: Christoph Heiss 
> ---
> Formatted the patch with `--irreversible-delete` for now to be more
> suitable for review, as otherwise it would be pretty big (~400kB) for
> the mailing list.
> 
>  www/css/font-awesome.css| 2086 ---
>  www/css/fonts/FontAwesome.otf   |  Bin 109688 -> 0 bytes
>  www/css/fonts/Makefile  |   12 -
>  www/css/fonts/README|   11 -
>  www/css/fonts/fontawesome-webfont.eot   |  Bin 70807 -> 0 bytes
>  www/css/fonts/fontawesome-webfont.svg   |  655 ---
>  www/css/fonts/fontawesome-webfont.ttf   |  Bin 142072 -> 0 bytes
>  www/css/fonts/fontawesome-webfont.woff  |  Bin 83588 -> 0 bytes
>  www/css/fonts/fontawesome-webfont.woff2 |  Bin 66624 -> 0 bytes
>  9 files changed, 2764 deletions(-)
>  delete mode 100644 www/css/font-awesome.css
>  delete mode 100644 www/css/fonts/FontAwesome.otf
>  delete mode 100644 www/css/fonts/Makefile
>  delete mode 100644 www/css/fonts/README
>  delete mode 100644 www/css/fonts/fontawesome-webfont.eot
>  delete mode 100644 www/css/fonts/fontawesome-webfont.svg
>  delete mode 100644 www/css/fonts/fontawesome-webfont.ttf
>  delete mode 100644 www/css/fonts/fontawesome-webfont.woff
>  delete mode 100644 www/css/fonts/fontawesome-webfont.woff2
> 
>

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-access-control 1/1] rpcenv: compute_api_permission : add default root dc.Sys.Modify

2023-06-07 Thread Thomas Lamprecht
Am 27/03/2023 um 12:18 schrieb Alexandre Derumier:
> Signed-off-by: Alexandre Derumier 
> ---
>  src/PVE/RPCEnvironment.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



Re: [pve-devel] [PATCH common/access-control/wt/manager v3] add realm sync jobs

2023-06-07 Thread Thomas Lamprecht
Am 17/01/2023 um 12:46 schrieb Dominik Csapak:
> pve-manager:
> 
> Dominik Csapak (4):
>   Jobs: include existing types in state file regex for deletion
>   Jobs: add RealmSync Plugin and register it
>   api: add realm-sync crud api to /cluster/jobs
>   ui: add Realm Sync panel
> 
>  PVE/API2/Cluster/Jobs.pm|   7 +
>  PVE/Jobs.pm |   7 +-
>  www/manager6/Makefile   |   1 +
>  www/manager6/dc/Config.js   |   7 +
>  www/manager6/dc/RealmSyncJob.js | 364 
>  5 files changed, 385 insertions(+), 1 deletion(-)
>  create mode 100644 www/manager6/dc/RealmSyncJob.js
> 

Now applied those too finally, thanks!

Some thoughts:

* would clarify that a existing LDAP/AD realm is required via some UX change in 
the
  add/edit widnow, maybe:

  - add an empty text in the field

  - disable the "scope" field until an realm is selected, as otherwise the 
invalid
state is slightly confusing.

* Merge this into realm, as panel at the bottom there.
  Having many realms is rather the exception, the two built-ins plus one or 
maybe
  two external ones (e.g., a LDAP and a OIDC) are probably enough for most 
setups.
  That means we got a lot of "free" reals^W space estate in the existing realm 
panel,
  putting that to better use and avoiding a tree entry might maybe improve 
cognitive
  load imposed by our UI minimally (or at least not increase it).

* A "Run now" button is missing?


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



[pve-devel] applied-series: [PATCH access-control 1/2] roles: restrict Permissions.Modify to Administrator

2023-06-07 Thread Thomas Lamprecht
Am 05/06/2023 um 16:21 schrieb Fabian Grünbichler:
> to reduce the chances of accidentally handing out privilege modification
> privileges. the old default setup of having Permissions.Modify in PVESysAdmin
> and PVEAdmin weakened the distinction between those roles and Administrator.
> 
> Signed-off-by: Fabian Grünbichler 
> ---
> 
> Notes:
> this is obviously a breaking change
> 
> can be detected in pve7to8 if any ACLs with PVESysAdmin or PVEAdmin are
> configured, with a hint about adding a custom role if desired.
> 
>  src/PVE/AccessControl.pm | 5 +++--
>  src/test/perm-test1.pl   | 4 
>  src/test/perm-test8.pl   | 4 ++--
>  src/test/test1.cfg   | 2 ++
>  src/test/test8.cfg   | 2 +-
>  5 files changed, 12 insertions(+), 5 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 v3 storage 1/1] fix #2920: cifs: add options parameter

2023-06-07 Thread Thomas Lamprecht
Am 01/03/2023 um 13:13 schrieb Fiona Ebner:
> From: Stefan Hrdlicka 
> 
> This makes it possible to add all mount options offered by mount.cifs.
> NFS & CIFS now share the options parameter since they use it for the
> same purpose.
> 
> Signed-off-by: Stefan Hrdlicka 
> [FE: rebase + style fixes]
> Signed-off-by: Fiona Ebner 
> ---
> 
> Changes from v2:
> * minor improvements in commit message
> * adapt to recently changed cifs_mount function interface
> 
>  PVE/Storage/CIFSPlugin.pm | 4 +++-
>  PVE/Storage/NFSPlugin.pm  | 4 
>  PVE/Storage/Plugin.pm | 6 ++
>  3 files changed, 9 insertions(+), 5 deletions(-)
> 
>

applied, with trivial context merge conflict resolved & Friedrich's T-b added,
thanks!


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



[pve-devel] applied: [PATCH access-control v3 2/2] add realm-sync plugin for jobs and CRUD api for realm-sync-jobs

2023-06-07 Thread Thomas Lamprecht
Am 17/01/2023 um 12:46 schrieb Dominik Csapak:
> to be able to define automated jobs that sync ldap/ad
> 
> The jobs plugin contains special handling when no node is given, since
> we only want it to run on a single node when that triggers. For that,
> we save a statefile in /etc/pve/priv/jobs/ which contains the
> node/time/upid of the node that runs the job. The first node that
> is able to lock the realm (via cfs_lock_domain) "wins" and may
> sync from the ldap.
> 
> in case a specific node was selected, this is omitted and the Jobs
> handling will not let it run on other nodes anyway
> 
> the API part is our usual sectionconfig CRUD api, but specialized
> for the specific type of job.
> 
> the api will be at /cluster/jobs/realm-sync
> (this must be done in pve-manager)
> 
> Signed-off-by: Dominik Csapak 
> ---
>  src/PVE/API2/AccessControl/Makefile |   6 +
>  src/PVE/API2/AccessControl/RealmSync.pm | 292 
>  src/PVE/API2/Makefile   |   4 +
>  src/PVE/Jobs/Makefile   |   6 +
>  src/PVE/Jobs/RealmSync.pm   | 201 
>  src/PVE/Makefile|   1 +
>  6 files changed, 510 insertions(+)
>  create mode 100644 src/PVE/API2/AccessControl/Makefile
>  create mode 100644 src/PVE/API2/AccessControl/RealmSync.pm
>  create mode 100644 src/PVE/Jobs/Makefile
>  create mode 100644 src/PVE/Jobs/RealmSync.pm
> 
>

applied, moving the API from AccessControl namespace to Jobs one, as it's 
confusing
if it's located in PVE::API2::AccessControl::, but isn't mounted there.

Some small code/import cleanups thrown in, 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-SERIES qemu-server/storage] improve RBD resize

2023-06-06 Thread Thomas Lamprecht
Am 28/04/2023 um 14:32 schrieb Fiona Ebner:
> Make the way the block_resize QMP command is used consistent with
> what other block device backed storages like ZFS and LVM(thin) do.
> 
> Avoid the --allow-shrink flag that should never be required in our
> code and avoid passing floating point numbers to the rbd resize
> command.
> 
> 
> qemu-server:
> 
> Fiona Ebner (1):
>   block resize: avoid passing zero size to QMP command
> 
>  PVE/QemuServer.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> pve-storage:
> 
> Fiona Ebner (2):
>   rbd: don't specify allow-shrink flag
>   rbd: volume resize: avoid passing floating point value to rbd
> 
>  PVE/Storage/RBDPlugin.pm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 


applied series, wrapping the ceil in an int(), just to be sure - thanks!


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



Re: [pve-devel] [PATCH storage 2/2] rbd: volume resize: avoid passing floating point value to rbd

2023-06-06 Thread Thomas Lamprecht
Am 28/04/2023 um 14:32 schrieb Fiona Ebner:
> which causes an error "the argument for option '--size' is invalid".
> Just round up to the nearest integer to have at least the requested
> size. This is similar to what is done for ZFS with d3e3e5d ("When
> resizing a ZFS volume, align size to 1M") and makes commands like 'qm
> resize 102 scsi1 +0.01G' work.
> 
> Signed-off-by: Fiona Ebner 
> ---
>  PVE/Storage/RBDPlugin.pm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
> index 9d59a14..c9e70a2 100644
> --- a/PVE/Storage/RBDPlugin.pm
> +++ b/PVE/Storage/RBDPlugin.pm
> @@ -7,6 +7,7 @@ use Cwd qw(abs_path);
>  use IO::File;
>  use JSON;
>  use Net::IP;
> +use POSIX qw(ceil);
>  
>  use PVE::CephConfig;
>  use PVE::Cluster qw(cfs_read_file);;
> @@ -779,7 +780,7 @@ sub volume_resize {
>  
>  my ($vtype, $name, $vmid) = $class->parse_volname($volname);
>  
> -my $cmd = $rbd_cmd->($scfg, $storeid, 'resize', '--size', 
> ($size/1024/1024), $name);
> +my $cmd = $rbd_cmd->($scfg, $storeid, 'resize', '--size', 
> ceil($size/1024/1024), $name);


Hmm, but POSIX ceil is also returning a double `double ceil(double x)`, maybe 
wrap
that into an int(), hedging against (future) perl is often a relatively good 
idea ^^

FWIW: we often to something like int(($size + 1023)/1024/1024); (untested for 
this
specific case).


>  run_rbd_command($cmd, errmsg => "rbd resize '$volname' error");
>  return undef;
>  }



___
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 1/2] ui: storage: backup: refactor extraColumns assignment

2023-06-06 Thread Thomas Lamprecht
Am 25/04/2023 um 09:21 schrieb Dominik Csapak:
> makes it easier to add columns, and uses less indentation
> 
> Signed-off-by: Dominik Csapak 
> ---
> new in v2
>  www/manager6/storage/BackupView.js | 40 +++---
>  1 file changed, 20 insertions(+), 20 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] fix #4605: drop rsyncable from zstd invocation

2023-06-06 Thread Thomas Lamprecht
Am 17/04/2023 um 09:04 schrieb Fabian Grünbichler:
> it causes severe slow downs on fast disks, and we still have other rsyncable
> compressors available.
> 
> it was originally added based on wrong documentation that made the performance
> impact look a lot smaller than it actually is.
> 
> Signed-off-by: Fabian Grünbichler 
> ---
> 
> Notes:
> IMHO, we can always re-introduce it as configurable opt-in option if 
> somebody
> really needs this combination.. and most things for which you might want
> rsyncable are better off just using PBS nowadays anyway.
> 
>  PVE/VZDump.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: [PATCH-SERIES ha-manager] avoid rebalance-on-start for newly added already running service

2023-06-06 Thread Thomas Lamprecht
Am 14/04/2023 um 14:38 schrieb Fiona Ebner:
> As reported in the community forum[0], the current behavior when
> adding an already running service to HA is wrong. If rebalance
> selected a different node, the service would be stopped, offline
> migrated and started again. Since the rebalance-on-start is only
> intended to trigger on the stopped->start transition[1], this series
> makes sure the service is not migrated at all for rebalance-on-start
> if already running.
> 
> [0]: https://forum.proxmox.com/threads/125597/
> [1]: 
> https://pve.proxmox.com/pve-docs/chapter-ha-manager.html#_crs_scheduling_points
> 
> Fiona Ebner (4):
>   sim: hardware: commands: fix documentation for add
>   sim: hardware: commands: make it possible to add already running
> service
>   tools: add IGNORED return code
>   lrm: do not migrate if service already running upon rebalance on start
> 
>  src/PVE/HA/LRM.pm |  5 ++
>  src/PVE/HA/Manager.pm |  6 ++
>  src/PVE/HA/Sim/Hardware.pm| 16 +++--
>  src/PVE/HA/Tools.pm   |  3 +-
>  src/test/test-crs-static-rebalance2/README|  3 +
>  src/test/test-crs-static-rebalance2/cmdlist   |  9 +++
>  .../test-crs-static-rebalance2/datacenter.cfg |  7 +++
>  .../hardware_status   |  5 ++
>  .../test-crs-static-rebalance2/log.expect | 63 +++
>  .../test-crs-static-rebalance2/manager_status |  1 +
>  .../test-crs-static-rebalance2/service_config |  1 +
>  .../static_service_stats  |  1 +
>  12 files changed, 115 insertions(+), 5 deletions(-)
>  create mode 100644 src/test/test-crs-static-rebalance2/README
>  create mode 100644 src/test/test-crs-static-rebalance2/cmdlist
>  create mode 100644 src/test/test-crs-static-rebalance2/datacenter.cfg
>  create mode 100644 src/test/test-crs-static-rebalance2/hardware_status
>  create mode 100644 src/test/test-crs-static-rebalance2/log.expect
>  create mode 100644 src/test/test-crs-static-rebalance2/manager_status
>  create mode 100644 src/test/test-crs-static-rebalance2/service_config
>  create mode 100644 src/test/test-crs-static-rebalance2/static_service_stats
> 


applied series, thanks!

But, I split the last commit into two, first one adding the (still broken) test,
then in the second the fix with the change to log.expect from broken -> fixed
encoded also in git. IME, this makes such changes a bit easier to understand and
evaluate when checking out the git log in the future.


___
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] net: Skip and warn of interfaces without bridge

2023-06-06 Thread Thomas Lamprecht
Am 12/04/2023 um 10:45 schrieb Christian Ebner:
> Handle and warn about network interfaces which are not attached to
> any bridge because the user actively removed it from the VM config.
> 
> Signed-off-by: Christian Ebner 
> ---
>  PVE/QemuServer.pm | 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-series: [PATCH proxmox-apt 1/2] fallback to Release file for Origin retrieval

2023-06-06 Thread Thomas Lamprecht
Am 12/04/2023 um 09:17 schrieb Fabian Grünbichler:
> APT will not store the InRelease file in some cases, and some repositories
> might not even have one in the first place.
> 
> Signed-off-by: Fabian Grünbichler 
> ---
>  src/repositories/repository.rs | 19 ---
>  1 file changed, 12 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: [RFC manager] pveceph: add osd details command

2023-06-06 Thread Thomas Lamprecht
Am 19/12/2022 um 15:46 schrieb Aaron Lauterer:
> To provide similar output on the CLI as is possible in the GUI/API
> regaring OSD details.
> 
> By default (output-format=text) a more concise output is shown. Using
> json or yaml as output format will print all the available data.
> 
> The 'verbose' flag causes json-pretty output to be used.
> 
> The functionality is split between the actual function and the output
> formatter as not all options/parameters are available in each.
> 
> Signed-off-by: Aaron Lauterer 
> ---
> Depends on the OSD Details patches which are not yet applied [0].
> 
> RFC because in order to check against the output format and the verbose
> flag, the code ended up split between the actual cmdline method and
> the formatting function.
> 
> The verbose flag is only available in the formatter and the verbose in
> the method itself.
> 
> If there would be a nicer way to achieve a similar result, I would be
> happy to get hints.
> 
> [0] https://lists.proxmox.com/pipermail/pve-devel/2022-December/055154.html
> 
>  PVE/CLI/pveceph.pm | 66 ++
>  1 file changed, 66 insertions(+)
> 
>

applied, with some (opinionated) code cleanups, thanks!

Looking at current CLI commands it seems a osd list is missing, as if I tried
this one first it asked for an OSD ID, and I thought well, how do I get one,
and yes `ceph osd tree` and co naturally, I know that, but a user that just
used pveceph might expect that there's some (osd) command that provides it
there too - so maybe we can add one.


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



[pve-devel] applied: [PATCH pve-container] fix #4457: use bridge mtu if no mtu is defined

2023-06-06 Thread Thomas Lamprecht
Am 11/04/2023 um 14:44 schrieb Alexandre Derumier:
> Signed-off-by: Alexandre Derumier 
> ---
>  src/PVE/LXC.pm | 12 ++--
>  1 file changed, 6 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 qemu 2/2] update reentrancy patches to version in upstream git

2023-06-06 Thread Thomas Lamprecht
Am 06/06/2023 um 10:58 schrieb Fiona Ebner:
> The previous version was picked from the mailing list and still had
> an object_dynamic_cast call in a hot path, which is avoided with the
> version that landed in git.
> 
> Also adds a few more exceptions for devices that need reentrancy.
> 
> Signed-off-by: Fiona Ebner 
> ---
>  ...-memory-prevent-dma-reentracy-issues.patch | 118 
>  ...s-Internal-cdbs-have-16-byte-length.patch} |   0
>  ...al-deadlock-when-draining-during-tr.patch} |   0
>  ...en-getting-cursor-without-a-console.patch} |   0
>  ...isabling-re-entrancy-checking-per-MR.patch |  38 -
>  ...-memory-prevent-dma-reentracy-issues.patch | 130 ++
>  ...le-reentrancy-detection-for-script-R.patch |   8 +-
>  ...-disable-reentrancy-detection-for-io.patch |  37 +
>  ...sable-reentrancy-detection-for-iomem.patch |  35 +
>  ...le-reentrancy-detection-for-apic-msi.patch |  36 +
>  debian/patches/series |  12 +-
>  11 files changed, 252 insertions(+), 162 deletions(-)
>  delete mode 100644 
> debian/patches/extra/0002-memory-prevent-dma-reentracy-issues.patch
>  rename 
> debian/patches/extra/{0003-scsi-megasas-Internal-cdbs-have-16-byte-length.patch
>  => 0002-scsi-megasas-Internal-cdbs-have-16-byte-length.patch} (100%)
>  rename 
> debian/patches/extra/{0004-ide-avoid-potential-deadlock-when-draining-during-tr.patch
>  => 0003-ide-avoid-potential-deadlock-when-draining-during-tr.patch} (100%)
>  rename 
> debian/patches/extra/{0007-ui-return-NULL-when-getting-cursor-without-a-console.patch
>  => 0004-ui-return-NULL-when-getting-cursor-without-a-console.patch} (100%)
>  delete mode 100644 
> debian/patches/extra/0005-memory-Allow-disabling-re-entrancy-checking-per-MR.patch
>  create mode 100644 
> debian/patches/extra/0005-memory-prevent-dma-reentracy-issues.patch
>  create mode 100644 
> debian/patches/extra/0007-bcm2835_property-disable-reentrancy-detection-for-io.patch
>  create mode 100644 
> debian/patches/extra/0008-raven-disable-reentrancy-detection-for-iomem.patch
>  create mode 100644 
> debian/patches/extra/0009-apic-disable-reentrancy-detection-for-apic-msi.patch
> 
>

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 1/2] update submodule and patches to QEMU 8.0.2

2023-06-06 Thread Thomas Lamprecht
Am 06/06/2023 um 10:58 schrieb Fiona Ebner:
> Signed-off-by: Fiona Ebner 
> ---
>  ...le-reentrancy-detection-for-script-R.patch |   4 +-
>  ...39-fix-large_send_mss-divide-by-zero.patch |  72 
>  ...en-getting-cursor-without-a-console.patch} |   2 +-
>  ...-Fix-crash-when-executing-HMP-commit.patch |  48 ---
>  ...our-channel-order-for-PNG-screenshot.patch |  77 
>  ...arm-Fix-vd-vm-overlap-in-sve_ldff1_z.patch |  41 --
>  ...e-incorrect-computation-in-float32_e.patch |  56 ---
>  ...ge-wrong-XFRM-value-in-SGX-CPUID-lea.patch |  39 --
>  ...t-assert_bdrv_graph_readable-by-defa.patch | 106 -
>  ...CI_ERR_UNCOR_MASK-register-for-machi.patch | 100 -
>  ...after-free-in-blockdev_mark_auto_del.patch |  57 ---
>  ...ly-call-bdrv_activate-outside-corout.patch |  64 ---
>  ...o_unref-for-calls-in-coroutine-conte.patch | 373 --
>  ...-no_coroutine_fns-in-qmp_block_resiz.patch |  43 --
>  ...-tcg-Fix-atomic_mmu_lookup-for-reads.patch |  36 --
>  debian/patches/series |  15 +-
>  qemu  |   2 +-
>  17 files changed, 5 insertions(+), 1130 deletions(-)
>  delete mode 100644 
> debian/patches/extra/0007-rtl8139-fix-large_send_mss-divide-by-zero.patch
>  rename 
> debian/patches/extra/{0009-ui-return-NULL-when-getting-cursor-without-a-console.patch
>  => 0007-ui-return-NULL-when-getting-cursor-without-a-console.patch} (97%)
>  delete mode 100644 
> debian/patches/extra/0008-block-monitor-Fix-crash-when-executing-HMP-commit.patch
>  delete mode 100644 
> debian/patches/extra/0010-ui-Fix-pixel-colour-channel-order-for-PNG-screenshot.patch
>  delete mode 100644 
> debian/patches/extra/0011-target-arm-Fix-vd-vm-overlap-in-sve_ldff1_z.patch
>  delete mode 100644 
> debian/patches/extra/0012-softfloat-Fix-the-incorrect-computation-in-float32_e.patch
>  delete mode 100644 
> debian/patches/extra/0013-target-i386-Change-wrong-XFRM-value-in-SGX-CPUID-lea.patch
>  delete mode 100644 
> debian/patches/extra/0014-block-compile-out-assert_bdrv_graph_readable-by-defa.patch
>  delete mode 100644 
> debian/patches/extra/0015-hw-pci-Disable-PCI_ERR_UNCOR_MASK-register-for-machi.patch
>  delete mode 100644 
> debian/patches/extra/0016-block-Fix-use-after-free-in-blockdev_mark_auto_del.patch
>  delete mode 100644 
> debian/patches/extra/0017-block-Consistently-call-bdrv_activate-outside-corout.patch
>  delete mode 100644 
> debian/patches/extra/0018-block-bdrv-blk_co_unref-for-calls-in-coroutine-conte.patch
>  delete mode 100644 
> debian/patches/extra/0019-block-Don-t-call-no_coroutine_fns-in-qmp_block_resiz.patch
>  delete mode 100644 
> debian/patches/extra/0020-accel-tcg-Fix-atomic_mmu_lookup-for-reads.patch
> 
>

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 docs] pvesm: mark ZFS level as both

2023-06-06 Thread Thomas Lamprecht
Am 11/04/2023 um 13:04 schrieb Aaron Lauterer:
> ZFS can do both, and we do use both, block and file level functionality.
> 
> Signed-off-by: Aaron Lauterer 
> ---
> changes since v1: rephrased the footnote for ZFS
> 
>  pvesm.adoc | 39 +--
>  1 file changed, 21 insertions(+), 18 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-SERIES storage/docs/guest-common/qemu-server/common] Improve and unify documentation of bwlimit parameter

2023-06-06 Thread Thomas Lamprecht
Am 29/03/2023 um 14:34 schrieb Stefan Hanreich:
> While looking through our documentation for the bwlimit parameter
> I noticed that the descriptions are inconsistent and sometimes
> wrong / unclear about the actual unit used for the parameter. This
> patch series fixes some inconsistencies and errors related to this
> parameter.
> 

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: [RFC storage] content-dirs: check that all content dirs are pairwise inequal

2023-06-06 Thread Thomas Lamprecht
Am 21/03/2023 um 18:03 schrieb Friedrich Weber:
> This prevents strange interactions in case the same content directory
> is used for multiple content types.
> 
> Signed-off-by: Friedrich Weber 
> ---
>  I guess technically this is a breaking change, as users may have an
>  iso+vztmpl storage that symlinks 'templates/iso' to 'templates/cache',
>  which would now throw an error.

Well, then lets apply this for upcoming 8, maybe you can add a note to
our breaking changes list so that users are aware.

Also, checking this upfront and erroring in pve7to8 checker script should
not be that hard and def. help some to avoid problems during/after the
upgrade.

> 
>  PVE/Storage/Plugin.pm | 11 +++
>  1 file changed, 11 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 manager] close #4513: ui: backup job: add performance tab

2023-06-06 Thread Thomas Lamprecht
stumbled upon this again when checking out old(er) patches without comments,
some higher level comments inline.

Am 15/03/2023 um 14:24 schrieb Fiona Ebner:
> pigz is not exposed, because it only works after manually installing
> the pigz package.
> 
> ionice is not exposed, because it only works in combination with the
> BFQ scheduler and even then not in all cases (only affects the
> compressor when doing snapshot/suspend mode backup of a VM).
> 
> These can still be added with appropriate notes if there is enough
> user demand.
> 
> Signed-off-by: Fiona Ebner 
> ---
>  www/manager6/Makefile   |   1 +
>  www/manager6/dc/Backup.js   |  12 +++
>  www/manager6/panel/BackupPerformance.js | 110 
>  3 files changed, 123 insertions(+)
>  create mode 100644 www/manager6/panel/BackupPerformance.js
> 
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index b73b729a..0c92b984 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -88,6 +88,7 @@ JSSRC=  
> \
>   grid/ResourceGrid.js\
>   panel/ConfigPanel.js\
>   panel/BackupJobPrune.js \
> + panel/BackupPerformance.js  \
>   panel/HealthWidget.js   \
>   panel/IPSet.js  \
>   panel/RunningChart.js   \
> diff --git a/www/manager6/dc/Backup.js b/www/manager6/dc/Backup.js
> index d0046177..8da6e7d7 100644
> --- a/www/manager6/dc/Backup.js
> +++ b/www/manager6/dc/Backup.js
> @@ -172,6 +172,11 @@ Ext.define('PVE.dc.BackupEdit', {
>   
> PVE.Utils.unEscapeNotesTemplate(data['notes-template']);
>   }
>  
> + if (data.performance) {
> + Object.assign(data, data.performance);
> + delete data.performance;
> + }
> +
>   view.setValues(data);
>   },
>   });
> @@ -411,6 +416,13 @@ Ext.define('PVE.dc.BackupEdit', {
>   },
>   ],
>   },
> + {
> + xtype: 'pveBackupPerformancePanel',
> + title: gettext('Performance'),
> + cbind: {
> + isCreate: '{isCreate}',
> + },
> + },
>   ],
>   },
>  ],
> diff --git a/www/manager6/panel/BackupPerformance.js 
> b/www/manager6/panel/BackupPerformance.js
> new file mode 100644
> index ..17e0f34c
> --- /dev/null
> +++ b/www/manager6/panel/BackupPerformance.js
> @@ -0,0 +1,110 @@
> +/*
> + * Input panel for backup performance settings intended to be used as part 
> of an edit/create window.
> + */
> +Ext.define('PVE.panel.BackupPerformance', {
> +extend: 'Proxmox.panel.InputPanel',
> +xtype: 'pveBackupPerformancePanel',
> +mixins: ['Proxmox.Mixin.CBind'],
> +
> +cbindData: function() {
> + let me = this;
> + me.isCreate = !!me.isCreate;
> + return {};
> +},
> +
> +onGetValues: function(formValues) {
> + if (this.needMask) { // isMasked() may not yet be true if not rendered 
> once
> + return {};
> + }
> +
> + let options = { 'delete': [] };
> +
> + let performance = {};
> + let performanceOptions = ['max-workers'];
> +
> + for (const [key, value] of Object.entries(formValues)) {
> + if (performanceOptions.includes(key)) {
> + performance[key] = value;
> + // deleteEmpty is not currently implemented for pveBandwidthField
> + } else if (key === 'bwlimit' && value === '') {
> + options.delete.push('bwlimit');
> + } else if (key === 'delete') {
> + if (Array.isArray(value)) {
> + value.filter(opt => 
> !performanceOptions.includes(opt)).forEach(
> + opt => options.delete.push(opt),
> + );
> + } else if (!performanceOptions.includes(formValues.delete)) {
> + options.delete.push(value);
> + }
> + } else {
> + options[key] = value;
> + }
> + }
> +
> + if (Object.keys(performance).length > 0) {
> + options.performance = PVE.Parser.printPropertyString(performance);
> + } else {
> + options.delete.push('performance');
> + }
> +
> + if (this.isCreate) {
> + delete options.delete;
> + }
> +
> + return options;
> +},
> +

hmm, maybe we should go for a layout with one field per row, but besides that a
short description about what it controls, like:

Label [ Field ] Description

Where the descriptions are sorta in the style of the CPU flag ones for VM CPU
edit, i.e., provide some context and hint of what this does.

Because max-workers "VM Worker Count" is not 

[pve-devel] applied-series: [PATCH widget-toolkit/http-server/apiclient 0/4] Set SameSite=Strict on Auth Cookies

2023-06-06 Thread Thomas Lamprecht
Am 15/03/2023 um 17:26 schrieb Max Carrara:
> This series sets the `SameSite` attribute of authentication cookies
> to `Strict` as per RFC 6265[1]. This prevents browsers from nagging;
> for example, FireFox 102.8.0esr would complain in the following manner:
> 
>> Cookie “PVEAuthCookie” does not have a proper “SameSite” attribute 
>> value. Soon, cookies without the “SameSite” attribute or with an
>> invalid value will be treated as “Lax”. This means that the cookie
>> will no longer be sent in third-party contexts. If your application
>> depends on this cookie being available in such contexts, please add
>> the “SameSite=None“ attribute to it. To know more about the
>> “SameSite“ attribute, read 
>> https://developer.mozilla.org/docs/Web/HTTP/Headers/Set-Cookie/SameSite
> 
> Since setting `SameSite` to `Strict` enforces that the cookie be only
> sent in a first-party context - so, only to the web UI and no other
> site - it seemed like the best thing to choose. I'm not aware of the
> cookie being used in any other contexts; if that's the case, I'll
> gladly provide a v2.

now, with the upcomming beta, it's the best time to find that out ^^

> 
> The attribute is set wherever it makes sense; the only repo in which
> it's not set would be 'pve-client', as that one's apparently not being
> used at all (it wouldn't even build). Please let me know if I have
> missed any spots.
> 
> [1] 
> https://httpwg.org/http-extensions/draft-ietf-httpbis-rfc6265bis.html#name-the-samesite-attribute
> 
> 
> proxmox-widget-toolkit:
> 
> Max Carrara (2):
>   toolkit/utils: set SameSite attr of auth cookie to 'strict'
>   toolkit/utils: fix whitespace
> 
>  src/Toolkit.js | 513 ++---
>  src/Utils.js   |   6 +-
>  2 files changed, 276 insertions(+), 243 deletions(-)
> 
> 
> pve-http-server:
> 
> Max Carrara (1):
>   formatter/bootstrap: set SameSite attr of auth cookie to 'strict'
> 
>  src/PVE/APIServer/Formatter.pm   | 2 +-
>  src/PVE/APIServer/Formatter/Bootstrap.pm | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> 
> pve-apiclient:
> 
> Max Carrara (1):
>   lwp: set SameSite attr of auth cookie to 'strict'
> 
>  PVE/APIClient/LWP.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: [PATCH manager] ui: fix duplicate references when using multiple DiskStorageSelectors

2023-06-06 Thread Thomas Lamprecht
Am 21/03/2023 um 14:27 schrieb Dominik Csapak:
> by removing the references and change the one place where we used
> one of the references.
> 
> Signed-off-by: Dominik Csapak 
> ---
> while i tested all places where we have a disk selector
> (wizard, clone, efidisk, add hd) i am not super sure i found all uses
> i grepped after the references, and only found one use where we used it
> in a lookup/lookupReference, but maybe someone else too can check that
> everything still works...
> 
> 
>  www/manager6/form/DiskStorageSelector.js | 4 
>  www/manager6/window/Clone.js | 2 +-
>  2 files changed, 1 insertion(+), 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 v2 manager] fix #2641: expose CIFS subdir parameter through GUI

2023-06-06 Thread Thomas Lamprecht
Am 08/02/2023 um 10:05 schrieb Leo Nunner:
> makes it possible to optionally set the 'subdir' parameter when adding a
> new CIFS storage.
> 
> Signed-off-by: Leo Nunner 
> ---
> Changes from v1:
> - use gettext for the Subdirectory label
> 
>  www/manager6/storage/CIFSEdit.js | 11 +++
>  1 file changed, 11 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] api: nodes: add 'migrateall' to index

2023-06-06 Thread Thomas Lamprecht
Am 29/03/2023 um 13:36 schrieb Fiona Ebner:
> Signed-off-by: Fiona Ebner 
> ---
>  PVE/API2/Nodes.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



Re: [pve-devel] [PATCH v3 manager] gui: expose content-dirs property in storage edit/create

2023-06-06 Thread Thomas Lamprecht
Am 24/03/2023 um 17:12 schrieb Leo Nunner:
> Add a separate tab for the storage edit/create panels to set the
> recently introduced "content-dirs" property which overrides the
> default directory locations. Analogous to the API implementation,
> the tab was added for Directory, CIFS and NFS storages.
> 
> Signed-off-by: Leo Nunner 
> ---
> Changes since v2:
> - Factor out edit tab into separate component
> - Add vtype for instant validation feedback to the user
> - Introduce tab for CephFS
> 
>  www/manager6/Makefile  |   1 +
>  www/manager6/Toolkit.js|   2 +
>  www/manager6/panel/ContentDirs.js  | 102 +
>  www/manager6/storage/CIFSEdit.js   |   9 +++
>  www/manager6/storage/CephFSEdit.js |   9 +++
>  www/manager6/storage/DirEdit.js|   9 +++
>  www/manager6/storage/NFSEdit.js|   9 +++
>  7 files changed, 141 insertions(+)
>  create mode 100644 www/manager6/panel/ContentDirs.js
> 
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index 2b577c8e..cb07b5b8 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -96,6 +96,7 @@ JSSRC=  
> \
>   panel/GuestSummary.js   \
>   panel/TemplateStatusView.js \
>   panel/MultiDiskEdit.js  \
> + panel/ContentDirs.js\
>   tree/ResourceTree.js\
>   tree/SnapshotTree.js\
>   window/Backup.js\
> diff --git a/www/manager6/Toolkit.js b/www/manager6/Toolkit.js
> index f69c376a..0c801d45 100644
> --- a/www/manager6/Toolkit.js
> +++ b/www/manager6/Toolkit.js
> @@ -15,6 +15,8 @@ Ext.apply(Ext.form.field.VTypes, {
>  IP64AddressListMask: /[A-Fa-f0-9,:.; ]/,
>  PciIdText: gettext('Example') + ': 0x8086',
>  PciId: v => /^0x[0-9a-fA-F]{4}$/.test(v),
> +ContentDirText: gettext('Example') + ': custom/subdir/location',
> +ContentDir: v => /([^.]*(?:\.?[^.]+)+)/.test(v),
>  });
>  
>  Ext.define('PVE.form.field.Display', {
> diff --git a/www/manager6/panel/ContentDirs.js 
> b/www/manager6/panel/ContentDirs.js
> new file mode 100644
> index ..57939101
> --- /dev/null
> +++ b/www/manager6/panel/ContentDirs.js
> @@ -0,0 +1,102 @@
> +Ext.define('PVE.panel.ContentDirsPanel', {
> +extend: 'Proxmox.panel.InputPanel',
> +xtype: 'pveContentDirsTab',
> +mixins: ['Proxmox.Mixin.CBind'],
> +
> +onGetValues: function(values) {
> + let me = this;
> + let ret = me.values ?? {};
> + Object.keys(values || {}).forEach(function(name) {
> + ret[name] = values[name];
> + });
> + return { "content-dirs": PVE.Parser.printPropertyString(ret) };
> +},
> +
> +onSetValues: function(values) {
> + let me = this;
> + me.values = values?.["content-dirs"];
> + return me.values;
> +},
> +
> +column1: [
> + {
> + xtype: 'pmxDisplayEditField',
> + name: 'vztmpl',
> + fieldLabel: gettext('CT Template'),
> + allowBlank: true,
> + emptyText: "template/cache",
> + vtype: "ContentDir",
> + editable: true,
> + },
> + {
> + xtype: 'pmxDisplayEditField',
> + name: 'iso',
> + fieldLabel: gettext('ISO Image'),
> + allowBlank: true,
> + emptyText: "template/iso",
> + vtype: "ContentDir",
> + editable: true,
> + },
> + {
> + xtype: 'pmxDisplayEditField',
> + name: 'snippets',
> + fieldLabel: gettext('Snippets'),
> + allowBlank: true,
> + emptyText: "snippets",
> + vtype: "ContentDir",
> + editable: true,
> + },

I'd add an advanced section  and move above into it.

> +],
> +column2: [
> + {
> + xtype: 'pmxDisplayEditField',
> + name: 'backup',
> + fieldLabel: gettext('Backup Files'),
> + allowBlank: true,
> + emptyText: "dump",
> + vtype: "ContentDir",
> + cbind: {
> + editable: '{isCreate}',
> + },
> + },
> + {
> + xtype: 'pmxDisplayEditField',
> + name: 'rootdir',
> + fieldLabel: gettext('Container'),

this is misleading, it's not for Container volumes but rather expanded Container
root-directories, something from OpenVZ times, which we don't fully support 
anymore
IIRC. If, I'd place this into advanced and use a different name to convey that.

> + allowBlank: true,
> + emptyText: "private",
> + vtype: "ContentDir",
> + cbind: {
> + editable: '{isCreate}',
> + },
> + },
> + {
> + xtype: 'pmxDisplayEditField',
> + name: 'images',
> + fieldLabel: gettext('Disk Image'),

Maybe call this 'Guest Volumes' or 'Guest Images' if it really has to be images.

> + allowBlank: true,
> + 

[pve-devel] applied: [PATCH docs] tree-wide: properly use {pve} instead of PVE

2023-06-06 Thread Thomas Lamprecht
Am 28/03/2023 um 14:03 schrieb Fabian Grünbichler:
> where applicable, or expand/replace where it's not a good fit or automatic
> expansion doesn't work.
> 
> there are a few more in generated files, those need to be cleaned up
> separately.
> 
> Signed-off-by: Fabian Grünbichler 
> ---
> 
> Notes:
> specifically, HA resources has \{pve\}, where the perl schema doesn't 
> have the
> back slashes, so somewhere in the formatting code escaping happens when it
> shouldn't, but removing the escaping might break something else..
> 
> pve-access-control has a few bare PVE instances in the LDAP schema, and 
> one in
> a deprecation note "PVE-8.0" in PVE::API2::AccessControl.
> 
>  getting-help.adoc  | 4 ++--
>  pmxcfs.adoc| 4 ++--
>  pve-firewall.adoc  | 2 +-
>  pve-package-repos.adoc | 2 +-
>  pvesm.adoc | 2 +-
>  pvesr.adoc | 6 +++---
>  6 files changed, 10 insertions(+), 10 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 docs] network: rephrase corosync and bonds recommendations

2023-06-06 Thread Thomas Lamprecht
Am 24/03/2023 um 14:03 schrieb Aaron Lauterer:
> I suspect that the old one seems to be related to multicast traffic and
> LACP bonds.
> 
> The link in the comment is dead by now. It seems this is one occasion
> where the internet actually forgets as I cannot find the actual message
> of that mailing list thread anymore. Therefore I cannot say for sure
> what the exact issue was. But it was introduced in commit
> 649098a64ecaffc7215ec0556e76787595b38e88 which unfortunately also
> doesn't have more information.
> 
> Since with Corosync 3, unicast is used, that recommentation is probably
> not accurate anymore. At least I am not aware of any issues with
> Corosync on LACP bonds in recent years. Therefore, rather recommend to
> configure Corosync on multiple networks instead of bonds to follow best
> practice.
> 
> Signed-off-by: Aaron Lauterer 
> ---
>  pve-network.adoc | 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 widget-toolkit 2/3] DateTimeField: Extend and refactor to make field value bindable

2023-06-06 Thread Thomas Lamprecht
Am 23/03/2023 um 15:42 schrieb Christian Ebner:
> Extends the date time field so that bindings are updated on value changes.
> Also adds a config to disable child components and avoid modification of
> current values by cloning the referenced object for min/max value calculation.
> 
> Signed-off-by: Christian Ebner 
> ---
>  src/form/DateTimeField.js | 105 +-
>  1 file changed, 81 insertions(+), 24 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 widget-toolkit 1/3] DateTimeField: fix typo in xtype

2023-06-06 Thread Thomas Lamprecht
Am 23/03/2023 um 15:42 schrieb Christian Ebner:
> Signed-off-by: Christian Ebner 
> ---
>  src/form/DateTimeField.js | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, but added the misspelled variant back as alias in a follow-up, that 
allows us to upgrade the use sites later, without requiring a Breaks for older
pmg-gui 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 edk2-firmware] add patch to work around older guest kernel bug

2023-06-06 Thread Thomas Lamprecht


nit: a slightly more telling subject could have been something like:

backport limiting the phys-bits to 46 for bug in old guest kernel

Am 05/06/2023 um 09:43 schrieb Fiona Ebner:
> by limiting the phys-bits to 46 instead of 47. On Ubuntu 18.04 with
> kernel 4.15, using 47 leads to a strange issue where initialization of
> VirtIO devices would fail.
> 
> Reported in the community forum:
> https://forum.proxmox.com/threads/127410/
> 
> Signed-off-by: Fiona Ebner 
> ---
>  ...latformInitLib-limit-phys-bits-to-46.patch | 46 +++
>  debian/patches/series |  1 +
>  2 files changed, 47 insertions(+)
>  create mode 100644 
> debian/patches/0001-OvmfPkg-PlatformInitLib-limit-phys-bits-to-46.patch
> 
>

applied, but fixed line endings of patch, edk2 (sadly) uses Windows-Style \r\n,
but the patch only got \n, so failed building a DSC:

cd pve-edk2-firmware-3.20230228; dpkg-buildpackage -S -uc -us -d
# ...
patching file OvmfPkg/Library/PlatformInitLib/MemDetect.c
Hunk #1 FAILED at 657 (different line endings).
1 out of 1 hunk FAILED
dpkg-source: info: the patch has fuzz which is not allowed, or is malformed
dpkg-source: info: if patch 
'0001-OvmfPkg-PlatformInitLib-limit-phys-bits-to-46.patch' is correctly applied 
by quilt, use 'quilt refresh' to update it
dpkg-source: info: if the file is present in the unpacked source, make sure it 
is also present in the orig tarball
dpkg-source: info: restoring quilt backup files for 
0001-OvmfPkg-PlatformInitLib-limit-phys-bits-to-46.patch
dpkg-source: error: LC_ALL=C patch -t -F 0 -N -p1 -u -V never -E -b -B 
.pc/0001-OvmfPkg-PlatformInitLib-limit-phys-bits-to-46.patch/ --reject-file=- < 
pve-edk2-firmware-3.20230228/debian/patches/0001-OvmfPkg-PlatformInitLib-limit-phys-bits-to-46.patch
 subprocess returned exit status 1


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



Re: [pve-devel] [PATCH common v2 1/3] JSONSchema: add support for array parameter in api calls, cli and config

2023-06-06 Thread Thomas Lamprecht
Am 06/06/2023 um 13:19 schrieb Dominik Csapak:
> ---8<---
> use Storable qw(dclone);
> 
> my $normalize;
> $normalize = sub {...};
> 
> my $data = /* create large hash here, with nested data */;
> 
> while(1) {
>     my $newdata = dclone($data);
>     $newdata = $normalize->($newdata);
> }
> --->8---
> 
> 
> executed it and monitored the rss usage, while letting it run for multiple 
> minutes
> the memory usage increased after the initial creation of the hash, and the 
> first
> dclone, but not after.
> 
> is that a sufficient test?

yeah, seems fine enough to me.


> maybe i'd use something like 'normalize_legacy_param_formats' ?

would be an improvement over the original convert_params and fine enough for me.


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


Re: [pve-devel] [PATCH common v2 1/3] JSONSchema: add support for array parameter in api calls, cli and config

2023-06-06 Thread Thomas Lamprecht
Am 06/06/2023 um 11:41 schrieb Dominik Csapak:
>>>   +my $untaint_recursive;
>>
>> I got flash backs w.r.t. refcount cycles here keeping all variables, and 
>> thus memory
>> inside the body alive forever, don't we need a weaken?
>>
>> E.g., like we had to do in PVE::Status::Graphite's assemble.
> 
> mhmm isn't that because there we use variables from outside the
> function? here we only use the parameters themselves

I'm not 100% sure about the details, but since then, seeing something like
this pattern triggers my cycle instincts, I'd like to have that checked out
closely.

> 
> anyway the solution there is to set the sub to undef after use, but
> we can do that here only if we move the sub into the regular
> function
> 
> i can also make it a proper sub if that's better?
> 



> how can i test for these things properly?

easiest: pass large hashes and loop, then see if memory goes up. For metrics
back then you could see the RSS of pbvestatd grow by ~ the metric data size
every update.


What I also used back then, IIRC, was the Devel::Cycle module, it should give
you a more specific answer if there's a cycle, but naturally has no idea what
the practical implications are.


>>> +# convert arrays to strings where we expect a '-list' format and convert 
>>> scalar
>>> +# values to arrays when we expect an array (because of www-form-urlencoded)
>>> +#
>>> +# only on the top level, since www-form-urlencoded cannot be nested anyway
>>> +#
>>> +# FIXME: change gui/api calls to not rely on this during 8.x, mark the
>>> +# behaviour deprecated with 9.x, and remove it with 10.x
>>> +my $convert_params = sub { my ($param, $schema) = @_;
>>
>> please keep the method paramethers on it's own line.
> 
> oops, one shift+j to many without noticing^^
> 
>>
>> Also, maybe go for a more telling names, as convert_params could mean 
>> everytrhing
>> and nothing ^^
>>
> 
> sure, any suggestions? 

sure, lets start with what this actually does in more explicit steps:

1) normalize_form_data
still a bit general but we now know that it handles form data and normalizes it 
to a
single representation


2) normalize_param_list_to_array
A bit more telling about what happens.


3) convert_legacy_list_format_to_array
very telling, but as this is a internal helper, and thus not used elsewhere, 
that
wouldn't hurt, having "legacy" in it underlines that we want to drop it 
sometimes.

Personally, I'd favor 3) or 2).
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


Re: [pve-devel] [PATCH common v2 1/3] JSONSchema: add support for array parameter in api calls, cli and config

2023-06-06 Thread Thomas Lamprecht
Am 06/06/2023 um 10:39 schrieb Dominik Csapak:
> a few things were missing for it to work:
> * on the cli, we have to get the option as an array if the type is an
>   array
> * the untainting must be done recursively, otherwise, the regex matching
>   converts an array hash into the string 'ARRAY(0x123412341234)'
> * JSONSchema::parse_config did not handle array formats specially, but
>   we want to allow to specify them multiple time
> * the biggest point: in the RESTHandler, to be compatible with the
>   current gui behavior, we have to rewrite two parameter types:
>   - when the api defines a '-list' format for a string type, but we get
> a list (because of the changes in http-server), we join the list
> with a comma into a string
>   - when the api defines an 'array' type, but we get a scalar value,
> wrap the value in an array (because for www-form-urlencoded, you
> cannot send an array with a single value) add tests for this
> behavior, some of which we want to deprecate and remove in the
> future
> 
> Signed-off-by: Dominik Csapak 
> ---
> changes from v1:
> * include wolfangs feedback
> * include auto-conversion from string <-> list where appropriate and add
>   tests for it
> 
>  src/PVE/JSONSchema.pm  |  12 +
>  src/PVE/RESTHandler.pm |  61 ++
>  test/Makefile  |   9 +++-
>  test/api_parameter_test.pl | 100 +
>  4 files changed, 172 insertions(+), 10 deletions(-)
>  create mode 100755 test/api_parameter_test.pl
> 
> diff --git a/src/PVE/JSONSchema.pm b/src/PVE/JSONSchema.pm
> index 527e409..526fc2b 100644
> --- a/src/PVE/JSONSchema.pm
> +++ b/src/PVE/JSONSchema.pm
> @@ -1709,6 +1709,8 @@ sub get_options {
>   } else {
>   if ($pd->{format} && $pd->{format} =~ m/-a?list/) {
>   push @getopt, "$prop=s@";
> + } elsif ($pd->{type} eq 'array') {
> + push @getopt, "$prop=s@";
>   } else {
>   push @getopt, "$prop=s";
>   }
> @@ -1869,6 +1871,16 @@ sub parse_config : prototype($$$;$) {
>  
>   $value = parse_boolean($value) // $value;
>   }
> + if ($schema->{properties}->{$key} &&
> + $schema->{properties}->{$key}->{type} eq 'array') {

code style, and can be fixed up:
for multi-line if's place the closing parenthesis and opening block { on it's 
own line:

It also doesn't hurt to move all expressions part of the condition in a 
separate line
(albeit that part is not a rule in our style guide):

if (
$schema->{properties}->{$key}
&& $schema->{properties}->{$key}->{type} eq 'array'
) {
# ...

> +
> + if (defined($cfg->{$key})) {
> + push $cfg->{$key}->@*, $value;
> + } else {
> + $cfg->{$key} = [$value];
> + }

Could be written shorter, but just fine as above

$cfg->{$key} //= [];
push $cfg->{$key}->@*, $value;

> + next;
> + }
>   $cfg->{$key} = $value;
>   } else {
>   warn "ignore config line: $line\n"
> diff --git a/src/PVE/RESTHandler.pm b/src/PVE/RESTHandler.pm
> index db86af2..369e302 100644
> --- a/src/PVE/RESTHandler.pm
> +++ b/src/PVE/RESTHandler.pm
> @@ -426,6 +426,56 @@ sub find_handler {
>  return ($handler_class, $method_info);
>  }
>  
> +my $untaint_recursive;

I got flash backs w.r.t. refcount cycles here keeping all variables, and thus 
memory
inside the body alive forever, don't we need a weaken?

E.g., like we had to do in PVE::Status::Graphite's assemble.
> +
> +$untaint_recursive = sub {
> +my ($param) = @_;
> +
> +my $ref = ref($param);
> +if ($ref eq 'HASH') {
> + $param->{$_} = $untaint_recursive->($param->{$_}) for keys $param->%*;
> +} elsif ($ref eq 'ARRAY') {
> + for (my $i = 0; $i < scalar($param->@*); $i++) {
> + $param->[$i] = $untaint_recursive->($param->[$i]);
> + }
> +} else {
> + if (defined($param)) {

could be merged into upper branch as elsif, but no hard feelings.

> + my ($newval) = $param =~ /^(.*)$/s;
> + $param = $newval;
> + }
> +}
> +
> +return $param;
> +};
> +
> +# convert arrays to strings where we expect a '-list' format and convert 
> scalar
> +# values to arrays when we expect an array (because of www-form-urlencoded)
> +#
> +# only on the top level, since www-form-urlencoded cannot be nested anyway
> +#
> +# FIXME: change gui/api calls to not rely on this during 8.x, mark the
> +# behaviour deprecated with 9.x, and remove it with 10.x
> +my $convert_params = sub { my ($param, $schema) = @_;

please keep the method paramethers on it's own line.

Also, maybe go for a more telling names, as convert_params could mean 
everytrhing
and nothing ^^ 



> +
> +return if !$schema->{properties};
> +return if (ref($param) // '') ne 'HASH';

doesn't this breaks the assignment when used below? I.e.,:

$param = $convert_params->($param, $schema);

or 

[pve-devel] applied: [PATCH pve-network 0/6] sdn multiples fixes

2023-06-06 Thread Thomas Lamprecht
Am 20/04/2023 um 23:36 schrieb Alexandre Derumier:
> This is a resend of the 4 last patches for pve-network
> + 2 new patches
> 
> 
> Alexandre Derumier (6):
>   fix #4657 : evpn: fix exit-node with multiple vrf
>   fix #4425: vxlan|evpn: add vxlan-port option
>   fix #4662 : frr: fix config generation ordering
>   fix #4389 : evpn: exit nodes : null routes subnets from other zones
>   fix #4683 : zones: qinq: fix vlan-protocol with bridge vlan aware
>   network reload: fix UPID parsing
> 
>  PVE/API2/Network/SDN.pm   |  3 +-
>  PVE/Network/SDN/Controllers/BgpPlugin.pm  |  2 +-
>  PVE/Network/SDN/Controllers/EvpnPlugin.pm | 83 ++--
>  PVE/Network/SDN/Vnets.pm  |  4 +-
>  PVE/Network/SDN/Zones/EvpnPlugin.pm   |  4 +
>  PVE/Network/SDN/Zones/QinQPlugin.pm   |  8 +-
>  PVE/Network/SDN/Zones/VxlanPlugin.pm  |  9 ++
>  .../ebgp_loopback/expected_controller_config  |  3 +-
>  .../evpn/exitnode/expected_controller_config  |  1 +
>  .../expected_controller_config|  4 +-
>  .../exitnode_snat/expected_controller_config  |  1 +
>  .../expected_controller_config| 98 +++
>  .../exitnodenullroute/expected_sdn_interfaces | 81 +++
>  test/zones/evpn/exitnodenullroute/interfaces  |  7 ++
>  test/zones/evpn/exitnodenullroute/sdn_config  | 42 
>  .../multiplezones/expected_controller_config  | 49 ++
>  .../multiplezones/expected_sdn_interfaces | 81 +++
>  test/zones/evpn/multiplezones/interfaces  |  7 ++
>  test/zones/evpn/multiplezones/sdn_config  | 37 +++
>  .../evpn/vxlanport/expected_controller_config | 41 
>  .../evpn/vxlanport/expected_sdn_interfaces| 44 +
>  test/zones/evpn/vxlanport/interfaces  |  7 ++
>  test/zones/evpn/vxlanport/sdn_config  | 26 +
>  .../expected_sdn_interfaces   |  4 +
>  .../vxlan/vxlanport/expected_sdn_interfaces   | 16 +++
>  test/zones/vxlan/vxlanport/interfaces |  7 ++
>  test/zones/vxlan/vxlanport/sdn_config | 11 +++
>  27 files changed, 640 insertions(+), 40 deletions(-)
>  create mode 100644 
> test/zones/evpn/exitnodenullroute/expected_controller_config
>  create mode 100644 test/zones/evpn/exitnodenullroute/expected_sdn_interfaces
>  create mode 100644 test/zones/evpn/exitnodenullroute/interfaces
>  create mode 100644 test/zones/evpn/exitnodenullroute/sdn_config
>  create mode 100644 test/zones/evpn/multiplezones/expected_controller_config
>  create mode 100644 test/zones/evpn/multiplezones/expected_sdn_interfaces
>  create mode 100644 test/zones/evpn/multiplezones/interfaces
>  create mode 100644 test/zones/evpn/multiplezones/sdn_config
>  create mode 100644 test/zones/evpn/vxlanport/expected_controller_config
>  create mode 100644 test/zones/evpn/vxlanport/expected_sdn_interfaces
>  create mode 100644 test/zones/evpn/vxlanport/interfaces
>  create mode 100644 test/zones/evpn/vxlanport/sdn_config
>  create mode 100644 test/zones/vxlan/vxlanport/expected_sdn_interfaces
>  create mode 100644 test/zones/vxlan/vxlanport/interfaces
>  create mode 100644 test/zones/vxlan/vxlanport/sdn_config
> 


applied, with `git am --directory=src `, as I separated packaging from 
source
recently, thanks!

ps. would be great to mock the access to CFS for the test system, otherwise 
building this
in isolated environments like via sbuild is broken (I just disabled tests there 
for now).


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



[pve-devel] applied: [PATCH frr 0/4] update to 8.5.1

2023-06-06 Thread Thomas Lamprecht
Am 23/05/2023 um 08:08 schrieb Alexandre Derumier:
> Hi,
> 
> This patch serie update frr to 8.5.1.
> (mirror need to be update to 8.5.1 tag 
> https://github.com/FRRouting/frr/tree/frr-8.5.1)
> 
> I have removed old upstreamed patches, and added 2 importants evpn
> patches released just after 8.5.1.
> 
> 
> 
> Alexandre Derumier (4):
>   patches: remove old upstreamed patches
>   patches : update autort patch
>   patches : add evpn mac mobility fixes from stable/8.0
>   bump version to 8.5.1
> 
>  debian/changelog  |   6 +
>  debian/patches/frr/0001-zebra-buffering.patch |  92 
>  .../0001-zebra-fix-evpn-dup-detected.patch|  46 ++
>  debian/patches/frr/0002-zebra-buffering.patch | 420 --
>  .../0002-zebra-evpn-handle-del-event.patch|  71 +++
>  debian/patches/frr/0003-zebra-buffering.patch | 283 
>  .../frr/ospf6d-fix-infinite-loop.patch|  76 
>  ...on-for-RT-auto-derivation-to-force-A.patch |  24 +-
>  debian/patches/series |   8 +-
>  9 files changed, 138 insertions(+), 888 deletions(-)
>  delete mode 100644 debian/patches/frr/0001-zebra-buffering.patch
>  create mode 100644 debian/patches/frr/0001-zebra-fix-evpn-dup-detected.patch
>  delete mode 100644 debian/patches/frr/0002-zebra-buffering.patch
>  create mode 100644 debian/patches/frr/0002-zebra-evpn-handle-del-event.patch
>  delete mode 100644 debian/patches/frr/0003-zebra-buffering.patch
>  delete mode 100644 debian/patches/frr/ospf6d-fix-infinite-loop.patch
> 

applied, and some general packaging cleanup done in frr, 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] evpn: update config to frr 8.5.1

2023-06-06 Thread Thomas Lamprecht
Am 23/05/2023 um 08:47 schrieb Alexandre Derumier:
> add default values:
>  "no bgp hard-administrative-reset"
>  "no bgp graceful-restart notification"
> 
> to avoid frr-reload warning
> 
> Signed-off-by: Alexandre Derumier 
> ---
>  PVE/Network/SDN/Controllers/EvpnPlugin.pm | 7 ++-
>  .../evpn/advertise_subnets/expected_controller_config | 6 +-
>  .../disable_arp_nd_suppression/expected_controller_config | 6 +-
>  test/zones/evpn/ebgp/expected_controller_config   | 6 +-
>  test/zones/evpn/ebgp_loopback/expected_controller_config  | 6 +-
>  test/zones/evpn/exitnode/expected_controller_config   | 6 +-
>  .../exitnode_local_routing/expected_controller_config | 6 +-
>  .../evpn/exitnode_primary/expected_controller_config  | 6 +-
>  test/zones/evpn/exitnode_snat/expected_controller_config  | 6 +-
>  .../evpn/exitnodenullroute/expected_controller_config | 8 +++-
>  test/zones/evpn/ipv4/expected_controller_config   | 6 +-
>  test/zones/evpn/ipv4ipv6/expected_controller_config   | 6 +-
>  .../evpn/ipv4ipv6nogateway/expected_controller_config | 6 +-
>  test/zones/evpn/ipv6/expected_controller_config   | 6 +-
>  .../zones/evpn/multipath_relax/expected_controller_config | 6 +-
>  test/zones/evpn/multiplezones/expected_controller_config  | 8 +++-
>  test/zones/evpn/rt_import/expected_controller_config  | 6 +-
>  test/zones/evpn/vxlanport/expected_controller_config  | 6 +-
>  18 files changed, 95 insertions(+), 18 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 frr] fix #4040 : patch : ospf6d: fix infinite loop when adding ASBR route

2023-06-06 Thread Thomas Lamprecht
Am 13/04/2023 um 13:48 schrieb Alexandre Derumier:
> ---
>  .../frr/ospf6d-fix-infinite-loop.patch| 76 +++
>  debian/patches/series |  1 +
>  2 files changed, 77 insertions(+)
>  create mode 100644 debian/patches/frr/ospf6d-fix-infinite-loop.patch
> 
>

applied,  thanks!

And yeah, rather late, but your frr update to 8.5.1 depends on that so this
was the easiest thing ^^


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



[pve-devel] applied: [PATCH] pve7to8: ceph version check: ignore commit hash

2023-06-05 Thread Thomas Lamprecht
Am 02/06/2023 um 18:04 schrieb Aaron Lauterer:
> The commit hash of the Ceph version might be different between major
> releases. For example:
> ceph version 17.2.6 (810db68029296377607028a6c6da1ec06f5a2b27) quincy (stable)
> ceph version 17.2.6 (995dec2cdae920da21db2d455e55efbc339bde24) quincy (stable)
> 
> This can lead to unnecessary warnings of multiple detected versions.
> Therefore, extract the version, e.g. 'ceph version 17.2.6', and the
> commit hash. Use the simplified version for the version checks and show
> an info line when the commit is different instead of a warning.
> If the commit hashes are the only difference, we are likely in the
> middle of the upgrade.
> 
> Signed-off-by: Aaron Lauterer 
> ---
>  PVE/CLI/pve7to8.pm | 25 +++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
>

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/RFC widget-toolkit 2/5] apt repositories: just ignore unknown info rather than throwing an error

2023-06-05 Thread Thomas Lamprecht
Am 05/06/2023 um 17:43 schrieb Fiona Ebner:
> This will avoid breaking older UI when extending the backend.
> 
> Signed-off-by: Fiona Ebner 
> ---
>  src/node/APTRepositories.js | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/src/node/APTRepositories.js b/src/node/APTRepositories.js
> index cb08bb6..ed58e5b 100644
> --- a/src/node/APTRepositories.js
> +++ b/src/node/APTRepositories.js
> @@ -660,8 +660,6 @@ Ext.define('Proxmox.node.APTRepositories', {
>   if (!repoGrid.majorUpgradeAllowed) {
>   infos[path][idx].warnings.push(info);
>   }
> - } else {
> - throw 'unknown info';

otherwise we could use some Ext.Msg or at least log via console.warn (maybe it 
helps detecting
a missing thing someday), would both not break UI.

>   }
>   }
>  



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



Re: [pve-devel] [PATCH proxmox-i18n] en: fix spelling mistake, unkown => unknown

2023-06-05 Thread Thomas Lamprecht
Am 15/05/2023 um 05:56 schrieb Marlin Sööse:
> This just fixes a spelling mistake.
> 

Thanks for reporting this and appreciating that you sent a patch already, but
the POT files are auto-generated and come directly from the source.

So, here one would need to fix that, and those are indicated as comment above
each msgid:

pve-manager/www/manager6/form/USBSelector.js:128

As it's just a small typo I fixed it directly (in my hurry I forgot to attribute
you with the findings through a Reported-by tag, sorry about that...)


https://git.proxmox.com/?p=pve-manager.git;a=commitdiff;h=3211e833d9140b8e3241993029c7693756bb9950


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


[pve-devel] applied: [PATCH cluster v4 1/1] add cfg files for resource mapping

2023-06-05 Thread Thomas Lamprecht
Am 25/05/2023 um 12:17 schrieb Dominik Csapak:
> resource/pci.cfg and
> resource/usb.cfg
> 
> to PVE/Cluster.pm
> and status.c
> 
> Signed-off-by: Dominik Csapak 
> ---
>  src/PVE/Cluster.pm  | 2 ++
>  src/pmxcfs/status.c | 2 ++
>  2 files changed, 4 insertions(+)
> 
>

applied, with finalizing our wording for this through s/resource/mapping/, as
talked off-list - thanks!


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



[pve-devel] applied: little PATCH proxmox-i18n for ja.po (for 3.0.0)

2023-06-05 Thread Thomas Lamprecht
Am 03/06/2023 um 05:41 schrieb ribbon:
> little Japanese translation update for 3.0.0
> 
> --- ja.po 2023-06-03 11:10:29.097009127 +0900
> +++ jan.po2023-06-03 11:10:08.708856465 +0900

applied, thanks!

Would be great if you could submit those via git directly:
https://pve.proxmox.com/wiki/Developer_Documentation#Preparing_Patches

While I can work with those diffs, and appreciate any translation update,
it would still simplify things for me a bit.

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 qemu-server 1/1] api2: add check_bridge_access for create/update vm

2023-06-05 Thread Thomas Lamprecht
Am 05/06/2023 um 01:37 schrieb Alexandre Derumier:
> test first if user have access to the full zone (any bridge/vlan)
> if a tag is defined, test if user have a specific access to the vlan (or 
> propagate from full bridge acl)
> if no tag, test if user have access to full bridge. (if trunks are defined, 
> it need also access to full bridge)
> 
> Signed-off-by: Alexandre Derumier 
> ---
>  PVE/API2/Qemu.pm | 38 +-
>  1 file changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 587bb22..4de7b32 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -46,6 +46,12 @@ use PVE::SSHInfo;
>  use PVE::Replication;
>  use PVE::StorageTunnel;
>  
> +my $have_sdn;
> +eval {
> +require PVE::Network::SDN;
> +$have_sdn = 1;
> +};
> +
>  BEGIN {
>  if (!$ENV{PVE_GENERATING_DOCS}) {
>   require PVE::HA::Env::PVE2;
> @@ -601,6 +607,34 @@ my $check_vm_create_usb_perm = sub {
>  return 1;
>  };
>  
> +my $check_bridge_access = sub {
> +my ($rpcenv, $authuser, $param) = @_;
> +
> +return 1 if $authuser eq 'root@pam';
> +
> +foreach my $opt (keys %{$param}) {
> + next if $opt !~ m/^net\d+$/;
> + my $net = PVE::QemuServer::parse_net($param->{$opt});
> + my $bridge = $net->{bridge};
> + my $tag = $net->{tag};

should below move to a method in pve-guest-common, taking $bridge (or name it 
already $vnet) and $tag
as additional parameter, and then be also used by container?

> + my $zone = 'local';
> +
> + if ($have_sdn) {
> + my $vnet_cfg = PVE::Network::SDN::Vnets::config();
> + if (defined(my $vnet = 
> PVE::Network::SDN::Vnets::sdn_vnets_config($vnet_cfg, $bridge, 1))) {
> + $zone = $vnet->{zone};
> + }
> + }
> + # test first if user have access to the full zone (any bridge/vlan)
> + return 1 if $rpcenv->check_any($authuser, "/sdn/zones/$zone", 
> ['SDN.Audit', 'SDN.Allocate'], 1);
> + # if a tag is defined, test if user have a specific access to the vlan 
> (or propagate from full bridge acl)
> + return 1 if $tag && $rpcenv->check_any($authuser, 
> "/sdn/vnets/$bridge/$tag", ['SDN.Audit', 'SDN.Allocate'], 1);
> + # if no tag, test if user have access to full bridge. (if trunks are 
> defined, it need also access to full bridge)
> + $rpcenv->check_any($authuser, "/sdn/vnets/$bridge", ['SDN.Audit', 
> 'SDN.Allocate']);
> +}
> +return 1;
> +};
> +
>  my $check_vm_modify_config_perm = sub {
>  my ($rpcenv, $authuser, $vmid, $pool, $key_list) = @_;
>  
> @@ -878,7 +912,7 @@ __PACKAGE__->register_method({
>  
>   &$check_vm_create_serial_perm($rpcenv, $authuser, $vmid, $pool, 
> $param);
>   &$check_vm_create_usb_perm($rpcenv, $authuser, $vmid, $pool, 
> $param);
> -
> + &$check_bridge_access($rpcenv, $authuser, $param);
>   &$check_cpu_model_access($rpcenv, $authuser, $param);
>  
>   $check_drive_param->($param, $storecfg);
> @@ -1578,6 +1612,8 @@ my $update_vm_api  = sub {
>  
>  &$check_storage_access($rpcenv, $authuser, $storecfg, $vmid, $param);
>  
> +&$check_bridge_access($rpcenv, $authuser, $param);
> +
>  my $updatefn =  sub {
>  
>   my $conf = PVE::QemuConfig->load_config($vmid);



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



[pve-devel] applied: [PATCH cluster] pvecm: fix cluster join over ssh with newer rsync

2023-06-04 Thread Thomas Lamprecht
Am 02/06/2023 um 15:20 schrieb Dominik Csapak:
> since rsync 3.2.4, the syntax to give multiple files in one parameter
> does not work anymore, so instead add both files explicitly
> 
> this fixes the cluster join over ssh on bookworm
> 
> Signed-off-by: Dominik Csapak 
> ---
>  src/PVE/CLI/pvecm.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: [PATCH v2 proxmox 1/5] apt: drop older Ceph standard repositories

2023-06-04 Thread Thomas Lamprecht
Am 02/06/2023 um 10:48 schrieb Fiona Ebner:
> On Proxmox VE 8, only Quincy and newer will be supported.
> 
> Signed-off-by: Fiona Ebner 
> ---
> 
> No changes in v2.
> 
> Changes for the series in v2:
> * create temporary test directories inside CARGO_TARGET_TMPDIR
> * mention that deprecated 'main' component maps to no-subscription
>   in description of the repo.

A cover letter would make this a bit easier to notice. ^^

> 
>  proxmox-apt/src/repositories/mod.rs  |  4 --
>  proxmox-apt/src/repositories/standard.rs | 58 
>  proxmox-apt/tests/repositories.rs|  4 --
>  3 files changed, 66 deletions(-)
> 
>

applied, thanks!

But I'd favor that the repo infos are namespaced by dist (bullseye, bookworm, 
..), that way
we could keep historical records and add future ones up-front (for a future 
"change repos
to next release" helper), that would then also allow to generate some docs for 
available
repos automatically, e.g.: https://pve.proxmox.com/wiki/Package_Repositories

For the "Add Repo" UI it might be nicer to split repo & product, i.e., have 
another field
above the current repository one where one selects "Proxmox VE" or "Ceph", can 
simply be
disabled+hidden in Proxmox Backup Server and Proxmox Mail Gateway web UI's (as 
IIRC we
share the UI down to the whole panel).


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



Re: [pve-devel] [PATCH-SERIES v3 qemu-server/manager/common] add and set x86-64-v2 as default model for new vms and detect best cpumodel

2023-06-03 Thread Thomas Lamprecht
Am 02/06/2023 um 11:13 schrieb DERUMIER, Alexandre:
> at minimum, it could be interesting to expose the flag in the gui, for
> users really needed intel-amd migration.

acked also from my side, we can use the flag description for the a note
that it might benefit cross vendor migration (with some overal slowdown).


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



Re: [pve-devel] [PATCH-SERIES v3 qemu-server/manager/common] add and set x86-64-v2 as default model for new vms and detect best cpumodel

2023-06-03 Thread Thomas Lamprecht
Am 01/06/2023 um 11:06 schrieb DERUMIER, Alexandre:
>> Maybe the easiest would be to extract the aes flag out of the grid
>> into
>> the non-advanced part?
>>
> Couldn't be easier to keep aes enable by default in a single model
> (even if it's doesn't match the x86-64 spec). and allow user to optin
> disable it.
> The only server where you need to disable aes if for nahelem, and I
> don't think that a lot of users still have this cpu in production.
> (so keeping the aes flag in advanced section make sense).
> Also, user with really old servers, could keep to use kvm64 model,
> where aes is not enabled.


I also think that we should not bend to much for Nehalem, and fwiw if
we go for a v2+aes CPU model (which IMO is one of the easies solutions)
we would only need that for v2, as v3 could always have that enabled by
default FWIWCT?

So:

x86-64-v2
x86-64-v2-aes   (UI default)
x86-64-v3

(and that as real models, not just UI fakery) would work,

But tbh., I'd not be completely against just enabling it for v2 and warning,
or even erroring, if the VM gets created on a host that doesn't supports it
(which might be a good idea for any vX level); as Alexandre is right, a lot of
users needlessly get slower VMs that they could be and production setups with
a 14 year old CPU are just very unlikely, and they simply can disable AES 
again...


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



Re: [pve-devel] [PATCH-SERIES v3 qemu-server/manager/common] add and set x86-64-v2 as default model for new vms and detect best cpumodel

2023-06-03 Thread Thomas Lamprecht
Am 01/06/2023 um 10:34 schrieb Fiona Ebner:
> Am 31.05.23 um 16:34 schrieb DERUMIER, Alexandre:
>> Le mercredi 31 mai 2023 à 13:36 +0200, Fiona Ebner a écrit :
>>> Am 22.05.23 um 12:25 schrieb Alexandre Derumier:
 In addition to theses model, I have enabled aes too.
 I think it's really important, because a lot of users use default
 values and have
 bad performance with ssl and other crypto stuffs.

>>>
>>> So there is the answer to my aes question :) But shouldn't we rather
>>> set
>>> it via the UI as a default than change the CPU definition itself?
>>> That
>>> feels cleaner as we'd not diverge from how they defined the ABI.
>>
>> I don't have looked pve-manager code yet, but do you think it's easy
>> to auto enable/disable the aes flag in the grid when we choose theses
>> models ?
> 
> I also haven't looked at the code, but yeah, it is an issue that it's in
> the advanced part and we shouldn't hide it from the user that it's on.
> 
>> Maybe could it be better to have 2 differents models, with/without aes
>> (like some qemu models versions like -IBRS,  
>> here we could have
>>
>> x86-64-v2
>> x86-64-v2-aes   (default)
>> x86-64-v3
>> x86-64-v3-aes
> 
> That might work, but if we do that, please only in the UI. Also not
> ideal, because how would interaction with the flag in the grid work?
> E.g. don't show it, force it on if an -aes model is selected?

Please no, I would find it very odd to see a CPU model in the Web UI that
doesn't exist in the API/Backend.

> 
> Maybe the easiest would be to extract the aes flag out of the grid into
> the non-advanced part?

Yes, rather that – but possibly without the radio-group UI but simply a
combobox with "Yes" <- default, "Auto (CPU model)" or "no" options


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


Re: [pve-devel] [PATCH-SERIES pve-manager/qemu-server] fix#4689 autofind node with proxyto_callback

2023-06-03 Thread Thomas Lamprecht
Hi!

Am 01/06/2023 um 00:28 schrieb Alexandre Derumier:
> Currently, to manage qemu && lxc vms, we always need to specify nodename in 
> uri.
> 
> This is a problem with automation tools like terraform, where is node is 
> registered
> in the state of terraform.

What do you need here, the whole API, just some operation on existing VMs
like start, stop, migrate, or just that + VM creation?

> (That mean, than if we move the vm on another node, terraform don't known it, 
> and try to create the vm
> again or can't delete the vm,...)
> https://github.com/Telmate/terraform-provider-proxmox/issues/168
> 
> This can also be a potential problem with race, if we need to query 
> /cluster/ressources to find the node, then another
> query on the vm.
> 
> I have some discussion with fabian about it:
> https://bugzilla.proxmox.com/show_bug.cgi?id=4689
> 
> 
> This patch series, find the nodename with proxyto_callback.
>> a new api endpoint /guests/  is defined:
> 
> /guests/(qemu|lxc)/vmid
> 

exposing the same API through multiple points isn't really REST-y design,
could even break things and would def. need special handling to make this
actually visible in the API schema, and thus pvesh and the api viewer, among
other things.

> 
> This patch series currently implement callback for api2::qemu
> 
> I'm not sure how to create vm_create when vmid && nodename is not defined.
> Currently the callback return localhost, so the vm is created on the called
> node.
> 
> todo (if this patch serie is ok for you):


ATM I'd rather strongly object this, for one to avoid that more work is done 
that
then might not get accepted, which would be avoidable waste for all of us, and 
as
temporary reason: this definitively won't get into Proxmox VE 8.0, but as new
functionality, which doesn't breaks existing stuff, it can be added just fine to
8.1 or 8.2 – and so I'd like to postpone in-depth review and accepting it into
the source tree until a few weeks after 8.0 Release where I got a bit more time
to calmly think about this – because the base idea of having such a feature
is definitively compelling and I think quite a few admins and devs that 
re-integrate
our API would like to see this.

Still, some thoughts that I couldn't suppress ;P:

- the datacenter manager might avoid a lot of such issues already, there we
  need to resolve Guest ID's to nodes anyway. But, it'd require having that
  setup always, which might not be wanted.

- could putting an adapter between Terraform <-> Proxmox VE API work?
  Did not really use Terraform, so I'm just guesstimating here.

- FWIW; the HA stack already is somewhat of a automagic node resolver, but
  naturally only for operative things like start/stop/migrate, but if one would
  just require those actions then it might be a feasible way that already 
exists.

- We got the Batch-process API calls Endpoint already and while I actually 
planned
  to remove that for PVE 8 (for other, mostly security reasons), if we'd keep 
(and
  fix) that, one could potentially also add proxying  and relaying support 
there.

That said, I have to admit that the solution you choose, while being a bit 
hacky is
really not a invasive change code-wise; But, and here still assuming we go that
direction in the first place, I  still don't like doing that in such a subtle 
way.

Rather, I'd add something like a "inherit" property that register_method 
understands
and then we could re-register API endpoint paths under another path, while 
letting
the schema and routing actually know about it. 

I'd would probably do that even in a lightweight way, i.e., resolved 
dynamically and
then also calling the "actual" original code site, to avoid potential bugs 
w.r.t.
imports and global variables from more in-depth copies.

I.e., usage would look something like:


pve-manager:# cat PVE/API2/Guests/Qemu.pm

# ...

use PVE::API2::Qemu; # <- required to ensure the methods we want to inherit 
from got registered

use base qw(PVE::RESTHandler);

__PACKAGE__->register_method({
name => 'index',
path => '{vmid}',
# ...
code => sub {
return [
{ name => 'config' },
# ...
];
});

__PACKAGE__->register_method({
name => 'set_config',
path => '{vmid}',
method => 'GET',
inherit => 'nodes/{nodes}/qemu/{vmid}/config',
proxyto_callback => \::API2Tools::resolve_vmid_node,
});

# ... other api endpoint's we like to expose in a node-independent way.

1;


For that to work we'd (probably, I did not check _that_ closely) need to adapt 
the base
RestHandler's register_method and map_path_to_methods, but for either the 
changes should
be in reasonable size.

For starters I'd only allow-list a few properties that can be overridden on 
such a
inheritance, as e.g., exposing the same thing with different privileges might be
questionable at best and give us some security woes.

I had something like above approach in my mind for a few other things already 
in the past,
e.g., in the 

Re: [pve-devel] [PATCH widget-toolkit 3/3] fix #3892: NetworkEdit: add bridge vids field for bridge_vids

2023-06-01 Thread Thomas Lamprecht
Don't we reuse that on PBS/PMG too, and if is it working there?

The commit message isn't excactly telling... ;-)


Am 13/04/2023 um 17:10 schrieb Aaron Lauterer:
> Signed-off-by: Aaron Lauterer 
> ---
>  src/node/NetworkEdit.js | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/src/node/NetworkEdit.js b/src/node/NetworkEdit.js
> index bb9add3..e4ab158 100644
> --- a/src/node/NetworkEdit.js
> +++ b/src/node/NetworkEdit.js
> @@ -57,11 +57,26 @@ Ext.define('Proxmox.node.NetworkEdit', {
>   }
>  
>   if (me.iftype === 'bridge') {
> + let vids = Ext.create('Ext.form.field.Text', {
> + fieldLabel: gettext('Bridge VIDS'),
> + name: 'bridge_vids',
> + emptyText: '2-4094',
> + disabled: true,
> + autoEl: {
> + tag: 'div',
> + 'data-qtip': gettext('Space-separated list of VLANs and 
> ranges, for example: 2 4 100-200'),
> + },
> + });
>   column2.push({
>   xtype: 'proxmoxcheckbox',
>   fieldLabel: gettext('VLAN aware'),
>   name: 'bridge_vlan_aware',
>   deleteEmpty: !me.isCreate,
> + listeners: {
> + change: function(f, newVal) {
> + vids.setDisabled(!newVal);
> + },
> + },
>   });
>   column2.push({
>   xtype: 'textfield',
> @@ -72,6 +87,7 @@ Ext.define('Proxmox.node.NetworkEdit', {
>   'data-qtip': gettext('Space-separated list of interfaces, 
> for example: enp0s0 enp1s0'),
>   },
>   });
> + advancedColumn2.push(vids);
>   } else if (me.iftype === 'OVSBridge') {
>   column2.push({
>   xtype: 'textfield',



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



Re: [pve-devel] [PATCH widget-toolkit 2/2] utils: format_size: show negative size as NA

2023-06-01 Thread Thomas Lamprecht
Am 19/04/2023 um 12:34 schrieb Aaron Lauterer:
> Signed-off-by: Aaron Lauterer 
> ---
> 
> AFAIK we do not have negative sizes anywhere, and if, it is an
> indication that something is wrong.

above belongs in the commit message, additionaly some background for why doing
this now (i.e., did you run into this or what made you make this change?)

> 
>  src/Utils.js | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/Utils.js b/src/Utils.js
> index ef72630..8cdbe86 100644
> --- a/src/Utils.js
> +++ b/src/Utils.js
> @@ -688,6 +688,9 @@ utilities: {
>  },
>  
>  format_size: function(size, useSI) {
> + if (size < 0) {
> + return gettext("N/A");

catching this seems OK, but I'd rather just return the value then, as "N/A" (Not
Applicable) doesn't really makes sense here and just hides a potential 
underlying
problem.

> + }
>   let units = ['', 'K', 'M', 'G', 'T', 'P', 'E', 'Z', 'Y'];
>   let order = 0;
>   const baseValue = useSI ? 1000 : 1024;



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



[pve-devel] applied: [PATCH proxmox-widget-toolkit v2 1/1] fix 4551: ui: translate byte unit in `format_size`

2023-06-01 Thread Thomas Lamprecht
Am 06/04/2023 um 13:38 schrieb Noel Ullreich:
> Some languages translate byte units like 'GiB' or write them in their
> own script.
> 
> By `gettext`ing the units in the `format_size` function, we can
> translate the units for (almost) all of the web interface.
> 
> Signed-off-by: Noel Ullreich 
> ---

-> add the v1 -> v2 changelog (filtered for the current patch) here too please

>  src/Utils.js | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
>

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 proxmox-widget-toolkit v2 1/1] fix #4551: ui: use gettext on hardcoded byte units

2023-06-01 Thread Thomas Lamprecht
Used wrong tag, this is for manager, not widget-toolkit...


Am 06/04/2023 um 13:38 schrieb Noel Ullreich:
> Since some languages translate byte units like 'GiB' or write them in their
> own script, this patch wraps units in the `gettext` function.
> 
> While most occurrences of byte strings can be translated within the
> `format_size` function in `proxmox-widget-toolkit/src/Utils.js`, this patch
> catches those instances that are not translated.
> 
> Signed-off-by: Noel Ullreich 
> ---
>  www/manager6/ceph/OSD.js | 4 ++--
>  www/manager6/form/DiskStorageSelector.js | 2 +-
>  www/manager6/lxc/MPResize.js | 2 +-
>  www/manager6/qemu/HDResize.js| 2 +-
>  www/manager6/qemu/HardwareView.js| 2 +-
>  5 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/www/manager6/ceph/OSD.js b/www/manager6/ceph/OSD.js
> index 2f12f94d..61fe5b06 100644
> --- a/www/manager6/ceph/OSD.js
> +++ b/www/manager6/ceph/OSD.js
> @@ -83,7 +83,7 @@ Ext.define('PVE.CephCreateOsd', {
>   {
>   xtype: 'numberfield',
>   name: 'db_dev_size',
> - fieldLabel: gettext('DB size') + ' (GiB)',
> + fieldLabel: `${gettext('DB size')} 
> (${gettext('GiB')})`,
>   minValue: 1,
>   maxValue: 128*1024,
>   decimalPrecision: 2,
> @@ -137,7 +137,7 @@ Ext.define('PVE.CephCreateOsd', {
>   {
>   xtype: 'numberfield',
>   name: 'wal_dev_size',
> - fieldLabel: gettext('WAL size') + ' (GiB)',
> + fieldLabel: `${gettext('WAL size')} 
> (${gettext('GiB')})`,
>   minValue: 0.5,
>   maxValue: 128*1024,
>   decimalPrecision: 2,
> diff --git a/www/manager6/form/DiskStorageSelector.js 
> b/www/manager6/form/DiskStorageSelector.js
> index abd46deb..d408b815 100644
> --- a/www/manager6/form/DiskStorageSelector.js
> +++ b/www/manager6/form/DiskStorageSelector.js
> @@ -148,7 +148,7 @@ Ext.define('PVE.form.DiskStorageSelector', {
>   itemId: 'disksize',
>   reference: 'disksize',
>   name: 'disksize',
> - fieldLabel: gettext('Disk size') + ' (GiB)',
> + fieldLabel: `${gettext('Disk size')} (${gettext('GiB')})`,
>   hidden: me.hideSize,
>   disabled: me.hideSize,
>   minValue: 0.001,
> diff --git a/www/manager6/lxc/MPResize.js b/www/manager6/lxc/MPResize.js
> index 881c037b..d560b788 100644
> --- a/www/manager6/lxc/MPResize.js
> +++ b/www/manager6/lxc/MPResize.js
> @@ -52,7 +52,7 @@ Ext.define('PVE.window.MPResize', {
>   maxValue: 128*1024,
>   decimalPrecision: 3,
>   value: '0',
> - fieldLabel: gettext('Size Increment') + ' (GiB)',
> + fieldLabel: `${gettext('Size Increment')} (${gettext('GiB')})`,
>   allowBlank: false,
>   });
>  
> diff --git a/www/manager6/qemu/HDResize.js b/www/manager6/qemu/HDResize.js
> index f9c7290d..29ff253b 100644
> --- a/www/manager6/qemu/HDResize.js
> +++ b/www/manager6/qemu/HDResize.js
> @@ -49,7 +49,7 @@ Ext.define('PVE.window.HDResize', {
>   maxValue: 128*1024,
>   decimalPrecision: 3,
>   value: '0',
> - fieldLabel: gettext('Size Increment') + ' (GiB)',
> + fieldLabel: `${gettext('Size Increment')} (${gettext('GiB')})`,
>   allowBlank: false,
>   });

this seems to be cut-off here, missign the full context, thus I get:


Applying: fix #4551: ui: use gettext on hardcoded byte units
error: corrupt patch at line 70



___
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] fix regression in api/html (bootstrap) viewer

2023-05-30 Thread Thomas Lamprecht
Am 30/05/2023 um 13:56 schrieb Dominik Csapak:
> recent versions of URI::Escape seem to handle the 'unsafe characters'
> parameter differently than before, enforcing what is documented:
> 
>  The set is specified as a string that can be used in a regular
>  expression character class (between [ ]).
> 
> so the leading/trailing [] were never supposed to be there.


Since liburi-perl 5.15 we could also use a qr// regex object [0], maybe that
would be a bit more explicit here; Debian Bookworm has already 5.17 available.

[0]: 
https://github.com/libwww-perl/URI/commit/a3f96169b002e1fa747713654edfa9f528d17cbb

> 
> Signed-off-by: Dominik Csapak 
> ---
>  src/PVE/APIServer/Formatter/Bootstrap.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

Anyhow, we don't have to touch this often and its for a dev/debug features, so
for now:

applied, with commit message amended with referencing the commit that broke
our usage and the possibility of regex objects, thanks!


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



Re: [pve-devel] Plan for (invasive) shrink of pve-manager git repository

2023-05-28 Thread Thomas Lamprecht
Am 28/05/2023 um 20:38 schrieb Thomas Lamprecht:
> For re-aligning your local master branch you can do a hard-reset, BUT check
> if you got any local commits yet (move them over to another branch with e.g.
> `git checkout -b feature-to-re-apply-on-master`
> 
> git checkout master
> git reset --hard origin/master
> 
> Then re-create your active development branches freshly from the master
> and cherry-pick the relevant patches from the old branch.
> 
> After that you can delete the old branches.
> 

Two things I forgot to mention, after above and ensuring no remote or branch
refers to the old git repo anymore, you can use the following to shrink:

git gc --aggressive --prune=now

But, moving the current pve-manager dir to a backup location and just cloning
freshly is waay faster

The other thing was that I had to split out sencha-touch ZIP into it's own repo
before the filter-repo clean up, it lives now in a libjs-sencha-touch package
and its source can be found here: 
https://git.proxmox.com/?p=sencha-touch.git;a=summary

(and just for completeness sake, note that this was only done for pure compat
reasons only, the mobile UI in PVE that uses it is pretty bare bones and doesn't
gets much love, we should replace it by something slightly more future proof 
some
day).

cheers,
 Thomas


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



Re: [pve-devel] Plan for (invasive) shrink of pve-manager git repository

2023-05-28 Thread Thomas Lamprecht
Am 26/05/2023 um 11:45 schrieb Thomas Lamprecht:
> I'll use the git filter-repo [0] tool, a replacement for filter-branch with
> better UX and less potential for getting it wrong, to rewrite the history,
> filtering out any problematic artefact or directory.
> 
> For this I'll use the following file-list
> 
> www/ext6
> www/ext5
> www/ext4
> www/touch
> po
> glob:*.zip
> 
> used as inverted match via the following command:
> 
> git filter-repo --invert-paths --paths-from-file
> ~/pve-manager-inverted-filter-paths
> 
> Then, I'd rename the current "pve-manager.git" hosted at git.proxmox.com to
> "pve-manager-legacy.git", so it will still be able as reference for ancient
> history, providing the possibility to build pre PVE 5 pve-manager packages
> (why ever one would want/needs to do that).
> 
> A new repo, with the same name "pve-manager.git", would then get created and
> the now cleaned up git repo pushed to it.

Above has been carried out now.

Old repo is still available here:
https://git.proxmox.com/?p=pve-manager-legacy.git;a=summary


If you fetch in an existing pve-manager.git repository you'll see something 
like:
>From git://git.proxmox.com/git/pve-manager
 + f548e4fca...4a8501a8b master -> origin/master  (forced update)
 + 40ccc11c4...d26a7c43e stable-3   -> origin/stable-3  (forced update)
 + 08ba4d2dd...789b4067b stable-4   -> origin/stable-4  (forced update)
 + d0ec33c69...b80838a2f stable-5   -> origin/stable-5  (forced update)
 + 6ba2c0bcf...b31a318d0 stable-6   -> origin/stable-6  (forced update)

For re-aligning your local master branch you can do a hard-reset, BUT check
if you got any local commits yet (move them over to another branch with e.g.
`git checkout -b feature-to-re-apply-on-master`

git checkout master
git reset --hard origin/master

Then re-create your active development branches freshly from the master
and cherry-pick the relevant patches from the old branch.

After that you can delete the old branches.

> # Fallout
> 
> This naturally has some fallout for developers currently working patch series,
> similar to any force-push (which we normally avoid at all cost).
> 
> Rebasing won't work IIUC, but as the source file layout won't change, you can
> simply use "git cherry-pick " if you have the before filter and
> after filter remotes & branches in the same git repo.  Otherwise, one can
> always use "git format-patch -o ~/patches/ " in the old repo to
> export patches cleanly, and then use "git am -3 ~/patches/*.patch" in the new
> repo.

FWIW, I migrated over my branches, and cherry-picking worked well.

> 
> Note that git commit hash references inside commit messages of pve-manager 
> will
> get rewritten, so here won't notice anything.  Commit references from other
> repos are naturally untouched, but pve-manager being a leave package means 
> that
> it won't have that many in other repos.

Note that above is wrong, my test for that was misguided, but filter-repo does
check for this and outputs it as "suboptimal-issues" file (see below), it 
luckily
ain't that many as we only (relatively) recently began to track stuff like 
"Fixes"
in there.

> I'll safe a copy of the old -> new commit reference map that git filter-repo
> produces, ensuring we got full transparency.

This is publicly available here:

https://pve.proxmox.com/pve-manager-filter-repo-result/

Most interesting will be the "commit-map" file.
In the "ref-map" I marked those branches which I did not copy over, mostly some
ancient hot fix branches; OTOH, all stable-X branches *got* copied over.

Again, sorry for any trouble and headache this may cause, if you have specific
question (or see something that is off) -> ask me (e.g., reply to this mail on
the list)

cheers,
 Thomas


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



Re: [pve-devel] [PATCH access-control] ldap: fix ldap distinguished names regex

2023-05-28 Thread Thomas Lamprecht
Am 17/05/2023 um 15:42 schrieb Stefan Sterz:
> sorry just noticed i forgot: adding tests for this was
> 
> Suggested-by: Dominik Csapak 

I mean, it's really great that we finally get some tests here and if only
Dominik statement made you do it, then chapeau to him! But, Suggested-by's
should be mostly for suggestion on implementation (details), as you all
definitively have a blanket statement from me, and I'd think it's safe to say
also from all other maintainers, to add tests!

I mean, great if that was all needed to solve the motivation to write tests
in developers ;-P




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



[pve-devel] applied: [PATCH widget-toolkit] tfa: paperkey: cleanup iframes for printing after window close

2023-05-28 Thread Thomas Lamprecht
Am 03/04/2023 um 14:28 schrieb Aaron Lauterer:
> Signed-off-by: Aaron Lauterer 
> ---
> similar as in
> https://git.proxmox.com/?p=pve-manager.git;a=commitdiff;h=b380cb5814ec551588dfcbd6c5ed190f01d54029

added the infor of above into the commit message, such cross references
have always some merit.

> 
>  src/window/AddTfaRecovery.js | 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



Re: [pve-devel] [PATCH v4 manager] ui: ceph: improve discoverability of warning details

2023-05-28 Thread Thomas Lamprecht
Am 26/05/2023 um 10:39 schrieb Aaron Lauterer:
> ping? Considering that some users are rather hesitant to upgrade to a major 
> version, it might be a good idea to still get this into Proxmox VE 7 to make 
> it easier for users to discover more details about any issues they experience.

I think that's probably the best argument can make here, as this is to much
in "new feature" territory for my taste.
But, wouldn't this be possible besser solved in the checker script though?
If one gets as far as seeing warnings in the UI overview but just can't figure
out how to view details on them I'd think that they either ask on a support
channel or could see more details in the checker script which they should run
anyway, avoiding any breakage potential.


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



[pve-devel] Plan for (invasive) shrink of pve-manager git repository

2023-05-26 Thread Thomas Lamprecht
Hi all!

It follows a head's up for the plan of making it easier to work with our
pve-manager git repository by rewriting its history to filter out huge
artefacts.

This will only affect developers, nothing in the current pve-manager Debian
package will change.


# Background

Our current pve-manager git repository is huge (> 500 MB) and this is mostly
due to hosting various huge copies of ExtJS, both as ZIP archive and as
extracted version directly in its git history.

Nowadays, well since Q1 of 2017 (before Proxmox VE 5), those huge artefacts are
not used anymore, as we slit the one still in use, like the ExtJS GPL source
code, out to its own repo, without any ZIP archives.  But, git being git and
providing a full history of every change still needs to hold copies of those
artefacts in its CAS object store, one cannot really mask those in any (for
development) ergonomic way.


# Proposed Solution

I'll use the git filter-repo [0] tool, a replacement for filter-branch with
better UX and less potential for getting it wrong, to rewrite the history,
filtering out any problematic artefact or directory.

For this I'll use the following file-list

www/ext6 www/ext5 www/ext4 www/touch po glob:*.zip

used as inverted match via the following command:

git filter-repo --invert-paths --paths-from-file
~/pve-manager-inverted-filter-paths

Then, I'd rename the current "pve-manager.git" hosted at git.proxmox.com to
"pve-manager-legacy.git", so it will still be able as reference for ancient
history, providing the possibility to build pre PVE 5 pve-manager packages
(why ever one would want/needs to do that).

A new repo, with the same name "pve-manager.git", would then get created and
the now cleaned up git repo pushed to it.


# Result

The result of above command measured by .git disk usage:

Before:  551 MB After:26 MB

So a huge reduction.


# Fallout

This naturally has some fallout for developers currently working patch series,
similar to any force-push (which we normally avoid at all cost).

Rebasing won't work IIUC, but as the source file layout won't change, you can
simply use "git cherry-pick " if you have the before filter and
after filter remotes & branches in the same git repo.  Otherwise, one can
always use "git format-patch -o ~/patches/ " in the old repo to
export patches cleanly, and then use "git am -3 ~/patches/*.patch" in the new
repo.

Note that git commit hash references inside commit messages of pve-manager will
get rewritten, so here won't notice anything.  Commit references from other
repos are naturally untouched, but pve-manager being a leave package means that
it won't have that many in other repos.

I'll safe a copy of the old -> new commit reference map that git filter-repo
produces, ensuring we got full transparency.


# Date of Change

I'll probably carry above out tomorrow, Saturday 2023-05-27, sometimes between
10:00 CEST and day's end, but writing today for a short heads-up.

For the record: this plan was discussed with Dietmar Maurer and Dominik, and as
said, this is "only" affecting developers.  And yes, it is a bit of a nuisance
and generating some churn, but we talked about doing this every other year, and
it won't get better on it's own, so let's just finally go for it.

cheers
 Thomas


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



Re: [PVE-User] vGPU scheduling

2023-05-25 Thread Thomas Lamprecht
Am 25/05/2023 um 09:43 schrieb Dominik Csapak:
>>
>> Do you think it'll be merged for proxmox 8 ?
> 
> i don't know, but this also depends on the capacity of my colleagues to 
> review 

making it easy to digest and adding (good) tests will surely help to
accelerate this ;-P

But, you're naturally right, and tbh., while I'll try hard to get the 
access-control
and some other fundaments in, especially those where we can profit from the 
higher
freedom/flexibility of a major release, I cannot definitely say that the actual 
HW
mapping will make it for initial 8.0.

For initial major release I prefer having a bit less features but focus more on 
that
the existing features keep working and that there's a stable and well tested 
upgrade path.


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


[pve-devel] applied: [PATCH qemu-server] fix #4737: qmeventd: gracefully handle interrupted epoll_wait call

2023-05-24 Thread Thomas Lamprecht
Am 24/05/2023 um 12:30 schrieb Fiona Ebner:
> Signed-off-by: Fiona Ebner 
> ---
>  qmeventd/qmeventd.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
>

applied and cherry-picked to stable-7, 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-SERIES v2 qemu] update to QEMU 8.0

2023-05-23 Thread Thomas Lamprecht
Am 15/05/2023 um 15:39 schrieb Fiona Ebner:
> After weeks and weeks of (sometimes painful) debugging, it's finally
> here. And got a load of stable fixes on top already. More testing is
> always appreciated, especially backup, PBS live restore and snapshots,
> which needed quite a few changes!
> 
> Changes from v1:
> * Add fix for lintian overrides.
> * Add patch squashing related changes (not required for 8.0 but will
> make life easier going forward).
> 
> Fiona Ebner (7):
>   d/rules: drop virtiofsd switch
>   d/rules: set job flag for make based on DEB_BUILD_OPTIONS
>   buildsys: fix lintian overrides
>   update submodule and patches to QEMU 8.0.0
>   add stable patches for 8.0.0
>   PVE backup: don't call no_co_wrapper function from coroutine
>   squash related patches
> 

applied, rebased onto your bookworm-build-cleanup branch with some additional
cleanups added, thanks!

Albeit I did not really test much for now, and w.r.t. packaging the switch
from manual d/rules to using dh wildcard rule is still standing out, but I
figured that's easier to do later and should not change the build result..


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



[pve-devel] applied: [PATCH ifupdown2 0/6] bump to 3.2.0

2023-05-20 Thread Thomas Lamprecht
Am 16/05/2023 um 00:47 schrieb Alexandre Derumier:
> This patch series bump version to 3.2.0.
> 
> This need update of proxmox ifupdown2 mirror to last master
> a0522546b848435115a20eb647f87ade01761a33 commit , as
> the 3.2.0 tagged version is missing 1 patch fix.

FYI: After checking out the submodule at that commit, it can add
be added as normal commit change in the outer repository, that way
you can already send it along to ensure that the one applying
it doesn't forgets to do that (or chooses a wrong commit by mistake).

> 
> I have remove old upstreamed patches,
> and add a new patch for ipv6 slaac support.
> 
> I only have tested with debian11 currently.
> 
> 
> Alexandre Derumier (6):
>   patch: patch5: fix code nit
>   patch: remove old upstreamed patches
>   patch: reorder patches
>   patch: add ipv6 slaac support upstream patch
>   patch: remove lacp bond min-links=0 warning
>   bump version to 3.2.0-1+pmx1
applied, thanks!

FYI: I reworked the build system a bit, i.e., merged upstreams debian/ into ours
and dropped our patches related to that. This means that we have check if there
are any packaging specific changes on new releases, but also allows us to build
source pacakages for clean building and to make changes w.r.t. packaging much
easier. So if anything has to change in the debian/ folder in the future you can
just edit that directly in the top-level one, no need for patches there any 
more.


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



Re: [pve-devel] [PATCH pve-qemu 1/1] patch: add 0001-add-cpu-models-x86-64-abi.patch

2023-05-19 Thread Thomas Lamprecht
Am 17/05/2023 um 11:08 schrieb Fiona Ebner:
>> (BTW, what do you think about bumping the default model ? is it ok for
>> you ?)
>>
> Yes, I do think it should be done, because most currently in-use CPUs
> should be fine with x86-64-abi2 and it's not nice to users if the
> defaults don't work out-of-the-box[0]. And other people can just select
> kvm64. Changing the default in the UI for new VMs should not lead to any
> breakage and your series does just that. Still, something we'll want to
> mention in the PVE 8 release notes to not catch people off guard.

+1 from my side.


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



[pve-devel] applied: [PATCH docs] pmxcfs: update size limit to 128 MiB

2023-05-18 Thread Thomas Lamprecht
Am 11/04/2023 um 10:16 schrieb Aaron Lauterer:
> Signed-off-by: Aaron Lauterer 
> ---
> 
> I also did a reformat of that paragraph.
> 
>  pmxcfs.adoc | 8 
>  1 file changed, 4 insertions(+), 4 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: [pbs-devel] [PATCH widget-toolkit 0/2] Proxmox Dark Theme Fix-ups - Round 6

2023-05-17 Thread Thomas Lamprecht
Am 14/04/2023 um 15:28 schrieb Stefan Sterz:
> this patch series adds a couple more tweaks to the dark theme. first
> it improves the contrast ratios on panel header tool icons. then the
> chart and gauge colors are brightened to match the primary color.
> 
> for the backup server first the path to the dark theme css in the api
> viewer is fixed. then the theme switcher menu item is renamed to
> "color theme" to match promox ve. the final patch also renames this
> menu item for the mail gateway.
> 
> Stefan Sterz (2) @ widget-toolkit:
>   dark-mode: adjust panel header tool icons
>   fix #4618: dark-mode: lighten critical/warning charts/gauges colors
> 
>  src/proxmox-dark/scss/extjs/_panel.scss| 37 +++---
>  src/proxmox-dark/scss/extjs/_progress.scss |  4 +--
>  src/proxmox-dark/scss/other/_charts.scss   |  4 +--
>  3 files changed, 36 insertions(+), 9 deletions(-)
> 
> Stefan Sterz (2) @ proxmox-backup:
>   docs: fix api viewer dark theme path
>   ui: main view: rename "Theme" selector to "Color Theme"
> 
>  docs/api-viewer/index.html | 2 +-
>  www/MainView.js| 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> Stefan Sterz (1) @ pmg-gui:
>   main view/quarantine: rename "Theme" selector to "Color Theme"
> 
>  js/MainView.js   | 2 +-
>  js/QuarantineView.js | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 


applied all the patches in the respective three repos, 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 firewall 1/2] icmp: factor out check for relevant protocols

2023-05-16 Thread Thomas Lamprecht
Am 16/05/2023 um 11:09 schrieb Fabian Grünbichler:
> this were not entirely consistent and sometimes the checks were repeated.
> 
> Signed-off-by: Fabian Grünbichler 
> ---
>  src/PVE/Firewall.pm | 17 -
>  1 file changed, 12 insertions(+), 5 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 v4 3/6] added Config for Shared Filesystem Directories

2023-05-04 Thread Thomas Lamprecht
Am 04/05/2023 um 10:57 schrieb Dominik Csapak:
> On 5/4/23 10:42, Thomas Lamprecht wrote:
>> Am 04/05/2023 um 10:31 schrieb Dominik Csapak:
>>> On 5/4/23 10:13, Thomas Lamprecht wrote:
>>>> Am 03/05/2023 um 13:26 schrieb Dominik Csapak:
>>>>> just a short comment since this series overlaps a bit with my
>>>>> cluster resource mapping series (i plan on sending a v4 soon).
>>>>>
>>>>> i'd prefer to have the configuration endpoints for mapping bundled in a 
>>>>> subdirectory
>>>>> so instead of /nodes//dirs/ i'd put it in 
>>>>> /nodes//mapping/dirs/
>>>>> (or /nodes//map/dirs )
>>>>>
>>>>> @thomas, @fabian, any other input on that?
>>>>>
>>>>
>>>> huh? aren't mappings per definition cluster wide i.e. 
>>>> /cluster/resource-map/
>>>> than then allows to add/update the mapping of a resource on a specific 
>>>> node?
>>>> A node specific path makes no sense to me, at max it would if 
>>>> adding/removing a mapping
>>>> is completely decoupled from adding/removing/updaing entries to it – but 
>>>> that seems
>>>> convoluted from an usage POV and easy to get out of sync with the actual 
>>>> mapping list.
>>>
>>> in markus series the mapping are only ever per node, so each node has it's
>>> own dir mapping
>>
>> Every resource maping is always per node, so that's not really changing 
>> anything.
>>
> 
> i meant there is no cluster aspect in the current version of markus series at 
> all

only if you lock down the vm hard to be kept at the node its mapping was 
configured.

> 
>> Rather what about migrations? Would be simpler from migration and ACL POV to 
>> have
>> it cluster wide,
> 
> sure, but how we get the info during migration is just an implementation 
> detail and
> for that shouldn't matter where the configs/api endpoints are


no it really isn't an implementation detail how ACLs govern access to mappings, 
which
in turn dictactes where the ACL object paths should be, which then again ahs 
implications
for where the API endpoints and configs should be – node level isn't the one.

>>>
>>> in my series, the actual config was cluster-wide, but the api endpoint to 
>>> configure
>>> them were sitting in the node path (e.g. /node//hardware-map/pci/*)
>>  > Please no.
> 
> was that way since the very first version, would have been nice to get that 
> feedback
> earlier
> 

IIRC I always talked about this being on cluster level, especially w.r.t. to 
making it
more general resource mapping, maybe I didn't looked to closely and was thrown 
off
by the series name, i.e., "add *cluster-wide* hardware device mapping" and on 
the ACL
path discussion (which IIRC never had any node-specific param in there, and 
thus only
work if it's either a clusterwide config or if ID uniquness is guaranteed by 
the pmxcfs
like we do for VMIDs)

Anyhow, sorry, I'll try to check for such things more closely earlier in the 
future.

>>>
>>> we *could* put them into the cluster path api, but we'd need to send a node 
>>> parameter
>>> along and forward it there anyway, so that wouldn't really make a difference
>>
>> no need for that, see above.
> 
> there is a need for the node parameter, because you always need to know for 
> which node the
> mapping is anyway ;) or did you mean the 'forward' part?

yes, I certainly meant the forward part – otherwise my "Every resource mapping 
is always
per node" sentence and mentioned the node selector above would be really odd. A
node:local-id map always needs to exist, I figured that's the core thing this 
series wants
to solve, but as it's not a hard requirement to forward every request (and a 
huge PITA
to get in sync if done that way, or at least would reaquire using a 
cfs_domain_lock),
so no (hard) need for proxying.


>>>
>>> for reading the mappings, that could be done there, but in my series in the 
>>> gui at least,
>>> i have to make a call to each node to get the current state of the mapping
>>> (e.g. if the pci device is still there)
>>
>> For now not ideal but ok, in the future I'd rather go in the direction of 
>> broadcasting
>> some types of HW resources via pmxcfs KV and then this isn't an issue 
>> anymore.
> 
> yeah can be done, but the question is if we want to broadcast the whole 
> pci/usb list
> from all nodes? (i don't believe that scales well?)

how is one or two dozen of PCI IDs + some meta info, broadcasted normally once 
per 

Re: [pve-devel] [PATCH manager v4 3/6] added Config for Shared Filesystem Directories

2023-05-04 Thread Thomas Lamprecht
Am 04/05/2023 um 10:31 schrieb Dominik Csapak:
> On 5/4/23 10:13, Thomas Lamprecht wrote:
>> Am 03/05/2023 um 13:26 schrieb Dominik Csapak:
>>> just a short comment since this series overlaps a bit with my
>>> cluster resource mapping series (i plan on sending a v4 soon).
>>>
>>> i'd prefer to have the configuration endpoints for mapping bundled in a 
>>> subdirectory
>>> so instead of /nodes//dirs/ i'd put it in /nodes//mapping/dirs/
>>> (or /nodes//map/dirs )
>>>
>>> @thomas, @fabian, any other input on that?
>>>
>>
>> huh? aren't mappings per definition cluster wide i.e. 
>> /cluster/resource-map/
>> than then allows to add/update the mapping of a resource on a specific node?
>> A node specific path makes no sense to me, at max it would if 
>> adding/removing a mapping
>> is completely decoupled from adding/removing/updaing entries to it – but 
>> that seems
>> convoluted from an usage POV and easy to get out of sync with the actual 
>> mapping list.
> 
> in markus series the mapping are only ever per node, so each node has it's
> own dir mapping

Every resource maping is always per node, so that's not really changing 
anything.

Rather what about migrations? Would be simpler from migration and ACL POV to 
have
it cluster wide,

> 
> in my series, the actual config was cluster-wide, but the api endpoint to 
> configure
> them were sitting in the node path (e.g. /node//hardware-map/pci/*)

Please no.
 
> the reason is that to check the validity of the mapping (at least for 
> creating/updating)
> need to happen at the node itself anyway since only that node can check it
> (e.g. for pci devices, if it exists, the ids are correct etc.)

That check is most relevant on using the map, not on updating/configuring it as 
there
the UX of getting the right one can be solved of providing a node selector per 
entry
that then loads the actual available devices/resources on that node. 

> 
> we *could* put them into the cluster path api, but we'd need to send a node 
> parameter
> along and forward it there anyway, so that wouldn't really make a difference

no need for that, see above.

> 
> for reading the mappings, that could be done there, but in my series in the 
> gui at least,
> i have to make a call to each node to get the current state of the mapping
> (e.g. if the pci device is still there)

For now not ideal but ok, in the future I'd rather go in the direction of 
broadcasting
some types of HW resources via pmxcfs KV and then this isn't an issue anymore.

> 
> if a mapping exists (globally) is not interesting most of the time, we only 
> need to know
> if it exists at a specific node

that's looking at it backwards, the user and ACL only care for global mapings, 
how
the code implements that is then, well an implementation detail.

> 
> also, after seeing markus' patches, i also leaned more in the direction of 
> splitting
> my global mapping config into a config per type+node (so node1/usb.conf, 
> node1/pci.conf,

no, please no forest^W jungle of config trees :/

A /etc/pve/resource-map/.conf must be enough, even a 
/etc/pve/resource-map.conf
should be tbh., but I could imagine that splitting per resource type makes some 
(schema)
things a bit easier and reduce some bug potential, so not to hard feelings on 
having one
cluster-wide per type; but really not more.



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


Re: [pve-devel] [PATCH manager v4 3/6] added Config for Shared Filesystem Directories

2023-05-04 Thread Thomas Lamprecht
Am 03/05/2023 um 13:26 schrieb Dominik Csapak:
> just a short comment since this series overlaps a bit with my
> cluster resource mapping series (i plan on sending a v4 soon).
> 
> i'd prefer to have the configuration endpoints for mapping bundled in a 
> subdirectory
> so instead of /nodes//dirs/ i'd put it in /nodes//mapping/dirs/
> (or /nodes//map/dirs )
> 
> @thomas, @fabian, any other input on that?
> 

huh? aren't mappings per definition cluster wide i.e. 
/cluster/resource-map/
than then allows to add/update the mapping of a resource on a specific node?
A node specific path makes no sense to me, at max it would if adding/removing a 
mapping
is completely decoupled from adding/removing/updaing entries to it – but that 
seems
convoluted from an usage POV and easy to get out of sync with the actual 
mapping list.


___
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 2/2] Polish translation update

2023-05-02 Thread Thomas Lamprecht
Am 15/04/2023 um 01:12 schrieb Daniel Koć:
> Signed-off-by: Daniel Koć 
> ---
>  pl.po | 3906 +
>  1 file changed, 2295 insertions(+), 1611 deletions(-)
> 
>

applied this one, thanks!

It seems like the first patch was (mostly?) already applied, maybe recheck the
current state and if anything is off please ensure that you base off current 
master
branch when sending a patch.


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


Re: [pve-devel] pct-gentoo-systemd-plugin-fix.patch

2023-05-02 Thread Thomas Lamprecht
Am 29/04/2023 um 21:47 schrieb Jorge Ventura:
> Allow install gentoo rootfs based on systemd as container. Tested with
> rootfs images from https://jenkins.linuxcontainers.org/.

Thanks for your contribution, but applying fails with "error: corrupt
patch at line 11" probably due to not using git send-email for submission,
also missing an actual subject and your Signed-off-by trailer line + a
signed CLA sent to off...@proxmox.com

See: https://pve.proxmox.com/wiki/Developer_Documentation#Sending_Patches


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



[pve-devel] applied: little PATCH proxmox-i18n for ja.po

2023-05-02 Thread Thomas Lamprecht
Am 30/04/2023 um 10:07 schrieb ribbon:
> little Japanese translation update
> 
> --- proxmox-i18n/ja.po2023-04-30 16:32:55.922938861 +0900
> +++ jan.po2023-04-30 16:58:03.294743009 +0900
> @@ -8,7 +8,7 @@

applied, thanks!


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



Re: [PVE-User] MIME type of SPICE configuration file

2023-04-29 Thread Thomas Lamprecht
Am 28/04/2023 um 23:24 schrieb Uwe Sauter:
> I'm having trouble finding out the MIME type of the configuration file that 
> is presented as download. As mentioned Firefox has no automatic association 
> but in order do the configuration I'd need to know which MIME type I need to 
> associate with virt-viewer.

The MIME media type set is "application/x-virt-viewer", as that's what
the main SPICE client "virt-viewer" uses, e.g. on my Debian workstation:

# grep mime-type /usr/share/mime/packages/virt-viewer-mime.xml
  

For completeness sake: we set it here:
https://git.proxmox.com/?p=pve-manager.git;a=blob;f=www/manager6/Utils.js;h=d5dd2999081ec6970bd2122da9d61caf43d3ce0b;hb=f548e4fca214a409d638a59e9a6dc0080b9f0f6a#l1416

cheers,
Thomas


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


Re: [pve-devel] [PATCH v2 qemu-server 2/2] remote-migration: add target-cpu param

2023-04-29 Thread Thomas Lamprecht
Am 28/04/2023 um 11:12 schrieb Fabian Grünbichler:
>> It's was more about "offline" term, because we don't offline the source
>> vm until the disk migration is finished. (to reduce downtime)
>> More like "online-restart" instead "offline".
>>
>> Offline for me , is really, we shut the vm, then do the disk migration.
> hmm, I guess how you see it. for me, online means without interruption,
> anything else is offline  but yeah, naming is hard, as always 

FWIW, in Proxmox Container land that's currently basically the "most online"
it gets, and there it's named "restore migration" – at least if we go for the
"clean reboot for actual moving the guest over" approach.


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


Re: [pve-devel] is the next pve version 8.0 with debian 12 ? (any planning on patches merge ?)

2023-04-28 Thread Thomas Lamprecht
Am 28/04/2023 um 09:15 schrieb DERUMIER, Alexandre:
> I would like to known if the next pve version will be 8.0 on debian12 ?

yes.

> and if they are any planning for patches merging ? (I would like to
> schedule time on my agenda if my patches need to be rework).

Debian Bookworkm release is planned for June 10th, we have no hard
planned timeline to make public, but if I would need to guess from
top of my mind I'd say a beta a bit before that and a release a bit
after that, where "bit" here means roughly one to two weeks.

And we probably won't accept bigger breaking change after the beta
is made public. So, continuing the strong guess of mine, I'd say
if you get anything bigger submitted before ~ mid-may, we got still
some time for patches rework until ~ end of may.

> 
> 
> We had discussed about it last year, but I would like to implement
> permissions on vmbrX && sdn vnets, as it a breaking change.
> https://git.proxmox.com/?p=pve-manager.git;a=commit;h=a37ff602ff742403f01d6682bf9948183f87eadb
> 

yes, here the questions is more if we want to map in below some more
general resource-mapping ACL path as slightly/peripherally hinted in
a reply[0] to Dominik's HW mapping series. Or if we want to keep netdevs
in the /sdn path, which might be fine too as network devices is a big
and important enough class to be kept all in their own "access category"

[0]: https://lists.proxmox.com/pipermail/pve-devel/2022-November/054557.html

Fabian has some more spelled out questions/ideas IIRC.

>   
> 
> Also, I have a bunch of patches waiting for review or merge. (sorry for
> the flood ^_^)
> 

I'll probably jump start the final bootstrapping (we did some internal
ones already) soon, afterwards we can apply more stuff (would like to only
do bug fixes in 7.x from now on).

- Thomas


___
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 1/2] ui: clone: validate name

2023-04-24 Thread Thomas Lamprecht
On 17/04/2023 09:09, Fiona Ebner wrote:
> As reported in the community forum[0], as opposed to VM/LXC creation,
> there is no validation for the name in the clone dialog. Use the same
> validation as the guest creation wizards do to catch errors early,
> before sending the API request.
> 
> [0]: https://forum.proxmox.com/threads/125883/#post-549304
> 
> Signed-off-by: Fiona Ebner 
> ---
>  www/manager6/window/Clone.js | 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



[pve-devel] applied: [PATCH pve-manager 1/1] adding missing gettext

2023-04-24 Thread Thomas Lamprecht
On 16/04/2023 05:29, Daniel Koć wrote:
> ---
>  www/manager6/ceph/FS.js | 4 ++--
>  www/manager6/ceph/OSD.js| 6 +++---
>  www/manager6/ceph/OSDDetails.js | 2 +-
>  www/manager6/form/VLanField.js  | 2 +-
>  www/manager6/ha/Fencing.js  | 4 ++--
>  www/manager6/ha/GroupEdit.js| 2 +-
>  www/manager6/node/Config.js | 2 +-
>  7 files changed, 11 insertions(+), 11 deletions(-)
> 
>

applied, with adding your S-o-b and rewriting the commit subject a bit, thanks!


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


Re: [pve-devel] [PATCH proxmox-offline-mirror 1/2] setup wizard: add subscription keys

2023-04-24 Thread Thomas Lamprecht
On 18/04/2023 10:58, Fabian Grünbichler wrote:
> to make it a bit easier to configure access to the enterprise repositories.
> 
> Signed-off-by: Fabian Grünbichler 
> ---
>  docs/offline-keys.rst |   3 +-
>  src/bin/proxmox-offline-mirror.rs | 102 ++
>  .../subscription.rs   |   2 +-
>  3 files changed, 105 insertions(+), 2 deletions(-)
> 

applied, with a minor fix up (see below), thanks!

> diff --git a/src/bin/proxmox-offline-mirror.rs 
> b/src/bin/proxmox-offline-mirror.rs
> index bec366a..93e8dfa 100644
> --- a/src/bin/proxmox-offline-mirror.rs
> +++ b/src/bin/proxmox-offline-mirror.rs
> @@ -2,6 +2,8 @@ use std::fmt::Display;
>  use std::path::Path;
>  
>  use anyhow::{bail, Error};

fixed up adding missing use for format_err here.


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


[pve-devel] applied: Re: [PATCH proxmox-offline-mirror 2/2] fix #4614: add note about key requirements to mirror docs

2023-04-24 Thread Thomas Lamprecht
On 18/04/2023 10:58, Fabian Grünbichler wrote:
> and reference the key part of the documentation.
> 
> Signed-off-by: Fabian Grünbichler 
> ---
>  docs/offline-keys.rst   | 2 ++
>  docs/offline-mirror.rst | 5 +
>  2 files changed, 7 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 manager 1/3] fix #4678: ui: don't sort storage backup content by vmid by default

2023-04-22 Thread Thomas Lamprecht
On 22/04/2023 09:38, Thomas Lamprecht wrote:
> , would potentially also split this

ignore that part, send to early and patch is actually fine in
that regard.


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



[pve-devel] applied: Re: [PATCH v2 manager] ui: utils: monitor_ceph_installed: avoid setting nodename to localhost

2023-04-22 Thread Thomas Lamprecht
On 20/04/2023 14:31, Aaron Lauterer wrote:
> If a user is accessing the Ceph panel via Datacenter -> Ceph, then the
> install & config wizard might be shown. The nodename that is passed to
> the wizard  will decide the ID of the initial MON and MGR services.
> 
> Therefore, don't fall back to 'localhost' but the actual name of the
> node to which we are connected to. The result will be that the first MON
> and MGR will have the expected ID instead of 'localhost'.
> 
> Signed-off-by: Aaron Lauterer 
> ---
> 
> changes since  ui: CephInstallWizard: avoid using localhost on first 
> configuration:
> 
> * set the node name in the utils function that calls the wizard and not
>   within the wizard.
> 
>  www/manager6/Utils.js | 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



Re: [pve-devel] [PATCH manager 1/3] fix #4678: ui: don't sort storage backup content by vmid by default

2023-04-22 Thread Thomas Lamprecht
On 20/04/2023 10:06, Dominik Csapak wrote:
> instead, add the vmid as extra column, so that the user can still sort
> by vmid if they wish to
> 

missing Reported-by tag, would potentially also split this 

> Signed-off-by: Dominik Csapak 
> ---
>  www/manager6/storage/BackupView.js | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/www/manager6/storage/BackupView.js 
> b/www/manager6/storage/BackupView.js
> index fbdf573d3..9ad1a4915 100644
> --- a/www/manager6/storage/BackupView.js
> +++ b/www/manager6/storage/BackupView.js
> @@ -220,16 +220,20 @@ Ext.define('PVE.storage.BackupView', {
>   },
>   },
>   };
> + } else {
> + me.extraColumns = {};

this is a bit odd, would rather set it upfront and change the the isPBS
branch to set each column explicitly, but has nothing to do with your
patch directly and if, the cleanup should happen in a separate one in
any way.

>   }
> + me.extraColumns.vmid = {
> + header: gettext('VMID'),
> + dataIndex: 'vmid',
> + hidden: true,
> + sorter: (a, b) => a.data.vmid - b.data.vmid,

doesn't this breaks, or at least behaves possibly confusingly, on
custom file names? At least we don't handle the 'vmid' field explicitly
in the 'pve-storage-content' model.

Note that our regex here is very liberal, it basically returns any file
that ends with tar or vma and an optional gz, lzo or zst compressor.

> + };
>  
>   me.callParent();
>  
>   me.store.getSorters().clear();
>   me.store.setSorters([
> - {
> - property: 'vmid',
> - direction: 'ASC',
> - },
>   {
>   property: 'vdate',
>   direction: 'DESC',



___
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 3/3] ui: enable multiColumnSort for storage backup content

2023-04-22 Thread Thomas Lamprecht
On 20/04/2023 10:06, Dominik Csapak wrote:
> this enables the user to sort the grid by multiple columns
> simultaneously, e.g. by vmid and then by date
> 
> Signed-off-by: Dominik Csapak 
> ---
> sending as rfc because i'm not so sure about this.
> 
> on one hand, this allows to recreate the original sorting if users want
> that, but the selection is a bit weird. there is no way to 'unsort'

the original sorting was strange and confusing, while it would be
at least not as confusing with the columns shown it's IMO just not
a good fit for a simple grid.

If we want to make this better, we should adopt a tree view, Fiona
even made a patch for that IIRC, either when she reworked the content
view or when adding group pruning in Proxmox VE (we definitively
talked about doing that off list, the patch might be imagination —
didn't check). Albeit tree views and sorting are naturally not the
greatest thing, an configurable "group by VMID" checkbox could be the
most flexible variant (meaning most edge cases and probabky code too..)

> columns again, it simply uses the last 3 columns that were clicked
> 
> especially with the last patch (statefulness) it becomes weird, but
> maybe we want this more than we want it to be stateful?

tbh. I want neither ;-) At least the chosen full-grid stateful way,
see reply to respective patch.


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


Re: [pve-devel] [PATCH manager 2/3] ui: make storage backup content stateful

2023-04-22 Thread Thomas Lamprecht
On 20/04/2023 10:06, Dominik Csapak wrote:
> so that e.g. a custom sort/column order is saved
> 
> Signed-off-by: Dominik Csapak 
> ---
>  www/manager6/storage/BackupView.js | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/www/manager6/storage/BackupView.js 
> b/www/manager6/storage/BackupView.js
> index 9ad1a4915..bb045f5b6 100644
> --- a/www/manager6/storage/BackupView.js
> +++ b/www/manager6/storage/BackupView.js
> @@ -5,6 +5,9 @@ Ext.define('PVE.storage.BackupView', {
>  
>  showColumns: ['name', 'notes', 'protected', 'date', 'format', 'size'],
>  
> +stateful: true,
> +stateId: 'storage-backup-content',

I'm not the biggest fan of stateful grids, if it would only save sorting
it would be OK (can easily be done, but needs to be done manual IIRC), but
by default much more state is saved and reapplied. E.g. the grid column
flex'd widths are then also easily made fixed. Personally I quite often use
the "Reset Layout" just due to that, after having my browser resized and the
columns not adapting in a responsive way – one also cannot clear the state
in a fine grained way, neither by component (at least not easily by changing
to a per, e.g. for here, stateful ID that includes the storage ID) nor by
single thing, like sort order (e.g., a user has no simple way to know what
the original sort order, deemed as best by the devs, was).

> +
>  initComponent: function() {
>   let me = this;
>  



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


Re: [pve-devel] [PATCH ifupdown2 2/2] patch: add vlan interface ifdown/ifup when changes on reload, like for vxlan

2023-04-22 Thread Thomas Lamprecht
On 20/04/2023 23:37, Alexandre Derumier wrote:
> Signed-off-by: Alexandre Derumier 
> ---
>  ...-down-up-vxlan-interfaces-when-ifreload_down.patch | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git 
> a/debian/patches/pve/0005-ifreload-down-up-vxlan-interfaces-when-ifreload_down.patch
>  
> b/debian/patches/pve/0005-ifreload-down-up-vxlan-interfaces-when-ifreload_down.patch
> index c9964af..63494c9 100644
> --- 
> a/debian/patches/pve/0005-ifreload-down-up-vxlan-interfaces-when-ifreload_down.patch
> +++ 
> b/debian/patches/pve/0005-ifreload-down-up-vxlan-interfaces-when-ifreload_down.patch
> @@ -1,13 +1,16 @@
>  From 2c38d0a157c8946f35a7da1c7c05484d33e6986f Mon Sep 17 00:00:00 2001
>  From: Alexandre Derumier 
>  Date: Wed, 5 Jun 2019 14:47:05 +0200
> -Subject: [PATCH 6/7] ifreload: down/up vxlan interfaces when
> +Subject: [PATCH 6/7] ifreload: down/up vxlan && vlan interfaces when
>   ifreload_down_changed=0
>  
>  almost all attributes of vxlan interfaces can't be updated
> -in current kernel (<= 5.2). (including vxlan-id)
> +(including vxlan-id).
>  
> -so when ifreload_down_changed=0, ifreload can't update vxlan.
> +Same for vlan interfaces (vlan-protocol, vlan-id)
> +
> +so when ifreload_down_changed=0, ifreload can't update vxlan or vlan
> + attributes.
>  
>  fix: https://github.com/CumulusNetworks/ifupdown2/issues/50
>  Signed-off-by: Alexandre Derumier 
> @@ -25,7 +28,7 @@ index b4e1864..9313573 100644
>   
> ifaceLinkKind.to_str(lastifaceobjlist[0].link_kind)))
>   ifacedownlist.append(newifaceobjlist[objidx].name)
>  -if not down_changed:
> -+if not down_changed and 
> ifaceLinkKind.to_str(lastifaceobjlist[0].link_kind) != 'vxlan':
> ++if not down_changed and 
> ifaceLinkKind.to_str(lastifaceobjlist[0].link_kind) != 'vxlan' and 
> ifaceLinkKind.to_str(lastifaceobjlist[0].link_kind) != 'vlan':

style nit: could be slightly nicer if we pull out the link_kind
into an intermediate variable, i.e., something like:

link_kind = ifaceLinkKind.to_str(lastifaceobjlist[0].link_kind)

if not down_changed and link_kind != 'vxlan' and link_kind != 'vlan':

>   continue
>   if len(newifaceobjlist) != len(lastifaceobjlist):
>   ifacedownlist.append(ifname)



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



[pve-devel] applied: Re: [PATCH ifupdown2 0/2] fix #4683 : fix reload of vlan interfaces when attribute change

2023-04-22 Thread Thomas Lamprecht
On 20/04/2023 23:37, Alexandre Derumier wrote:
> This patch series fix the reload of vlan interface,
> when attribute change like vlan-protocol  (only use for qinq currently).
> 
> Like vxlan, we need to ifdown/ifup the interface on reload.
> 
> Alexandre Derumier (2):
>   patch : vlan: fix vlan-protocol query check
>   patch: add vlan interface ifdown/ifup when changes on reload, like for
> vxlan
> 
>  ...-vxlan-interfaces-when-ifreload_down.patch | 11 ++-
>  debian/patches/series |  3 +-
>  ...-check-vlan-protocol-for-not-dotted-.patch | 88 +++
>  3 files changed, 97 insertions(+), 5 deletions(-)
>  create mode 100644 
> debian/patches/upstream/0001-vlan-query_check-check-vlan-protocol-for-not-dotted-.patch
> 

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 pve-docs v5] update the PCI(e) docs

2023-04-17 Thread Thomas Lamprecht
Am 17/04/2023 um 14:45 schrieb Noel Ullreich:
> Sorry for the borked v4's. Those can be ignored. This is the (hopefully) 
> correctly formatted patch

please reply to the series/patch mail that should be ignored due to $problems,
not the one fixing it. That way, one that has marked the thread with the old
version for review gets the info correctly and can unmark that thread.

As comparison, putting a post it on a light switch stating that another light
switch should not be used as it gives people shocks, is not really _that_ much
of a help to anyone trying to turn on the light and getting to the broken one
first ⚡ ;-)


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


Re: [pve-devel] [PATCH pve-manager 1/1] adding missing gettext

2023-04-17 Thread Thomas Lamprecht
Am 16/04/2023 um 05:29 schrieb Daniel Koć:

Ok, but is missing your Signed-off-by developer certificate of origin, you can 
either
reply here with the S-o-b line, or resend the patch with it amended to the 
commit
message (in that case please also change the commit subject to start with "ui: 
"),
whatever you prefer.

> ---
>  www/manager6/ceph/FS.js | 4 ++--
>  www/manager6/ceph/OSD.js| 6 +++---
>  www/manager6/ceph/OSDDetails.js | 2 +-
>  www/manager6/form/VLanField.js  | 2 +-
>  www/manager6/ha/Fencing.js  | 4 ++--
>  www/manager6/ha/GroupEdit.js| 2 +-
>  www/manager6/node/Config.js | 2 +-
>  7 files changed, 11 insertions(+), 11 deletions(-)
> 





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


Re: [pve-devel] [PATCH cluster/manager/ha-manager/proxmox{, -perl-rs} 00/18] fix #4156: introduce new notification module

2023-04-14 Thread Thomas Lamprecht
Am 27/03/2023 um 17:18 schrieb Lukas Wagner:
> The purpose of this patch series is to overhaul the existing mail
> notification infrastructure in Proxmox VE.

nice! Some high level review, but I didn't checked the code basically at all,
so sorry if some comments of it are moot, as they're either already implemented
or discussed.

> A later patch could mark individual mail recipients in vzdump jobs as
> being deprecated in favor of the new notification endpoints. 

Hmmm, this is something that I did not think to closely about when drafting
the (very rough) design for the notification overhaul, but it has some merit
to be able to configure this per job – but naturally it also adds some
redundancy with a global filtering system. IOW., we still need something 
not all too compley for handling a job for, say, not so important test virtual
guests and one for production virtual guests. FWIW this could also be handled
by having a fixed mapping for some overides and jobs, i.e., the global
notification config could hold stanzas that match to a specific job (or other
notification emitting type) which is then also exposed when editing said job
directly. Otherwise, as we have now a pretty generic datacenter wide config
for almost all relevant jobs Proxmox VE can handle, we could also add a property
there that allows to match some notification policy/filter/config – that might
be cleaner as it avoids having to read/handle two configs in one API call.


> The
> drawback of this approach is that we might send a notification twice
> in case a user has created another sendmail endpoint with the same
> recipient. However, this is still better than not sending a
> notification at all. However, I'm open for suggestions for other
> approaches for maintaining compatibility.
> 
> Alternative approaches that came to my mind are:
>   - Explicitly break existing mail notifications with a major
> release, maybe create a default sendmail endpoint where *all*
> notifications are sent to root@pam via a sendmail endpoint.
> In this variant, the custom mail recipients for vzdump jobs would
> be removed
>   - On update, create endpoints in the new configuration file for all
> possible email addresses, along with appropriate filters so that
> the behavior for every component currently sending mails remains
> unchanged. I don't really like this approach, as it might lead to
> a pretty messy 'notifications.cfg' with lots of endpoints/filters, if
> the user has lots of different backup jobs configured.


If we go that way we could do the ephermal sendpoint only heuristically,
i.e., check if there's another (email) notifaction endpoint that matches
ours and the `root@localhost || $configured_root_at_pam_email` and silence
it then, as that means that the admin switched over to the new mechanism.

Or, much simpler and transparent, some node/cluster global flag in e.g.,
vzdump.conf.

 
> Side effects of the changes:
>   - vzdump loses the HTML table which lists VMs/CTs. Since different
> endpoints support different markup styles for formatting, it
> does not really make sense to support this in the notification API, at
> least not without 'cross-rendering' (e.g. markdown --> HTML)


This I'd think should be avoided if just anyhow possible, as that is
quite the nice feature. For keeping that possible we could add a optional
strucutred data object that can be passed to the notifaction system and
that (some) plugins can then use to show some more structured data.

Simplest might be a Option that has a defined structure
like for example (psuedo json):

{
  schema: {
type: "table", // or object
rows: ["vmid", "job", "..."],
// ... ?
  },
  data: [
 { vmid: 100, job: "foo", "...": "..."},
 ...
  ],
}

The mail plugin could then produce the two tables again, for the text/plain
and text/html multiparts again.

> 
> 
> Short summary of the introduced changes per repo:
>   - proxmox:
> Add new proxmox-notification crate, including configuration file
> parsing, two endpoints (sendmail, gotify) and per-endpoint
> filtering
>   - proxmox-perl-rs:
> Add bindings for proxmox-notification, so that we can use it from
> perl. Also configure logging in such a way that log messages from
> proxmox-notification integrate nicely in task logs.
>   - proxmox-cluster:
> Register new configuration file, 'notifications.cfg'
>   - pve-manager:
> Add some more glue code, so that the notification functions are
> callable in a nice way. This is needed since we need to read the
> configuration file and feed the config to the rust bindings as a
> string.
> Replace calls to 'sendmail' in vzdump,
> apt, and replication with calls to the new notification module.
>   - pve-ha-manager:
> Also replace calls to 'sendmail' with the new notification module
> 
> 
> Follow-up work (in no particular order):
>   - Add CRUD API routes for managing notification endpoints 

Re: [pve-devel] VNC Console using our own portal

2023-04-13 Thread Thomas Lamprecht via pve-devel
--- Begin Message ---
Hi,

Am 13/04/2023 um 15:06 schrieb Mark Schouten:
> Not sure if this thread should exist on this list, if not, let me know and 
> I’ll repost to pve-users. However, I think there are more people here that 
> know how to achieve what I’m looking for. :)

That's fine here too.

> We’re building a portal for our VPS services and so far we’ve been happy 
> users of the PVE API. However, we’re now trying to make the Console 
> available, and that seems to be quite difficult. As far as I can see, it’s 
> not documented either, so I’m kinda stuck reverse engineering all this.
> 
> So far I’ve found out that I can just proxy /novnc/ to the relevant cluster 
> to get the HTML/CSS/JS in my browser. But to get VNC working, I need to:
> 1: Start the vncproxy using POST /api2/json/nodes/{node}/qemu/{vmid}/vncproxy
> 2: Use that output to create a new request
> 
> But what should that new request be? And do I need API-Auth keys as well, or 
> is the ticket created by the API call enough? Should I add all the data as 
> HTTP headers, or can I put them in the url parameters?

You'll need to use the information you get from the first API call to pass
along to noVNC, which then uses that to open a websocket connection.

> 
> Have I missed documentation on how to set this up?

No, we have no explicit (developer) documentation for that, but besides our
open source code there are a few forum threads about this, e.g. a recent one:

https://forum.proxmox.com/threads/no-ticket-error-even-with-the-ticket-in-api2-json-nodes-pve-qemu-100-vncwebsocket.123530/

Wile chunked into multiple posts it contains a lot of relevant info for how
this works and what one needs to do.

- Thomas


--- End Message ---
___
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] file upload: don't calculate MD5

2023-04-13 Thread Thomas Lamprecht
Am 12/04/2023 um 16:22 schrieb Matthias Heiserer:
> Until now, we calculated the MD5 hash of any uploaded file during the upload, 
> regardless
> of whether the user chose to provide a hash sum and algorithm.
> The hash was only logged in the syslog.
> 
> As the user can provide a hash algorithm and a checksum when uploading a file,
> which gets automatically checked (after the upload), this is not needed 
> anymore.
> Instead, the file name is logged.
> 
> Depending on the speed of the network and the cpu, upload speed or CPU usage 
> might improve:
> All tests were made by uploading a 3.6GB iso from the PVE host to a local VM.
> First line is with md5, second without.
> 
> no networklimit
> multipart upload complete (size: 3826831360B time: 20.310s rate: 179.69MiB/s 
> md5sum: 8c651682056205967d530697c98d98c3)
> multipart upload complete (size: 3826831360B time: 16.169s rate: 225.72MiB/s 
> filename: ubuntu-22.04.1-desktop-amd64.iso)
> 
> 125MB/s network
> In this test, pveproxy worker used x % CPU during the upload. As you can see, 
> the reduced CPU usage is noticable in slower networks.
> ~75% CPU: multipart upload complete (size: 3826831360B time: 30.764s rate: 
> 118.63MiB/s md5sum: 8c651682056205967d530697c98d98c3)
> ~60% CPU: multipart upload complete (size: 3826831360B time: 30.763s rate: 
> 118.64MiB/s filename: ubuntu-22.04.1-desktop-amd64.iso)
> 
> qemu64 cpu, no network limit
> multipart upload complete (size: 3826831360B time: 46.113s rate: 79.14MiB/s 
> md5sum: 8c651682056205967d530697c98d98c3)
> multipart upload complete (size: 3826831360B time: 41.492s rate: 87.96MiB/s 
> filename: ubuntu-22.04.1-desktop-amd64.iso)
> 
> qemu64, -aes, 1 core, 0.7 cpu
> multipart upload complete (size: 3826831360B time: 79.875s rate: 45.69MiB/s 
> md5sum: 8c651682056205967d530697c98d98c3)
> multipart upload complete (size: 3826831360B time: 66.364s rate: 54.99MiB/s 
> filename: ubuntu-22.04.1-desktop-amd64.iso)
> 
> Signed-off-by: Matthias Heiserer 
> ---
>  src/PVE/APIServer/AnyEvent.pm | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
>

applied, thanks!


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



<    5   6   7   8   9   10   11   12   13   14   >