Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
[ TL: fix wrong comparison with >= and avoid undef warning if file
does not yet exist at all ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
[ TL: squash removal of both subs into one commit ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Sun, 10 Mar 2024 18:25:34 +0000 (19:25 +0100)]
api: import-metadata: make warnings structured & merge ignored-volumes
This allows the frontends to translate them and avoids somewhat
duplicated info by having some warnings explicitly (ignored-volumes)
while others are in the warnings array.
By passing along the key and the value the frontend can also show the
warnings in-line, e.g. by marking a disk-entry in a grid as having
potential problems.
Ideally we'd have a central list of known types used for the API
return schema enum and to check when calling the $warn closure, but as
we only got three warnings keep this as is and only add a comment.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This will be used for returning the base meta information of a
external VM that is about to be imported into Proxmox VE.
A front-end can use this endpoint to show the proposed configs with
potential override switches to the user, so that they can adapt the
most important options to ensure that import can work.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
[ TL: add more commit message with some background ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Add a new 'import' content type which will be the corner stone for a
better API and UI integrated way to import virtual guests into Proxmox
VE.
For starters this will be used to implement a ESXi adapter, so that
those VMs can get imported nicely.
Later we want to integrate the OVF/OVA import skeletons we got in
qemu-server to something more usable here.
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
[ TL: add more commit message with some background ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Mon, 19 Feb 2024 16:13:41 +0000 (17:13 +0100)]
fix #5254: api: allow usage of download-url with Sys.AccessNetwork
The download-url API endpoint has some implications that admins are
unaware of, namely that it basically allow to scan the whole network
via HTTP URLs, and potentially even download some image that the user
should not have access to and adding to a VM that the user controls.
That's why in addition to the Datastore.AllocateTemplate privilege on
the storage, the Sys.Modify on the whole Cluster was required to use
the API call. That design was chosen as we were not fully sure if a
separate privilege is warranted, but user feedback has shown that the
(not so big) cost of adding such a new privilege is justified.
Change the permission check to allow the combination of
Datastore.AllocateTemplate on the storage and either 'Sys.Modify' on
/, for backwards compatibility, or the newer 'Sys.AccessNetwork' on
the node that handles the download.
Using a node-specific ACL path allows admins to e.g. prepare one
specific node's firewall so that pveproxy can access only a safe set
of hosts via outgoing HTTP (not stemming from valid connection
tracking to the PVE API), and thus even further limit the privileges
of users or tools that are trusted to download images to a storage.
Buglink: https://bugzilla.proxmox.com/show_bug.cgi?id=5254 Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com> Tested-by: Hannes Duerr <h.duerr@proxmox.com> Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Thomas Lamprecht [Sun, 19 Nov 2023 19:05:50 +0000 (20:05 +0100)]
btrfs: fix calling parent create_base method in fall-back
If we want to forward to the create_base of the directory plugin while
making that use our $class for the operations that call might do, we
cannot use the -> notation (which would resolve the next actual
implementation) but rather pass the class directly.
But, DirPlugin reuses the create_base method from the base Plugin
method, so we also need to call that, because on direct call notation
the inheritance fallback to super methods isn't available.
Reported in the forum:
https://forum.proxmox.com/threads/95684/post-606535
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
fix #254: iscsi: add support for multipath targets
With this patch Proxmox now tries to login to all discovered portals
in case some of them are not logged yet.
In case of multipath configuration when initially configured portal is
missing for some reason Proxmox don't lose iSCSI storage now and can
successfully restore iSCSI connection between reboots.
Fiona Ebner [Tue, 27 Jun 2023 07:48:49 +0000 (09:48 +0200)]
cifs: bubble up NT_STATUS_INVALID_PARAMETER during connection check
instead of claiming that the storage is not online.
Would've made the issue fixed by b27da68 ("cifs: fix check connection
call") more obvious, because (the UI passes along an empty string for
domain if not set and) the smbclient call returns that status with
> -W ''
in Bookworm.
Philipp Hufnagl [Mon, 14 Aug 2023 14:42:17 +0000 (16:42 +0200)]
fix #4849: download-url: allow download and decompression of compressed ISOs
adds information for how to decompress isos.
generates the compressor regex from a list of comression formats (to
avoid redundancy)
extends the download_url wtih the functionality to handley compression
for images
Signed-off-by: Philipp Hufnagl <p.hufnagl@proxmox.com>
Thomas Lamprecht [Sat, 17 Jun 2023 12:53:05 +0000 (14:53 +0200)]
disk api: only ask for Datastore.Allocate if adding to storage config
The Proxmox VE storage systems doesn't cares at all if the
Datastore.Allocate privilege is present if no Proxmox VE storage will
be allocated.
Note, if we want to restrict this further as Sys.Modify on /, which
is already quite a powerful permission, we should probably add a new
one under the Sys. space, e.g., Sys.Disk.Use or the like.
This is a step in splitting the disk manage code out of the
pve-storage package, and maybe even repository
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Sat, 17 Jun 2023 12:22:28 +0000 (14:22 +0200)]
disk api: avoid using unrelated Datastore.Audit priv for disk management
Local disk and storage creation and listing is something rather
different than the Proxmox VE storage client ABI that provides an
abstract access to a variety of storage types, specifically targeted
to virtual guests images, templates and backups.
The Datastore.* privilege group is specifically made for auditing the
abstract configuration, here the name must be interpreted in context
and not just assumed that due to "datastore" sounding like it could
have to do something with disks or creation of local storage it just
must be a good fit.
Luckily, Sys.Audit was already used too, which is the correct one
here, this is for node specific (HW) details, not some config for
accessing datastore in a restricted way.
This is a step in splitting the disk manage code out of the
pve-storage package, and maybe even repository.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Friedrich Weber [Thu, 15 Jun 2023 11:36:58 +0000 (13:36 +0200)]
content-dirs check: silently skip paths that cannot be resolved
Since commit 8e623a2930f7aee4b3309b1f297613a250ee4698, the inequality
check for content-dirs prints a warning if a content directory path
could not be resolved, i.e., if `abs_path` returns undef. Among other
things, `abs_path` returns undef if the path has an inner (= any but
last) component that does not exist. This can happen for a storage
with content type `iso,vztmpl` and `create-subdirs` set to 0, in case
`template/` does not exist. In this case, the warnings printed by
pvestatd are quite noisy.
As missing content directories are not a problem per se, remove the
warning and just ignore the directory during the inequality check.
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
Fiona Ebner [Mon, 12 Jun 2023 14:27:33 +0000 (16:27 +0200)]
api: config: add/update storage: check for type mismatch first
This avoids confusing errors about other properties when the storage
type doesn't match. By highlighting that the type doesn't match, users
should know right away what the issue is.
content dirs: skip creation if either mkdir or create-subdirs is false
This is slightly confusing due to both options, the legacy convoluted
one and the new targeted one, exist, but before the rework we skip if
either of those sub-expressions was true, so doing it needs both to
be true.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
activate storage: ensure content directories are created before checking them
checking the content dirs for clashes via abs_path must be done after
the logic for creating them ran, as abs_path is working on actual
filesystem level, so it will return undf if the directory does not
exist, in which case we then set a hash entry for "undef", and the
next for loop round then resolved again to "undef", resulting in a
false-positive of the check.
Avoid the dangerous "return if" stanzas and reverse them to an actual
if block, which is much safer to adapt. Then move the check for
duplicate content-dir usage after that.
best viewed with white space change ignored: git show -w
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Stefan Hrdlicka [Wed, 1 Mar 2023 12:13:25 +0000 (13:13 +0100)]
fix #2920: cifs: add options parameter
This makes it possible to add all mount options offered by mount.cifs.
NFS & CIFS now share the options parameter since they use it for the
same purpose.
Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
[FE: rebase + style fixes] Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Tested-by: Friedrich Weber <f.weber@proxmox.com>
[T: fix merge conflict ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
rbd: volume resize: avoid passing floating point value to rbd
which causes an error "the argument for option '--size' is invalid".
Just round up to the nearest integer to have at least the requested
size. This is similar to what is done for ZFS with d3e3e5d ("When
resizing a ZFS volume, align size to 1M") and makes commands like 'qm
resize 102 scsi1 +0.01G' work.
It was introduced by commit 4b7dd9d ("allow --allow-shrink on RBD
resize"), but doesn't give a rationale. A mail gives more[0],
indicating that the user also uses the function to shrink images.
However, the volume_resize function is only reachable via the resize
API endpoints for VMs and containers, which have an explicit check to
disallow shrinkage. If somebody really wants to shrink the image, just
let them use the storage's tools directly. Calling into Proxmox VE's
perl functions directly is not supported.
plugin: simplify and fix create-base-path vs mkdir logic
In the previous code, if `create-base-path` was explicitly
set to false, it would be treated the same as if it was
undef, falling through to whatever 'mkdir' was.
Instead, the new options should always be preferred, and the
logic can be simplified to a single line.
Here's the table showing the difference, 'u' being 'undef':
config: mkdir: u 0 1 u 0 1 u 0 1
create: u u u 0 0 0 1 1 1
=========================
mkpath: old: 1 0 1 0 0 1 1 1 1
new: 1 0 1 0 0 0 1 1 1
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Aaron Lauterer [Wed, 31 May 2023 12:46:09 +0000 (14:46 +0200)]
deprecate mkdir option for create-base-path and create-subdirs
The `mkdir` option has two meanings[0][1] which are split up in `create-path`
and `create-sub-dirs`.
The `create-base-path` option decides if the path to the storage is
automatically created or not.
The `create-subdirs` options decides if the default directory
structure (dump, images, ...) at the storage location is created.
The `mkdir` option is still working but will trigger a warning in the
logs.
As a side effect, this also fixes #3214 because the `create-base-path` option
is now run after the `is_mountpoint` check in the `activate_storage`
method in DirPlugin.pm.
The 'mkpath' command has been moved into a new helper function that
first determines if the conditions to create the path is true, called
'config_aware_base_mkdir'.
Friedrich Weber [Thu, 4 May 2023 09:13:34 +0000 (11:13 +0200)]
api: upload: add pattern to tmpfilename parameter
The `tmpfilename` is generated by pve-http-server and always adheres
to this pattern, so make this explicit to prevent confusion and have
a more complete JSON API schema, useful for e.g., the API viewer.
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
[ T: slightly extended commit message ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Stoiko Ivanov [Tue, 21 Mar 2023 11:12:40 +0000 (12:12 +0100)]
cifs: use empty string instead of / as default directory
this keeps the mount sources consistent with previous versions
without this patch there is a small regression, which leads to the
storage not being recognized as being mounted on upgrade:
* pvestatd in older version mount the storage with out trailing /
```
//cifsstore/ISO on /mnt/pve/cifsstore type cifs...
```
* the cifs_is_mounted helper does not recognize it as being mounted
(as the source now has a / in the end)
* attempting to mount leads to
```
mount error(16): Device or resource busy
```
noticed after upgrading and having a cifs storage mounted
Christian Ebner [Thu, 9 Mar 2023 09:41:23 +0000 (10:41 +0100)]
api: fix get content call response type for RBD/ZFS/iSCSI volumes
`pvesh get /nodes/{node}/storage/{storage}/content/{volume}` failed for
several storage types, because the respective storage plugins returned
only the volumes `size` on `volume_size_info` calls, while also the format
is required.
This patch fixes the issue by returning also `format` and where possible `used`.
The issue was reported in the forum:
https://forum.proxmox.com/threads/pvesh-get-nodes-node-storage-storage-content-volume-returns-error.123747/
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
[ T: fixup white space error ] Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Leo Nunner [Thu, 1 Dec 2022 11:32:55 +0000 (12:32 +0100)]
fix #2641: allow mounting of CIFS subdirectories
CIFS/SMB supports directly mounting subdirectories, so it makes sense to
also allow the --subdir parameter for these storages. The subdir
parameter was moved from CephFSPlugin.pm to Plugin.pm, because it isn't
specific to CephFS anymore.
Fiona Ebner [Mon, 23 Jan 2023 09:14:50 +0000 (10:14 +0100)]
nfs: check connection: support NFSv4-only servers without rpcbind
by simply doing a ping with the expected port as a fallback when the
rpcinfo command fails.
The timeout was chosen to be 2 seconds, because that's what the
existing callers of tcp_ping() in the iSCSI and GlusterFS plugins use.
Alternatively, the existing check could be replaced, but that would
1. Dumb down the check.
2. Risk breakage in some corner case that's yet to be discovered.
3. It would still be necessary to use rpcinfo (or dumb the check down
even further) in case port=0; from 'man 5 nfs' about the NFSv4 'port'
option:
> If the specified port value is 0, then the NFS client uses the NFS
> service port number advertised by the server's rpcbind service.
Reported in the community forum:
https://forum.proxmox.com/threads/118466/post-524449
https://forum.proxmox.com/threads/120774/
Fiona Ebner [Fri, 9 Dec 2022 10:30:41 +0000 (11:30 +0100)]
fix #4390: rbd: snapshot delete: avoid early return to fix handling TPM drive
The only caller where $running can even be truthy is QemuServer.pm's
qemu_volume_snapshot_delete(). But there, a check if snapshots should
be done with QEMU is already made and the storage function is only
called if snapshots should not be done with QEMU (like for TPM drives
which are not attached directly). So rely on the caller and do not
return early to fix removing snapshots in such cases.
Even if a stray call ends up here (can happen already by changing the
krbd setting while a VM is running to flip the above-mentioned check
and the early return check removed by this patch), it might not even
be problematic. At least a quick test worked fine:
1. take snapshot via a monitor command in QEMU
2. remove snapshot via the storage layer
3. create a new file in the VM
4. take a snapshot with the same name via monitor command in QEMU
5. roll back to the snapshot
6. check that the file in the VM is as expected
Using the storage layer to take the snapshots and QEMU to remove the
snapshot also worked doing the same test. Even if it were problematic,
the check in qemu-server should rather be improved then.
(Trying to issue a snapshot mon command for a krbd-mapped image fails
with an error on the other hand, but that is also not too bad and not
relevant to the storage code. Again, it rather would be necessary to
improve the check in qemu-server).
The fact that the pve-container code didn't even pass $running is the
reason removing snapshots worked for containers on a storage with krbd
disabled (the pve-container code calls map_volume() explicitly, so
containers can work regardless of the krbd setting in the storage
configuration; see commit 841fba6 ("call map_volume before using
volumes.") in pve-container).
For volume_snapshot(), the early return with $running was already
removed (or rather the relevant logic moved to QemuServer.pm) in 2015
by commit f5640e7 ("remove running from Storage and check it in
QemuServer"), even before krbd support was added in RBDPlugin.pm.
Fiona Ebner [Tue, 10 Jan 2023 12:52:43 +0000 (13:52 +0100)]
zfs: list zvol: limit recursion depth to 1
To be correct in all cases, it's still necessary to filter by "pool"
in zfs_parse_zvol_list(), because $scfg->{pool} could be e.g.
'foo/vm-123-disk-0' which looks like an image name and would pass the
other "should skip"-checks in zfs_parse_zvol_list().
No change in the result of zfs_list_zvol() is intended.
projects / pve-storage.git / log