* [pve-devel] [PATCH storage manager docs] Allow mounting of CIFS subdirectories @ 2022-12-01 11:32 Leo Nunner 2022-12-01 11:32 ` [pve-devel] [PATCH storage 1/1] fix #2641: allow " Leo Nunner ` (3 more replies) 0 siblings, 4 replies; 7+ messages in thread From: Leo Nunner @ 2022-12-01 11:32 UTC (permalink / raw) To: pve-devel CIFS supports mounting subdirectories inside a share, so it makes sense to also have the 'subdir' parameter for the CIFS backend. I'm also looking into allowing overrides for all the fixed directories, but even then, I think it makes sense to support this parameter (for either providing a prefix to all the other directories, or just changing the base directory). storage: Leo Nunner (1): fix #2641: allow mounting of CIFS subdirectories PVE/Storage/CIFSPlugin.pm | 39 ++++++++++++++++++++----------------- PVE/Storage/CephFSPlugin.pm | 4 ---- PVE/Storage/Plugin.pm | 5 +++++ 3 files changed, 26 insertions(+), 22 deletions(-) manager: Leo Nunner (1): fix #2641: expose CIFS subdir parameter through GUI www/manager6/storage/CIFSEdit.js | 11 +++++++++++ 1 file changed, 11 insertions(+) docs: Leo Nunner (1): fix #2641: document subdir parameter for CIFS backend pve-storage-cifs.adoc | 5 +++++ 1 file changed, 5 insertions(+) -- 2.30.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH storage 1/1] fix #2641: allow mounting of CIFS subdirectories 2022-12-01 11:32 [pve-devel] [PATCH storage manager docs] Allow mounting of CIFS subdirectories Leo Nunner @ 2022-12-01 11:32 ` Leo Nunner 2023-02-07 14:58 ` [pve-devel] applied: " Thomas Lamprecht 2022-12-01 11:32 ` [pve-devel] [PATCH manager 1/1] fix #2641: expose CIFS subdir parameter through GUI Leo Nunner ` (2 subsequent siblings) 3 siblings, 1 reply; 7+ messages in thread From: Leo Nunner @ 2022-12-01 11:32 UTC (permalink / raw) To: pve-devel 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. Signed-off-by: Leo Nunner <l.nunner@proxmox.com> --- PVE/Storage/CIFSPlugin.pm | 39 ++++++++++++++++++++----------------- PVE/Storage/CephFSPlugin.pm | 4 ---- PVE/Storage/Plugin.pm | 5 +++++ 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm index 982040a..dc41e0b 100644 --- a/PVE/Storage/CIFSPlugin.pm +++ b/PVE/Storage/CIFSPlugin.pm @@ -13,11 +13,16 @@ use base qw(PVE::Storage::Plugin); # CIFS helper functions -sub cifs_is_mounted { - my ($server, $share, $mountpoint, $mountdata) = @_; +sub cifs_is_mounted : prototype($$) { + my ($scfg, $mountdata) = @_; + + my $mountpoint = $scfg->{path}; + my $server = $scfg->{server}; + my $share = $scfg->{share}; + my $subdir = $scfg->{subdir} // "/"; $server = "[$server]" if Net::IP::ip_is_ipv6($server); - my $source = "//${server}/$share"; + my $source = "//${server}/$share$subdir"; $mountdata = PVE::ProcFSTools::parse_proc_mounts() if !$mountdata; return $mountpoint if grep { @@ -63,11 +68,16 @@ sub get_cred_file { return undef; } -sub cifs_mount { - my ($server, $share, $mountpoint, $storeid, $smbver, $user, $domain) = @_; +sub cifs_mount : prototype($$$$$) { + my ($scfg, $storeid, $smbver, $user, $domain) = @_; + + my $mountpoint = $scfg->{path}; + my $server = $scfg->{server}; + my $share = $scfg->{share}; + my $subdir = $scfg->{subdir} // "/"; $server = "[$server]" if Net::IP::ip_is_ipv6($server); - my $source = "//${server}/$share"; + my $source = "//${server}/$share$subdir"; my $cmd = ['/bin/mount', '-t', 'cifs', $source, $mountpoint, '-o', 'soft', '-o']; @@ -130,6 +140,7 @@ sub options { path => { fixed => 1 }, server => { fixed => 1 }, share => { fixed => 1 }, + subdir => { optional => 1 }, nodes => { optional => 1 }, disable => { optional => 1 }, maxfiles => { optional => 1 }, @@ -204,12 +215,8 @@ sub status { $cache->{mountdata} = PVE::ProcFSTools::parse_proc_mounts() if !$cache->{mountdata}; - my $path = $scfg->{path}; - my $server = $scfg->{server}; - my $share = $scfg->{share}; - return undef - if !cifs_is_mounted($server, $share, $path, $cache->{mountdata}); + if !cifs_is_mounted($scfg, $cache->{mountdata}); return $class->SUPER::status($storeid, $scfg, $cache); } @@ -221,17 +228,15 @@ sub activate_storage { if !$cache->{mountdata}; my $path = $scfg->{path}; - my $server = $scfg->{server}; - my $share = $scfg->{share}; - if (!cifs_is_mounted($server, $share, $path, $cache->{mountdata})) { + if (!cifs_is_mounted($scfg, $cache->{mountdata})) { mkpath $path if !(defined($scfg->{mkdir}) && !$scfg->{mkdir}); die "unable to activate storage '$storeid' - " . "directory '$path' does not exist\n" if ! -d $path; - cifs_mount($server, $share, $path, $storeid, $scfg->{smbversion}, + cifs_mount($scfg, $storeid, $scfg->{smbversion}, $scfg->{username}, $scfg->{domain}); } @@ -245,10 +250,8 @@ sub deactivate_storage { if !$cache->{mountdata}; my $path = $scfg->{path}; - my $server = $scfg->{server}; - my $share = $scfg->{share}; - if (cifs_is_mounted($server, $share, $path, $cache->{mountdata})) { + if (cifs_is_mounted($scfg, $cache->{mountdata})) { my $cmd = ['/bin/umount', $path]; run_command($cmd, errmsg => 'umount error'); } diff --git a/PVE/Storage/CephFSPlugin.pm b/PVE/Storage/CephFSPlugin.pm index 4976747..944f0b8 100644 --- a/PVE/Storage/CephFSPlugin.pm +++ b/PVE/Storage/CephFSPlugin.pm @@ -127,10 +127,6 @@ sub properties { description => "Mount CephFS through FUSE.", type => 'boolean', }, - subdir => { - description => "Subdir to mount.", - type => 'string', format => 'pve-storage-path', - }, 'fs-name' => { description => "The Ceph filesystem name.", type => 'string', format => 'pve-configid', diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm index 8a41df1..af53a99 100644 --- a/PVE/Storage/Plugin.pm +++ b/PVE/Storage/Plugin.pm @@ -169,6 +169,11 @@ my $defaultData = { type => 'boolean', optional => 1, }, + subdir => { + description => "Subdir to mount.", + type => 'string', format => 'pve-storage-path', + optional => 1, + }, 'format' => { description => "Default image format.", type => 'string', format => 'pve-storage-format', -- 2.30.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] applied: [PATCH storage 1/1] fix #2641: allow mounting of CIFS subdirectories 2022-12-01 11:32 ` [pve-devel] [PATCH storage 1/1] fix #2641: allow " Leo Nunner @ 2023-02-07 14:58 ` Thomas Lamprecht 0 siblings, 0 replies; 7+ messages in thread From: Thomas Lamprecht @ 2023-02-07 14:58 UTC (permalink / raw) To: Proxmox VE development discussion, Leo Nunner Am 01/12/2022 um 12:32 schrieb Leo Nunner: > 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. > > Signed-off-by: Leo Nunner <l.nunner@proxmox.com> > --- > PVE/Storage/CIFSPlugin.pm | 39 ++++++++++++++++++++----------------- > PVE/Storage/CephFSPlugin.pm | 4 ---- > PVE/Storage/Plugin.pm | 5 +++++ > 3 files changed, 26 insertions(+), 22 deletions(-) > > the patch does more at once than ideal, I'd recommend doing such things as the signature cleanups, which should not have any change functional effect after all, first - that simplify review and also working with the history (e.g., bisect or just wanting to see how a specific feature was done if adding a similar one). But this was relatively minor crowding here, and I did not wanted to delay this further... so applied, with some cleanups as follow up, thanks! ps. the safer fallback for $subdir would have been a empty string '', as then it'd be 1:1 the exact same value as before if one doesn't have any subdir configured - but lets hope no cifs client/server throws up on a newly added trailing slash. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH manager 1/1] fix #2641: expose CIFS subdir parameter through GUI 2022-12-01 11:32 [pve-devel] [PATCH storage manager docs] Allow mounting of CIFS subdirectories Leo Nunner 2022-12-01 11:32 ` [pve-devel] [PATCH storage 1/1] fix #2641: allow " Leo Nunner @ 2022-12-01 11:32 ` Leo Nunner 2022-12-01 11:32 ` [pve-devel] [PATCH docs 1/1] fix #2641: document subdir parameter for CIFS backend Leo Nunner 2023-02-02 14:20 ` [pve-devel] [PATCH storage manager docs] Allow mounting of CIFS subdirectories Leo Nunner 3 siblings, 0 replies; 7+ messages in thread From: Leo Nunner @ 2022-12-01 11:32 UTC (permalink / raw) To: pve-devel makes it possible to optionally set the 'subdir' parameter when adding a new CIFS storage. Signed-off-by: Leo Nunner <l.nunner@proxmox.com> --- RFC: I'm not sure whether this should be exposed, since it's not available for CephFS either, but it might be a good idea to make this more obvious to the user. www/manager6/storage/CIFSEdit.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/www/manager6/storage/CIFSEdit.js b/www/manager6/storage/CIFSEdit.js index 71415401..4c06de5e 100644 --- a/www/manager6/storage/CIFSEdit.js +++ b/www/manager6/storage/CIFSEdit.js @@ -129,6 +129,9 @@ Ext.define('PVE.storage.CIFSInputPanel', { if (values.username?.length === 0) { delete values.username; } + if (values.subdir?.length === 0) { + delete values.subdir; + } return me.callParent([values]); }, @@ -216,6 +219,14 @@ Ext.define('PVE.storage.CIFSInputPanel', { }, }, }, + { + xtype: 'pmxDisplayEditField', + editable: me.isCreate, + name: 'subdir', + fieldLabel: 'Subdirectory', + allowBlank: true, + emptyText: gettext('/some/path'), + }, ]; me.callParent(); -- 2.30.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] [PATCH docs 1/1] fix #2641: document subdir parameter for CIFS backend 2022-12-01 11:32 [pve-devel] [PATCH storage manager docs] Allow mounting of CIFS subdirectories Leo Nunner 2022-12-01 11:32 ` [pve-devel] [PATCH storage 1/1] fix #2641: allow " Leo Nunner 2022-12-01 11:32 ` [pve-devel] [PATCH manager 1/1] fix #2641: expose CIFS subdir parameter through GUI Leo Nunner @ 2022-12-01 11:32 ` Leo Nunner 2023-02-02 14:20 ` [pve-devel] [PATCH storage manager docs] Allow mounting of CIFS subdirectories Leo Nunner 3 siblings, 0 replies; 7+ messages in thread From: Leo Nunner @ 2022-12-01 11:32 UTC (permalink / raw) To: pve-devel Signed-off-by: Leo Nunner <l.nunner@proxmox.com> --- pve-storage-cifs.adoc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pve-storage-cifs.adoc b/pve-storage-cifs.adoc index bb4b902..15f8034 100644 --- a/pve-storage-cifs.adoc +++ b/pve-storage-cifs.adoc @@ -57,6 +57,10 @@ path:: The local mount point. Optional, defaults to `/mnt/pve/<STORAGE_ID>/`. +subdir:: + +The subdirectory of the share to mount. Optional, defaults to the root directory of the share. + .Configuration Example (`/etc/pve/storage.cfg`) ---- cifs: backup @@ -66,6 +70,7 @@ cifs: backup content backup username anna smbversion 3 + subdir /data ---- -- 2.30.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pve-devel] [PATCH storage manager docs] Allow mounting of CIFS subdirectories 2022-12-01 11:32 [pve-devel] [PATCH storage manager docs] Allow mounting of CIFS subdirectories Leo Nunner ` (2 preceding siblings ...) 2022-12-01 11:32 ` [pve-devel] [PATCH docs 1/1] fix #2641: document subdir parameter for CIFS backend Leo Nunner @ 2023-02-02 14:20 ` Leo Nunner 2023-02-07 14:58 ` [pve-devel] applied: " Thomas Lamprecht 3 siblings, 1 reply; 7+ messages in thread From: Leo Nunner @ 2023-02-02 14:20 UTC (permalink / raw) To: pve-devel On 2022-12-01 12:32, Leo Nunner wrote: > CIFS supports mounting subdirectories inside a share, so it makes sense > to also have the 'subdir' parameter for the CIFS backend. I'm also > looking into allowing overrides for all the fixed directories, but > even then, I think it makes sense to support this parameter (for either > providing a prefix to all the other directories, or just changing the > base directory). > > storage: > > Leo Nunner (1): > fix #2641: allow mounting of CIFS subdirectories > > PVE/Storage/CIFSPlugin.pm | 39 ++++++++++++++++++++----------------- > PVE/Storage/CephFSPlugin.pm | 4 ---- > PVE/Storage/Plugin.pm | 5 +++++ > 3 files changed, 26 insertions(+), 22 deletions(-) > > manager: > > Leo Nunner (1): > fix #2641: expose CIFS subdir parameter through GUI > > www/manager6/storage/CIFSEdit.js | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > docs: > > Leo Nunner (1): > fix #2641: document subdir parameter for CIFS backend > > pve-storage-cifs.adoc | 5 +++++ > 1 file changed, 5 insertions(+) Bump due to continued user demand. storage and manager seem to still apply, while docs produces a merge conflict because it clashes with the "content-dirs" documentation. Does this warrant a v2? ^ permalink raw reply [flat|nested] 7+ messages in thread
* [pve-devel] applied: [PATCH storage manager docs] Allow mounting of CIFS subdirectories 2023-02-02 14:20 ` [pve-devel] [PATCH storage manager docs] Allow mounting of CIFS subdirectories Leo Nunner @ 2023-02-07 14:58 ` Thomas Lamprecht 0 siblings, 0 replies; 7+ messages in thread From: Thomas Lamprecht @ 2023-02-07 14:58 UTC (permalink / raw) To: Proxmox VE development discussion, Leo Nunner Am 02/02/2023 um 15:20 schrieb Leo Nunner: > On 2022-12-01 12:32, Leo Nunner wrote: >> CIFS supports mounting subdirectories inside a share, so it makes sense >> to also have the 'subdir' parameter for the CIFS backend. I'm also >> looking into allowing overrides for all the fixed directories, but >> even then, I think it makes sense to support this parameter (for either >> providing a prefix to all the other directories, or just changing the >> base directory). >> >> storage: >> >> Leo Nunner (1): >> fix #2641: allow mounting of CIFS subdirectories >> >> PVE/Storage/CIFSPlugin.pm | 39 ++++++++++++++++++++----------------- >> PVE/Storage/CephFSPlugin.pm | 4 ---- >> PVE/Storage/Plugin.pm | 5 +++++ >> 3 files changed, 26 insertions(+), 22 deletions(-) >> >> manager: >> >> Leo Nunner (1): >> fix #2641: expose CIFS subdir parameter through GUI >> >> www/manager6/storage/CIFSEdit.js | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> docs: >> >> Leo Nunner (1): >> fix #2641: document subdir parameter for CIFS backend >> >> pve-storage-cifs.adoc | 5 +++++ >> 1 file changed, 5 insertions(+) > > Bump due to continued user demand. storage and manager seem to still storage produces a merge conflict, that git could auto-merge. Would be great if you could send a v2 for manager and docs, as those I'd apply then once storage is bumped. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-02-07 14:58 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-12-01 11:32 [pve-devel] [PATCH storage manager docs] Allow mounting of CIFS subdirectories Leo Nunner 2022-12-01 11:32 ` [pve-devel] [PATCH storage 1/1] fix #2641: allow " Leo Nunner 2023-02-07 14:58 ` [pve-devel] applied: " Thomas Lamprecht 2022-12-01 11:32 ` [pve-devel] [PATCH manager 1/1] fix #2641: expose CIFS subdir parameter through GUI Leo Nunner 2022-12-01 11:32 ` [pve-devel] [PATCH docs 1/1] fix #2641: document subdir parameter for CIFS backend Leo Nunner 2023-02-02 14:20 ` [pve-devel] [PATCH storage manager docs] Allow mounting of CIFS subdirectories Leo Nunner 2023-02-07 14:58 ` [pve-devel] applied: " Thomas Lamprecht
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.Service provided by Proxmox Server Solutions GmbH | Privacy | Legal