pvecm: updatecerts: check and report if we got a timeout
If timeout is reached, run_fork_with_timeout will warn "got timeout".
When processing triggers for pve-manager (because of ExecStartPre of
pveproxy.service invoking pvecm updatecerts) that warning can appear
in the apt output without any context (output of the forked sub
doesn't appear there). So give it some actual content using the
$got_timeout returned in wantarray list context of the
run_fork_with_timeout method.
Note that this could be seen as clash with the silent flag, but IMO
that is a bit misnamed, we mostly want to avoid outputting the info
messages from updatecerts_and_ssh, and that silent (i.e., quite) gets
convoluted with a no-error behavior seems also odd.
TBH., I actually thought about printing the "waiting for quorum" at
least once for the silent case too, but for now omitted it – lets
wait on actual feedback and then maybe restructure this completely.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Fiona Ebner [Fri, 30 Jun 2023 11:59:42 +0000 (13:59 +0200)]
pvecm: updatecerts: wait for quorum
Mostly useful for the updatecerts call triggered via the ExecStartPre
hook for pveproxy.service.
When starting a node that's part of a cluster, there is a time window
between the start of pve-cluster.service and when quorum is reached
(from the node's perspective). pveproxy.service is ordered after
pve-cluster.service, but that does not prevent the ExecStartPre hook
from being executed before the node is part of the quorate partition.
The pvecm updatecerts command won't do much without quorum. Generating
local (non-pmxcfs) files is still done before waiting on quorum.
In particular, it might happen that the base directories for observed
files will not get created during/after the upgrade from Proxmox VE 7
to 8 (reported in the community forum [0] and reproduced right away in
a virtual test cluster).
Waiting on quorum should highly increase the chances for successful
execution of the ExecStartPre hook.
[0]: https://forum.proxmox.com/threads/129644/
Suggested-by: Thomas Lamprecht <t.lamprecht@proxmox.com> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Fri, 30 Jun 2023 09:29:18 +0000 (11:29 +0200)]
pmxcfs: check all addresses from getaddrinfo to find non-loopback one
Make the resolution of the node name to a local non-loopback address
through getaddrinfo more flexible.
Don't just ask for one, but for all addresses, then loop through the
returned linked list until the first non-loopback one is found.
It seems that some cloud-init templates hosting providers, like e.g.
OVH use, are misconfigured in always adding 127.0.1.1 for the
hostname to /etc/hosts, even if another mapping for the hostname
exists already. This popped up in our community forum with the
upgrade to bookworm, so might be related to the cloud-init version
from Bookworm.
Some examples pointed out by Alexandre:
https://forum.proxmox.com/threads/129678/
https://forum.proxmox.com/threads/129669/#post-568172
https://forum.proxmox.com/threads/129398/#post-568290
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Dominik Csapak [Thu, 25 May 2023 10:17:43 +0000 (12:17 +0200)]
add cfg files for resource mapping
There might be more mapping types in the future, but for now it's:
- mapping/pci.cfg
- mapping/usb.cfg
Add them to both, the perl side (PVE/Cluster.pm) and pmxcfs side
(status.c).
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[ TL: Dominik, Fabian and I decided to do a s/resource/mapping/
rename on the cfg & API paths and privileges, so ammend that ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Fri, 19 May 2023 15:29:08 +0000 (17:29 +0200)]
pmxcfs: migrate to g_memdup2
As the previously used g_memdup is deprecated since GLib 2.68, due to
being prone to integer overflows when converting the argument from a
gsize to a guint, while that only produces a warning we compile with
Werror, so this FTBFS.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Friedrich Weber [Thu, 9 Mar 2023 12:30:19 +0000 (13:30 +0100)]
pvecm add: require user to navigate out of /etc/pve
If `pvecm add` is issued from /etc/pve (or any subdirectory), it
prints some errors:
[...]
shell-init: error retrieving current directory: getcwd: cannot
access parent directories: Transport endpoint is not connected
[...]
successfully added node 'pve-c2' to cluster.
cannot fetch initial working directory: Transport endpoint is not
connected at /usr/share/perl5/PVE/CLI/pvecm.pm line 446.
The reason is that `pvecm add` restarts pmxcfs, which re-mounts the
fuse mount at /etc/pve, invalidating pvecm's working directory.
The error messages give the impression that something went wrong.
Indeed, the second error indicates the temporary directory is not
cleaned up. The cluster join itself actually works, though.
The issue could be fixed by chdir'ing to / in `pvecm add`. However,
the user's shell would still remain in the now-invalid /etc/pve,
potentially leading to confusing "transport endpoint not connected"
messages in future interactions.
To avoid this, require the user to chdir out of /etc/pve before
running `pvecm add`.
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
Dominik Csapak [Wed, 16 Nov 2022 15:47:58 +0000 (16:47 +0100)]
datacenter config: add options to control tag usage rights
By adding a 'user-tag-privileges' and 'registered-tags' option.
The former sets the general policy by which "normal" users (with just
'VM.Config.Options' on the respective guest) can set or delete tags
on a guest, and the latter is a list of registered tags only settable
by users with admin-like permission ('Sys.Modify' on '/'); it is
designed to be used as source/target for actions like backup jobs in
the future.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
[T: drop outdated stuff & reword/work commit message] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Dominik Csapak [Wed, 16 Nov 2022 15:47:56 +0000 (16:47 +0100)]
cluster: add get_guest_config_properties
Akin to the singular get_guest_config_property, but with the
possibility to query a list of properties.
Uses the CFS_IPC_GET_GUEST_CONFIG_PROPERTIES introduced in the
previous patch.
Note that the same details apply w.r.t. parsing and permissions as
the singular variant, iow. one needs to take caution and filter
allowed guests views on call site when using this.
Dominik Csapak [Wed, 16 Nov 2022 15:47:55 +0000 (16:47 +0100)]
pmxcfs: add IPC call to get multiple guest config properties at once
Previously we used the existing IPC call just for getting the `lock`
property of virtual guests in the cluster resource API call, but for
the tag display we'll to get another one and calling this twice seems
rather non-ideal.
Thus introduce a successor: CFS_IPC_GET_GUEST_CONFIG_PROPERTIES
It allows one to get multiple properties from a single, or all
virtual guest in-memory configs in one go. Keep the existing IPC call
as is for backward compatibility and add this as separate, new one.
The new IPC command basically behaves the same as the previous
CFS_IPC_GET_GUEST_CONFIG_PROPERTY, but takes a list of properties
instead and returns multiple properties per guest.
The existing perl wrapper around the IPC call to get a single
property was switched over to use the new IPC call too, so we'll
be able to drop the old IPC command with the next major release if
nothing comes up.
= Benchmark =
== Setup ==
Proxmox VE in a VM with CPU type host (Intel 12700k) and 4 cores
10000 typical configs with both 'lock' and 'tags' set at the end, and
fairly long tags ('test-tag1,test-tag2,test-tag3') (normal VM with a
snapshot, ~ 1 KiB)
== Test ==
Average of 100 runs each with time in milliseconds
== Previous Results ==
num props total time time per iteration
1 1054.2 10.2
== Results with this Patch ==
num props total time time per iter function
2 1332.2 13.2 get_properties
1 1051.2 10.2 get_properties
2 2082.2 20.2 get_property, 2 separate calls
1 1034.2 10.2 get_property
So, a call with the new code for one property is the same as with the
old code, and adding a second property only adds a bit of additional
time (in this case ~30%).
if processing a corosync.conf update is delayed on a single node,
reloading the config too early can have disastrous results (loss of
token and HA fence). artifically delay the reload command by one second
to allow update propagation in most scenarios until a proper solution
(e.g., using broadcasting/querying of locally deployed config versions)
has been developed and fully tested.
reported on the forum:
https://forum.proxmox.com/threads/expanding-cluster-reboots-all-vms.110903/
reported issue can be reproduced by deploying a patched pmxcfs on
non-reloading node that sleeps before writing out a broadcasted
corosync.conf update and adding a node to the cluster, leading to the
following sequence of events:
- corosync config reload command received
- corosync config update written out
which causes that particular node to have a different view of cluster
topology, causing all corosync communication to fail for all nodes until
corosync on the affected node is restarted (the on-disk config is
correct after all, just not in effect).
Stefan Sterz [Wed, 11 May 2022 09:27:07 +0000 (11:27 +0200)]
cluster config: mark qdevice end point as protected
The qdevice status endpoint retrieves some required info via writing
a status command to the qdevice socket
(/var/run/corosync-qdevice/corosync-qdevice.sock) and then parsing
the response.
Since the socket has 0755 permissions and is owned by root it can not
be written by the unprivileged pveproxy that runs as www-data user.
Relay the endpoint to the privileged pvedaemon by marking it as
`protected` to allow the end point to retrieve data from the qdevice
properly.
Fixes an issue where the api would return an empty object.
Signed-off-by: Stefan Sterz <s.sterz@proxmox.com> Tested-by: Oguz Bektas <o.bektas@proxmox.com>
[ T: extend commit message ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Stefan Sterz [Thu, 24 Feb 2022 14:21:48 +0000 (15:21 +0100)]
parse datacenter config: remove "\s*" from comment regex
To be consistent with PBS's implementation of multi-line comments
remove "\s*" here too. Since the regex isn't lazy .* matches
everything \s* would anyway. (Note that new lines occurs after "$").
struct clog_base {
uint32_t size; // total size of this clog_base
uint32_t cpos; // index into data, starts counting at start of clog_base, initially 0
char data[];
};
an entry consists of indices of the next and previous entries and
various fields (fixed-length ones omitted here):
typedef struct {
uint32_t prev; // index of previous entry, or 0 if none exists
uint32_t next; // index of next entry
[..] // fixed-length fields
uint8_t node_len;
uint8_t ident_len;
uint8_t tag_len;
uint32_t msg_len;
char data[]; // node+ident+tag+msg - variable-length fields
} clog_entry_t;
the next and prev indices are calculated when allocating a new entry,
and the position of the current entry 'cpos' is updated accordingly
(clog_alloc_entry):
- size of the entry is padded with up to 7 bytes
- first entry goes to index 8
- second and subsequent entries go to the current entry's 'next' index
- if the current entry's 'next' index is out of bonds, the first entry
is overwritten => wrap-around
- the 'prev' index of the new entry is set to cpos
- cpos is set to the index of the new entry
- the 'next' index of the new entry is set to its index+padded size
when iterating over the entries, the following bounds are used to follow
the 'prev' links starting at the current entry:
while this handles a not-yet-wrapped around ring buffer (cpos would be 0
when reaching the first entry), and tries to handle wrap-arounds by
terminating when reaching a 'red-zone' of 'CLOG_MAX_ENTRY_SIZE' starting
at the current entry (this covers the current entry which was already
visited as first entry during the iteration, and the next entry after it
which might have been overwritten) - but it's possible that entries line
up so that the wrap-around 'prev' index of the first entry points to a
location *before* the current entry.
for example, looking at clog_base with S being the size field, C being
the cpos field, followed by the actual data. N/P are the next/prev
indices of the entry at C, Q denotes the 'prev' index of the first entry
in the data array, and 'R' the red zone used for the loop check in case
of wrap-around.
first, fill up the buffer with six large entries:
Q P C N
| | | |
| | | |
v v v v
+-+-+------+------+------+------+------+------+-+
| | | | | | | | |x|
| | | 1 | 2 | 3 | 4 | 5 | 6 |x|
| | | | | | | | |x|
+-+-+------+------+------+------+------+------+-+
S C RRRRRRRRRRR
iterating from C backwards ends up at Q being 0, terminating the loop
without a wrap-around after having visit 6->1
now the next (in this example, smaller) entry that gets allocated/insert
needs to wrap around, because the empty space at the end (denoted by
XXX) is too small:
C N QP
| | ||
| | ||
v v vv
+-+-+------+------+------+------+------+------+-+
| | | | | | | | |x|
| | | 7 | 2 | 3 | 4 | 5 | 6 |x|
| | | | | | | | |x|
+-+-+------+------+------+------+------+------+-+
S C RRRRRRRRRRR
iterating backwards from C terminates the loop when reaching the red
zone, with the (second) entry no longer being considered since it partly
overlaps it. only 7->3 are visited.
adding more entries we end up with the following layout:
P QC N
| || |
| || |
v vv v
+-+-+------+---+---+---+---+---+---+---+---+--+-+
| | | | | | | | | | | |##|x|
| | | 7 | 8 | 9 |10 |11 |12 |13 |14 |15 |#6|x|
| | | | | | | | | | | |##|x|
+-+-+------+---+---+---+---+---+---+---+---+--+-+
S C RRRRRRRRRRR
with # denoting space previously occupied the last large entry (#6)
which is still unmodified (the rest of that entry's data has been
overwritten by entries #14 and #15).
iterating from C (to the left/P) the loop ends up at entry #7, follows
the link to Q (which satisfies the loop bounds as Q < C), and the data
starting at (invalid index) Q gets interpreted as an entry. it is
possible (though even more unlikely than the partial overwrite case)
that Q and C line up perfectly, which would cause the loop to become an
infinite loop. the loop *should* terminate after having visited 15-7,
without wrapping around.
note that the actual sizes of the entries are not relevant, the
requirements are:
- entry before last wrap-around must be big enough that entry of current
index can overtake it without another wrap-around
- method that does iteration must be called before next wrap-around
the fix is obviously trivial once the issue became apparent - when
wrapping around during iteration, additionally check that we are not
jumping across the red zone into already invalidated parts of data.
clusterlog_merge is technically not affected since it aborts before a
wrap-around anyway, but it doesn't hurt to have the checks consistently
in case this ever changes.
thanks to @kev1904 on our community forums for reporting and providing the data
to nail the cause down fast!
the recommended way is to first shutdown, then delnode, and never let it
come back online, in which case corosync-cfgtool won't be able to kill
the removed (offline) node.
also, the order was wrong - if we first update corosync.conf to remove
the node entry from the nodelist, corosync doesn't know about the nodeid
anymore, so killing will fail even if the node is still online.
currently, when veth or tap interfaces are plugged to bridge,
an igmp v3 report is broadcasted to the network, with the
bridge mac adddress.
Users have reported problems with hetzner for example, blocking the server
because of the unknown mac flooding the network.
https://forum.proxmox.com/threads/proxmox-claiming-mac-address.52601/page-6#post-421676
some traces:
ip addr:
190: fwbr109i0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
link/ether 22:5f:0b:cb:ac:42 brd ff:ff:ff:ff:ff:ff
ebtable log:
Oct 6 09:46:24 kvmformation3 kernel: [437256.753355] MAC-FLOOD-F IN=fwpr109p0 OUT=eno1 MAC source = 22:5f:0b:cb:ac:42 MAC dest = 01:00:5e:00:00:16 proto = 0x0800 IP SRC=0.0.0.0 IP DST=224.0.0.22, IP tos=0xC0, IP proto=2
Stoiko Ivanov [Thu, 11 Nov 2021 15:42:09 +0000 (16:42 +0100)]
sysctl snippet: move to /usr/lib and prefix with 10-
following best-practices according to `sysctl.d(5)`:
* Packages should install their configuration files in /usr/lib/ ...
* It is recommended to prefix all filenames with a two-digit number
and a dash ...
the conffile removal is inspired by how it was done in `procps` (one
of the few packages in the debian repository, which did this
transition) and by following `dpkg-maintscript-helper(1)` and
`deb-conffiles(5)` (the former recommending the latter)
The choice of 10- as prefix is due to pve-container shipping its
snippet with that prefix already. other packages use higher numbers
(e.g. systemd - 50-)
Tested on 2 VMs (one with modifications, the other without) - worked
as advertised (the modified file was kept as
/etc/sysctl.d/pve.conf.dpkg-old and the upgrade notified me of the
change)
projects / pve-cluster.git / log