pbs: activate storage: fully validate if storage config works
improves UX of on_update and on_add hooks *a lot*.
This is a bit more expensive than the TCP ping, or even just an
unauthenticated ping, but not as bad as a full datastore status - as
this only reads the datastore config file (which is normally in page
cache anyway).
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Fabian Ebner [Fri, 27 Nov 2020 09:35:44 +0000 (10:35 +0100)]
plugin: hooks: add explicit returns
to avoid returning something unexpected. Finish what afeda182566292be15413d9b874720876eac14c9 already started for all the other
plugins. At least for ZFS's on_add_hook this is necessary (adding a ZFS storage
currently fails as reported here [0]), but it cannot hurt
in the other places either as the only hooks we expect to return something
currently are PBS's on_add_hook and on_update_hook.
Dominik Csapak [Tue, 24 Nov 2020 09:09:32 +0000 (10:09 +0100)]
Storage/Plugin: add get/update_volume_comment and implement for dir
and add the appropriate api call to set and get the comment
we need to bump APIVER for this and can bump APIAGE, since
we only use it at this new call that can work with the default
implementation
Fabian Ebner [Mon, 23 Nov 2020 12:33:09 +0000 (13:33 +0100)]
convert maxfiles to prune_backups when reading the storage configuration
If there are already prune options configured, simply delete the maxfiles
setting. Having set both is invalid from vzdump's perspective anyways, and any
backup job on such a storage failed, meaning a user would've noticed.
If there are no prune options, translate the maxfiles value to keep-last,
except for maxfiles being zero (=unlimited), in which case we use keep-all.
If both are not set, don't set anything, so:
1. Storages don't suddenly have retention options set.
2. People relying on vzdump defaults can still use those.
Fabian Ebner [Mon, 23 Nov 2020 12:33:08 +0000 (13:33 +0100)]
prune: introduce keep-all option
useful to have an alternative to the old maxfiles = 0. There has to
be a way for vzdump to distinguish between:
1. use the /etc/vzdump.conf default (when no options are configured for the storage)
2. use no limit (when keep-all=1)
Dominic Jäger [Wed, 28 Oct 2020 10:04:54 +0000 (11:04 +0100)]
lvmthin: Match snapshot remove regex to allowed names
We allow snapshot names that match pve-configid but during qm destroy we have
not removed all snapshots that match pve-configid so far. For example, the name
x-y was allowed but the resulting snap_vm-105-disk-0_x-y was not removed.
Reported-by: Hannes Laimer <h.laimer@proxmox.com> Signed-off-by: Dominic Jäger <d.jaeger@proxmox.com>
Fabian Ebner [Fri, 13 Nov 2020 13:08:55 +0000 (14:08 +0100)]
prune: allow having all prune options zero/missing
This is basically necessary for the GUI's prune widget, because we want to
pass along all options equal to zero when all the number fields are cleared.
And it's more similar to how it's done in PBS now.
Bumped the APIAGE and APIVER, in case some external plugin needs to adapt to
the now less restrictive schema for 'prune-backups'.
Thomas Lamprecht [Thu, 12 Nov 2020 17:01:41 +0000 (18:01 +0100)]
api: storage create/update: return parts of the configuration
First, doing such things can make client work slightly easier, as the
submitted values do not need to be made available in any callback
handling the response.
But the actual reason for doing this now is, that this is a
preparatory step for allowing the user to download/print/.. an
autogenerated PBS client encryption key.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Thomas Lamprecht [Thu, 12 Nov 2020 16:31:16 +0000 (17:31 +0100)]
avoid unecessary try for over optimization
That was lots of code and hash map touching for the case where one
avoided a extra stat, which result probably was in the page cache
anyway, for the case that a backup has a comment.
A case which is rather be unlikely - comments are normally done for
the occasional explicit backup (e.g., before major upgrade, before a
configuration change in that guest, ...), at least not worth some
relatively complicated effort making that sub harder to read and
maintain.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Fabian Ebner [Fri, 6 Nov 2020 14:30:55 +0000 (15:30 +0100)]
fix #3030: always activate volumes in storage_migrate
AFAICT the snapshot activation is not necessary for our plugins at the moment,
but it doesn't really hurt and might be relevant in the future or for external
plugins.
Deactivating volumes is up to the caller, because for example, for replication
on a running guest, we obviously don't want to deactivate volumes.
Stoiko Ivanov [Fri, 6 Nov 2020 14:19:40 +0000 (15:19 +0100)]
add check for fsfreeze before snapshot
In order to take a snapshot of a container volume, which can be mounted
read-only with RBD, the volume needs to be frozen (fsfreeze (8)) before taking
the snapshot.
This commit adds helpers to determine if the FIFREEZE ioctl needs to be called
for the volume.
Jan-Jonas Sämann [Fri, 30 Oct 2020 03:57:22 +0000 (04:57 +0100)]
Diskmanage: Use S.M.A.R.T. attributes for SSDs wearout lookup
This replaces a locally maintained hardware map in
get_wear_leveling_info() by commonly used register names of
smartmontool. Smartmontool maintains a labeled register database that
contains a majority of drives (including versions). The current lookup
produces false estimates, this approach hopefully provides more reliable
data.
Jan-Jonas Sämann [Fri, 30 Oct 2020 03:57:21 +0000 (04:57 +0100)]
Update disk_tests/ssd_smart/sde data
Provides recent test data for disk_tests/ssd_smart/sde_smart. The
previous data was created using an older smartmontools version, which
did not yet support the drive and therefore had bogus attribute mapping.
Fabian Ebner [Thu, 1 Oct 2020 08:11:36 +0000 (10:11 +0200)]
fix #1452: also log stderr of remote command with insecure storage migration
Commit 8fe00d99449b7c80e81ab3c9826625a4fcd89aa4 already
introduced the necessary logging for the secure code path,
so presumably the bug was already fixed for most people.
Delay the potential die for the send command to be able to log
the ouput+error from the receive command. Like this we also see e.g.
'volume ... already exists' instead of just 'broken pipe'.
Fabian Ebner [Thu, 1 Oct 2020 08:11:35 +0000 (10:11 +0200)]
avoid output of zfs get command on volume import
quiet takes care of both the error and success case.
Without this, there are lines like:
myzpool/vm-4352-disk-0@__replicate_4352-7_1601538554__ name myzpool/vm-4352-disk-0@__replicate_4352-7_1601538554__ -
in the log if the dataset exists, and this information is
already present in more readable form.
Thomas Lamprecht [Tue, 27 Oct 2020 06:03:17 +0000 (07:03 +0100)]
fix #3097: cifs, nfs: increase connection check timeout to 10s
we already have the ZFS pool plugin as precedent to use 10s, at for
network with remote off-site storage one can get to 200 - 300ms
RTT latency, which means that for a protocol needing multiple rounds of
communication, one can easily get over 2s while not being in a broken
network.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Stoiko Ivanov [Mon, 12 Oct 2020 15:34:57 +0000 (17:34 +0200)]
LIO: untaint values read from remote config
The LIO backend for ZFS over iSCSI fetches the json-config periodically from
the target.
This patch reduces the stored config values to those which are actually used
and additonally untaints the values read from the remote host's config-file.
Since the LUN index is used in calls to targetcli on the remote host (via
run_command), untainting prevents the call to crash when run with '-T'.
Tested by creating a zfs over iscsi backed VM, starting it, adding disks,
resizing disks, removing disks, creating snapshots, rolling back to a snapshot.
Stoiko Ivanov [Fri, 9 Oct 2020 15:13:44 +0000 (17:13 +0200)]
ZFSPlugin: untaint lun number
ZFS over iSCSI fetches information about the disk-images via ssh, thus
the obtainted data is tainted (perlsec (1)).
Since pvedaemon runs with '-T' enabled trying to start a VM via GUI/API failed,
while it still worked via `qm` or `pvesh`.
The issue surfaced after commit cb9db10c1a9855cf40ff13e81f9dd97d6a9b2698 in
pve-common ('run_command: improve performance for logging and long lines'),
and results from concatenating the original (tainted) buffer to a variable,
instead of a captured subgroup.
Untainting the value in ZFSPlugin should not cause any regressiosn, since the
other 3 target providers already have a match on '\d+' for retrieving the
lun number.
reported via pve-user [0].
reproduced and tested by setting up a LIO-target (on top of a virtual PVE),
adding it as storage and trying to start a guest (with a disk on the
ZFS over iSCSI storage) with `perl -T /usr/sbin/qm start $vmid`
When creating a new ZFS storage, also instantiate an import-unit for the pool.
This should help mitigate the case where some pools don't get imported during
boot, because they are not listed in an existing zpool.cache file.
This patch needs the corresponding addition of 'zfs-import@.service' in
the zfsonlinux repository.
commit 815df2dd08ac4c7295135262e60d64fbb57b8f5c introduced a small issue
when activating linked clone volumes - the volname passed contains
basevol/subvol, which needs to be translated to subvol.
using the path method should be a robust way to get the actual path for
activation.
Found and tested by building the package as root (otherwise the zfs
regressiontests are skipped).
Reported-by: Thomas Lamprecht <t.lamprecht@proxmox.com> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Makes it possible to clone and start a container whose
ZFS subvols are not yet mounted for some reason. If a
subvol cannot be mounted, there's a better error now:
zfs error: cannot mount '/myzpool/subvol-103-disk-0': directory is not empty
Previously, cloning would quietly do an "empty" clone,
and startup would fail with:
mount_autodev: 1074 Permission denied - Failed to create "/dev" directory
lxc_setup: 3238 Failed to mount "/dev"
do_start: 1224 Failed to setup container "103"
__sync_wait: 41 An error occurred in another process (expected sequence number 5)
Fabian Ebner [Mon, 31 Aug 2020 08:48:02 +0000 (10:48 +0200)]
prune-backups: improve command description
This is shown in the man page, so it's not important to mention
that this is a wrapper. Also mention the fact that the keep options
from the storage configuration serve as a fallback, which was previously
mentioned in the description of the (now removed) prune-backups parameter.
Thomas Lamprecht [Fri, 24 Jul 2020 14:47:47 +0000 (16:47 +0200)]
config: add hint for set encryption key, die on unlink error
as else the API cannot easily know if this is set, it cannot check
with -f as the key is in a restricted area and we do not want that a
GET runs as protected.
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
vdisk_list: skip scanning storages which cannot have images/rootdisks
Do not try to scan (and thus activate) storages which aren't
configured to support (or cannot support) "vdisks" anyway.
Avoids seemingly strange failures of VM migrations due to a backup storage
not being currently online - even if that storage isn't referenced in
the VM config anywhere..
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
we already differentiate between standard and non-standard names anyway
and don't detect and return the VMID in the latter case anyway. drop it
from the RE as well to allow names like 'vzdump-qemu-template.vma.lzo'
without the need for a fake VMID.
Because we always have 4-digit years, we can simply pass
the year itself to timelocal instead of subtracting 1900.
Like this it will also work for years not in the range 2000-2999.
See also:
https://perldoc.perl.org/Time/Local.html#Year-Value-Interpretation
Mira Limbeck [Tue, 7 Jul 2020 14:14:54 +0000 (16:14 +0200)]
fix #2460: use dm-uuid-mpath- as fallback for multipath
With Debian Buster it looks like the 'scsi-' method is no longer
reliable. In addition to that, which is also used for non-multipath
systems, add the 'dm-uuid-mpath-' method as fallback. This is also used
by openstack (see os-brick
https://github.com/openstack/os-brick/blob/39b201160bbff39117fca46b2709ff3f74c53d80/os_brick/initiator/linuxscsi.py#L400)
Also sort the output of readdir so 'scsi-' is always after
'dm-uuid-mpath-' so the output of pvesm list does not change for systems
that worked before.
Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
pbs: run_raw_client_cmd: fix typo, and drop repetive info
It would be s/bps/pbs/ but as we already have "proxmox-backup-client"
included in the log through the executable name, so it should be
clear that this is a PBS command - so drop that part entirely.
Now using:
> run: /usr/bin/proxmox-backup-client ...
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
projects / pve-storage.git / log