5
0
mirror of git://git.proxmox.com/git/pve-storage.git synced 2024-12-22 13:34:16 +03:00
Commit Graph

1919 Commits

Author SHA1 Message Date
Fiona Ebner
0756b484d4 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.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2023-01-11 16:42:42 +01:00
Fiona Ebner
d05015f887 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.

Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-01-10 14:31:21 +01:00
Fiona Ebner
4c82e7b61c zfs: list zvol: skip different pools during parsing already
The 'pool' property in the result of zfs_parse_zvol_list() was not
used for anything else.

No functional change is intended.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-01-10 14:29:53 +01:00
Fiona Ebner
ebcb12ee09 zfs: list zvol: return empty hash rather than undef
Avoids the need for the fallback for the (only existing) caller.

Note that the old my $list = (); is a rather intransparent way of
assigning undef.

Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2023-01-10 14:28:14 +01:00
Leo Nunner
b539bb46a8 plugin: change name, separator and error message for dir overrides
Rename the "dirs" parameter to "content-dirs". Switch from a "vtype:/dir"
format to "vtype=/dir", and remove the misleading error message talking
about "absolute" paths. One might expect these to be absolute over the
whole system, while in reality they are relative to the mountpoint of
the storage.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
2023-01-09 10:47:20 +01:00
Leo Nunner
3e7bd7dac3 config: add overrides for default directory locations
Allowing overrides for the default directory locations seems to
integrate rather well into the existing system. Custom locations
are specified using the "dirs" parameter as a comma-separated list
of "vtype:/location" values.

For now, the option has been enabled for the Directory, CIFS and NFS
backends.

Signed-off-by: Leo Nunner <l.nunner@proxmox.com>
2023-01-05 14:33:16 +01:00
Fiona Ebner
39eb268662 zfs: list images: code cleanup
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2022-12-21 10:46:15 +01:00
Fiona Ebner
4470f0cbe9 zfs: list images: don't use cache
There is no caller using $cache and the same $storeid multiple times,
so there is no need to keep the cache.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2022-12-21 10:46:15 +01:00
Fiona Ebner
c2e6a90d4f zfs: list: only cache and list images for requested storage/pool
The plugin for remote ZFS storages currently also uses the same
list_images() as the plugin for local ZFS storages. There is only
one cache which does not remember the target host where the
information originated.

This is problematic for rescan in qemu-server:
1. Via list_images() and zfs_list_zvol(), ZFSPlugin.pm's zfs_request()
   is executed for a remote ZFS.
2. $cache->{zfs} is set to the result of scanning the images there.
3. Another list_images() for a local ZFS storage happens and uses the
   cache with the wrong information.

The only two operations using the cache currently are
1. Disk rescan in qemu-server which is affected by the issue. It is
   done as part of (or as a) long-running operations.
2. prepare_local_job for pvesr, but the non-local ZFS plugin doesn't
   support replication, so it cannot trigger there. The cache only
   helps if there is a replicated guest with volumes on different
   ZFS storages, but otherwise it will be less efficient than no
   cache or querying every storage by itself.

