Replace `foreach` with `for` and use postfix deref instead of block
(circumfix) dereference (`$foo->%*` instead of `%$foo`).
Furthermore, make `format_config_line` a private sub instead of
unnecessarily declaring it as an anonymous subroutine, which avoids
the `&$sub_ref(...)` syntax altogether.
Signed-off-by: Max Carrara <m.carrara@proxmox.com>
We can just save both $! and %! and use the latter to check for
specific errors. This is not really pretty but perl does the same
internally, so...
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
when starting a vm with passthrough, we have to bind all normal pci
devices to vfio-pci. This happens by
* unbinding from current driver
* telling vfio-pci the 'vendorid modelid' combo so it knows this device
class can use the driver (by writing to 'new_id')
* actually binding the device to vfio-pci
if there are multiple devices of the same 'vendorid modelid' class on
the host (and passed through), only the first write to 'new_id' is
successful, all subsequent ones return EEXIST.
This could happen e.g. for setups with multiple GPUs that have the same
audio chip.
To fix this, ignore the EEXIST error for this write to new_id, by adding
a new parameter to file_write for this.
If we need to ignore other errors in the future, we can still make this
more generic.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
the actual error and path is useful to know when trying to debug or
figure out what did not work, so warn here if there was an error.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Commit e68ebda ("fix #545: interfaces: allow arbitrary bridge names in
network config") introduced a cyclic usage between
PVE::RESTEnvironment and PVE::INotify, making code like the following
fail:
> perl -e "use PVE::RESTEnvironment qw(log_warn);"
Note, including the PVE::INotify module first would still work, i.e.:
> perl -e "use PVE::INotify; use PVE::RESTEnvironment qw(log_warn);"
The rest of the PVE::INotify module alredy uses syslog(), which could
be used here as well to get rid of the cyclic usage. Wolfgang argued
that the whole point of commit e68ebda was to remove coupling between
the name and the type of the interface. If there still is some code
about a name starting with 'vmbr' being classified wrong, that should
rather be fixed. Because of the very commit, the frontend already
doesn't show e.g. a non-bridge with name 'vmbr7' in bridge selectors.
Suggested-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Fixes: e68ebda ("fix #545: interfaces: allow arbitrary bridge names in network config")
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
This can be useful to have, e.g., when requiring different behaviors
the nearer an expiry gets.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
When configuring an OVS network device via web interface,
any OVS option set to value=0 is ignored upon saving. This happens
because value=0 is evaluated as false in $parse_ovs_option.
Signed-off-by: Tiomet Pelston <tiometpelston@gmail.com>
Reviewed-By: Aaron Lauterer <a.lauterer@proxmox.com>
Tested-By: Aaron Lauterer <a.lauterer@proxmox.com>
they should not be expensive (only reading/file checking in sysfs; the
parsed vendor/id names are not required) so we should include them
always.
We need at least the mdev part later at a point where we're not
interested in the rest of the verbose mode.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
with kernel 6.8 NVIDIAs vGPU driver has a different api than the
previous 'mediated devices'. Adapt our sysfcode to also recognize this
for the 'mdev' paths and add another 'nvidia' property so we can detect
this.
Also parse the new api when they exist instead of the mediated devices.
The biggest difference to the existing mdev api for our use is that the
devices don't report all generally available devices, only the
createable ones. So if a user wants to configure a VM, the selection is
restricted by what may currently run on the GPU (depending ont the exact
settings, e.g. mixed mode gpus where different models can be mixed on a
single GPU; not the default though)
We could overcome this, when we'd parse the general info from the
'nvidia-smi' tool, though I'm currently unsure if that interface is
stable and intended to be parsed (there is no json output or similar
AFAIK)
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Reviewed-by: Christoph Heiss <c.heiss@proxmox.com>
since `print` is doing buffered IO, we don't always get an error there,
even if the underlying write does not work.
To properly catch that, do an unbuffered `syswrite` which circumvents
all buffers and writes directly to the file handle.
We aren't actually interested in the specific error here, but only if
the write was successful or not.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Christoph Heiss <c.heiss@proxmox.com>
Since commit ef0bcc9 ("tools: file_set_contents: use syswrite instead
of print") we're using PerlIO's scalar layer to ensure we encode any
potential unicode before passing the data to syswrite, which does not
support writing code points above 255.
Add an explicit use statement for PerlIO::scalar to avoid some odd
failures.
Some more background on why this seems odd:
After the pve-common version that included this change got moved to
public repos we got some reports in our Forum about CTs failing to
start [0] due to the lxc-pve-prestart-hook failing with an error
message like:
> Can't locate PerlIO.pm in @INC [...] at /usr/share/perl5/PVE/Tools.pm line 293.
Which points to the recently added `open(my $data_fh, '>', \$data)`
line in file_set_contents. The call chain from there upwards was
$lxc_setup->ct_file_set_contents <- $lxc_setup->pre_start_hook <-
closure <- lxc_hook <- lxc-pve-prestart-hook.
This seemed especially odd as we use `file_set_contents` in a lot of
other places and there was no all to obvious breakage from the change
on our test systems.
During evaluation I noticed some additional strange behavior, if one
called `file_set_contents` inside the closure before the call to
`pre_start_hook`, the error just goes away and one can observer that
%INC, which contains all loaded modules, suddenly does have a entry
for the PerlIO module, or well, it's scalar layer, which it did not
have without that call. So why the PerlIO can get automatically loaded
just fine most of the time but not inside the `pre_start_hook` is not
really clear yet; still loading PerlIO explicitly makes the issue go
away and seems sensible, so go for that and keep a comment to remind
more explicitly of this oddity. Once it's explained it can be removed
with a commit that mentions the explanation.
Further, if the PerlIO scalar layer cannot be loaded, the result is
that the passed reference is used as filename, which is far from
ideal, see the report in the perl GH [1] and the PR that fixes this
[2] by moving PerlIO::scalar into perl core proper, which will be
available in Perl v5.40 and thus our next Debian Trixie based major
release. This might have well to do with the original symptom that
embarked me on this odd (and not 100% finished) quest..
[0]: https://forum.proxmox.com/threads/156188/
[1]: https://github.com/Perl/perl5/issues/21275
[2]: https://github.com/Perl/perl5/pull/21282
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
In perl the `or` and the `||` operator do mostly the same thing but
with a different precedence level [0].
A statement like:
`$foo += bar() or die "error"`
is basically equivalent to:
`($foo += bar()) or die "error"`
That means as long as bar only returns zero or positive integers the
`or die` can only happen the first time, as otherwise $foo is bigger
than zero and thus will never evaluate to false. This can be
reproduced by perl -we 'my $foo = 1; $foo += 0 or die "wont happen";'
While one could switch to the `||` operator, this is a bit to subtle,
so to fix this, separate tracking the total bytes written from getting
the bytes written by the current call, this avoids the error potential
completely.
[0]: https://perldoc.perl.org/perlop#Logical-or-and-Exclusive-Or
Reported-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
instead of whether the key exists in the schema instance, just in case somebody
wants to set "download => 0".
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
with the newly introduced strict checks in the API handler, the download key
actually marks which endpoints/methods are allowed to use the download
functionality of the REST server, and the "directly return filename to be
downloaded" part is no longer supported as a result.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
The function can be called with
- neither quota nor period
- only a period (quota will be 'max')
- both
$quota was therefore defaulted to 'max' and the check for whether
values were provided should use $period instead of $quota.
Also move the defaulting-assignment into the condition for clarity.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
all text that is going through encode_text will at a later point be
decoded by 'decode_text'. The latter is decoding all percent encoded
characters, even those not originally encoded by 'encode_text'.
This means, to preserve the original data, we first have to at least
percent encode the '%' itself, otherwise it's impossible to properly
store e.g. '%20' there.
It would get saved as '%20' directly, but on the next read, it gets
decoded to ' ', which is not the original data. instead we have to save
it as '%2520', which gets then correctly decoded to '%20' again
This is especially important for the vm/ct/node description, as there
users can store external links, which already include percent encoded
characters.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
In order to make the parser somewhat more maintainable in the future,
this commit cleans up its logic and makes its control flow easier to
follow.
Signed-off-by: Max Carrara <m.carrara@proxmox.com>
Replace `foreach` with `for` and use postfix deref instead of block
(circumfix) dereference (`$foo->%*` instead of `%$foo`).
Furthermore, make `format_config_line` a private sub instead of
unnecessarily declaring it as an anonymous subroutine, which avoids
the `&$sub_ref(...)` syntax altogether.
Signed-off-by: Max Carrara <m.carrara@proxmox.com>
b792e8df81 introduced a bug that can cause this:
Undefined subroutine &PVE::Syscall::SYS_mknod called at /usr/share/perl5/PVE/Syscall.pm line 11
It should be mknod, not SYS_mknod. This caused other pve perl lib failing
to build. I couldn't reproduce this on amd64 build, but I could reproduce this
on arm64 build; however this didn't seem to fix the issue, unless I revert
b792e8df81.
cf: b792e8df81
Signed-off-by: Jing Luo <jing@jing.rocks>
This is allowed in ifupdown2 and previously interfaces named
'vmbr\d+' were recognized as bridges even if they used this mode.
With commit e68ebda4f1 this is no longer the case.
Fixes: e68ebda4f1 ("fix #545: interfaces: allow arbitrary bridge names in network config")
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Similar to other interface types, we can detect a bridge by the
presence of its bridge_ports attribute, rather than solely relying on
the "vmbr" ifname prefix heuristic. For OVS bridges we need to examine
the OVS type instead.
The check needs to be moved up since other prefixes could
theoretically be included in a bridge name and then would otherwise
get picked up wrongly.
Also added a warning for interfaces named vmbrX that are not bridges
to catch possible misconfigurations.
Originally-by: Jillian Morgan <jillian.morgan@primordial.ca>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
include 'PVEAPIToken=' prefix in the example for target-endpoint which
is mainly used for remote migrations.
Signed-off-by: Folke Gleumes <f.gleumes@proxmox.com>
$1 and friends are not cleared if a RE fails to match, in which case they will
contain the captured values from a previous successful match in the same scope.
deduplicate the two branches to avoid accidental re-introduction.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
makes the behaviour easier to analyze, and also helps when testing since it
allows constructing test cases that trigger certain order of parsing.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
