[pve-devel] applied: [PATCH qemu-server v3 3/5] fix #4474: qemu api: add overrule-shutdown parameter to stop endpoint
Am 12/04/2024 um 16:15 schrieb Friedrich Weber: > The new `overrule-shutdown` parameter is boolean and defaults to 0. If > it is 1, all active `qmshutdown` tasks for the same VM (which are > visible to the user/token) are aborted before attempting to stop the > VM. > > Passing `overrule-shutdown=1` is forbidden for HA resources. > > Signed-off-by: Friedrich Weber > --- > > Notes: > changes v2 -> v3: > - broke over-long lines > - avoid printing empty list of overruled tasks > - removed Wolfgang's Acked-by because patch changed > - rephrased parameter description/commit to reflect changed logic > - adapt to rename to `abort_guest_tasks` > > no changes v1 -> v2 > > PVE/API2/Qemu.pm | 19 ++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH guest-common v3 1/5] guest helpers: add helper to abort active guest tasks of a certain type
Am 12/04/2024 um 16:15 schrieb Friedrich Weber: > Given a `(type, user, vmid)` tuple, the helper aborts all tasks of the > given `type` for guest `vmid` that `user` is allowed to abort: > > - If `user` has `Sys.Modify` on the node, they can abort any task > - If `user` is an API token, it can abort any task it started itself > - If `user` is a user, they can abort any task started by themselves > or one of their API tokens. > > The helper is used to overrule any active qmshutdown/vzshutdown tasks > when attempting to stop a VM/CT (if requested). > > Signed-off-by: Friedrich Weber > --- > > Notes: > As the computation of `$can_abort_task` essentially duplicates logic > from PVE/API2/Tasks.pm, I considered reusing that, but this would have > required moving it to one of the dependencies of pve-guest-common > (Thomas suggested pve-access-control off-list). Seeing that the logic > boils down to 4 lines in `abort_guest_tasks`, I didn't consider it > worth the trouble in the end. Happy to reconsider, though. > > changes v2 -> v3: > - improved readability: renamed subroutine to describe what it does, > renamed return value, added comment, clarified commit message (thx > Thomas) > - better align logic with current permission model for stopping tasks: > - allow users with Sys.Modify to abort *any* task (thx Thomas) > - allow users to abort tasks of their tokens > > no changes v1 -> v2 > > src/PVE/GuestHelpers.pm | 35 +++ > 1 file changed, 35 insertions(+) > > applied, with some (very) tiny efficiency improvement as follow-up, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH container v3 2/5] fix #4474: lxc api: add overrule-shutdown parameter to stop endpoint
Am 12/04/2024 um 16:15 schrieb Friedrich Weber: > The new `overrule-shutdown` parameter is boolean and defaults to 0. If > it is 1, all active `vzshutdown` tasks for the same CT (which are > visible to the user/token) are aborted before attempting to stop the > CT. > > Passing `overrule-shutdown=1` is forbidden for HA resources. > > Signed-off-by: Friedrich Weber > --- > > Notes: > changes v2 -> v3: > - avoid printing empty list of overruled tasks > - rephrased parameter description/commit to reflect changed logic > - adapt to rename to `abort_guest_tasks` > > changes v1 -> v2: > - move overrule code into worker, as suggested by Wolfgang > - print to worker stdout instead of syslog > > src/PVE/API2/LXC/Status.pm | 19 +++ > 1 file changed, 19 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 ksm-control-daemon] ksmtuned: use PSS instead of RSZ for caluculating memory usage
Am 11/04/2024 um 12:04 schrieb Roland: > where arcsize is not taken into account > > https://bugzilla.proxmox.com/show_bug.cgi?id=3859 I think this bug should be split, as those are two completely different things implementation wise. The existing one could be kept for RRD, and a new one added for ksmtuned. For the latter it might be simpler to fix, as we do not have to care about upgrading some RRD schema in a cluster, which has a few orders of complexity more compared to checking the ARC on-demand in ksmtuned. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH ha-manager] d/postinst: make deb-systemd-invoke non-fatal
Am 11/04/2024 um 12:10 schrieb Fabian Grünbichler: > else this can break an upgrade for unrelated reasons. > > this also mimics debhelper behaviour more (which we only not use here because > of lack of reload support) - restructured the snippet to be more similar with > an explicit `if` as well. > > Signed-off-by: Fabian Grünbichler > --- > debian/pve-ha-manager.postinst | 10 ++ > 1 file changed, 6 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: [PATCH manager] d/postinst: make deb-systemd-invoke non-fatal
Am 11/04/2024 um 12:10 schrieb Fabian Grünbichler: > else this can break an upgrade for unrelated reasons (regular debhelper also > constructs the restart invocations like this, it even redirects output to > /dev/null) > > Signed-off-by: Fabian Grünbichler > --- > debian/postinst | 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
Re: [pve-devel] [PATCH storage] plugin: move definition for 'port' option to base plugin
Am 15/04/2024 um 14:48 schrieb Fiona Ebner:
> Commit 7020491 ("esxi: add 'port' config parameter") started using
> the 'port' option in a second plugin, but the definition stayed in the
> PBS plugin. Avoid the hidden dependency and move the definition to the
> base plugin instead.
>
> It is necessary to mark it as optional or it would be required always.
>
> Signed-off-by: Fiona Ebner
> ---
> src/PVE/Storage/PBSPlugin.pm | 6 --
> src/PVE/Storage/Plugin.pm| 8
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/src/PVE/Storage/PBSPlugin.pm b/src/PVE/Storage/PBSPlugin.pm
> index 08ceb88..0808bcc 100644
> --- a/src/PVE/Storage/PBSPlugin.pm
> +++ b/src/PVE/Storage/PBSPlugin.pm
> @@ -49,12 +49,6 @@ sub properties {
> description => "Base64-encoded, PEM-formatted public RSA key. Used
> to encrypt a copy of the encryption-key which will be added to each encrypted
> backup.",
> type => 'string',
> },
> - port => {
> - description => "For non default port.",
> - type => 'integer',
> - minimum => 1,
> - maximum => 65535,
> - },
> };
> }
>
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index 22a9729..5f49830 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -205,6 +205,14 @@ my $defaultData = {
> format => 'pve-storage-options',
> optional => 1,
> },
> + port => {
> + description => "For PBS/ESXi, use this port to connect to the
> storage instead of the"
I'd probably avoid hard-coding "PBS/ESXi" here, it would work as good if that
part would be omitted:
"Use this port to connect to the storage instead of the default one."
In the long run we should switch to a per-plugin schema, like the (IIRC)
mappings have, as then we could correctly define defaults and descriptions
without having to be overly general to fit a common denominator.
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH docs] storage: pbs: document port option
Am 15/04/2024 um 14:48 schrieb Fiona Ebner: > Signed-off-by: Fiona Ebner > --- > pve-storage-pbs.adoc | 4 > 1 file changed, 4 insertions(+) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH kernel 1/1] cherry-pick improved erratum 1386 workaround
Am 15/04/2024 um 14:56 schrieb Folke Gleumes: > The original fix disabled the xsaves feature for zen1/2. The issue has > since been fixed in the cpus microcode and this patch keeps the feature > enabled > if the microcode version is recent enough to contain the fix. > > Signed-off-by: Folke Gleumes > --- > > Tested this on an AMD Epyc 7302P v2. > > ...-improve-the-erratum-1386-workaround.patch | 82 +++ > 1 file changed, 82 insertions(+) > create mode 100644 > patches/kernel/0013-improve-the-erratum-1386-workaround.patch > > applied both patches, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied-series: [PATCH-SERIES v4 manager/docs] close #4513: add advanced tab for backup jobs and improve performance fallback/default
Am 16/04/2024 um 14:09 schrieb Fiona Ebner: > Changes in v4 (Thanks to Thomas for feedback!): > * rename tab from 'Performance' to 'Advanced' > * move repeat-missed setting there too > * update docs to clarify that those settings can be found in the > advanced tab > > Changes in v3 (Thanks to Thomas for feedback!): > * new patch to actually honor default values for performance > format/schema > * new patch to switch to per-property fallback for performance > properties > * also handle new (came in after v2) pbs-entries-max in UI (make > sure to preserve value, but don't expose it) > * drop already applied patch > > Improve fallback for the 'performance' sub-properties by using a > per-property fallback and honor schema defaults. > > Expose commonly used advanced properties in the backup job UI under a > new tab. > > > manager: > > Fiona Ebner (5): > vzdump: actually honor schema defaults for performance > vzdump: use per-property fallback for performance settings > close #4513: ui: backup job: add tab for advanced options > ui: backup job: disable zstd thread count field when zstd isn't used > ui: backup job: move repeat-missed option to advanced tab > > PVE/VZDump.pm | 33 +++- > www/manager6/Makefile | 1 + > www/manager6/dc/Backup.js | 42 +++-- > www/manager6/panel/BackupAdvancedOptions.js | 169 > 4 files changed, 228 insertions(+), 17 deletions(-) > create mode 100644 www/manager6/panel/BackupAdvancedOptions.js > > > docs: > > Fiona Ebner (2): > backup: update information about performance settings > backup: clarify where repeat-missed option can be found now > > vzdump.adoc | 15 --- > 1 file changed, 8 insertions(+), 7 deletions(-) > applied series, with some slight rework of the empty texts and hints done as follow-up, 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-flutter-frontend] node overview: power menu: reorder/reword confirm buttons
Am 17/04/2024 um 10:53 schrieb Dominik Csapak: > move the confirm action to the right as mentioned in the material spec[0] > also rewords the buttons to 'cancel' and 'shutdown/reboot' > for that to work properly slightly rename the confirm message > > 0: > https://m3.material.io/components/dialogs/guidelines#befd7f4d-1029-4957-b1b5-da13fc0bbf3c > > Signed-off-by: Dominik Csapak > --- > replaces v3 of my previous patch since v2 was applied: > https://lists.proxmox.com/pipermail/pve-devel/2024-April/063106.html > > lib/widgets/pve_node_power_settings_widget.dart | 14 +++--- > 1 file changed, 7 insertions(+), 7 deletions(-) > > applied, with rewording the commit message a bit to make it easier to read (flow wise) and adding a Suggested-by trailer to give Folke credit for his review/suggestion – 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] fix insecure migration failing if waiting on lock
Am 17/04/2024 um 11:48 schrieb Mira Limbeck:
> both STDOUT and STDERR are written into `$info` which is then parsed for
> IP and port of the target socket listening.
> when the ports file can't be locked immediately `trying to acquire
> lock...` is printed on STDERR and in turn written into `$info`.
> trying to parse the IP then fails, resulting in a migration or
> replication failing.
>
> the bare open3 call is replaced by the run_command wrapper from
> pve-common to use a safe wrapper around open3 with the same
> functionality.
> STDERR is read separatey from STDOUT and the last line of STDERR is
> kept in case of errors.
>
> Fixes: 57acd6a ("fix #1452: also log stderr of remote command with
> insecure storage migration")
>
> Signed-off-by: Mira Limbeck
> ---
> v3:
> - added log prefix for remote error logs
> - fixed style issues
>
> v2:
> - incorporated Fiona's suggestions
>- added `Fixes: ...` to commit message
>- kept old ip/port matching including # untaint comments
>- added logging for all messages in STDERR
>- simplified branches
>
> src/PVE/Storage.pm | 91 --
> 1 file changed, 55 insertions(+), 36 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 v5 manager 0/2] add edit window for device passthrough
Am 17/04/2024 um 10:44 schrieb Filip Schauer:
> Changes since v4:
> * Simplify cbind
> * Fix selection of custom devid not being applied on creation in
> onGetValues
>
> Changes since v3:
> * Pass confid in via cbind instead of manually setting it in the view
> model
> * Check me.isCreate instead of !me.confid for whether to find the next
> free device slot
>
> Changes since v2:
> * Clarify naming of mount point and device passthrough related utils
> * Remove unnecessary cbind
> * Make the device index selectible
> * Add default values as emptyText to the UI elements
> * Reorder UI elements to improve the layout
> * Disable the Device Passthrough menu entries for non-root users
> * Change var to let
> * Minor code cleanup of DeviceEdit.js
>
> Changes since v1:
> * Remove usb mapping
> * Add mode, uid and gid fields
>
> Filip Schauer (2):
> utils: clarify naming of LXC mount point utils
> ui: lxc: add edit window for device passthrough
>
> www/manager6/Makefile| 1 +
> www/manager6/Utils.js| 23 ++-
> www/manager6/lxc/DeviceEdit.js | 176 +++
> www/manager6/lxc/MPEdit.js | 4 +-
> www/manager6/lxc/MultiMPEdit.js | 4 +-
> www/manager6/lxc/Resources.js| 33 -
> www/manager6/window/GuestDiskReassign.js | 6 +-
> 7 files changed, 232 insertions(+), 15 deletions(-)
> create mode 100644 www/manager6/lxc/DeviceEdit.js
>
applied, with a follow-up that made the forEachLxc{MP,Dev} methods also
pass the property ID directly to the closure, thanks!
I also made some clean-ups, e.g. dropping the right-alignment of the field
label, that look just way to odd..
Allowing manual control over the ID also seems to not provide much advantage
here, so I hid that field like we do for VM PCI passthrough.
Also noticed something not related to the UI side: if I enter some bogus path,
like `/dev/enoent`, I correctly get an error that this does not exist, but the
config entry is added nonetheless!
Which then also means that if I keep the dialogue open and correct the dev
path, I won't be able to submit as the config digest changed, I need to close
and re-open the add/edit dialogue again. This is not only bad UX, but seems
completely broken?
Also, if I got some bogus devX entries already, the error I get when saving
a new one is often from them, not from the one I add, but the change also
goes through here...
Please check that out.
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH storage 1/9] copy OVF.pm from qemu-server
Am 17/04/2024 um 11:19 schrieb Fiona Ebner: > Am 16.04.24 um 17:02 schrieb Thomas Lamprecht: >> high-level nit: this, and most of the ESXi one, should go into another module >> name space, e.g. PVE::GuestImport:: (or if that's to long, or we really are >> sure >> that other stuff can be imported (I doubt it), then just PVE::Import might be >> fine too). >> > > Hmm, ESXiPlugin.pm is a storage plugin, so it does fit. But no Yes, ESXiPlugin _is_ a storage plugin, and it must stay there, but about 80% of it's code is not related to being a storage plugin but for importing only, parts of it might be even shareable with other such import related stuff. So what I meant with "**most** of the ESXi one" is that I'd separate these parts from the storage plugin specific code, not moving it completely. > objections to moving it from my side either. And fully agree that OVF.pm > should live somewhere else, it is not a storage plugin. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH pve-flutter-frontend v2] node overview: add power settings menu
Am 17/04/2024 um 08:45 schrieb Dominik Csapak: > similar to how it works for qemu, but add a confirmation dialog > so one does not accidentally shutdown or reboot a node. > > Signed-off-by: Dominik Csapak > --- > changes from v1: > * add an AlertDialog as confirmation before executing the action > > lib/bloc/pve_node_overview_bloc.dart | 11 +++ > lib/widgets/pve_node_overview.dart| 24 ++ > .../pve_node_power_settings_widget.dart | 84 +++ > 3 files changed, 119 insertions(+) > create mode 100644 lib/widgets/pve_node_power_settings_widget.dart > > applied this already a bit ago [0] with some code style follow-up [1], so can you please send the changes to adapt to Folke's review as patch on-top of current master? [0]: https://git.proxmox.com/?p=flutter/pve_flutter_frontend.git;a=commitdiff;h=16ddf759f338e59501520068867624b549215553 [1]: https://git.proxmox.com/?p=flutter/pve_flutter_frontend.git;a=commitdiff;h=e883ee3b9c9c7c6cee85d5ac77dcc86dbf7c12c0 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH pve-flutter-frontend] update flutter_inappwebview to 6.0.0
Am 17/04/2024 um 10:41 schrieb Dominik Csapak: > only 2 changes for the URI necessary > as mentioned off-list I tried this already locally, so pushed that similar change out instead replacing this: https://git.proxmox.com/?p=flutter/pve_flutter_frontend.git;a=commit;h=b7ea60bc40fb542bbf13835369e7c9d7372fc275 ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer v5 00/36] add automated/unattended installation
Am 16/04/2024 um 17:32 schrieb Aaron Lauterer: > patches until 31 got a [0,1] > > Tested-by: Christoph Heiss > Reviewed-by: Christoph Heiss why don't these patches include above trailers then? ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH flutter-repositories] some improvements/fixes for the app
Am 15/04/2024 um 12:30 schrieb Dominik Csapak: > some improvements: > * better error handling for saving the password > * adding node power actions (shutdown/restart) > * change webview so we can show it without a trusted certificate > * improve back button behavior > > flutter-frontend patch 1 depends on login-manager patch 2 > flutter-frontend patch 4 depends on dart-api-client to work correctly > > this is intended on top of my last series: > https://lists.proxmox.com/pipermail/pve-devel/2024-April/062891.html > > proxmox-dart-api-client: > > Dominik Csapak (1): > client: correctly set parameter for node actions > > lib/src/client.dart | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > proxmox-login-manager: > > Dominik Csapak (2): > login: show custom alert dialog for password saving errors > general settings: add trustedFingerprints list > > lib/proxmox_general_settings_model.dart | 6 - > lib/proxmox_login_form.dart | 32 + > 2 files changed, 33 insertions(+), 5 deletions(-) > > pve-flutter-frontend: > > Dominik Csapak (5): > console: use flutter inappwebview as webview > console: wrap console with appbar > node overview: use correct color for rrd icons > nove overview: add power settings menu > improve back button behavior > > lib/bloc/pve_node_overview_bloc.dart | 11 ++ > lib/pages/main_layout_slim.dart | 4 +- > lib/widgets/pve_console_menu_widget.dart | 139 -- > lib/widgets/pve_node_overview.dart| 36 - > .../pve_node_power_settings_widget.dart | 59 > pubspec.lock | 42 ++ > pubspec.yaml | 3 +- > 7 files changed, 240 insertions(+), 54 deletions(-) > create mode 100644 lib/widgets/pve_node_power_settings_widget.dart > applied all but the patch adding a power menu for the nodes, 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-flutter-frontend 4/5] nove overview: add power settings menu
Am 15/04/2024 um 12:30 schrieb Dominik Csapak: > similar to how it works for qemu the one for virtual guests is already borderline dangerous without having a confirmation prompt, but this one here is far from being borderline here IMO, as for guests one can at least start them for sure again through, e.g., this App, but for a node one might not be easily able to without out-of-band management. I would not mind being a bit on the scare-mongering side in the prompt. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH proxmox-login-manager 1/2] login: show custom alert dialog for password saving errors
Am 15/04/2024 um 12:30 schrieb Dominik Csapak: > in those cases, we sometimes get ugly stack traces/exceptions, so > instead of just showing that and aborting, show a custom dialog with > the basic info that we could not save the password (+details box with > the original error) and continue instead. > > Signed-off-by: Dominik Csapak > --- > lib/proxmox_login_form.dart | 32 > 1 file changed, 28 insertions(+), 4 deletions(-) > > applied, with a follow-up to fix the unsafe context access over an async gap, that `dart analyze` reported, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH proxmox-dart-api-client 1/1] client: correctly set parameter for node actions
Am 15/04/2024 um 12:30 schrieb Dominik Csapak: > using '?' in the url will be escaped and not used for the get > parameters. Instead add the command to the parameters map. > > Signed-off-by: Dominik Csapak > --- > lib/src/client.dart | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH v2 proxmox-ve] apt hook: disable on remove
Am 16/04/2024 um 14:32 schrieb Fabian Grünbichler: > and (re-)enable on install. adapted from apt-listbugs/apt-listchanges, which > solve the issue of removing (instead of purging) the conffile and hook binary > providing package in the same fashion. > > Suggested-by: Fiona Ebner > Signed-off-by: Fabian Grünbichler > Reviewed-by: Fiona Ebner > --- > v2: > - add addition file existence check in remove case > - add missing trailer(s) > > debian/proxmox-ve.postrm | 35 +++ > debian/proxmox-ve.preinst | 13 + > 2 files changed, 48 insertions(+) > create mode 100644 debian/proxmox-ve.postrm > create mode 100644 debian/proxmox-ve.preinst > > 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 storage 1/9] copy OVF.pm from qemu-server
Am 16/04/2024 um 15:18 schrieb Dominik Csapak: > copies the OVF.pm and relevant ovf tests from qemu-server. > We need it here, and it uses PVE::Storage already, and since there is no > intermediary package/repository we could put it, it seems fitting in > here. > > Signed-off-by: Dominik Csapak > --- > src/PVE/Storage/Makefile | 1 + > src/PVE/Storage/OVF.pm| 242 ++ > src/test/Makefile | 5 +- > src/test/ovf_manifests/Win10-Liz-disk1.vmdk | Bin 0 -> 65536 bytes > src/test/ovf_manifests/Win10-Liz.ovf | 142 ++ > .../ovf_manifests/Win10-Liz_no_default_ns.ovf | 142 ++ > .../ovf_manifests/Win_2008_R2_two-disks.ovf | 145 +++ > src/test/ovf_manifests/disk1.vmdk | Bin 0 -> 65536 bytes > src/test/ovf_manifests/disk2.vmdk | Bin 0 -> 65536 bytes > src/test/run_ovf_tests.pl | 71 + > 10 files changed, 747 insertions(+), 1 deletion(-) > create mode 100644 src/PVE/Storage/OVF.pm > create mode 100644 src/test/ovf_manifests/Win10-Liz-disk1.vmdk > create mode 100755 src/test/ovf_manifests/Win10-Liz.ovf > create mode 100755 src/test/ovf_manifests/Win10-Liz_no_default_ns.ovf > create mode 100755 src/test/ovf_manifests/Win_2008_R2_two-disks.ovf > create mode 100644 src/test/ovf_manifests/disk1.vmdk > create mode 100644 src/test/ovf_manifests/disk2.vmdk > create mode 100755 src/test/run_ovf_tests.pl > > diff --git a/src/PVE/Storage/Makefile b/src/PVE/Storage/Makefile > index d5cc942..2daa0da 100644 > --- a/src/PVE/Storage/Makefile > +++ b/src/PVE/Storage/Makefile > @@ -14,6 +14,7 @@ SOURCES= \ > PBSPlugin.pm \ > BTRFSPlugin.pm \ > LvmThinPlugin.pm \ > + OVF.pm \ > ESXiPlugin.pm > > .PHONY: install > diff --git a/src/PVE/Storage/OVF.pm b/src/PVE/Storage/OVF.pm > new file mode 100644 > index 000..90ca453 > --- /dev/null > +++ b/src/PVE/Storage/OVF.pm > @@ -0,0 +1,242 @@ > +# Open Virtualization Format import routines > +# https://www.dmtf.org/standards/ovf > +package PVE::Storage::OVF; > + high-level nit: this, and most of the ESXi one, should go into another module name space, e.g. PVE::GuestImport:: (or if that's to long, or we really are sure that other stuff can be imported (I doubt it), then just PVE::Import might be fine too). ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH pve-flutter-frontend 0/9] small improvements
Am 12/04/2024 um 10:04 schrieb Dominik Csapak: > a series of smaller improvements, noticed by rechecking the app > after updating to new dart/flutter version. > > not all of them have to be applied, most are rather independent. > e.g. use of the NavigationBar, and the icon of the warning task logs are > just my preferences. > > Dominik Csapak (9): > replace BottomNavigationBar with NavigationBar > resource tab: improve colors for headers > node overview: don't throw permission errors on every update > adapt to material 3 changes for themes > fix clipping for data card widget > set the color of the template indicator to that of the icon > task logs: don't always say 'Last Task: ' > task logs: use separate color for warnings > task log list: make warning icon more distinct > > lib/bloc/pve_node_overview_bloc.dart | 11 +++- > lib/main.dart | 8 ++- > lib/pages/main_layout_slim.dart | 62 ++- > lib/widgets/pve_guest_icon_widget.dart| 2 +- > .../pve_resource_data_card_widget.dart| 2 + > .../pve_task_log_expansiontile_widget.dart| 23 +-- > lib/widgets/pve_task_log_widget.dart | 5 +- > 7 files changed, 74 insertions(+), 39 deletions(-) > applied series, thanks! Made one follow-up to avoid suggesting that there are no updates if the user did not have the permission to query that. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH http-server] http: support Content-Encoding=deflate
Am 16/04/2024 um 13:44 schrieb Maximiliano Sandoval:
> Add support for compressing the body of responses with
> `Content-Encoding=deflate` as per [RFC9110]. Note that in this context
> `deflate` is actually a "zlib" data format as defined in [RFC1950].
>
> [RFC9110] https://www.rfc-editor.org/rfc/rfc9110#name-deflate-coding
> [RFC1950] https://www.rfc-editor.org/rfc/rfc1950
>
> Signed-off-by: Maximiliano Sandoval
> ---
> src/PVE/APIServer/AnyEvent.pm | 23 +--
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
> index b60b825..a43d6bb 100644
> --- a/src/PVE/APIServer/AnyEvent.pm
> +++ b/src/PVE/APIServer/AnyEvent.pm
> @@ -123,6 +123,7 @@ sub cleanup_reqstate {
> delete $reqstate->{request};
> delete $reqstate->{proto};
> delete $reqstate->{accept_gzip};
> +delete $reqstate->{accept_deflate};
> delete $reqstate->{starttime};
>
> if ($reqstate->{tmpfilename}) {
> @@ -288,7 +289,7 @@ sub response {
> $reqstate->{hdl}->timeout($self->{timeout});
>
> $nocomp = 1 if !$self->{compression};
> -$nocomp = 1 if !$reqstate->{accept_gzip};
> +$nocomp = 1 if !$reqstate->{accept_gzip} && !$reqstate->{accept_deflate};
>
> my $code = $resp->code;
> my $msg = $resp->message || HTTP::Status::status_message($code);
> @@ -333,11 +334,17 @@ sub response {
> $content_length = length($content);
>
> if (!$nocomp && ($content_length > 1024)) {
> - my $comp = Compress::Zlib::memGzip($content);
> - $resp->header('Content-Encoding', 'gzip');
> - $content = $comp;
> - $content_length = length($content);
> + if ($reqstate->{accept_gzip}) {
> + my $comp = Compress::Zlib::memGzip($content);
> + $resp->header('Content-Encoding', 'gzip');
> + $content = $comp;
> + } elsif ($reqstate->{accept_deflate}) {
> + my $comp = Compress::Zlib::compress($content);
> + $resp->header('Content-Encoding', 'deflate');
> + $content = $comp;
> + }
> }
> + $content_length = length($content);
> $resp->header("Content-Length" => $content_length);
> $reqstate->{log}->{content_length} = $content_length;
>
> @@ -735,7 +742,10 @@ sub proxy_request {
> if $auth->{api_token};
> $headers->{'CSRFPreventionToken'} = $auth->{token}
> if $auth->{token};
> - $headers->{'Accept-Encoding'} = 'gzip' if ($reqstate->{accept_gzip} &&
> $self->{compression});
> + if ($self->{compression}) {
> + $headers->{'Accept-Encoding'} = 'deflate' if
> $reqstate->{accept_deflate};
> + $headers->{'Accept-Encoding'} = 'gzip' if $reqstate->{accept_gzip};
it might warrant a comment (and a hint in the commit message) that gzip is
favored, otherwise it might seem like a bug that if both, accept_deflate and
accept_gzip are set, only the gzip one is set (even though passing a list
would be supported), or was this overlooked?
___
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 manager/docs] close #4513: add performance tab for backup jobs
Am 25/03/2024 um 15:09 schrieb Fiona Ebner:
> Am 07.11.23 um 14:49 schrieb Fiona Ebner:
>> Improve fallback for the 'performance' sub-properties by using a
>> per-property fallback and honor schema defaults.
>>
>> Expose commonly used performance-related properties in the backup job
>> UI under a new tab.
>>
>
> Ping. Still applies. I need to tell forum users how to do this via CLI
> rather frequently, so would be nice to have in the UI too.
>
I do not like using "Performance" here, that conveys that this includes
important options that one must set to improve performance, and it's
settings won't help with overall performance (buy a "faster" HW for that).
In short, while the name is not wrong in a technical sense, all knobs will
change performance on some axis, that's not really matching with user
expectations IMO.
Even just calling this "Options" would be better IMO, even better might
be to call it "Advanced" and move also the single advanced option we
currently have ("Repeat missed" in General tab) over there.
That way newer users are made aware off that these options are
definitively not a must to adapt to get good performance, but rather for
some advanced options for tuning/limiting/etc. ...
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH pve-kernel] fix #5373: cherry-pick USB ethernet naming fix
Am 12/04/2024 um 15:25 schrieb Fabian Grünbichler: > Signed-off-by: Fabian Grünbichler > --- > test-built 6.8, but I assume 6.5 works as well since the patch applies cleanly > there (build hasn't finished yet ;)) > > I also assume this will be picked up fairly fast by stable point releases, but > not sure how fast those will be folded atm on the Ubuntu side with the freeze > going on ;) > > ...178a-avoid-the-interface-always-conf.patch | 50 +++ > 1 file changed, 50 insertions(+) > create mode 100644 > patches/kernel/0013-net-usb-ax88179_178a-avoid-the-interface-always-conf.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 pve-zsync] parse disks: improve error messages
On 12/09/2023 14:29, Fiona Ebner wrote: > The one with the backup flag was reported in the community forum: > https://forum.proxmox.com/threads/77254/ > > Signed-off-by: Fiona Ebner > --- > > Changes in v2: > * Further improve message as suggested by Sterzy. > > pve-zsync | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > applied, thanks! should this immediately bumped or is there some other patches, existing or short-term planned? Just not that we forget this for too long now (FWIW, feel free to do the bump yourself if you want). ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] partially-applied: [PATCH qemu v3 06/22] backup: add minimum cluster size to performance options
On 11/04/2024 11:29, Fiona Ebner wrote: > Useful to make discard-source work in the context of backup fleecing > when the fleecing image has a larger granularity than the backup > target. > > Backup/block-copy will use at least this granularity for copy operations > and in particular, discard requests to the backup source will too. If > the granularity is too small, they will just be aligned down in > cbw_co_pdiscard_snapshot() and thus effectively ignored. > > Signed-off-by: Fiona Ebner > --- > block/backup.c| 2 +- > block/copy-before-write.c | 2 ++ > block/copy-before-write.h | 1 + > blockdev.c| 3 +++ > qapi/block-core.json | 9 +++-- > 5 files changed, 14 insertions(+), 3 deletions(-) > > applied the qemu part, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH guest-common v3 10/22] vzdump: schema: make storage for fleecing semi-optional
On 11/04/2024 11:29, Fiona Ebner wrote: > so it doesn't need to be set when explicitly disabling fleecing. Needs > a custom verifier to enforce it being set when enabled. > > Suggested-by: Fabian Grünbichler > Signed-off-by: Fiona Ebner > --- > src/PVE/VZDump/Common.pm | 14 +- > 1 file changed, 13 insertions(+), 1 deletion(-) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH guest-common v3 10/22] vzdump: schema: make storage for fleecing semi-optional
On 11/04/2024 11:29, Fiona Ebner wrote:
> so it doesn't need to be set when explicitly disabling fleecing. Needs
> a custom verifier to enforce it being set when enabled.
>
> Suggested-by: Fabian Grünbichler
> Signed-off-by: Fiona Ebner
> ---
> src/PVE/VZDump/Common.pm | 14 +-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/src/PVE/VZDump/Common.pm b/src/PVE/VZDump/Common.pm
> index 30bd044..f7cdb66 100644
> --- a/src/PVE/VZDump/Common.pm
> +++ b/src/PVE/VZDump/Common.pm
> @@ -95,8 +95,20 @@ PVE::JSONSchema::register_format('backup-fleecing', {
> description => "Use this storage to storage fleecing images. For
> efficient space usage, "
> ."it's best to use a local storage that supports discard and either
> thin provisioning "
> ."or sparse files.",
> + optional => 1,
> }),
> -});
> +}, \_backup_fleecing);
> +
> +sub verify_backup_fleecing {
> +my ($param, $noerr) = @_;
> +
> +if (!$param->{storage} && $param->{enabled}) {
> + return if $noerr;
> + die "'storage' parameter is required when 'enabled' is set\n";
> +}
> +
> +return $param;
> +}
>
> PVE::JSONSchema::register_format('backup-performance', {
> 'max-workers' => {
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: Re: [PATCH guest-common v3 09/22] vzdump: schema: add fleecing property string
On 11/04/2024 11:29, Fiona Ebner wrote: > It's a property string, because that avoids having an implicit > "enabled" as part of a 'fleecing-storage' property. And there likely > will be more options in the future, e.g. threshold/limit for the > fleecing image size. > > Storage is non-optional, so the storage choice needs to be a conscious > decision. Can allow for a default later, when a good choice can be > made further down the stack. The original idea with "same storage as > VM disk" is not great, because e.g. for LVM, it would require the same > size as the disk up front. > > Signed-off-by: Fiona Ebner > --- > src/PVE/VZDump/Common.pm | 25 + > 1 file changed, 25 insertions(+) > > applied, but massaged in having the whitespace on the continued line for multi-line strings, not on the previous (we should actually add that to the style guide), thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH common v3 08/22] json schema: add format description for pve-storage-id standard option
On 11/04/2024 11:29, Fiona Ebner wrote: > so that the option can be used as part of a property string. > > Signed-off-by: Fiona Ebner > --- > src/PVE/JSONSchema.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] [RFC qemu-server v3 17/22] parse config: allow config keys with minus sign
On 11/04/2024 11:29, Fiona Ebner wrote: > In preparation for the upcoming 'fleecing-images' key. To avoid mixing > of options with - and options with _, which is not very user-friendly, > it would be nice to add aliases for existing options with _. And > long-term, backup restore handlers could switch to the modern keys > with -. a long-term way out could be to switch all formats over to using kebab-case and normalize through s/_/- here. Writing that out again naturally need either checking if all nodes support the new format already actively, or adding support to it in a minor release now and then enable it on a major release. All churn, but so is having an format mess. I ponder about doing something like this for the API, but for endpoints not being backed by a config (schema) it would be less problematic w.r.t. accepting both variants for a while, but API docs showing only the new one and then making warnings noisier over the time of a full major release.. Anyhow, this on it's own would be fine by me, but in any case we should at least document why the formats are mixed would be good, users can be confused by it, and cannot know that most of the "bad" options isn't the fault from the devs currently mainly working on PVE. ___ 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/9] report: add kernel command line including boot time
On 11/04/2024 19:07, Alexander Zeidler wrote: > On Mon, 2024-03-25 at 08:52 +0100, Thomas Lamprecht wrote: >> journalctl --list-boots > > On slow servers / storage drives this execution can easily take 10 seconds > or longer. But there is an alternative that is quite fast and even shows > the booted kernel version: > > # last reboot -F -n10 ack, `last` is using /var/log/wtmp as source which is maintained by login, and/or init and/or getty, i.e., not through any traditional log mechanism like the now optional rsyslog, so this would be fine by me. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH guest-common v2 2/5] mapping: pci: rework properties check
On 10/04/2024 13:03, Dominik Csapak wrote:
> refactors the actual checking out to its own sub, so we can reuse it
> later
>
> Signed-off-by: Dominik Csapak
> ---
> src/PVE/Mapping/PCI.pm | 43 +-
> 1 file changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/src/PVE/Mapping/PCI.pm b/src/PVE/Mapping/PCI.pm
> index 725e106..fcf07c0 100644
> --- a/src/PVE/Mapping/PCI.pm
> +++ b/src/PVE/Mapping/PCI.pm
> @@ -129,6 +129,26 @@ sub options {
> };
> }
>
> +my sub check_properties {
s/check/assert/ and ideally some words that better describe what is
actually asserted here.
> +my ($correct, $configured, $path, $name) = @_;
maybe s/correct/expected/ would be slightly better in conveying that the
passed $configured one do not only need to be all in the first hash, but
that all keys of the first hash
> +for my $prop (sort keys %$correct) {
> + next if !defined($correct->{$prop}) && !defined($configured->{$prop});
> +
> + die "no '$prop' for device '$path'\n"
pre-existing, but maybe this would be slightly better worded like:
"missing expected property '$prop' for device '$path'\n"
(no hard feelings though)
> + if defined($correct->{$prop}) && !defined($configured->{$prop});
> + die "'$prop' configured but should not be\n"
also pre-existing, but I would adapt the error message to something like:
"unknown property '$prop' configured for device '$path'\n"
(slightly hard feelings here ;-))
(btw. would it make sense to also add $name?)
> + if !defined($correct->{$prop}) && defined($configured->{$prop});
can above check even trigger if we just go through the expected ($correct)
set of properties? Or are existing, but undef, entries in $correct the
forbidden ones, and other extra properties in $configured do not matter?
(I dind't check the full picture, so excuse me if this would be obvious,
but them IMO some comments would be warranted)
> +
> + my $correct_prop = $correct->{$prop};
> + $correct_prop =~ s/0x//g;
> + my $configured_prop = $configured->{$prop};
> + $configured_prop =~ s/0x//g;
> +
> + die "'$prop' does not match for '$name' ($correct_prop !=
> $configured_prop)\n"
> + if $correct_prop ne $configured_prop;
> +}
> +}
> +
> # checks if the given config is valid for the current node
> sub assert_valid {
> my ($name, $cfg) = @_;
> @@ -150,30 +170,19 @@ sub assert_valid {
>
> my $correct_props = {
> id => "$info->{vendor}:$info->{device}",
> - iommugroup => $info->{iommugroup},
> };
>
> + # check iommu only on the first device
> + if ($idx == 0) {
> + $correct_props->{iommugroup} = $info->{iommugroup};
> + }
is this really the same than what got removed in the loop?
As if the next ID
> +
> if (defined($info->{'subsystem_vendor'}) &&
> defined($info->{'subsystem_device'})) {
> $correct_props->{'subsystem-id'} =
> "$info->{'subsystem_vendor'}:$info->{'subsystem_device'}";
> }
>
> - for my $prop (sort keys %$correct_props) {
> - next if $prop eq 'iommugroup' && $idx > 0; # check iommu only on
> the first device
> -
> - next if !defined($correct_props->{$prop}) &&
> !defined($cfg->{$prop});
> - die "no '$prop' for device '$path'\n"
> - if defined($correct_props->{$prop}) && !defined($cfg->{$prop});
> - die "'$prop' configured but should not be\n"
> - if !defined($correct_props->{$prop}) && defined($cfg->{$prop});
> + check_properties($correct_props, $cfg, $path, $name);
>
> - my $correct_prop = $correct_props->{$prop};
> - $correct_prop =~ s/0x//g;
> - my $configured_prop = $cfg->{$prop};
> - $configured_prop =~ s/0x//g;
> -
> - die "'$prop' does not match for '$name' ($correct_prop !=
> $configured_prop)\n"
> - if $correct_prop ne $configured_prop;
> - }
> $idx++;
> }
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: Re: [PATCH guest-common v2 1/5] mapping: pci: fix missing description/default for mdev
On 10/04/2024 13:03, Dominik Csapak wrote: > Signed-off-by: Dominik Csapak > --- > src/PVE/Mapping/PCI.pm | 2 ++ > 1 file changed, 2 insertions(+) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied-series: [PATCH-SERIES v2 guest-common] replication improvements
On 13/12/2023 15:17, Fiona Ebner wrote: > Improve error when finding a common base snapshot and fix the check if > a snapshot is needed by replication when there are volumes with > replicate setting turned off. > > First version: > https://lists.proxmox.com/pipermail/pve-devel/2023-January/055513.html > > Changes since v1: > * add an additional fix > > Fiona Ebner (3): > replication: prepare: include volumes without snapshots in the result > replication: find common base: improve error when no common base > snapshot exists > abstract config: fix snapshot needed by replication check > > src/PVE/AbstractConfig.pm | 2 +- > src/PVE/Replication.pm| 17 - > 2 files changed, 13 insertions(+), 6 deletions(-) > applied, thanks! Had some slight reservations about recommending deletion of volumes to user, but maybe I'm to paranoid about them not reading the "target" part and disregarding any basic sense.. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager v9 3/3] ui: machine: add viommu ComboBox
On 11/04/2024 12:48, Markus Frank wrote:
> Added a proxmoxKVComboBox for selecting a vIOMMU implementation for a
> VM. If i440fx is selected, a hint tells that q35 is required for Intel vIOMMU.
>
> The UI also needs to parse the new machine parameter as PropertyString.
>
> Signed-off-by: Markus Frank
> ---
> www/manager6/qemu/MachineEdit.js | 45
> 1 file changed, 45 insertions(+)
>
> diff --git a/www/manager6/qemu/MachineEdit.js
> b/www/manager6/qemu/MachineEdit.js
> index f928c80c..45f3d34d 100644
> --- a/www/manager6/qemu/MachineEdit.js
> +++ b/www/manager6/qemu/MachineEdit.js
> @@ -1,6 +1,7 @@
> Ext.define('PVE.qemu.MachineInputPanel', {
> extend: 'Proxmox.panel.InputPanel',
> xtype: 'pveMachineInputPanel',
> +onlineHelp: 'qm_system_settings',
nit: this could be done in a separate patch.
>
> controller: {
> xclass: 'Ext.app.ViewController',
> @@ -40,12 +44,30 @@ Ext.define('PVE.qemu.MachineInputPanel', {
> delete values.delete;
> }
> delete values.version;
> + if (values.machine === undefined) {
> + if (values.viommu) {
> + delete values.delete;
> + values.machine = "pc";
> + } else {
> + values.delete = "machine";
> + }
> + }
> + if (values.viommu) {
> + values.machine += ",viommu=" + values.viommu;
> + }
> + if (values.delete === "viommu") {
> + delete values.delete;
> + }
> + delete values.viommu;
can't we use printPropertyString here (with a bit less code preparing the object
passed to it), or at least try making it a bit less convoluted..
> fieldLabel: gettext('Note'),
> value: gettext('Machine version change may affect hardware layout
> and settings in the guest OS.'),
> },
> + {
> + xtype: 'proxmoxKVComboBox',
> + fieldLabel: gettext('vIOMMU'),
> + name: 'viommu',
> + reference: 'viommu',
> + value: '__default__',
> + comboItems: [
> + ['__default__', Proxmox.Utils.defaultText + ' (None)'],
> + ['intel', 'Intel'],
> + ['virtio', 'VirtIO'],
> + ],
> + },
> + {
> + xtype: 'displayfield',
> + name: 'q35Hint',
> + reference: 'q35Hint',
> + userCls: 'pmx-hint',
> + value: gettext('Intel vIOMMU needs the q35 machine type'),
maybe we could mark the field invalid (if that text is returned in the
validity check it will be shown as tooltip) instead of using the hint?
> + hidden: true,
> + },
> ],
> });
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: Re: [PATCH docs v9 2/3] add vIOMMU documentation
On 11/04/2024 12:48, Markus Frank wrote: > Signed-off-by: Markus Frank > --- > qm-pci-passthrough.adoc | 50 + > qm.adoc | 1 + > 2 files changed, 51 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: Re: [PATCH qemu-server v9 1/3] fix #3784: config: Parameter for guest vIOMMU + test-cases
On 11/04/2024 12:48, Markus Frank wrote: > vIOMMU enables the option to passthrough pci devices to L2 VMs > in L1 VMs via Nested Virtualisation and adds an extra isolation. > > Uses the new property-string from the "config: define machine schema > as property-string"-commit to add the viommu option to the machine > parameter. > > Currently there are two vIOMMU implementation in QEMU to choose: > intel or virtio > > Virtio-iommu is more recent but less used in production than intel-iommu. > > The assert_valid_machine_property function prevents using intel-iommu with > i440fx. > > Signed-off-by: Markus Frank > --- > PVE/API2/Qemu.pm | 2 ++ > PVE/QemuServer.pm | 12 +++ > PVE/QemuServer/Machine.pm | 17 ++- > test/cfg2cmd/i440fx-viommu-intel.conf | 2 ++ > test/cfg2cmd/i440fx-viommu-virtio.conf | 1 + > test/cfg2cmd/i440fx-viommu-virtio.conf.cmd | 25 ++ > test/cfg2cmd/q35-viommu-intel.conf | 1 + > test/cfg2cmd/q35-viommu-intel.conf.cmd | 23 > test/cfg2cmd/q35-viommu-virtio.conf| 1 + > test/cfg2cmd/q35-viommu-virtio.conf.cmd| 23 > 10 files changed, 106 insertions(+), 1 deletion(-) > create mode 100644 test/cfg2cmd/i440fx-viommu-intel.conf > create mode 100644 test/cfg2cmd/i440fx-viommu-virtio.conf > create mode 100644 test/cfg2cmd/i440fx-viommu-virtio.conf.cmd > create mode 100644 test/cfg2cmd/q35-viommu-intel.conf > create mode 100644 test/cfg2cmd/q35-viommu-intel.conf.cmd > create mode 100644 test/cfg2cmd/q35-viommu-virtio.conf > create mode 100644 test/cfg2cmd/q35-viommu-virtio.conf.cmd > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server v8 2/4] fix #3784: Parameter for guest vIOMMU + test-cases
Am 24/01/2024 um 10:49 schrieb Markus Frank:
> vIOMMU is the emulation of a hardware IOMMU within a virtual machine,
> providing improved memory access control and security for virtualized I/O
> devices.
> vIOMMU also enables the option to passthrough pci devices to L2 VMs
> in L1 VMs via Nested Virtualisation.
>
> Currently there are two vIOMMU implementation in QEMU to choose:
> intel & virtio
>
> Virtio-iommu is more recent but less used in production than intel-iommu.
>
> The check_machine_config function prevents using intel-iommu with
> i440fx.
>
> Signed-off-by: Markus Frank
> ---
> PVE/API2/Qemu.pm| 2 ++
> PVE/QemuServer.pm | 12
> PVE/QemuServer/Machine.pm | 17 -
> test/cfg2cmd/q35-viommu-intel.conf | 1 +
> test/cfg2cmd/q35-viommu-intel.conf.cmd | 23 +++
> test/cfg2cmd/q35-viommu-virtio.conf | 1 +
> test/cfg2cmd/q35-viommu-virtio.conf.cmd | 23 +++
> 7 files changed, 78 insertions(+), 1 deletion(-)
> create mode 100644 test/cfg2cmd/q35-viommu-intel.conf
> create mode 100644 test/cfg2cmd/q35-viommu-intel.conf.cmd
> create mode 100644 test/cfg2cmd/q35-viommu-virtio.conf
> create mode 100644 test/cfg2cmd/q35-viommu-virtio.conf.cmd
>
this one needs to be rebased.
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index c23b16a..4a5a833 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -1056,6 +1056,7 @@ __PACKAGE__->register_method({
> $conf->{machine} =
> PVE::QemuServer::Machine::print_machine($machine_conf);
> }
> }
> + PVE::QemuServer::Machine::check_machine_config($conf,
> $machine_conf);
>
> PVE::QemuConfig->write_config($vmid, $conf);
>
> @@ -1894,6 +1895,7 @@ my $update_vm_api = sub {
> $conf->{pending}->{$opt} = $param->{$opt};
> } elsif ($opt eq 'machine') {
> my $machine_conf =
> PVE::QemuServer::Machine::parse_machine($param->{$opt});
> + PVE::QemuServer::Machine::check_machine_config($conf,
> $machine_conf);
> $conf->{pending}->{$opt} = $param->{$opt};
> } else {
> $conf->{pending}->{$opt} = $param->{$opt};
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 6bb2ec3..92832f8 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -4070,6 +4070,18 @@ sub config_to_command {
> }
> push @$machineFlags, "type=${machine_type_min}";
>
> +PVE::QemuServer::Machine::check_machine_config($conf, $machine_conf);
> +
> +if ($machine_conf->{viommu}) {
> + if ($machine_conf->{viommu} eq 'intel') {
> + unshift @$devices, '-device',
> 'intel-iommu,intremap=on,caching-mode=on';
> + push @$machineFlags, 'kernel-irqchip=split';
> + }
> + if ($machine_conf->{viommu} eq 'virtio') {
could be merged with the line before as `} elsif (...) {`
> + push @$devices, '-device', 'virtio-iommu-pci';
> + }
> +}
> +
> push @$cmd, @$devices;
> push @$cmd, '-rtc', join(',', @$rtcFlags) if scalar(@$rtcFlags);
> push @$cmd, '-machine', join(',', @$machineFlags) if
> scalar(@$machineFlags);
> diff --git a/PVE/QemuServer/Machine.pm b/PVE/QemuServer/Machine.pm
> index 5e3a75c..71790c4 100644
> --- a/PVE/QemuServer/Machine.pm
> +++ b/PVE/QemuServer/Machine.pm
> @@ -23,12 +23,19 @@ my $machine_fmt = {
> format_description => 'machine type',
> optional => 1,
> },
> +viommu => {
> + type => 'string',
> + description => "Enable/disable guest vIOMMU"
> + ." (needs kvm to be enabled and q35 to be set as machine type).",
early newline
> + enum => ['intel', 'virtio'],
> + optional => 1,
> +},
> };
>
> PVE::JSONSchema::register_format('pve-qemu-machine-fmt', $machine_fmt);
>
> PVE::JSONSchema::register_standard_option('pve-qemu-machine', {
> -description => "Specify the QEMU machine type.",
> +description => "Specify the QEMU machine type & enable/disable vIOMMU.",
> type => 'string',
> optional => 1,
> format => PVE::JSONSchema::get_format('pve-qemu-machine-fmt'),
> @@ -48,6 +55,14 @@ sub print_machine {
> return print_property_string($machine_conf, $machine_fmt);
> }
>
> +sub check_machine_config {
maybe name that `assert_valid_machine_property` to better convey that it can die
> +my ($conf, $machine_conf) = @_;
> +my $q35 = $machine_conf->{type} && ($machine_conf->{type} =~ m/q35/) ? 1
> : 0;
> +if ($machine_conf->{viommu} && $machine_conf->{viommu} eq "intel" &&
> !$q35) {
> + die "to use Intel vIOMMU please set the machine type to q35\n";
> +}
> +}
> +
> sub machine_type_is_q35 {
> my ($conf) = @_;
>
> diff --git a/test/cfg2cmd/q35-viommu-intel.conf
> b/test/cfg2cmd/q35-viommu-intel.conf
> new file mode 100644
> index
[pve-devel] applied: [PATCH qemu-server v8 1/4] machine as property-string
Am 24/01/2024 um 10:49 schrieb Markus Frank: > Convert the machine parameter to a property-string and use the > machine type as the default key for backward compatibility. > > Signed-off-by: Markus Frank > --- > PVE/API2/Qemu.pm | 9 +++-- > PVE/QemuConfig.pm | 3 ++- > PVE/QemuServer.pm | 16 ++- > PVE/QemuServer/Machine.pm | 42 +-- > 4 files changed, 55 insertions(+), 15 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 manager 1/2] ui: dc: backup: improve UX for the different 'notification-mode's
Am 14/12/2023 um 11:26 schrieb Maximiliano Sandoval:
>
> Lukas Wagner writes:
>
>> - Switch order of 'mailto' and 'mailnotification' field
>> - When mode is 'auto', disable 'mailtnotification' field
>> - When mode is 'auto' and 'mailto' is empty, show
>> hint that the notification system will be used
>>
>> Signed-off-by: Lukas Wagner
>
> Tested both commits, they do what they promise.
>
>> +let notificationSystemHint = Ext.create({
>> +xtype: 'displayfield',
>> +padding: '0 0 0 5',
>> +userCls: 'pmx-hint',
>> +hidden: true,
>> +value: gettext('No email configured, the notification system will
>> be used'),
>> +viewModel,
>> +bind: {
>> +hidden: '{!hintTextVisible}',
>
> I think the `pmx-hint` being displayed as yellow suggests it is a
> warning/error rather than a hint. I wonder if there is a better
> approach, as this is certainly not a warning.
Yeah, the hint is even often used as warning, having something more like a
"notice" or well, actual "hint" level, that isn't as flashy, might be a
good idea in the long run..
For now, we could also set a 'Hint' label and then add the pmx-hint class only
to the labelClsExtra config?
But I'm fine with this as is if you, Lukas, think that it's warranted (and
maybe even not showed that often in practice anyway).
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [RFC container/firewall/manager/proxmox-firewall/qemu-server 00/37] proxmox firewall nftables implementation
Am 11/04/2024 um 07:21 schrieb Stefan Hanreich: >> Since `Command` is serializable anyway, we could have a nice test suite of >> firewall/VM config files and expected commands as JSON dumps. >> This will be tedious to setup at first, but will help to detect any unwanted >> regressions in the long-term. > > Yes, that is certainly something that is on the menu, as we've already > talked off-list using something like insta[1], which is already > packaged, would be a good approach to this imo. > > [1] https://github.com/mitsuhiko/insta Does a simple serialize config and then diff that to the reference really needs that elaborate crate that comes with its own cargo sub command? I mean it looks Like I do not need to use the latter for running the tests, so I guess if it's packaged in Debian we could try it if you really think it provides that much convenience. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied-series: [PATCH stable-7 qemu 1/2] update patches and submodule to QEMU stable 7.2.10
Am 10/04/2024 um 15:13 schrieb Fiona Ebner:
> Many stable fixes came in since the last bump, a few of which were
> actually already present. Notable ones not yet present include a few
> guest-triggerable assert fixes, some AHCI/IDE fixes (including the fix
> for bug #2784), TGC fixes for i386 and ARM, VirtIO fixes, fix to avoid
> VNC clipboard denial-of-service.
>
> The reentrancy patches that landed upstream/stable were a newer
> version than the ones backported initially here, so it was necessary
> to explicitly drop them before rebase (which then picked up the
> upstream version).
>
> There were no other conflicts.
>
> Signed-off-by: Fiona Ebner
> ---
> ...d-support-for-sync-bitmap-mode-never.patch | 16 +-
> ...check-for-bitmap-mode-without-bitmap.patch | 4 +-
> .../0006-mirror-move-some-checks-to-qmp.patch | 4 +-
> ...race-with-clients-disconnecting-earl.patch | 10 +-
> ...monize-defuse-PID-file-resolve-error.patch | 4 +-
> ...s-Internal-cdbs-have-16-byte-length.patch} | 0
> ...he-bitmap-index-of-the-section-offse.patch | 44 ---
> ...al-deadlock-when-draining-during-tr.patch} | 10 +-
> ...he-iterator-variable-in-a-vmem-rdl_l.patch | 36 ---
> ...ty-bitmap-syncing-when-vIOMMU-is-ena.patch | 141 -
> ...pci-fix-migration-compat-for-vectors.patch | 42 ---
> ...-zeroes-with-BDRV_REQ_REGISTERED_BUF.patch | 36 ---
> ...-memory-prevent-dma-reentracy-issues.patch | 118
> ...double-free-on-BUSY-or-similar-statu.patch | 32 --
> ...ing-endian-conversions-for-doorbell-.patch | 67
> ...fix-field-corruption-in-type-4-table.patch | 50 ---
> ...ix-transitional-migration-compat-for.patch | 35 ---
> ...er-hpet-Fix-expiration-time-overflow.patch | 80 -
> ...vdpa-stop-all-svq-on-device-deletion.patch | 71 -
> ...tential-use-of-an-uninitialized-vari.patch | 132
> ...ket-set-s-listener-NULL-in-char_sock.patch | 70 -
> ...il-MAP-notifier-without-caching-mode.patch | 41 ---
> ...-fail-DEVIOTLB_UNMAP-without-dt-mode.patch | 50 ---
> ...isabling-re-entrancy-checking-per-MR.patch | 38 ---
> ...le-reentrancy-detection-for-script-R.patch | 33 --
> ...uest-visible-maximum-access-size-to-.patch | 166 --
> ...Introduce-and-use-reg_t-consistently.patch | 286 --
> ...25-target-i386-Fix-BEXTR-instruction.patch | 97 --
> ...i386-Fix-C-flag-for-BLSI-BLSMSK-BLSR.patch | 47 ---
> ...arget-i386-fix-ADOX-followed-by-ADCX.patch | 192
> ...028-target-i386-Fix-BZHI-instruction.patch | 64
> ...djust-network-script-path-to-etc-kvm.patch | 4 +-
> ...he-CPU-model-to-kvm64-32-instead-of-.patch | 2 +-
> .../0007-PVE-Up-qmp-add-get_link_status.patch | 4 +-
> ...return-success-on-info-without-snaps.patch | 2 +-
> ...dd-add-osize-and-read-from-to-stdin-.patch | 12 +-
> ...E-Up-qemu-img-dd-add-isize-parameter.patch | 14 +-
> ...PVE-Up-qemu-img-dd-add-n-skip_create.patch | 10 +-
> ...virtio-balloon-improve-query-balloon.patch | 2 +-
> ...async-for-background-state-snapshots.patch | 10 +-
> ...-Add-dummy-id-command-line-parameter.patch | 10 +-
> ...3-PVE-monitor-disable-oob-capability.patch | 4 +-
> ...sed-balloon-qemu-4-0-config-size-fal.patch | 4 +-
> ...E-Allow-version-code-in-machine-type.patch | 12 +-
> ...VE-Backup-add-vma-backup-format-code.patch | 4 +-
> ...ckup-proxmox-backup-patches-for-qemu.patch | 8 +-
> ...estore-new-command-to-restore-from-p.patch | 4 +-
> ...irty-bitmap-tracking-for-incremental.patch | 6 +-
> .../pve/0032-PVE-various-PBS-fixes.patch | 6 +-
> ...k-driver-to-map-backup-archives-into.patch | 6 +-
> ...dd-query_proxmox_support-QMP-command.patch | 2 +-
> ...E-add-query-pbs-bitmap-info-QMP-call.patch | 2 +-
> ...ct-stderr-to-journal-when-daemonized.patch | 4 +-
> ...-transaction-to-synchronize-job-stat.patch | 2 +-
> ...-block-on-finishing-and-cleanup-crea.patch | 2 +-
> ...igrate-dirty-bitmap-state-via-savevm.patch | 4 +-
> ...all-back-to-open-iscsi-initiatorname.patch | 4 +-
> ...routine-QMP-for-backup-cancel_backup.patch | 6 +-
> .../pve/0044-PBS-add-master-key-support.patch | 6 +-
> ...accept-NULL-qiov-in-bdrv_pad_request.patch | 2 +-
> ...-add-l-option-for-loading-a-snapshot.patch | 14 +-
> .../pve/0052-pbs-namespace-support.patch | 6 +-
> ...e-jobs-correctly-cancel-in-error-sce.patch | 2 +-
> ...nsure-jobs-in-di_list-are-referenced.patch | 2 +-
> ...d-segfault-issues-upon-backup-cancel.patch | 2 +-
> ...-passing-max-workers-performance-set.patch | 6 +-
> debian/patches/series | 28 +-
> qemu | 2 +-
> 68 files changed, 122 insertions(+), 2114 deletions(-)
> rename
> debian/patches/extra/{0010-scsi-megasas-Internal-cdbs-have-16-byte-length.patch
> => 0003-scsi-megasas-Internal-cdbs-have-16-byte-length.patch} (100%)
> delete mode 100644
>
[pve-devel] applied: [PATCH qemu-server v2 1/2] fix #5363: cloudinit: make creation of scsi cloudinit discs possible again
Am 10/04/2024 um 13:17 schrieb Hannes Duerr: > Upon obtaining the device type, a check is performed to determine if it > is a CD drive. It is important to note that Cloudinit drives are always > assigned as CD drives. If the drive has not yet been allocated, the test > will fail due to the unset cd attribute. > To avoid this, an explicit check is now performed to determine if it is > a Cloudinit drive that has not yet been assigned. > > The mentioned error was introduced by this patch: > https://lists.proxmox.com/pipermail/pve-devel/2024-January/061311.html > > Signed-off-by: Hannes Duerr > --- > Changes since v1: > - fixed rephrased commit message > - added reference to the commit which introduced the bug > > PVE/QemuServer/Drive.pm | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > applied both patches, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH kernel] add apparmor patch to fix recvmsg returning EINVAL
Am 10/04/2024 um 14:17 schrieb Wolfgang Bumiller: > With apparmor 4, when recvmsg() calls are checked by the apparmor LSM > they will always return EINVAL. > This causes very weird issues when apparmor profiles are in use, and a > lot of networking issues in containers (which are always using > apparmor). > > When coming from sys_recvmsg, msg->msg_namelen is explicitly set to > zero early on. (see sys_recvmsg in net/socket.c) > We still end up in 'map_addr' where the assumption is that addr != > NULL means addrlen has a valid size. > > This is likely not a final fix, it was suggested by jjohansen on irc > to get things going until this is resolved properly. > > Signed-off-by: Wolfgang Bumiller > --- > ...pect-msg_namelen-0-for-recvmsg-calls.patch | 31 +++ > 1 file changed, 31 insertions(+) > create mode 100644 > patches/kernel/0012-apparmor-expect-msg_namelen-0-for-recvmsg-calls.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-series: [PATCH widget-toolkit 0/2] notification: set 'Remove' button text to 'Reset to default' for built-ins
Am 14/12/2023 um 10:48 schrieb Lukas Wagner: > Deleting a built-in target/matcher does not remove it, but resets it > to its default settings. This was not really obvious from the UI. > This patch changes the 'Remove' button text based on the > selected target/matcher. If it is a built-in, the button text is > changed to 'Reset to default'. Also, if the built-in is not actually > modified, the button is disabled. > > Lukas Wagner (2): > remove button: allow to set custom confirmation message > notify: change 'Remove' button to 'Reset to default' for built-ins > > src/button/Button.js| 10 - > src/panel/NotificationConfigView.js | 63 + > 2 files changed, 72 insertions(+), 1 deletion(-) > applied series, thanks! But I reduced the text from "Reset to default" to just "Reset", the confirmation message is telling enough for this nice case to avoid all to much jumping of UI elements. Note that we normally even use the proxmoxAltTextButton, which sets its width such, that it doesn't have to change size when changing between two (statically set) texts – like for "Remove" and "Detach" in the VM hardware panel for non-disks vs. disks. But this could be done afterwards, personally it's not a no-go for layout changes, not sure how good we're currently to avoid that, but maybe reusing that component could be worth it here. ___ 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 v2] i18n: mark strings as translatable
Am 07/12/2023 um 09:18 schrieb Maximiliano Sandoval: > Note that N/A is already translatable in other places. > > Signed-off-by: Maximiliano Sandoval > --- > Differences from v2: > - Translate the invalid subscription key message, this string is also in > two more places in pve-manager. This will be the content of a following patch. > - Do not translate 'ACME' > > src/Utils.js | 5 - > src/node/DNSView.js | 2 +- > src/panel/DiskList.js | 6 +++--- > src/window/NotificationMatcherEdit.js | 2 +- > 4 files changed, 9 insertions(+), 6 deletions(-) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server 1/1] fix #5365: drive: add drive_is_cloudinit check to get_scsi_devicetype
This is not bug #5365 [0] (which is about a ceph device class UX improvement)
but #5363 [0].
[0]: https://bugzilla.proxmox.com/show_bug.cgi?id=5365
[1]: https://bugzilla.proxmox.com/show_bug.cgi?id=5363
I mostly noticed because I had too loo what this is actually about, IMO the
subject could be a bit more telling, targeting more our users, not our
devs (and even for those it's most often not too helpful to name methods
directly).
How about something like:
fix #5363: cloudinit: allow using scsi for the CD-ROM again
That'd tell me roughly what was wrong and also that it worked previously
already, i.e. fixes a regression, rather than a new enhancement. But that
was just from top of my head, maybe there's ab even more telling one
possible, e.g. one alternative could be:
fix #5363: scsi device type: detect cloudinit also for new drives
Am 09/04/2024 um 16:47 schrieb Hannes Duerr:
> When we obtain the devicetype, we check whether it is a CD drive.
I know this quite often comes natural, but for commit messages its IMO
often a bit better to avoid first-person pronouns to avoid the reader
asking "who's we?".
> Cloudinit drives are always allocated CD drives, but if the drive has
> not yet been allocated, the test fails because the cd attribute has not
> yet been set.
> We therefore now explicitly check whether it is a cloudinit
> drive that has not yet been allocated.
As per the bug report this seems to be a regression, in which case a
reference to the commit this fixes would be nice.
> Signed-off-by: Hannes Duerr
> ---
> PVE/QemuServer/Drive.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/PVE/QemuServer/Drive.pm b/PVE/QemuServer/Drive.pm
> index 34c6e87..c829bde 100644
> --- a/PVE/QemuServer/Drive.pm
> +++ b/PVE/QemuServer/Drive.pm
> @@ -853,7 +853,7 @@ sub get_scsi_devicetype {
unrelated, but "get_scsi_device_type" would be a slightly easier to
read variant of that name.
>
> my $devicetype = 'hd';
> my $path = '';
> -if (drive_is_cdrom($drive)) {
> +if (drive_is_cdrom($drive) || drive_is_cloudinit($drive)) {
> $devicetype = 'cd';
> } else {
> if ($drive->{file} =~ m|^/|) {
___
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 v4] window: edit: avoid sharing custom config objects between subclasses
Am 09/04/2024 um 10:16 schrieb Friedrich Weber: > Currently, `Proxmox.window.Edit` initializes `extraRequestParams` and > `submitOptions` to two objects that, if not overwritten, are shared > between all instances of subclasses. This bears the danger of > modifying the shared object in a subclass instead of overwriting it, > which affects all edit windows of the current session and can cause > hard-to-catch GUI bugs. > > One such bug is the following: Currently, the `PVE.pool.AddStorage` > component inadvertently adds `poolid` to an `extraRequestParams` > object that is shared between all instances of `Proxmox.window.Edit`. > As a result, after adding a storage to a pool, opening any edit window > will send a GET request with a superfluous `poolid` parameter and > cause an error in the GUI: > >> Parameter verification failed. (400) >> poolid: property is not defined in schema and the schema does not >> allow additional properties > > This breaks all edit windows of the current session. A workaround is > to reload the current browser session. > > To avoid this class of bugs in the future, implement a constructor > that makes copies of `extraRequestParams` and `submitOptions`. This > ensures that any subclass instance modifies only its own copies, and > modifications do not leak to other subclass instances. > > Suggested-by: Stefan Sterz > Suggested-by: Thomas Lamprecht > Signed-off-by: Friedrich Weber > --- > > Notes: > @Thomas, I've added a Suggested-by, feel free to remove/keep as you > prefer. > > Changes from v3: > - Fix broken pool edit window (thx sterzy!) by passing all arguments > to `callParent`. The `initConfig` call is obsolete as the constructor > of `Ext.Component` [1] calls `initConfig` already. > > Changes from v1+v2: > - As suggested by sterzy (thx!), avoid this class of bugs in a more > generic fashion by introducing a `Proxmox.window.Edit` constructor > that copies custom config objects > - Added full error message to commit message for better searchability > > v3: https://lists.proxmox.com/pipermail/pve-devel/2024-April/062657.html > v2: https://lists.proxmox.com/pipermail/pve-devel/2024-April/062561.html > v1: https://lists.proxmox.com/pipermail/pve-devel/2024-March/062179.html > > [1] > https://docs.sencha.com/extjs/7.0.0/classic/src/Component.js.html#line2203 > > src/window/Edit.js | 9 + > 1 file changed, 9 insertions(+) > > applied, with Stefan's T-b, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH widget-toolkit] dark-mode: set intentionally black icons to `$icon-color`
Am 16/10/2023 um 18:28 schrieb Stefan Sterz: > some icons intentionally use black as their color in the light theme. > this includes the little pencil and check mark icon in the acme > overview. change their color to the regular dark-mode icon-color. for > this to work the filter inversion needed for some other icons needs to > be remove too. > > Signed-off-by: Stefan Sterz > --- > src/proxmox-dark/scss/other/_icons.scss | 9 + > 1 file changed, 9 insertions(+) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH pve-storage] esxi: add mapping for windows server 2016/2019
Am 09/04/2024 um 12:56 schrieb Stefan Sterz: > previously these were mapped to the linux 2.6 default > > Signed-off-by: Stefan Sterz > --- > src/PVE/Storage/ESXiPlugin.pm | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/PVE/Storage/ESXiPlugin.pm b/src/PVE/Storage/ESXiPlugin.pm > index 4212c36..e5082ea 100644 > --- a/src/PVE/Storage/ESXiPlugin.pm > +++ b/src/PVE/Storage/ESXiPlugin.pm > @@ -878,6 +878,8 @@ my %guest_types_windows = ( > winNetBusiness => 'w2k3', > windows9=> 'win10', > 'windows9-64' => 'win10', > +windows9srv => 'win10', > +'windows9srv-64'=> 'win10', just FYI: I had this also applied but with a follow-up wrapping all keys in single quotes to make the mappings slightly easier to read. I rebased and pushed that out on top. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager] Revert "ui: dc: remove notify key from datacenter option view"
Am 09/02/2024 um 11:16 schrieb Lukas Wagner: > This reverts commit c81bca2d28744616098448b81fa58e133d3ac5ed. > > In the first iteration of the notification system, notification > routing and enabling/disabling notifications was configured via > the (extended) `notify` parameter in `datacenter.cfg`. > Because of that, the configuration UI for this parameter was moved to > a new panel as a part of the notification UI. > When changing to the newer approach for notification routing (matcher > based), the "new" panel this setting was moved to was dropped from the > UI. > > Notification sending for package updates is still influenced by this > parameter (see bin/pveupdate, line 55), so there should be a way to > configure this from the GUI. At some point, the `notify` parameter > should be dropped, but that'd be a thing for a major release. > > Signed-off-by: Lukas Wagner > --- > https://forum.proxmox.com/threads/package-update-notifs-not-working.141182/ > > Notes: > Alternatively, we could just *always* send package update > notifications and just ignore that parameter from now on but this > might leave users wondering who have previously set > `package-updates=never`... I'd propose two changes: - add a hint to redirect users to the new mechanisms so that a future deprecation would be more expected (if we already plan that now) - only show it if defined? While that's a bit magic, it'd avoid that users set it, but rather use the new mechanism. If, I'd never delete the setting via the UI, so that it doesn't suddenly disappears if one switches it from some value to default. What do you think? ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH installer] run env: use default error message if country detection failed with empty string
Am 26/03/2024 um 14:29 schrieb Christoph Heiss:
> Bit of perl fun again.
> $err from detect_country_tracing_to() can be empty string under certain
> circumstances (according to a forum post [0]). The // operator
> evaluates an empty as true, thus `warn` receives an empty string to and
> just prints
>
> Warning: something wrong at /usr/share/perl5/proxmox/Install/RunEnv.pm line
> 305
>
> Which isn't particular helpful. Use the || operator instead, that
> evaluates an empty string as false and thus would fall back to the
> generic error message.
>
> A minimal reproducer/example for completeness sake:
>
> #!/usr/bin/env perl
> use strict;
> use warnings;
>
> warn ('' // "unable to detect country\n");
> warn ('' || "unable to detect country\n");
>
> gives
>
> Warning: something's wrong at ./test.pl line 5.
> unable to detect country
>
> [0] https://forum.proxmox.com/threads/blank-screen-while-installing.143928/
>
> Signed-off-by: Christoph Heiss
> ---
> Proxmox/Install/RunEnv.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 common] docs: add missing prefix
Am 27/03/2024 um 14:09 schrieb Folke Gleumes: > include 'PVEAPIToken=' prefix in the example for target-endpoint which > is mainly used for remote migrations. > > Signed-off-by: Folke Gleumes > --- > src/PVE/JSONSchema.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 pve-manager v2] sdn: evpn: allow empty primary exit node in zone form
Am 22/02/2024 um 17:40 schrieb Stefan Hanreich: > its broken since the change in semantics of the PUT endpoint [1] > > [1] > https://git.proxmox.com/?p=pve-network.git;a=commit;h=3e3cafabaf955d53c4c2d4e346bf5c3a5c6d1852 > > Signed-off-by: Stefan Hanreich > Originally-by: Alexandre Derumier > --- > www/manager6/sdn/zones/EvpnEdit.js | 2 ++ > 1 file changed, 2 insertions(+) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH v3 pve-network 00/12] SDN: Testing VNets as a blackbox and fixing found bugs
Am 05/04/2024 um 15:17 schrieb Stefan Lendl: > This add several tests for SDN VNets. > State setup as well as testing results is done only via the API to test on the > API boundaries and not against the internal state. Internal state and config > files are mocked to avoid requiring access to system files or pmxcfs. > > The first 4 commits fix functionality, identified as bugs thanks to the tests. > > The next 7 commits extract various functions to allow mocking them in the > tests. The tests are then added as the test_vnets_blackbox test. > The last commit removes the old vnets tests which are not working anyway. > > Tests validate the events of a nic joining a VNet or a nic staring on a VNet. > These events are tested with with different subnet configurations. > Mainly for IPv4 and/or IPv6 configurations and odd combinations. > Further descriptions in the commit. > > Differences v2 -> v3: > * Fix functionalitiy in VNet and Subnet so all tests pass > Thanks @s.hanreich for lots of testing > * Update and add more tests > * Make it build in sbuild > > Differences v1 -> v2: > * Add tests that expect a failure when no IP can be allocated > * Removed commented out debug stuff > > > Stefan Hanreich (2): > sdn: dhcp: only consider subnets that have dhcp-range configured > sdn: dhcp: rollback allocated ips on failure > > Stefan Lendl (10): > sdn: dhcp: get next free ip for a specific IP version > sdn: dhcp: request both IPv4 and IPv6 addresses on VM start > sdn: zones: extract function that reads datacenter config > dns: dnsmasq: extract function to systemctl command. > sdn: dnsmasq: extract function that generates the ethers file path > sdn: dnsmasq: extract function that updates dnsmasq lease via dbus > sdn: api: extract function that creates the sdn directory. > debian: blackbox tests depend on libpve-access-control at build > tests: test VNets functionality as a blackbox > tests: remove old Vnets tests > > debian/control| 1 + > src/PVE/API2/Network/SDN/Zones.pm | 6 +- > src/PVE/Network/SDN/Dhcp/Dnsmasq.pm | 47 +- > src/PVE/Network/SDN/Subnets.pm| 2 +- > src/PVE/Network/SDN/Vnets.pm | 29 +- > src/PVE/Network/SDN/Zones/EvpnPlugin.pm | 3 +- > src/PVE/Network/SDN/Zones/Plugin.pm | 5 + > src/PVE/Network/SDN/Zones/SimplePlugin.pm | 2 +- > src/test/Makefile | 5 +- > src/test/run_test_vnets.pl| 343 - > src/test/run_test_vnets_blackbox.pl | 894 ++ > 11 files changed, 962 insertions(+), 375 deletions(-) > delete mode 100755 src/test/run_test_vnets.pl > create mode 100755 src/test/run_test_vnets_blackbox.pl > applied series, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH] fix #4835: order zfs-import@ before -cache/-scan
Am 28/03/2024 um 10:41 schrieb Fabian Grünbichler: > this should fix failures of the template instances because either of the two > other import services picked up the pool in question first. > > Signed-off-by: Fabian Grünbichler > --- > ...dd-systemd-unit-for-importing-specific-pools.patch | 11 --- > 1 file changed, 8 insertions(+), 3 deletions(-) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH ksm-control-daemon] ksmtuned: use PSS instead of RSZ for caluculating memory usage
Am 08/04/2024 um 15:25 schrieb Stefan Lendl: > PSS properly accounts for memory usage of shared libraries and is > therefore better suited when summing up memory usage of multiple > processes. > > Signed-off-by: Stefan Lendl > --- > debian/patches/series | 1 + > debian/patches/use-pss-instead-of-rsz.diff | 11 +++ > 2 files changed, 12 insertions(+) > create mode 100644 debian/patches/use-pss-instead-of-rsz.diff > > applied, thanks! I made a follow-up that introduced a `KSM_PS_METRIC` variable which can be set in `/etc/ksmtuned.conf` to override the metric to something else, like `rsz` (resident set size) – if e.g. it turns out that for some setups the PSS one is rather slow. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server v3] QEMU AMD SEV enable
Am 09/12/2022 um 15:25 schrieb Markus Frank: > This Patch is for enabling AMD SEV (Secure Encrypted > Virtualization) support in QEMU > > VM-Config-Examples: > amd_sev: type=std,nodbg=1,noks=1 > amd_sev: es,nodbg=1,kernel-hashes=1 > > Node-Config-Example (gets generated automatically): > amd_sev: cbitpos=47,reduced-phys-bios=1 > > kernel-hashes, reduced-phys-bios & cbitpos correspond to the varibles > with the same name in qemu. > > kernel-hashes=1 adds kernel-hashes to enable measured linux kernel > launch since it is per default off for backward compatibility. > > reduced-phys-bios and cbitpos are system specific and can be read out > with QMP. If not set by the user, a dummy-vm gets started to read QMP > for these variables out and save them to the node config. > Afterwards the dummy-vm gets stopped. > > type=std stands for standard sev to differentiate it from sev-es (es) > or sev-snp (snp) when support is upstream. > > Qemu's sev-guest policy gets calculated with the parameters nodbg & noks > These parameters correspond to policy-bits 0 & 1. > If type=es than policy-bit 2 gets set to 1 to activate SEV-ES. > Policy bit 3 (nosend) is always set to 1, because migration > features for sev are not upstream yet and are attackable. > > see coherent doc patch > > Signed-off-by: Markus Frank > --- > I still could not get SEV-ES to work. > After a firmware update I got the same error like Daniel in his testing: > kvm: ../softmmu/vl.c:2568: qemu_machine_creation_done: Assertion > `machine->cgs->ready' failed. > This was one of the main turn-offs for me, but maybe the situation change here w.r.t newer HW, kernel and QEMU support. Can you please re-test this rather soonish? E.g. with kernel 6.5 and 6.8, also trying a newer QEMU like Fiona's 8.2 build and our newer AMD based HW would be good to check out. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH v2 ksm-control-daemon 1/2] ksmtuned: revert to use rsz instead of vsz to compute memory usage
Am 08/04/2024 um 14:17 schrieb Stefan Lendl: > For summing up actual memory usage of precesses rsz is more suitable > then vsz. > > This reverts commit cd5cf20cc8af53427dcb9b08486c68f376ce8743. > > Signed-off-by: Stefan Lendl > --- > debian/patches/series | 1 - > debian/patches/use-vsz-instead-of-rsz.diff | 13 - > 2 files changed, 14 deletions(-) > delete mode 100644 debian/patches/use-vsz-instead-of-rsz.diff > > applied both patches, thanks! Please still re-look into using PSS as metric, like asked in my reply to the previous discussion thread twenty minutes ago – if that works through `ps -C kvm -o pss` as it does on my machines, the change could be sent as a follow-up. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH widget-toolkit v3] window: edit: avoid sharing custom config objects between subclasses
Am 08/04/2024 um 12:36 schrieb Stefan Sterz:
> On Mon Apr 8, 2024 at 11:30 AM CEST, Friedrich Weber wrote:
>> +constructor: function(conf) {
>> +let me = this;
>> +// make copies in order to prevent subclasses from accidentally writing
>> +// to objects that are shared with other edit window subclasses
>> +me.extraRequestParams = Object.assign({}, me.extraRequestParams);
>> +me.submitOptions = Object.assign({}, me.submitOptions);
>> +me.initConfig(conf);
>> +me.callParent();
>
>
> so, this seems like a fix bug a) creates bug b) type of situation...
> this patch means that editing a pool allows changing the name suddenly,
> but since we don't support that in the backend, that just creates a new
> pool :/
>
> this is due to the `editable` attribute depending on `isCreate`, which
> in turn depends on the configs poolid being set. to fix this, the config
> needs to also be passed to `callParent` so it can set the configurations
> there too. so this line should be:
>
> me.callParent([conf]);
>
> sorry, could have noticed that earlier in my suggestion. also this needs
> to be an arrray as `callParent` expects a list of arguments to pass to
> parent's function and not the parameters themselves directly.
Using `me.callParent(arguments)` might be more proof to future changes and
arguments is an array (or well, iterator) already
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH ksm-control-daemon] ksmtuned: fix large number processing
Am 08/04/2024 um 14:04 schrieb Stefan Lendl: > I agree summing up processes it would make sense to use PSS. > Unfortunately, ps does not report the PSS. The `ps` from the Debian Bookworm version of the `procps` package does report it here if I use something like `ps -C kvm -o pss` though, FWICT this should be available here? Can you please re-check this? ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH container v2 3/6] fix #4474: lxc api: add overrule-shutdown parameter to stop endpoint
Am 30/01/2024 um 18:10 schrieb Friedrich Weber:
> The new `overrule-shutdown` parameter is boolean and defaults to 0. If
> it is 1, all active `vzshutdown` tasks by the current user for the same
> CT are aborted before attempting to stop the CT.
>
> Passing `overrule-shutdown=1` is forbidden for HA resources.
>
> Signed-off-by: Friedrich Weber
> ---
>
> Notes:
> changes v1 -> v2:
> * move overrule code into worker, as suggested by Wolfgang
> * print to worker stdout instead of syslog
>
> src/PVE/API2/LXC/Status.pm | 18 ++
> 1 file changed, 18 insertions(+)
>
> diff --git a/src/PVE/API2/LXC/Status.pm b/src/PVE/API2/LXC/Status.pm
> index 7741374..33f449c 100644
> --- a/src/PVE/API2/LXC/Status.pm
> +++ b/src/PVE/API2/LXC/Status.pm
> @@ -220,6 +220,12 @@ __PACKAGE__->register_method({
> node => get_standard_option('pve-node'),
> vmid => get_standard_option('pve-vmid', { completion =>
> \::LXC::complete_ctid_running }),
> skiplock => get_standard_option('skiplock'),
> + 'overrule-shutdown' => {
> + description => "Abort any active 'vzshutdown' task by the
> current user for this CT before stopping",
> + optional => 1,
> + type => 'boolean',
> + default => 0,
> + }
> },
> },
> returns => {
> @@ -237,10 +243,15 @@ __PACKAGE__->register_method({
> raise_param_exc({ skiplock => "Only root may use this option." })
> if $skiplock && $authuser ne 'root@pam';
>
> + my $overrule_shutdown = extract_param($param, 'overrule-shutdown');
> +
> die "CT $vmid not running\n" if !PVE::LXC::check_running($vmid);
>
> if (PVE::HA::Config::vm_is_ha_managed($vmid) && $rpcenv->{type} ne
> 'ha') {
>
> + raise_param_exc({ 'overrule-shutdown' => "Not applicable for HA
> resources." })
> + if $overrule_shutdown;
This probably could be implemented through a CRM command and would be nice
to have in the long-run. The more HA and non-HA resources can be managed
the same the better, as otherwise it's causing friction to use HA.
The same holds for the qemu-server patch.
But, I'm fine with doing this separately later on, as this on its own is
already an improvement for quite a few users.
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH guest-common v2 1/6] guest helpers: add helper to overrule active tasks of a specific type
Am 05/04/2024 um 15:13 schrieb Friedrich Weber:
> On 04/04/2024 17:20, Thomas Lamprecht wrote:
>> Or does it even make sense to check this at all?
>> As long as the user has the rights to execute a stop they probably
>> should also be able to force it at any time, even for other users?
>
> The usecase sounds reasonable, but would technically amount to a small
> privilege escalation: Currently, if user A and user B both have
> PVEVMUser and user A starts a `vzshutdown` task, user B must have
> `Sys.Modify` to kill the task.
>
Hmm, yeah, this is definitively something one could argue about as
we currently do not have much overruling functionality of tasks triggered
by other users that can manage a specific resource, but not the whole
system. So this is essential new territory w.r.t. semantics, and we can
also adapt new ones, albeit they should certainly not be completely
unexpected.
>> Maybe make this optional and only enforce it for users that do not
>> have some more powerful priv?
>
> We could introduce a similar check as for the DELETE
> /nodes/{node}/tasks/{upid} endpoint: Users can always overrule their own
> guest tasks, and with `Sys.Modify` they can overrule any guest task (for
> guests for which they have the necessary permission).
Yeah, I think that would be a safe bet for now.
> Still, right now I think the primary motivation for this overruling
> feature is to save GUI users some frustration and/or clicks. In this
> scenario, a user will overrule only their own tasks, which is possible
> with the current check. What do you think about keeping the check as it
> is for now, and making it more permissive once the need arises?
I think that allowing users that hold the respective Sys.Modify and
VM.PowerMgmt to overrule any tasks from the start wouldn't be to much
"speculative future-proofing" but rather something expected while still
safe.
FWIW, you could also drop the $authuser then and just get it from
the RPCEnv singleton available in all API call-paths and then do
the permission check in the helper directly.
This would IMO be also a bit better w.r.t. conveying why we do it this
way.
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH pve-kernel] revert 2 changes in thermal driver causing an early kernel Oops.
Am 05/04/2024 um 11:27 schrieb Stoiko Ivanov: > The second patch, that is reverted (first): > `thermal: trip: Drop lockdep assertion from thermal_zone_trip_id()` > only touches code introduced by the first patch. > The first patch causes the following Oops (reproduced on an old > HP DL380 G8): > ``` > [2.960519] ACPI: button: Power Button [PWRF] > [2.963126] BUG: kernel NULL pointer dereference, address: 000c > [2.965667] #PF: supervisor read access in kernel mode > [2.966954] #PF: error_code(0x) - not-present page > [2.966954] PGD 0 P4D 0 > [2.966954] Oops: [#1] PREEMPT SMP PTI > [2.966954] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G I > 6.5.13-4-pve #1 > [2.966954] Hardware name: HP ProLiant DL380p Gen8, BIOS P70 05/24/2019 > [2.966954] RIP: 0010:step_wise_throttle+0x48/0x360 > [2.966954] Code: 04 25 28 00 00 00 48 89 45 d0 31 c0 48 63 c6 48 8d 14 40 > 48 8b 87 50 03 00 00 4c 8d 24 90 e8 cf d0 ff ff c6 45 bf 00 89 45 b4 <41> 8b > 04 24 41 39 85 78 03 00 00 0f 8d a9 02 00 00 0f 1f 44 00 00 > [2.966954] RSP: :9e2b8014bae8 EFLAGS: 00010246 > [2.966954] RAX: 0002 RBX: 0001 RCX: > > [2.966954] RDX: RSI: RDI: > > [2.966954] RBP: 9e2b8014bb40 R08: R09: > > [2.966954] R10: R11: R12: > 000c > [2.966954] R13: 8c7ac421d000 R14: 0001 R15: > > [2.966954] FS: () GS:8c7def60() > knlGS: > [2.966954] CS: 0010 DS: ES: CR0: 80050033 > [2.966954] CR2: 000c CR3: 000513a34001 CR4: > 000606f0 > [2.966954] Call Trace: > [2.966954] > ``` > > the relevant mainline kernels (6.6.15), corresponding to the > Ubuntu-patchset (which mixes changes from 6.6.15, with ones from > 6.1.76) [0] - also boot happily - so I strongly assume that the > changes depend on one of the many commits introduced in linux-upstream > between v6.5.1 and v6.6.1. > As it looks like a refactoring (upon which later changes are based), > and not a bug-fix in itself - simply dropping it seems sensible. > > Signed-off-by: Stoiko Ivanov > --- > ...rip-Drop-lockdep-assertion-from-ther.patch | 24 ++ > ...ore-Store-trip-pointer-in-struct-the.patch | 343 ++ > 2 files changed, 367 insertions(+) > create mode 100644 > patches/kernel/0014-Revert-thermal-trip-Drop-lockdep-assertion-from-ther.patch > create mode 100644 > patches/kernel/0015-Revert-thermal-core-Store-trip-pointer-in-struct-the.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 container v2 2/6] api: status: move config locking from API handler into worker
Am 30/01/2024 um 18:10 schrieb Friedrich Weber: > Previously, container start/stop/shutdown/suspend would try to acquire > the config lock in the API handler prior to forking a worker. If the > lock was currently held elsewhere, this would block the API handler > and thus the pvedaemon worker thread until the 10s timeout expired (or > the lock could be acquired). > > To avoid blocking the API handler, immediately fork off a worker > process and try to acquire the config lock in that worker. > > Patch best viewed with `git show -w`. > > Suggested-by: Wolfgang Bumiller > Signed-off-by: Friedrich Weber > --- > > Notes: > The diff is somewhat messy without `-w` -- couldn't come up with a > better way. > > new in v2 > > src/PVE/API2/LXC/Status.pm | 91 ++ > 1 file changed, 42 insertions(+), 49 deletions(-) > > applied this one already, thanks! btw. in general getting some early timeout sent as response to the initial request would be good, but blocking the daemon worker is naturally a no-go. And bringing in Anyevent somehow to make it async-like might be a PITA, so for now this change is OK. Oh, and it might be worth mentioning explicitly in the next release notes, as it's a change in behavior that could theoretically throw up some tooling that depends on the $action not failing due to locking if the adapted endpoints returned – albeit IMO a bit silly to assume that (as the actions can fail in other ways inside the worker anyway). ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH guest-common v2 1/6] guest helpers: add helper to overrule active tasks of a specific type
Am 30/01/2024 um 18:10 schrieb Friedrich Weber:
Maybe start of with "Add a helper to abort all tasks from a specific
(type, user, vmid) tuple. It will be used ...
> This helper is used to abort any active qmshutdown/vzshutdown tasks
> before attempting to stop a VM/CT (if requested).
>
> Signed-off-by: Friedrich Weber
> ---
>
> Notes:
> no changes v1 -> v2
>
> src/PVE/GuestHelpers.pm | 18 ++
> 1 file changed, 18 insertions(+)
>
> diff --git a/src/PVE/GuestHelpers.pm b/src/PVE/GuestHelpers.pm
> index 961a7b8..bd94ed2 100644
> --- a/src/PVE/GuestHelpers.pm
> +++ b/src/PVE/GuestHelpers.pm
> @@ -416,4 +416,22 @@ sub check_vnet_access {
> if !($tag || $trunks);
> }
>
> +sub overrule_tasks {
hmm, overruling is the thing you want to do now, but one might
be use this for other reasons, so maybe naming it about what it
does would be a bit better compared to what is used for (now).
>From top of my head that could be "abort_guest_tasks", but maybe
you got a better idea.
> +my ($type, $user, $vmid) = @_;
> +
> +my $active_tasks = PVE::INotify::read_file('active');
> +my $res = [];
> +for my $task (@$active_tasks) {
> + if (!$task->{saved}
> + && $task->{type} eq $type
> + && $task->{user} eq $user
This also means that e.g. root@pam cannot overrule a task started by
apprentice@pam, which might be something admins what to do (they like
overruling users after all ;-P). Or some automation triggering the
shutdown (using a token with a separate user) might be also a good
examples of things I'd like to be able to overrule.
Maybe make this optional and only enforce it for users that do not
have some more powerful priv?
Or does it even make sense to check this at all?
As long as the user has the rights to execute a stop they probably
should also be able to force it at any time, even for other users?
> + && $task->{id} eq $vmid
> + ) {
meh, the pre-existing $killit param is way too subtle for my taste...
Some %param that takes a `kill => "reason"` property for this would be
much more telling.
But changing that is a bit out of scope, a comment would be great for
now though.
> + PVE::RPCEnvironment->check_worker($task->{upid}, 1);
> + push @$res, $task->{upid};
renaming $res to $killed_tasks would also help in reading this code out
of further context.
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH proxmox] notify: fix #5274: also set 'X-Gotify-Key' header for authentication
Am 03/04/2024 um 10:08 schrieb Lukas Wagner: > Versions of Gotify < 2.2.0 only supported the 'X-Gotify-Key' header > for passing the API token. This comment sets this header in addition > to the regular 'Authorization' header in order to be compatible with > older Gotify servers. > > Signed-off-by: Lukas Wagner > --- > proxmox-notify/src/endpoints/gotify.rs | 11 +++ > 1 file changed, 7 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: [PATCH v3 pve-network 0/5] SDN: Add mocking to run tests in sbuild
Am 02/04/2024 um 13:07 schrieb Stefan Lendl: > Extract and mock functions that otherwise access system files which is not > possible in a clean sbuild environment. > Namely /etc/network/interfaces as well as /etc/frr/frr.config.local > Added .gitignore for sbuild artifacts > > Changes v2 -> v3: > * Changed commit messages according to commit guide lines > * Not importing Dumper in zones test > * Re-Enabled DNS tests > * Added .gitignore which was previously a separate patch > > Changes v1 -> v2: > * Disabled DNS tests because they fail > > > Stefan Lendl (5): > controllers: extract read_etc_network_interfaces > evpn: extract read_local_frr_config > tests: mocking more functions to avoid system access > tests: run tests in sbuild > gitignore: build artifacts from sbuild > > .gitignore| 3 ++ > src/Makefile | 2 +- > src/PVE/Network/SDN/Controllers.pm| 16 +++--- > src/PVE/Network/SDN/Controllers/EvpnPlugin.pm | 10 -- > src/test/run_test_zones.pl| 31 +++ > 5 files changed, 54 insertions(+), 8 deletions(-) > applied series, with Stefan Hanreich's T-b, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH installer] html: pbs: fix missing in template after feature list
Am 03/04/2024 um 12:45 schrieb Christoph Heiss: > This adds an empty line between the feature list and the "more > information" paragraph, which looks a lot better. > > The exact same is already present in the HTML template for both other > products, probably a simple oversight. > > Signed-off-by: Christoph Heiss > --- > html/pbs/extract1-license.htm | 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 pve-kernel] revert cifs backport to 6.1 added between 6.5.13-1 and 6.5.13-2
Am 03/04/2024 um 13:46 schrieb Stoiko Ivanov: > copying files within a cifs-share currently result in the following > trace: > ``` > [ 495.388739] BUG: unable to handle page fault for address: fffe > [ 495.388744] #PF: supervisor read access in kernel mode > [ 495.388746] #PF: error_code(0x) - not-present page > [ 495.388747] PGD 172c3f067 P4D 172c3f067 PUD 172c41067 PMD 0 > [ 495.388752] Oops: [#2] PREEMPT SMP NOPTI > [ 495.388754] CPU: 1 PID: 3894 Comm: cp Tainted: G D > 6.5.0-32-generic #32-Ubuntu > >[ 495.388756] Hardware name: > QEMU Standard PC (Q35 + ICH9, 2009), BIOS 4.2023.08-4 02/15/2024 > [ 495.388758] RIP: 0010:cifs_flush_folio+0x41/0xf0 [cifs] > ... > ``` > > a quick check identified proxmox-kernel-6.5.13-2 as the first affected > version, and `2dc07a11e269bfbe5589e99b60cdbae0118be979` as likely > source of the issue. The commit adapts the changes from > `7b2404a886f8b91250c31855d287e632123e1746` to work with the code in > kernel 6.1. > This is not needed as the relevant changes were made in 6.4 and > are already part of the 6.5 tree - > `66dabbb65d673aef40dd17bf62c042be8f6d4a4b` > > reverting the commit fixes copying files within a samba share. > > Tested/reproduced with: > * a VM with the kernel as cifs-client > * one very crude samba-share allowing guest-write access on a Debian > bookworm host > * as well as a share using cifscreds + multiuser (`mount.cifs(8)`) > * mounting the share, copying any file from one directory to another > on the same share (with `cp` and Thunar and Nautilus). > > Reported to Ubuntu upstream at [1]. > > [0] https://lore.kernel.org/linux-mm/zzhrpnj3zxmr8...@eldamar.lan/ > [1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2055002 > > Reported-by: Daniela Häsler > Signed-off-by: Stoiko Ivanov > --- > ...flushing-folio-regression-for-6.1-ba.patch | 23 +++ > 1 file changed, 23 insertions(+) > create mode 100644 > patches/kernel/0014-Revert-cifs-fix-flushing-folio-regression-for-6.1-ba.patch > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH widget-toolkit 3/3] window: edit: avoid shared object for extra request params
Am 04/04/2024 um 12:10 schrieb Friedrich Weber:
> Maybe we could do:
>
> ```js
> extraRequestParams: {},
>
> constructor: function(conf) {
> let me = this;
> me.extraRequestParams = Ext.clone(me.extraRequestParams);
> me.initConfig(conf);
> me.callParent();
> },
> ```
>
> ... which, if I'm not missing anything, *should* cover everything (with
> the cost of allocating unnecessary empty objects)?
>
yeah, I just wanted to suggest something like that, albeit I first had the
(a few times used):
me.extraRequestParams = Ext.apply({}, me.extraRequestParams ?? {});
pattern in mind. Mostly an slight performance improvement as we can assume
that this is either undefined or an object, while Ext.clone checks for all
different types (in a legacy browser compat way).
Albeit nowadays one might be even better off to use something like the
spread operator:
me.extraRequestParams = { ...(me.extraRequestParams ?? {}) };
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax#spread_in_object_literals
Or maybe even nicer, the Object.assign operator, that does not throw if
the sources are null or undefined:
me.extraRequestParams = Object.assign({}, me.extraRequestParams);
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign
But all those are implementation details (of which I probably would prefer
the Object.assign one the most, albeit no hard feelings), in general your
proposed solution seems to be the best one, w.r.t sane new usage and
backward compat.
btw. hasn't the `submitOptions` config object the same issue?
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager v15 1/2] add clipboard comboBox to VM Options
subject should be more in the style of:
ui: qemu: add clipboard selector to options
Am 21/11/2023 um 13:39 schrieb Markus Frank:
> For SPICE and VNC, a different message is displayed.
>
possibly reference the backend work here,
> Save config in DisplayEdit so that the clipboard setting persist.
this sentence is not really that self-explaining, maybe extend
on the reasoning here.
>
> Signed-off-by: Markus Frank
> Reviewed-by: Dominik Csapak
> Tested-by: Dominik Csapak
> ---
> v15:
> * changed style of line break in vncHint field
>
> www/manager6/qemu/DisplayEdit.js | 8
> www/manager6/qemu/Options.js | 80
> 2 files changed, 88 insertions(+)
>
> diff --git a/www/manager6/qemu/Options.js b/www/manager6/qemu/Options.js
> index 7b112400..53d0beac 100644
> --- a/www/manager6/qemu/Options.js
> +++ b/www/manager6/qemu/Options.js
> @@ -154,6 +154,86 @@ Ext.define('PVE.qemu.Options', {
> },
> } : undefined,
> },
> + vga: {
> + header: gettext('Clipboard'),
> + defaultValue: false,
> + renderer: function(value) {
> + let vga = PVE.Parser.parsePropertyString(value, 'type');
> + if (vga.clipboard) {
> + return vga.clipboard.toUpperCase();
> + } else {
> + return Proxmox.Utils.defaultText + ' (SPICE)';
> + }
> + },
> + editor: caps.vms['VM.Config.HWType'] ? {
> + xtype: 'proxmoxWindowEdit',
> + subject: gettext('Clipboard'),
> + onlineHelp: 'qm_display',
> + items: {
> + xtype: 'pveDisplayInputPanel',
> + referenceHolder: true,
It might be avoidable to make this a referenceHolder by using a viewModel,
see below.
> + items: [
> + {
> + xtype: 'proxmoxKVComboBox',
> + name: 'clipboard',
> + reference: 'clipboard',
do we really need a reference here? I see no use of it (i.e., through lookup)
> + itemId: 'clipboardBox',
the itemId seems also unused, and those have to be globally unique, so best
to be avoided if unsure.
> + fieldLabel: gettext('Clipboard'),
> + deleteDefaultValue: true,
> + listeners: {
> + change: function(field, value) {
> + let inputpanel = field.up("inputpanel");
> + let isVnc = value === 'vnc';
> +
> inputpanel.lookup('vncHint').setVisible(isVnc);
> +
> inputpanel.lookup('defaultHint').setVisible(!isVnc);
> + },
a viewModel would be nicer to do this, e.g at the input panel level add:
viewModel: {
data: {
clipboard: '__default__',
},
formulas: {
isVnc: get => get('clipboard') === 'vnc',
},
},
Then in the clipboard field bind the value to the viewModel:
bind: {
value: {clipboard},
},
and in the hint fields bind the visibility state to the formula:
bind: {
hidden: {isVNC},
// or reverse: hidden: {!isVNC},
},
> + },
> + value: '__default__',
> + comboItems: [
> + ['__default__', Proxmox.Utils.defaultText +
> ' (SPICE)'],
> + ['vnc', 'VNC'],
> + ],
> + },
> + {
> + itemId: 'vncHint',
> + name: 'vncHint',
> + reference: 'vncHint',
view the viewModel you would avoid the explicit references and let ExtJS
built-in
logic handle this.
> + xtype: 'displayfield',
the xtype should be the first property in item object definitions. immediately
followed by the name.
> + userCls: 'pmx-hint',
> + hidden: true,
> + value: gettext('You cannot use the default
> SPICE clipboard if the VNC Clipboard is selected.') + ' ' +
> + gettext('VNC Clipboard requires spice-tools
> installed in the Guest-VM.'),
> + },
> + {
> + itemId: 'defaultHint',
> + name: 'defaultHint',
> + reference: 'defaultHint',
same here w.r.t. reference avoidance by using a viewModel.
> + xtype: 'displayfield',
same here with xtype definition having to move up.
> +
[pve-devel] applied: [PATCH docs] system-requirements: mention that SSDs with PLP should be used
Am 20/03/2024 um 09:56 schrieb Aaron Lauterer: > Signed-off-by: Aaron Lauterer > --- > pve-system-requirements.adoc | 2 ++ > 1 file changed, 2 insertions(+) > > applied this one, thanks! It's correct and avoids duplicate use of "recommend" ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied-series: [PATCH v2 pve-manager pve-docs 0/4] add optional WoL config options
Am 26/03/2024 um 10:16 schrieb Christian Ebner: > For certain network setups the default values currently used to send > a wake on lan magic packet are not correct, e.g. it will get send via > the interface for which the default gateway is configured. > > This patches add optional configuration options to set a bind > interface, over which to send the WoL packet and/or set a broadcast > address to use. > > The functionality was tested by listening on all interfaces of the > sending host via `tcpdump -i any udp port 9`, and testing the > combinations of > > `pvenode config set -wakeonlan XX:XX:XX:XX:XX:XX,bind-interface=` > > and > > `pvenode config set -wakeonlan > XX:XX:XX:XX:XX:XX,broadcast-address=`. > > See also the thread in the community forum > https://forum.proxmox.com/threads/123459/ > > pve-manager: > > Christian Ebner (3): > node: config: make wakeonlan a property string > fix #5255: node: wol: add optional bind interface > fix #5255: node: wol: configurable broadcast address > > PVE/API2/Nodes.pm | 23 > PVE/NodeConfig.pm | 53 +-- > 2 files changed, 66 insertions(+), 10 deletions(-) > > pve-docs: > > Christian Ebner (1): > pvenode/wake-on-lan: mention optional config options > > pvenode.adoc | 14 ++ > 1 file changed, 14 insertions(+) > applied, thanks! I did some very minor follow-ups mostly to document the current default in the schema and docs and a small style fix (well not even really style wise, but rather making the part with assigning and checking the $bind_interface variable slightly shorter). While this is slightly niche it might still make sense to add this to the web UI too (WoL is exposed via Node -> Options) for completeness sake. ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH docs] installation: add serial console grub entry
Am 23/02/2024 um 13:06 schrieb Christoph Heiss: > This was added with the 8.1 release ISO, so mention it in the > documentation too. > > Signed-off-by: Christoph Heiss > --- > pve-installation.adoc | 7 +++ > 1 file changed, 7 insertions(+) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH docs v2] installation: reword `nomodeset` section a bit, add link to it
Am 26/02/2024 um 18:59 schrieb Christoph Heiss: > The `nomodeset` section needs some massaging due to the text flow being > broken a bit. While at it, link to it above at the 'Terminal UI' > bootloader tip such that readers can find it more easily. > > Suggested-by: Alexander Zeidler > Signed-off-by: Christoph Heiss > --- > Changes v1 -> v2: > * dropped superflous `the` > * use `xref` instead of `<< >>` for linking paragraph > > pve-installation.adoc | 11 ++- > 1 file changed, 6 insertions(+), 5 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 docs v2] installation: reword `nomodeset` section a bit, add link to it
Am 26/02/2024 um 18:59 schrieb Christoph Heiss:
> The `nomodeset` section needs some massaging due to the text flow being
> broken a bit. While at it, link to it above at the 'Terminal UI'
> bootloader tip such that readers can find it more easily.
>
> Suggested-by: Alexander Zeidler
> Signed-off-by: Christoph Heiss
> ---
> Changes v1 -> v2:
> * dropped superflous `the`
> * use `xref` instead of `<< >>` for linking paragraph
>
> pve-installation.adoc | 11 ++-
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/pve-installation.adoc b/pve-installation.adoc
> index 6b44fc0..016aa91 100644
> --- a/pve-installation.adoc
> +++ b/pve-installation.adoc
> @@ -88,7 +88,8 @@ Both modes use the same code base for the actual
> installation process to
> benefit from more than a decade of bug fixes and ensure feature parity.
>
> TIP: The 'Terminal UI' option can be used in case the graphical installer
> does
> -not work correctly, due to e.g. driver issues.
> +not work correctly, due to e.g. driver issues. See also
> +xref:nomodeset_kernel_param[adding the `nomodeset` kernel parameter].
>
> Advanced Options: Install {pve} (Graphical, Debug Mode)::
>
> @@ -300,14 +301,14 @@ following command:
> # zpool add log
>
>
> +[[nomodeset_kernel_param]]
> Adding the `nomodeset` Kernel Parameter
> ~
>
> Problems may arise on very old or very new hardware due to graphics drivers.
> If
> -the installation hangs during the boot. In that case, you can try adding the
> -`nomodeset` parameter. This prevents the Linux kernel from loading any
> -graphics drivers and forces it to continue using the BIOS/UEFI-provided
> -framebuffer.
> +the installation hangs during boot, you can try adding the `nomodeset`
> +parameter. This prevents the Linux kernel from loading any graphics drivers
> and
> +forces it to continue using the BIOS/UEFI-provided framebuffer.
>
> On the {pve} bootloader menu, navigate to 'Install {pve} (Terminal UI)' and
> press `e` to edit the entry. Using the arrow keys, navigate to the line
> starting
> --
> 2.43.0
>
>
>
> ___
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied-series: [PATCH docs] local-zfs: mention `zfs_arc_max` clamping by the installer
Am 05/03/2024 um 13:13 schrieb Christoph Heiss: > This was forgotten to be updated when it changed it the installer and > now reported in the forum [0] that the docs are a bit outdated in this > regard. > > [0] > https://forum.proxmox.com/threads/hat-proxmox-8-neue-zfs_arc_max-settings.142754/ > > Signed-off-by: Christoph Heiss > --- > local-zfs.adoc | 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
Re: [pve-devel] applied: [PATCH manager] ui: guest import: allow setting mac addresses to unique
Am 27/03/2024 um 11:32 schrieb Aaron Lauterer: > I did choose the 'labelWidth' and 'fieldLabel' to match the look and > feel of the 'Prepare for Virtio-SCSI" checkbox further up for > consistency though. > > Maybe we want to change that back? No, those are separate grids, it's fine to not have them align all components 100% as they're visually separate anyway. ___ 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: guest import: allow setting mac addresses to unique
Am 26/03/2024 um 14:25 schrieb Aaron Lauterer: > by adding a new checkbox and render the grid accordingly. > > If unique MAC addresses are enabled, set them to undefined when getting > the values from the grid. > > Signed-off-by: Aaron Lauterer > --- > not sure if setting `uniqueMac` directly on the component is okay. > Though we do that dynamically for the `vmConfig` as well. So probably > okayish. > > www/manager6/window/GuestImport.js | 39 +- > 1 file changed, 38 insertions(+), 1 deletion(-) > > applied, with a follow-up to rework this using the viewModel for tracking the state instead of doing that in a config option of the widget, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH storage v2] esxi: use mac address when static, generated and vpx
Am 26/03/2024 um 13:04 schrieb Aaron Lauterer: > static -> defined manually > generated -> by ESXi > vpx -> generated by vCenter > > Signed-off-by: Aaron Lauterer > --- > src/PVE/Storage/ESXiPlugin.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 storage] esxi: use mac address when static, generated and vpx
Am 26/03/2024 um 12:03 schrieb Aaron Lauterer:
> static -> defined manually
> generated -> by ESXi
> vpx -> generated by vCenter
nice!
> Signed-off-by: Aaron Lauterer
> ---
> src/PVE/Storage/ESXiPlugin.pm | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/PVE/Storage/ESXiPlugin.pm b/src/PVE/Storage/ESXiPlugin.pm
> index 77fb6c0..c5ddfea 100644
> --- a/src/PVE/Storage/ESXiPlugin.pm
> +++ b/src/PVE/Storage/ESXiPlugin.pm
> @@ -793,7 +793,10 @@ sub for_each_netdev {
>
> my $ty = $dev->{addressType};
> my $mac = $dev->{address};
> - if ($ty && fc($ty) eq fc('generated')) {
> + if ($ty && (fc($ty) eq fc('static')
> + || fc($ty) eq fc('generated')
> + || fc($ty) eq fc('vpx') # vCenter generated
To compare the same string to different allowed values in Perl we
generally prefer to either use a hash map with known OK values as keys,
or alternatively a regex, e.g.:
if (fc($ty) =~ /^(static|generated|vpx)$/) {
# ...
}
If there are only a ~ handful of different option, like here, this is
still easy to read but avoids some line bloat.
If you want you can send a v2, else I can also change this on applying?
> + )) {
> $mac = $dev->{generatedAddress} // $mac;
> }
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH cluster] datacenter config: mark notification settings as deprecated/unused
Am 19/01/2024 um 11:30 schrieb Lukas Wagner: > These were part of the first version of the notification overhaul > which was already rolled out in pvetest. To avoid breakage for users > who may have used the version from pvetest, we do not remove them yet > and only mark them as unused or deprecated. They can be removed at > some point in the future. > > Signed-off-by: Lukas Wagner > --- > src/PVE/DataCenterConfig.pm | 47 ++--- > 1 file changed, 12 insertions(+), 35 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] installation: update link to installing on top of Debian to bookworm version
Am 22/03/2024 um 12:11 schrieb Christoph Heiss: > Seems this just was forgotten, Buster is quite old at this point. > > Signed-off-by: Christoph Heiss > --- > pve-installation.adoc | 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 9/9] report: add microcode info to better assess possible system impacts
On 22/03/2024 14:59, Alexander Zeidler wrote:
> * list availability and installation status of `*microcode` packages
> * grep for applied "Early OS Microcode Updates"
> * grep for (un)patched CPU vulnerability messages
>
> Signed-off-by: Alexander Zeidler
> ---
> PVE/Report.pm | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/PVE/Report.pm b/PVE/Report.pm
> index fe497b43..18c554ec 100644
> --- a/PVE/Report.pm
> +++ b/PVE/Report.pm
> @@ -108,6 +108,8 @@ my $init_report_cmds = sub {
> 'dmidecode -t bios -q',
> 'dmidecode -t memory | grep -E
> "Capacity|Devices|Size|Manu|Part" | sed -Ez "s/\n\t(M|P)[^:]*: (\S*)/\t\2/g"
> | sort',
> 'lscpu',
> + 'apt list *microcode 2>/dev/null | column -tL',
> + 'dmesg | grep -i "microcode\|vuln"',
I'm wondering if instead of having a handful of dmesg + grep instances
it makes more sense to just add the whole dmesg output as separate
file.
I.e., I would like to have a cluster-wide report collection API that
spawns a task, calls to all nodes to generate a report, saves all of
those reports, including commands or files with very long output as
separate files, and then assembles an archive with all that.
On the long run that would provide nicer UX and also avoid that some
to strict filter hides information that might be relevant for a
specific setup.
I'd much more prefer work time spent on something like that than on
adding the same command a few times with each having some different,
rather a bit brittle looking, pipe chains..
> 'lspci -nnk',
> ],
> },
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager 6/9] report: switch `dmidecode` to quiet to omit almost never needed info
On 22/03/2024 14:59, Alexander Zeidler wrote:
> like on this system:
>
> # dmidecode -t bios
> # dmidecode 3.4
> Getting SMBIOS data from sysfs.
> SMBIOS 3.0.0 present.
>
> Handle 0x, DMI type 0, 24 bytes
>
> Handle 0x005C, DMI type 13, 22 bytes
The manual page here states that this option also hides other entries:
> Unknown, inactive and OEM-specific entries are not displayed. Meta-data and
> handle references are hidden.
So especially on newer HW this might contain some actual info beyond
the SMBIOS version/metadata that gets hidden.
I mean, I have no hard feelings here, but I'm wondering if it's really
worth hiding that info if it can include potential relevant stuff.
>
> Signed-off-by: Alexander Zeidler
> ---
> PVE/Report.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/PVE/Report.pm b/PVE/Report.pm
> index 505629c7..3a81bdb2 100644
> --- a/PVE/Report.pm
> +++ b/PVE/Report.pm
> @@ -104,7 +104,7 @@ my $init_report_cmds = sub {
> hardware => {
> order => 70,
> cmds => [
> - 'dmidecode -t bios',
> + 'dmidecode -t bios -q',
> 'lscpu',
> 'lspci -nnk',
> ],
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH manager 5/9] report: move `lscpu` & cluster info to more appropriate sections
And why are those more appropriate? Both fit's the general "always important"
section, so even though they fit the section you moved them too, they also
fit the general one, so some actual reasoning here would be good..
On 22/03/2024 14:59, Alexander Zeidler wrote:
> Signed-off-by: Alexander Zeidler
> ---
> PVE/Report.pm | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/PVE/Report.pm b/PVE/Report.pm
> index 2c2a5e12..505629c7 100644
> --- a/PVE/Report.pm
> +++ b/PVE/Report.pm
> @@ -41,8 +41,6 @@ my $init_report_cmds = sub {
> 'cat /etc/apt/sources.list',
> sub { dir2text('/etc/apt/sources.list.d/', '.+\.list') },
> sub { dir2text('/etc/apt/sources.list.d/', '.+\.sources') },
> - 'lscpu',
> - 'pvesh get /cluster/resources --type node --output-format=yaml',
> ],
> },
> 'system-load' => {
> @@ -96,6 +94,7 @@ my $init_report_cmds = sub {
> order => 60,
> cmds => [
> 'pvecm nodes',
> + 'pvesh get /cluster/resources --type node --output-format=yaml',
> 'pvecm status',
> 'cat /etc/pve/corosync.conf 2>/dev/null',
> 'ha-manager status',
> @@ -106,6 +105,7 @@ my $init_report_cmds = sub {
> order => 70,
> cmds => [
> 'dmidecode -t bios',
> + 'lscpu',
> 'lspci -nnk',
> ],
> },
___
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/9] report: add `jobs.cfg` to debug related network/load/backup/etc issues
On 22/03/2024 14:59, Alexander Zeidler wrote:
> Suggested-by: Friedrich Weber
> Signed-off-by: Alexander Zeidler
> ---
> PVE/Report.pm | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/PVE/Report.pm b/PVE/Report.pm
> index d4191769..6014f13e 100644
> --- a/PVE/Report.pm
> +++ b/PVE/Report.pm
> @@ -35,6 +35,7 @@ my $init_report_cmds = sub {
> 'pveversion --verbose',
> 'cat /etc/hosts',
> 'pvesubscription get',
> + 'cat /etc/pve/jobs.cfg',
This does not really fits the general section, and makes it harder to
see the core subscription and repository state. I'd either add it to the
cluster one, as cluster-wide jobs config, or to a new "Jobs" section,
with a slight preference to the latter.
> 'cat /etc/apt/sources.list',
> sub { dir2text('/etc/apt/sources.list.d/', '.+\.list') },
> sub { dir2text('/etc/apt/sources.list.d/', '.+\.sources') },
___
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/9] report: add list of upgradable packages
On 22/03/2024 14:59, Alexander Zeidler wrote:
> * to easily see if APT already knows about old packages in use and
> their exact version
> * to reconsider asking for applying updates as a first recommendation
> if the list is empty and no updates have been released very recently
>
> # apt list --upgradable ...
> Listing...
> pve-manager testing 8.1.6 amd64 [upgradable from: 8.1.4]
>
> Signed-off-by: Alexander Zeidler
> ---
> PVE/Report.pm | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/PVE/Report.pm b/PVE/Report.pm
> index 6014f13e..ad5c2aa0 100644
> --- a/PVE/Report.pm
> +++ b/PVE/Report.pm
> @@ -36,6 +36,7 @@ my $init_report_cmds = sub {
> 'cat /etc/hosts',
> 'pvesubscription get',
> 'cat /etc/pve/jobs.cfg',
> + 'apt list --upgradable 2>/dev/null | sed "s/\//\t/g" | column
> -tL',
this can easily add a few 100s KiB of text to the report on major upgrades,
this is just to verbose for an initial report.
I'd rather add just the count of available upgrades, i.e. lie `apt update`
does, which allows you to draw similar conclusions without bloating the
report to unwieldy sizes.
Maybe we could even add that info to the pveversion --verbose output,
as then it'd be already included here and also help on other channels
like the forum.
> 'cat /etc/apt/sources.list',
> sub { dir2text('/etc/apt/sources.list.d/', '.+\.list') },
> sub { dir2text('/etc/apt/sources.list.d/', '.+\.sources') },
___
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/9] report: add kernel command line including boot time
On 22/03/2024 14:59, Alexander Zeidler wrote:
> While using `/proc/cmdline` would already provide an initial info for
> debugging passthrough and similar, the use of `dmesg` is an easy way
> to get the boot date as an absolute value for free (additional to the
> relative value in `uptime` from `top`).
>
> Signed-off-by: Alexander Zeidler
> ---
> PVE/Report.pm | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/PVE/Report.pm b/PVE/Report.pm
> index 53ffdcbb..d4191769 100644
> --- a/PVE/Report.pm
> +++ b/PVE/Report.pm
> @@ -31,6 +31,7 @@ my $init_report_cmds = sub {
> cmds => [
> 'hostname',
> 'date -R',
> + 'dmesg -T | head | grep Command',
Meh, I'd rather get the command line explicitly and the last boots
using:
journalctl --list-boots
As here you get much more info, e.g. about (recent) frequent reboots.
Please remember to add this also to the PMG and PBS reports.
> 'pveversion --verbose',
> 'cat /etc/hosts',
> 'pvesubscription get',
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: Re: [PATCH cluster] ssh: default to 4096 bit keys when generating
On 21/12/2023 10:46, Fabian Grünbichler wrote: > Signed-off-by: Fabian Grünbichler > --- > src/PVE/Cluster/Setup.pm | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > applied, (but was a bit to quick with the push before adding Fiona's R-b & T-b, they are still much appreciated) thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH storage] zfs: fix duplicate word typo in error message
On 30/01/2024 10:11, Fiona Ebner wrote: > Signed-off-by: Fiona Ebner > --- > src/PVE/Storage/ZFSPoolPlugin.pm | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > applied (since a while already), thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH installer] build: run shellcheck as part of `test` step
On 15/03/2024 11:23, Christoph Heiss wrote: > Especially unconfigured.sh is worth checking consistently. > > Running shellcheck also does not really have any notable impact on build > time, so no downside there either. > > Signed-off-by: Christoph Heiss > --- > Makefile| 14 +- > debian/control | 1 + > unconfigured.sh | 3 +++ > xinitrc | 2 ++ > 4 files changed, 15 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 installer] unconfigured: move terminal size setting before starting debug shell
On 12/03/2024 12:59, Christoph Heiss wrote: > Otherwise, when using the serial debug shell, the console size will be > 0x0. This in turn breaks the TUI installer, as it cannot detect the size > properly. > > It also adjust the size to the proper 80x24 instead of 80x25, as > advertised in the log message. > > Signed-off-by: Christoph Heiss > --- > unconfigured.sh | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > applied, with the info and references you provided in your reply massaged into the commit message, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied-series: [PATCH docs v3 0/5] improve & unify installation(-related) documentation
On 21/03/2024 16:29, Christoph Heiss wrote: > This series in short tries to bring the documentation for the > ISO installation flow and anything related to it in line the with > respective documentation for PMG. As both products use the same > installer (minus small differences such as LVM options and BTRFS > support) and overall same basic system setup, it is worth unifying them. > > There is (of course) still room for improvement, esp. regarding the apt > repo/update section as pointed out by Alex. Still like to get this in > rather sooner than later, to not diverge even more. > > I'll send the appropriate pmg-docs series (see v1 of that [0]) when this > has been finalized & applied, to keep it transparent and easier to > reason about. > > [0] https://lists.proxmox.com/pipermail/pmg-devel/2024-March/002870.html > > Revision history > > v1: https://lists.proxmox.com/pipermail/pve-devel/2024-March/062056.html > v2: https://lists.proxmox.com/pipermail/pve-devel/2024-March/062159.html > > Notable changes v1 -> v2: > * dropped already-applied patches > * slight rewording in some places based on suggestions > > Notable changes v2 -> v3: > * fixed small typo > > Christoph Heiss (5): > package-repos: improve wording partly based on pmg-docs > installation: iso: improve & align wording with pmg-docs > installation: lvm-options: improve & align wording with pmg-docs > installation: zfs-options: improve & align wording with pmg-docs > installation: iso: reflow location and password dialog screenshots > > pve-installation.adoc | 144 ++--- > pve-package-repos.adoc | 26 > 2 files changed, 107 insertions(+), 63 deletions(-) > applied series, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
[pve-devel] applied: [PATCH storage v2] esxi: detect correct os type in 'other' family
On 21/03/2024 10:07, Gabriel Goller wrote: > This patch introduces the conversion table for all possible OS Types > that are in the VMWare 'other' family and sets the pve counterpart. > Our default OS Type is 'linux', so including mappings to 'other' makes > sense. > > Signed-off-by: Gabriel Goller > --- > > v2, thanks @sterzy: > - removed perltidy output > > src/PVE/Storage/ESXiPlugin.pm | 43 +++ > 1 file changed, 39 insertions(+), 4 deletions(-) > > applied, thanks! ___ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