Fix the issue by making the cache $storeid-specific, which effectively
makes the cache superfluous, because there is no caller using the same
$storeid multiple times. As argued above, this should not really hurt
the callers described above much and actually improves performance for
all other callers.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2022-12-21 10:46:15 +01:00
Fiona Ebner
4be012a4cd disk manage: pass full NVMe device path to smartctl
This essentially reverts commit c9bd3d2 ("fix #1123: modify NVME
device path for SMART support").

The man page for smartctl states
> Use the forms "/dev/nvme[0-9]" (broadcast namespace) or
> "/dev/nvme[0-9]n[1-9]" (specific  namespace 1-9) for NVMe devices.
so it should be fine to pass the path with the specific namespace to
smartctl.

But that text was already present in the man page of version 6.5,
which is the version the commit c9bd3d2 talks about. It might be that
it was necessary to drop the specific namespace for the version
backported from Stretch to Jessie (the bug report mentions that that
version was used[0]), but it's not quite clear.

With current versions, passing in the path with the specific namespace
did work as expected[1], even on a device with multiple namespaces set
up tested locally. In PBS, the path queried via
udev::Device::from_syspath("/sys/block/{name}") is passed to smartctl
and that also included the specific namespace on the systems I tested
with a short script.

So pass the full path to make things a little bit simpler and to avoid
potential future issues like bug #2020[2].

[0]: https://bugzilla.proxmox.com/show_bug.cgi?id=1123#c3
[1]: https://forum.proxmox.com/threads/113962/post-493185
[2]: https://bugzilla.proxmox.com/show_bug.cgi?id=2020

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2022-12-13 13:26:53 +01:00
Fiona Ebner
668c15043f Revert "Fix #2020: use /sys to map nvmeXnY to nvmeX"
This reverts commit c3442aa554.

Nowadays, relying on 'readlink /sys/block/nvmeXnY/device' won't always
lead to the correct device, as reported in the community forum[0],
where it results in '../../nvme-subsys0' and there's no matching entry
under '/dev/'.

Since Linux kernel 5.4, in particular commit 733e4b69d508 ("nvme:
Assign subsys instance from first ctrl"), the problematic situation
from bug #2020 shouldn't happen anymore.

Stated more clearly by the commit's author here[1]:
> Indeed, that commit will make the naming a bit more sane and will
> definitely prevent mistaken identity. It is still possible to
> observe controllers with instances that don't match their
> namespaces, but it is impossible to get a namespace instance that
> matches a non-owning controller.

The only other user of get_sysdir_info() doesn't use the 'device'
entry, so reverting that part is fine too.

[0] https://forum.proxmox.com/threads/113962/
[1] https://github.com/linux-nvme/nvme-cli/issues/510#issuecomment-552508647

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
Tested-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
2022-11-30 16:33:09 +01:00
Thomas Lamprecht
4d5fb344ac bump version to 7.3-1
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-24 08:26:55 +01:00
Fiona Ebner
0227e28e8e get bandwidth limit: improve detecting if storages are involved
Previously, calling with e.g. $storage_list = [undef] would lead to an
early return of $override and not consider the limit from
datacenter.cfg.

Refactoring the bandwidth limit handling for migration introduced
calls such as described above, which broke applying the limit from
datacenter.cfg for VM RAM/state migration.

Reported in the community forum:
https://forum.proxmox.com/threads/37920/post-513005

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2022-11-24 08:25:12 +01:00
Fiona Ebner
b9e309d667 test: bwlimit: fix test description
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2022-11-24 08:25:12 +01:00
Thomas Lamprecht
7c3fd6b1ef bump version to 7.2-12
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-17 19:12:38 +01:00
Thomas Lamprecht
06deafa43e disk manage: fix dereferencing draid config
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-17 19:10:58 +01:00
Thomas Lamprecht
76e84829e5 bump version to 7.2-11
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-17 17:51:13 +01:00
Dominik Csapak
1de000bbfd api: FileRestore: make use of file-restores and guis timeout mechanism
file-restore has a 'timeout' parameter and if that is exceeded, returns
an error with the http code 503 Service Unavailable

When the web ui encounters such an error, it retries the listing a few
times before giving up.

To make use of these, add the 'timeout' parameter to the new
'extraParams' of 'file_restore_list'.

25 seconds are chosen because it's under pveproxy 30s limit, with a bit
of overhead to spare for the rest of the api call, like json decoding,
forking, access control checks, etc.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2022-11-15 11:03:08 +01:00
Dominik Csapak
14f99a1614 api: FileRestore: decode and return proper error of file-restore listing
since commit
ba690c40 ("file-restore: remove 'json-error' parameter from list_files")

in proxmox-backup, the file-restore binary will return the error as json
when called with '--output-format json' (which we do in PVE::PBSClient)

here, we assume that 'file-restore' will fail in that case, and we try
to use the return value as an array ref which fails, and the user never
sees the real error message.

To fix that, check the ref type of the return value and act accordingly

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
2022-11-15 11:03:08 +01:00
Thomas Lamprecht
676b2e9094 api: storage: fix indentation in free volume
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-11 13:49:08 +01:00
Thomas Lamprecht
e698cbb9af disk manage: draid: style clean ups
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-11 09:36:24 +01:00
Thomas Lamprecht
8a5ffcd991 disk manage: move "draid-config set only on draid level" assertion
so that there is a better code locality and also we avoid forgetting
to adapt the check for each specific draid-config parameter if a new
one gets added or an existing one changed.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-11 09:36:03 +01:00
Stefan Hrdlicka
59db1208c3 fix #3967: enable ZFS dRAID creation via API
It is possible to set the number of spares and the size of
data stripes via draidspares & dreaddata parameters.

Signed-off-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>
2022-11-11 09:35:59 +01:00
Thomas Lamprecht
8b06da647a zfs diskmanage: code/indentation cleanup in get_pool_data
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-11 09:35:59 +01:00
Thomas Lamprecht
0bcfafa471 pbs: prune: avoid getting all snapshots for group assembly if fixed anyway
If both type and vmid is defined we don't need to list the current
snapshots, we simply can derive the single backup group from that and
let the PBS client handle the rest.

Should be a not so small speedup for most setups using PBS backup and
pruning configured on PVE side, as vzdump calls this separately for
every vmid on backup jobs with multiple guests included.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-07 15:55:40 +01:00
Thomas Lamprecht
e6d7728171 pbs: wrap getting list volumes for pruning for error context
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-11-07 15:22:14 +01:00
Fabian Ebner
f8b890f1c0 api: pbs: file restore: don't use namespaced parameters
Instead, rely on PBSClient to set namespace according to the initial
configuration.

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2022-11-04 14:14:04 +01:00
Wolfgang Bumiller
950ed264ea bump common dependency to 7.2-4
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2022-11-04 14:14:04 +01:00
Thomas Lamprecht
bcbed84e8e bump version to 7.2-10
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-09-29 14:33:18 +02:00
Fabian Grünbichler
71460c8ace (remote) export: check and untaint format
this format comes from the remote cluster, so it might not be supported
on the source side - checking whether it's known (as additional
safeguard) and untainting (to avoid open3 failure) is required.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
 [ T: squashed in canonical perl array ref access ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-09-29 14:21:08 +02:00
Thomas Lamprecht
47d1125bfe api: disk SMART: fix details for depreacated return value comment
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-09-23 11:59:33 +02:00
Thomas Lamprecht
9aff3f3d8e disk manage: module wide code/style cleanup
fixing some issues reported by perlcritic along the way.

cutting down 70 lines, often with even improving readability.
Tried to recheck and be conservative, so there shouldn't be any
regression, but it's still perl after all...

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-09-23 11:56:55 +02:00
Matthias Heiserer
4c86c7111c fix #4165: disk: SMART: add normalized field
This makes it consistent with the naming scheme in PBS/GUI.
Keep value for API stability reasons and remove it in the next major version.

Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.cspak@proxmox.com>
2022-09-22 14:14:19 +02:00
Fabian Grünbichler
88f272b204 api: remove duplicate variable
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2022-09-20 10:50:12 +02:00
Fabian Grünbichler
f553e6e639 bump version to 7.2-9
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2022-09-20 09:20:14 +02:00
Aaron Lauterer
bd485fd4aa disks: allow add_storage for already configured local storage
One of the smaller annoyances, especially for less experienced users, is
the fact, that when creating a local storage (ZFS, LVM (thin), dir) in a
cluster, one can only leave the "Add Storage" option enabled the first
time.

On any following node, this option needed to be disabled and the new
node manually added to the list of nodes for that storage.

This patch changes the behavior. If a storage of the same name already
exists, it will verify that necessary parameters match the already
existing one.
Then, if the 'nodes' parameter is set, it adds the current node and
updates the storage config.
In case there is no nodes list, nothing else needs to be done, and the
GUI will stop showing the question mark for the configured, but until
then, not existing local storage.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>
2022-09-13 10:05:20 +02:00
Aaron Lauterer
55553bd432 disks: die if storage name is already in use
If a storage of that type and name already exists (LVM, zpool, ...) but
we do not have a Proxmox VE Storage config for it, it is possible that
the creation will fail midway due to checks done by the underlying
storage layer itself. This in turn can lead to disks that are already
partitioned. Users would need to clean this up themselves.

By adding checks early on, not only checking against the PVE storage
config, but against the actual storage type itself, we can die early
enough, before we touch any disk.

For ZFS, the logic to gather pool data is moved into its own function to
be called from the index API endpoint and the check in the create
endpoint.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>
2022-09-13 10:05:16 +02:00
Aaron Lauterer
4de6002558 diskmanage: add mounted_paths
returns a list of mounted paths with the backing devices

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>
2022-09-13 10:04:48 +02:00
Fiona Ebner
6a44cc417d RBD plugin: librados connect: increase timeout when in worker
The default timeout in PVE/RADOS.pm is 5 seconds, but this is not
always enough for external clusters under load. Workers can and should
take their time to not fail here too quickly.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2022-09-13 09:57:19 +02:00
Fiona Ebner
2be327abf6 RBD plugin: librados connect: pass along options
In preparation to increase the timeout for workers. Both existing
callers of librados_connect() don't currently use the parameter.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2022-09-13 09:57:19 +02:00
Fiona Ebner
e8e477112f RBD plugin: path: conditionalize get_rbd_dev_path() call
The return value of get_rbd_dev_path() is only used when $scfg->{krbd}
evaluates to true and the function shouldn't have any side effects
that are needed later, so the call can be avoided otherwise.

This also saves a RADOS connection and command with configurations for
external clusters with krbd disabled.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
2022-09-13 09:55:56 +02:00
Wolfgang Bumiller
c560cb58a5 fix #4189: pbs: bump list_volumes timeout to 2mins
When switching this from calling the external binary to
using the perl api client the timeout got reduced to 7
seconds, which is definitely insufficient for larger stores.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2022-08-17 13:12:49 +02:00
Fabian Grünbichler
78e3ebb040 bump version to 7.2-8
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2022-08-16 13:56:56 +02:00
Fabian Grünbichler
de0f92442a pbs: die if master key is missing
while the resulting backups are encrypted, they would not be restorable
using the master key (only) if the original PVE system is lost.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2022-08-16 13:55:43 +02:00
Fabian Grünbichler
de635c2668 pbs: warn about missing, but config master key
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
2022-08-16 13:52:24 +02:00
Fabian Grünbichler
2bc4cfb866 pbs: detect mismatch of encryption settings and key
if the key file doesn't exist (anymore), but the storage.cfg references
one, die on commands that should use encryption instead of falling back
to plain-text operations.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Tested-by: Stoiko Ivanov <s.ivanov@proxmox.com>
2022-08-16 13:51:38 +02:00
Thomas Lamprecht
0eb1803679 bump version to 7.2-7
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-07-15 13:36:39 +02:00
Fabian Ebner
ad3666d716 pbs: fix namespace handling in list_volumes
Before af07f67 ("pbs: use vmid parameter in list_snapshots") the
namespace was set via do_raw_client_command, but now it needs to be
set explicitly here.

Fixes: af07f67 ("pbs: use vmid parameter in list_snapshots")
Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
2022-07-15 13:35:37 +02:00
Thomas Lamprecht
752a0bcabb bump version to 7.2-6
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
2022-07-14 13:47:14 +02:00
Wolfgang Bumiller
af07f67a89 pbs: use vmid parameter in list_snapshots
Particularly for operations such as pruning backups after a
scheduled backups we do not want to list the entire
store.

(pbs_api_connect is moved up unmodified)

Note that the 'snapshots' CLI command only takes a full
group, but the API does allow specifying a backup-id without
a backup-type!

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
2022-07-14 13:30:16 +02:00