vzdump: have property string helpers always return the result
Previously, the result would only be returned implicitly and if not
already parsed. While callers do not strictly need the return value,
future callers might mistakenly rely on it and even work by chance in
some scenarios, because of the implicit return. Make the code more
future proof by explicitly returning the result in all cases.
There was an oversight with recent replication fixes that led to
attempting to remove snapshots that do not exist (in more scenarios).
While not an issue with real consequences, it's confusing to users.
This has since been fixed by pve-guest-common commit "replication:
snapshot cleanup: only attempt to remove snapshots that exist".
ui: acme: add External Account Binding (EAB) related fields
Adds fields for eab credentials. By default eab is optional, but if the
directory should report that eab is required, the eab credential fields
are marked as mandatory and prevent the form from being submittable
until credentials are provided.
Signed-off-by: Folke Gleumes <f.gleumes@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This patch allows the user to set a custom ACME directory by providing
a 'Custom' option in the directory dropdown. This in turn reveals an
input for the url. When using a custom directory the directory has to
be manually queried via button press to prevent from spamming the
directory on every input.
Signed-off-by: Folke Gleumes <f.gleumes@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
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)
Thomas Lamprecht [Wed, 17 Apr 2024 12:22:41 +0000 (14:22 +0200)]
ui: backup job: avoid calling max-workers VM workers
that could make some users (not reading the explanation on the right
closely) belief that this controls the amount of parallel VMs to be
backed up or the like.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
close #4513: ui: backup job: add tab for advanced options
pigz is not exposed, because it only works after manually installing
the pigz package.
ionice is not exposed, because it only works in combination with the
BFQ scheduler and even then not in all cases (only affects the
compressor when doing snapshot/suspend mode backup of a VM).
The pbs-entries-max performance option is not exposed. It is rather
niche and hard to understand. It serves as an escape hatch for
rare/extreme cases.
These can still be added with appropriate notes if there is enough
user demand.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
vzdump: use per-property fallback for performance settings
Currently, fallback for the 'performance' option is done as a whole,
taking away flexibility from the user. It also means that when only
one of the two sub-properties is specified, the other one will default
to the backend (i.e. QEMU or proxmox-backup-client) default rather
than the schema default. For the latter point in particular, it can be
argued to be incorrect. These limitations will only get worse in the
future with more sub-properties.
Switch to a per-property fallback mechanism to improve the situation,
having each go through the usual preference order (CLI/job > node-wide
default > schema default).
Technically, this is a breaking change, but pbs-entries-max is rather
new and potential for breakage seems rather low. Requirements for
breakage:
* job (or CLI) that defines only one of the performance options
* job also covers a guest where the other performance option applies
* the other performance option is defined in the node-wide configuration
* the node-wide setting is worse for the job than the implicit backend
default (because this change will have the node-wide default win over
the implicit backend default).
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
vzdump: actually honor schema defaults for performance
The 'performance' option itself defines no 'default' in the schema, so
what happened is that the defaults used by the backends (i.e. QEMU and
proxmox-backup-client) would be used. Luckily, they correspond to the
default values defined in the schema, i.e. in the 'backup-performance'
format. Make the code future-proof and use the actual defaults defined
in the schema instead of relying on that correspondence.
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Wed, 17 Apr 2024 09:33:55 +0000 (11:33 +0200)]
ui: lxc: keep passthrough ID internal for now
this is not like mount points, where the order can make a difference,
but rather like the PCI passthrough for VMs, for which we do not
expose editing the ID either.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Stefan Hanreich [Tue, 16 Jan 2024 14:30:22 +0000 (15:30 +0100)]
firewall: properly detect changes when ip / cidr is used in rule
With the current implementation using queryDelay, this means that the
change event for the input never completes. This in turn leads to
the input panel never changing its dirty status. By using the
beforequery event we can simply cancel the query without resorting to
the queryDelay hack.
Reported-By: Mira Limbeck <m.limbeck@proxmox.com> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com> Tested-by: Mira Limbeck <m.limbeck@proxmox.com> Reviewed-by: Mira Limbeck <m.limbeck@proxmox.com>
fall back to using v.ref as value when we do not have an alias or ipset
since scope and name are not set for ips / cidrs
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com> Tested-by: Filip Schauer <f.schauer@proxmox.com> Reviewed-by: Mira Limbeck <m.limbeck@proxmox.com> Tested-by: Mira Limbeck <m.limbeck@proxmox.com>
and guard it to only run on ceph-using systems (the regular 'inited' check
doesn't work as a guard for this, because it checks for new-style inits
including the dir existing).
Max Carrara [Tue, 2 Apr 2024 14:55:22 +0000 (16:55 +0200)]
fix #4759: ceph: configure ceph-crash.service and its key
Due to Ceph dropping privileges when running the 'ceph-crash' daemon
[0], it is necessary to allow the daemon to authenticate with its
cluster in a safe manner.
In order to avoid exposing sensitive keyrings or somehow escalating
its privileges again, 'ceph-crash' is therefore provided with its own
keyring in the '/etc/pve/ceph' directory. This directory, due to being
on 'pmxcfs', may be read by members of the 'www-data' group, which
'ceph-crash' is made part of [1].
Expected Configuration
----------------------
1. A keyring file named '/etc/pve/ceph/ceph.client.crash.keyring'
exists
2. A section named 'client.crash' exists in '/etc/pve/ceph.conf'
3. The 'client.crash' section has a key named 'keyring' which
references the keyring file as '/etc/pve/ceph/$cluster.$name.keyring'
4. The 'client.crash' section has *no* key named 'key'
New Clusters
------------
The keyring file is created and the conf file is updated after the first
monitor has been created (when calling `pveceph mon create`).
Existing Clusters
-----------------
A new helper script creates and configures the 'client.crash' keyring in
`postinst`, if:
* Ceph is installed
* Ceph is initialized ('/etc/pve/ceph.conf' and '/etc/pve/ceph' exist)
* Connection to RADOS is successful
If the above conditions are met, the helper script ensures that the
existing configuration matches the expected configuration mentioned
above.
The configuration is not changed if it is already as expected.
The helper script may be called again manually if the `postinst` hook
fails. It is installed to '/usr/share/pve-manager/helpers/pve-init-ceph-crash'.
Max Carrara [Tue, 2 Apr 2024 14:55:21 +0000 (16:55 +0200)]
ceph: introduce '/etc/pve/ceph'
This commit adds the '/etc/pve/ceph' directory to our overall expected
Ceph configuration.
This directory is meant to store cluster-wide, non-private
configuration files used by Ceph applications and services that are
executed with lower privileges, such as 'ceph-crash.service'.
The existence of the directory is now also checked for when checking
whether Ceph is configured correctly. This makes it easier for our
other tooling to rely on the directory's existence, reducing the
number of otherwise needless frequent checking.
* For new clusters: `pveceph init` now creates '/etc/pve/ceph' when
called.
* For existing clusters: The 'postinst' hook this commit adds ensures
that '/etc/pve/ceph' is created when updating.
Signed-off-by: Max Carrara <m.carrara@proxmox.com> Tested-by: Friedrich Weber <f.weber@proxmox.com>
Thomas Lamprecht [Thu, 28 Mar 2024 17:06:34 +0000 (18:06 +0100)]
fix #5337: ui: parse a port in the server field
Based on how we handle this for PBS but without the hidden fields,
which do not really make this simpler, at least not if the logic is
only required on creation due to the field being only editable then.
As example, if the port of the ESXi host would be 8080, one would
enter `192.168.1.2:8080` for an IPv4 address or `[2001:db8::42]:8443`
for an IPv6 one.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Christian Ebner [Tue, 26 Mar 2024 09:16:56 +0000 (10:16 +0100)]
node: config: make wakeonlan a property string
Moves the wakeonlan property to be a property string, with current mac
address as default key. This allows to later add further optional
properties such as bind-interface and broadcast-address.
Adds the `get_wakeonlan_config` helper function to parse the string
when read from the node config.
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
Thomas Lamprecht [Thu, 28 Mar 2024 16:25:31 +0000 (17:25 +0100)]
ui: guest import: separate disk mapping and rendering cleanly
handle mapping purely in the change handler for the prepare-for-virtio
checkbox and avoid any such mapping logic in the renderer, while
slightly longer the code is much easier to reason about.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Thu, 28 Mar 2024 16:09:21 +0000 (17:09 +0100)]
ui: guest import: actually copy vmconfig state before applying values
Otherwise changes will persist, e.g., disabling a CDROM drive won't
work if the values got assembled for the "Resulting Config" tab
before. Same for the "Prepare for VirtIO" checkbox, there one got the
disk twice, once as (old) mapped SATA and once as (original) SCSI.
To fix this do not work on a reference of the original VM config, but
rather always copy that to a new object before applying the form
values.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Wed, 27 Mar 2024 10:45:31 +0000 (11:45 +0100)]
ui: guest import: use box-label style for prepare-for-virtio checkbox
Aaron originally designed the new "unique MAC address" checkbox after
this one for alignment reasons [0]. I changed it and argued that's
fine if there is a misalignment between those components [1], which I
still think, but tbh. I also like the boxLabel more for the "Prepare
for VirtIO" checkbox, so lets use it there too and hope all are happy
again.
Thomas Lamprecht [Wed, 27 Mar 2024 10:40:21 +0000 (11:40 +0100)]
ui: guest import: fix internal name for the prepare-for-virtIO feature
Mapping to SATA is only one thing it does, and what we actually want
to do is preparing for VirtIO, the effects are implementation detail,
so name it after the reason not one specific step.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Wed, 27 Mar 2024 09:33:41 +0000 (10:33 +0100)]
ui: guest import: use view-model for unique-mac-addr state
Avoid some global widget config to store internal state, rather use
the existing viewModel for that.
Also avoid unnecessary references and switch to a boxLabel, drop also
the tooltip, as that doesn't add much here where the effect can be
seen in the grid.
Avoid reconfiguring all grids if only the network one can have changed
due to a change of this check box value.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Max Carrara [Thu, 14 Mar 2024 10:19:30 +0000 (11:19 +0100)]
api: ceph monitor: rework some code style to modern
Signed-off-by: Max Carrara <m.carrara@proxmox.com>
[ TL: also improve if-expression wrapping ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Max Carrara [Thu, 14 Mar 2024 10:19:29 +0000 (11:19 +0100)]
fix #5198: ceph: mon: fix mon existence check in mon removal assertion
The Ceph monitor removal assertion contains a condition that checks
whether the given mon ID actually exists and thus may be removed.
The first part of the condition checks whether the hash returned by
`get_services_info` [0] contains the key "mon.$monid". However, the
hash's keys are never prefixed with "mon.", which makes this check
incorrect.
This is fixed by just using "$monid" directly.
The second part checks whether the mon hashes returned by
Ceph contain the "name" key before comparing the key with the given
mon ID. This key existence check is also incorrect; in particular:
* If the lookup `$_->{name}` evaluates to e.g. "foo", the check
passes, because "foo" is truthy. [1]
* If the lookup `$_->{name}` evaluates to "0", the check fails,
because "0" is falsy (due to it being equivalent to the number 0,
according to Perl [1]).
This is solved by using the inbuilt `defined()` instead of relying on
Perl's definition of truthiness.
ui: user: password change: require a confirmation-password
To hedge against a scenario where an attacker has local or even
physical access to a computer where a user is logged in.
While that general scenario cannot neither get detected nor really
secured against, at least not without requiring re-authentication on
every API call that can have side-effect (i.e., all but GET method),
it still makes sense to ensure that credentials cannot be modified,
which would allow denial of service.
See the related pve-access-control commit 5bcf553 ("user: password
change: require confirmation-password parameter")
Reported-by: Wouter Arts <security@wth-security.nl> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Thomas Lamprecht [Thu, 21 Mar 2024 09:40:35 +0000 (10:40 +0100)]
ui: parse VM network: support floats without trailing zero
While on simple cases the frontend will translate a `.5` to `0.5` the
backend really doesn't care and takes either.
And it seems that editing from an exiting `0.5` to `.75` will often
cause this to submitted as is.
Independent of how such a value comes into the config, it broke parsing
the network rate property in the UI, where we assumed that there's
always a digit on the left side of the floating point separator.
Simply extend the regex to allow parsing those floats that directly
start with a separator too.
Dominik Csapak [Tue, 19 Mar 2024 13:00:32 +0000 (14:00 +0100)]
ui: guest import: add warning for losing efi state
and add a link to recreate the boot entries in ovmf
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[ TL: break up String.format to avoid overly long line ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Dominik Csapak [Tue, 19 Mar 2024 13:00:30 +0000 (14:00 +0100)]
ui: guest import: auto activate virtio preparation for win + ovmf
it seems on esxi, most windows vms with uefi are automatically
configured with an lsi scsi controller, which we can't currently support
(ovmf driver issue) so automatically activate the sata mapping + virtio
preparation in this case
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Dominik Csapak [Tue, 19 Mar 2024 13:00:29 +0000 (14:00 +0100)]
ui: guest import: fix isWindows check
while most of our 'windiows' ostypes start with 'win' not all of them do
(wxp, wvista), so just shorten the condition to 'starts with `w`', this
covers all our windows ostypes, while not including others.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Wed, 13 Mar 2024 13:52:31 +0000 (14:52 +0100)]
ui: guest import: avoid view-model race on setting os type
Use the default value of the field also as start value in the
viewModel, as other wise the two-way binding can cause the empty
string to be set to the field's value, not vice-versa.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Wed, 13 Mar 2024 12:01:45 +0000 (13:01 +0100)]
ui: import: fix top-padding of CD-ROM label
While it was required before commit ca9dce9e8 ("ui: guest import:
rework windows virtio-scsi preparation") to avoid making this crowded,
after that commit we already got the top-padding from the columnT
here, so no need for adding extra padding to that.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Wed, 13 Mar 2024 11:54:06 +0000 (12:54 +0100)]
ui: import: increase CPU limits to better match ESXi
On ESXi one can have at max 768 vCPUs and that over maximal 128
sockets. Further, by default ESXi will favor adding single-cpu
sockets, so if one just enters "give me 8 vCPUs", it will be 8
single-core sockets.
As we only supported up to 4 sockets in the web UI, which caused this
field to show up as invalid on initial load for such VMs, which is not
nice.
Having that many sockets is rather odd, so for us it doesn't make
sense to allow more than the ESXi limit that, but having more vCPUs
might make sense, so allow up to 1024 for that for now.
Note that QEMU and our API backend doesn't really cares, the kernels
NR_MAX_CPU is the limit there. Albeit it is questionable if using more
vCPU threads as there are host threads has some sense in any setup.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Dominik Csapak [Wed, 13 Mar 2024 08:35:57 +0000 (09:35 +0100)]
ui: guest import: rework windows virtio-scsi preparation
instead of having a separate iso selector that shows with an enabled
checkbox, just add a CDROM drive when windows is selected and there is
no such drive available yet.
The idea here is that the VM's OS is already fully set up, so a single
CDROM drive is enough to be used for installing VirtIO drivers, unlike
the VM create case, where the first one is already used for the
installation medium.
Also, rename the 'map to sata' checkbox to 'prepare for virtio-scsi'
that also changes the scsi controller to virtio-scsi-single
Additionally, change the positioning of the checkbox/scsihw selector
to be below the disk grid
With that we then only disable prepare-for-virtio checkbox for
non-windows OS types, as the scsi controller on the right looks like
it hangs in the air without any field on the left otherwise.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[ TL: rework commit message, squash in some fixes ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
projects / pve-manager.git / log