[pve-devel] [PATCH v1 pve-storage 1/2] example: sshfs plugin: add custom storage plugin for SSHFS
Max Carrara
m.carrara at proxmox.com
Fri Jul 4 14:33:34 CEST 2025
On Wed Jul 2, 2025 at 10:14 PM CEST, Thomas Lamprecht wrote:
> Am 16.04.25 um 14:47 schrieb Max Carrara:
> > This commit adds an example implementation of a custom storage plugin
> > that uses SSHFS [0] as the underlying filesystem.
> >
> > The implementation is very similar to that of the NFS plugin; as a
> > prerequisite, it is currently necessary to use pubkey auth and have
> > the host's root user's public key deployed to the remote host like so:
> >
> > ssh-copy-id -i ~/.ssh/id_my_private_key \
> > -o UserKnownHostsFile=/etc/pve/priv/known_hosts [USER]@[HOST]
> >
> > Then, the storage can be added as follows:
> >
> > pvesm add sshfs [STOREID] \
> > --username [USER] \
> > --server [HOST] \
> > --sshfs-remote-path [ABS PATH ON REMOTE] \
> > --path /mnt/path/to/storage \
> > --sshfs-private-key ~/.ssh/id_my_private_key
> >
> > If the host is part of a cluster, other nodes may connect to the
> > remote without any additional setup required. This is because we copy
> > the private key to `/etc/pve/priv/storage/$KEYNAME.key` and use the
> > cluster-wide `/etc/pve/priv/known_hosts` file. Also mark each SSHFS
> > storage as `shared` by default in order to make use of this.
> >
> > Note: Because there's currently no way to officially and permanently
> > mark a storage as shared (like some built-in plugins [1]) set
> > `$scfg->{shared} = 1;` in `on_add_hook`. This has almost the same
> > effect as modifying `@PVE::Storage::Plugin::SHARED_STORAGE` directly,
> > except that the `shared` is written to `/etc/pve/storage.cfg`.
> >
> > [0]: https://github.com/libfuse/sshfs
> > [1]: https://git.proxmox.com/?p=pve-storage.git;a=blob;f=src/PVE/Storage/Plugin.pm;h=4e16420f667f196e8eb99ae7c9f3f1d3e13791fb;hb=refs/heads/master#l37
>
> not a fully complete review but some comments inline, and there is now also a
> new repo that would be a great fit for hosting this example:
>
> https://git.proxmox.com/?p=pve-storage-plugin-examples.git;a=summary
Thanks for the pointer; v2 will be for that repo then!
>
> >
> > Signed-off-by: Max Carrara <m.carrara at proxmox.com>
> > ---
> > Changes rfc-v1 --> v1:
> > * rework most of the plugin
> > * cease to call methods of DirPlugin (plugins should be isolated;
> > we don't want to encourage third-party devs to do that)
> > * handle SSH private key as sensitive property and place it on pmxcfs
> > * make storage shared
> > * manually implement attribute handling ("notes", "protected" for
> > backups)
> >
> > .../lib/PVE/Storage/Custom/SSHFSPlugin.pm | 398 ++++++++++++++++++
> > 1 file changed, 398 insertions(+)
> > create mode 100644 example/sshfs-plugin/lib/PVE/Storage/Custom/SSHFSPlugin.pm
> >
> > diff --git a/example/sshfs-plugin/lib/PVE/Storage/Custom/SSHFSPlugin.pm b/example/sshfs-plugin/lib/PVE/Storage/Custom/SSHFSPlugin.pm
> > new file mode 100644
> > index 0000000..75b29c1
> > --- /dev/null
> > +++ b/example/sshfs-plugin/lib/PVE/Storage/Custom/SSHFSPlugin.pm
> > @@ -0,0 +1,398 @@
> > +package PVE::Storage::Custom::SSHFSPlugin;
> > +
> > +use strict;
> > +use warnings;
> > +
> > +use feature 'signatures';
>
> FYI: you can replace above four lines with:
>
> use v5.36;
>
> As that version implies strict, warnings and enables the signatures feature.
Oh, that's nice. Will change the above to that in v2.
>
> > +
> > +use Cwd qw();
> > +use Encode qw(decode encode);
> > +use File::Path qw(make_path);
> > +use File::Basename qw(dirname);
> > +use IO::File;
> > +use POSIX;
>
> The POSIX module exports a lot of stuff by default, even overriding some existing
> perl symbols IIRC, while most of the time one just needs some error constants or
> format time printing or the like, so explicitly importing what one uses is almost
> always the safer choice. There are some export groups for sets of exports, so that
> one can e.g. import qw(:errno_h) instead of every error number constant explicitly.
Oh good point, I hadn't considered that the POSIX module imports that
much.
>
> > +
> > +use PVE::ProcFSTools;
> > +use PVE::Tools qw(
> > + file_copy
> > + file_get_contents
> > + file_set_contents
> > + run_command
> > +);
> > +
> > +use base qw(PVE::Storage::Plugin);
> > +
> > +my $CLUSTER_KNOWN_HOSTS = "/etc/pve/priv/known_hosts";
>
> For intra-cluster ssh this shared known_host file is being deprecated in
> favor of using a per-node file, i.e. /etc/pve/nodes/NODENAME/ssh_known_hosts,
> it might be better to not re-use that here and rather use a dedicated file
> for this storage or directly encode it in the config entry (either passed by
> the user or TOFU on storage addition).
Do you mean a dedicated file for the entire storage plugin or one for
each configured SSHFS storage? Asking just in case because the
terminology can sometimes be a bit ambiguous.
I'm not sure if we should encode it in the config entry though as
there's no way to pass a single known_hosts entry directly
to `ssh` / `sshfs` AFAIK.
>
> > +
> > +# Plugin Definition
> > +
>
> > +sub properties {
> > + return {
> > + 'sshfs-remote-path' => {
> > + description => "Path on the remote filesystem used for SSHFS. Must be absolute.",
> > + type => 'string',
> > + format => 'pve-storage-path',
> > + },
> > + 'sshfs-private-key' => {
> > + description => "Path to the private key to use for SSHFS.",
> > + type => 'string',
> > + format => 'pve-storage-path',
> > + }
>
> We normally do not add the storage type as prefix for properties.
> FWIW, we could treat the private-key like a password and manage it ourselves
Yeah this was more catered towards external developers; my idea here was
that prefixing the property name in some way (doesn't necessarily need
to be the storage type) helps avoiding conflicts.
Specifically, if a plugin introduces a property that already exists it
will `die` on the call to `->init()`.
While the chance is low that users will install many different plugins
all at once (I assume), I've seen some plugins introduce properties with
names like "multipath", "apikey", "protocol", etc. that to me seem
common enough to eventually cause breakage if we introduce properties
with such names ourselves.
>
> > + };
> > +}
> > +
> > +sub options {
> > + return {
> > + disable => { optional => 1 },
> > + path => { fixed => 1 },
> > + 'create-base-path' => { optional => 1 },
> > + content => { optional => 1 },
> > + 'create-subdirs' => { optional => 1 },
> > + 'content-dirs' => { optional => 1 },
> > + 'prune-backups' => { optional => 1 },
> > + 'max-protected-backups' => { optional => 1 },
> > + format => { optional => 1 },
> > + bwlimit => { optional => 1 },
> > + preallocation => { optional => 1 },
> > + nodes => { optional => 1 },
> > + shared => { optional => 1 },
> > +
> > + # SSHFS Options
> > + username => {},
> > + server => {},
> > + 'sshfs-remote-path' => {},
> > + port => { optional => 1 },
> > + 'sshfs-private-key' => { optional => 1 },
> > + };
> > +}
> > +
> > +# SSHFS Helpers
> > +
> > +my sub sshfs_remote_from_config: prototype($) ($scfg) {
> > + my ($user, $host, $remote_path) = $scfg->@{qw(username server sshfs-remote-path)};
> > + return "${user}\@${host}:${remote_path}";
> > +}
> > +
> > +my sub sshfs_private_key_path: prototype($) ($storeid) {
> > + return "/etc/pve/priv/storage/$storeid.key";
> > +}
> > +
> > +my sub sshfs_common_ssh_opts: prototype($) ($storeid) {
> > + my $private_key_path = sshfs_private_key_path($storeid);
> > +
> > + my @common_opts = (
> > + '-o', "UserKnownHostsFile=${CLUSTER_KNOWN_HOSTS}",
> > + '-o', 'GlobalKnownHostsFile=none',
> > + '-o', "IdentityFile=${private_key_path}",
> > + );
> > +
> > + return @common_opts;
> > +}
> > +
> > +my sub sshfs_set_private_key: prototype($$) ($storeid, $src_key_path) {
> > + die "path of private key file not specified" if !defined($src_key_path);
> > + die "path of private key file does not exist" if ! -e $src_key_path;
> > + die "path of private key file does not point to a file" if ! -f $src_key_path;
>
> I mean, fine as this is more for better UX, but not really safe as there is
> a TOCTOU race here. If we really want to make it safe(r) we probably should
> not allow copying arbitrary files from the system and allow either passing a
> existing key to use as value, not the best UX, but it's not _that_ worse then
> the status quo IMO.
Oh yeah you're right, I hadn't considered that there's a TOCTOU race
here... I mean, I guess passing the key as value should work just fine
then, the user could always just do something like e.g.
pvesm add sshfs sshfs-storage [...] --sshfs-private-key $(cat ~/.ssh/id_sshfs-storage)
(Unless there's also something I'm overlooking here.)
>
> > +
> > + my $dest_key_path = sshfs_private_key_path($storeid);
> > +
> > + my $dest_key_parent_dir = dirname($dest_key_path);
> > + if (! -e $dest_key_parent_dir) {
> > + make_path($dest_key_parent_dir, { chmod => 0700 });
>
> chmod is useless for pmxcfs, as it controls those file attributes itself. While
> as is it shouldn't hurt, it might be confusing for devs basing off this plugin
> and wanting some other mode (on another path) and then wonder why this suddenly
> errors out; so maybe dropping the chmod and adding a comment that the file will
> reside below /etc/pve/priv, of which pmxcfs will always set the mode to 0700 for.
ACK, will remove the mode and add a comment. Will also do that below
for the call to `file_copy`.
>
> btw., does make_path even dies explicitly, or would you need to check the return
> value?
make_path will `die` on fatal errors, yes. All non-fatal errors will
emit a warning.
I double-checked just to be sure; `make_path` has only one "severe"
error that becomes fatal if it isn't trapped.
See: https://perldoc.perl.org/File::Path#ERROR-HANDLING
Even though none of the non-fatal errors for `make_path` seem to apply
in our case, I guess we could still use its trapping mechanism and
always `die` on our end. That would probably be the safest option.
>
> > + } else {
> > + die "'$dest_key_path' already exists" if -e $dest_key_path;
>
> Being able to override this on add might be nice though? And FWIW, there is
> some code deduplication with writing this file in the on_update_hook that
> might be avoidable I think.
You mean leaving the user to choose whether to overwrite the private key
or not? I mean, we could make the sshfs-private-key property optional,
and then still `die` if the private key doesn't exist after the storage
was added.
>
> > + }
> > +
> > + file_copy($src_key_path, $dest_key_path, undef, 600);
> > +
> > + return undef;
> > +}
> > +
> > +my sub sshfs_remove_private_key: prototype($) ($storeid) {
> > + my $key_path = sshfs_private_key_path($storeid);
> > + unlink($key_path) or $! == ENOENT or die "failed to remove private key '$key_path' - $!\n";
> > +
> > + return undef;
> > +}
> > +
> > +my sub sshfs_is_mounted: prototype($) ($scfg) {
> > + my $remote = sshfs_remote_from_config($scfg);
> > +
> > + my $mountpoint = Cwd::realpath($scfg->{path}); # Resolve symlinks
> > + return 0 if !defined($mountpoint);
> > +
> > + my $mountdata = PVE::ProcFSTools::parse_proc_mounts();
> > +
> > + my $has_found_mountpoint = grep {
> > + $_->[0] =~ m|^\Q${remote}\E$|
> > + && $_->[1] eq $mountpoint
> > + && $_->[2] eq 'fuse.sshfs'
> > + } $mountdata->@*;
> > +
> > + return $has_found_mountpoint != 0;
> > +}
> > +
> > +my sub sshfs_mount: prototype($$) ($scfg, $storeid) {
> > + my $remote = sshfs_remote_from_config($scfg);
> > + my ($port, $mountpoint) = $scfg->@{qw(port path)};
> > +
> > + my @common_opts = sshfs_common_ssh_opts($storeid);
> > + my $cmd = [
> > + '/usr/bin/sshfs', @common_opts,
> > + '-o', 'noatime',
> > + ];
> > +
> > + push($cmd->@*, '-p', $port) if $port;
> > + push($cmd->@*, $remote, $mountpoint);
> > +
> > + eval {
> > + run_command(
> > + $cmd,
> > + timeout => 10,
> > + errfunc => sub { warn "$_[0]\n"; },
> > + );
> > + };
> > + if (my $err = $@) {
> > + die "failed to mount SSHFS storage '$remote' at '$mountpoint': $@\n";
> > + }
> > +
> > + die "SSHFS storage '$remote' not mounted at '$mountpoint' despite reported success\n"
> > + if ! sshfs_is_mounted($scfg);
> > +
> > + return;
> > +}
> > +
> > +my sub sshfs_umount: prototype($) ($scfg) {
> > + my $mountpoint = $scfg->{path};
> > +
> > + my $cmd = ['/usr/bin/umount', $mountpoint];
> > +
> > + eval {
> > + run_command(
> > + $cmd,
> > + timeout => 10,
> > + errfunc => sub { warn "$_[0]\n"; },
> > + );
> > + };
> > + if (my $err = $@) {
> > + die "failed to unmount SSHFS at '$mountpoint': $err\n";
> > + }
> > +
> > + return;
> > +}
> > +
> > +# Storage Implementation
> > +
> > +sub on_add_hook ($class, $storeid, $scfg, %sensitive) {
> > + $scfg->{shared} = 1; # mark SSHFS storages as shared by default
> > +
> > + eval {
> > + my $src_key_path = $sensitive{'sshfs-private-key'};
> > + sshfs_set_private_key($storeid, $src_key_path);
> > + };
> > + die "error while adding SSHFS storage '${storeid}': $@\n" if $@;
> > +
> > + return undef;
> > +}
> > +
> > +sub on_update_hook ($class, $storeid, $scfg, %sensitive) {
> > + return undef if !exists($sensitive{'sshfs-private-key'});
> > +
> > + my $src_key_path = $sensitive{'sshfs-private-key'};
> > +
> > + if (!defined($src_key_path)) {
> > + warn "removing private key for SSHFS storage '${storeid}'";
> > + warn "the storage might not be mountable without a private key!";
> > +
> > + eval { sshfs_remove_private_key($storeid); };
> > + die $@ if $@;
> > +
> > + return undef;
> > + }
> > +
> > + my $dest_key_path = sshfs_private_key_path($storeid);
> > + my $dest_key_path_tmp = "${dest_key_path}.old";
> > +
> > + file_copy($dest_key_path, $dest_key_path_tmp) if -e $dest_key_path;
> > +
> > + eval { file_copy($src_key_path, $dest_key_path, undef, 600); };
> > +
> > + if (my $err = $@) {
> > + if (-e $dest_key_path_tmp) {
> > + warn "attempting to restore previous private key for storage '${storeid}'\n";
> > + eval { file_copy($dest_key_path_tmp, $dest_key_path, undef, 600); };
> > + warn "$@\n" if $@;
> > +
> > + unlink $dest_key_path_tmp;
> > + }
> > +
> > + die "failed to set private key for SSHFS storage '${storeid}': $err\n";
> > + }
> > +
> > + unlink $dest_key_path_tmp;
> > +
> > + return undef;
> > +}
> > +
> > +sub on_delete_hook ($class, $storeid, $scfg) {
> > + eval { sshfs_remove_private_key($storeid); };
> > + warn $@ if $@;
> > +
> > + eval { sshfs_umount($scfg) if sshfs_is_mounted($scfg); };
> > + warn $@ if $@;
> > +
> > + return undef;
> > +}
> > +
> > +sub check_connection ($class, $storeid, $scfg) {
> > + my ($user, $host, $port) = $scfg->@{qw(username server port)};
> > +
> > + my @common_opts = sshfs_common_ssh_opts($storeid);
> > + my $cmd = [
> > + '/usr/bin/ssh',
> > + '-T',
> > + @common_opts,
> > + '-o', 'BatchMode=yes',
> > + '-o', 'ConnectTimeout=5',
> > + ];
> > +
> > + push($cmd->@*, "-p", $port) if $port;
> > + push($cmd->@*, "${user}\@${host}", 'exit 0');
> > +
> > + eval {
> > + run_command(
> > + $cmd,
> > + timeout => 10,
> > + errfunc => sub { warn "$_[0]\n"; },
> > + );
> > + };
> > + if (my $err = $@) {
> > + warn "$err";
> > + return 0;
> > + }
> > +
> > + return 1;
> > +}
> > +
> > +sub activate_storage ($class, $storeid, $scfg, $cache) {
> > + my $mountpoint = $scfg->{path};
> > +
> > + if (!sshfs_is_mounted($scfg)) {
> > + if ($scfg->{'create-base-path'} // 1) {
> > + make_path($mountpoint);
> > + }
> > +
> > + die "unable to activate storage '$storeid' - directory '$mountpoint' does not exist\n"
> > + if ! -d $mountpoint;
> > +
> > + sshfs_mount($scfg, $storeid);
> > + }
> > +
> > + $class->SUPER::activate_storage($storeid, $scfg, $cache);
> > + return;
> > +}
> > +
> > +sub get_volume_attribute ($class, $scfg, $storeid, $volname, $attribute) {
> > + my ($vtype) = $class->parse_volname($volname);
> > + return if $vtype ne 'backup';
> > +
> > + my $volume_path = PVE::Storage::Plugin->filesystem_path($scfg, $volname);
> > +
> > + if ($attribute eq 'notes') {
> > + my $notes_path = $volume_path . $class->SUPER::NOTES_EXT;
> > +
> > + if (-f $notes_path) {
> > + my $notes = file_get_contents($notes_path);
> > + return eval { decode('UTF-8', $notes, 1) } // $notes;
> > + }
> > +
> > + return "";
> > + }
> > +
> > + if ($attribute eq 'protected') {
> > + return -e PVE::Storage::protection_file_path($volume_path) ? 1 : 0;
> > + }
> > +
> > + return;
> > +}
> > +
> > +sub update_volume_attribute ($class, $scfg, $storeid, $volname, $attribute, $value) {
> > + my ($vtype, $name) = $class->parse_volname($volname);
> > + die "only backups support attribute '$attribute'\n" if $vtype ne 'backup';
> > +
> > + my $volume_path = PVE::Storage::Plugin->filesystem_path($scfg, $volname);
> > +
> > + if ($attribute eq 'notes') {
> > + my $notes_path = $volume_path . $class->SUPER::NOTES_EXT;
> > +
> > + if (defined($value)) {
> > + my $encoded_notes = encode('UTF-8', $value);
> > + file_set_contents($notes_path, $encoded_notes);
> > + } else {
> > + unlink $notes_path or $! == ENOENT or die "could not delete notes - $!\n";
> > + }
> > +
> > + return;
> > + }
> > +
> > + if ($attribute eq 'protected') {
> > + my $protection_path = PVE::Storage::protection_file_path($volume_path);
> > +
> > + # Protection already set or unset
> > + return if !((-e $protection_path) xor $value);
> > +
> > + if ($value) {
> > + my $fh = IO::File->new($protection_path, O_CREAT, 0644)
> > + or die "unable to create protection file '$protection_path' - $!\n";
> > + close($fh);
> > + } else {
> > + unlink $protection_path or $! == ENOENT
> > + or die "could not delete protection file '$protection_path' - $!\n";
> > + }
> > +
> > + return;
> > + }
> > +
> > + die "attribute '$attribute' is not supported for storage type '$scfg->{type}'\n";
> > +}
> > +
> > +1;
More information about the pve-devel
mailing list