[pve-devel] [PATCH storage 1/2] plugin schema: improve description of 'shared' property

[pve-devel] [PATCH storage 1/2] plugin schema: improve description of 'shared' property

[Fiona Ebner]

It's not clear to users what this property does otherwise. Latest
report from the community forum:

https://forum.proxmox.com/threads/134393

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/Storage/Plugin.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index 815773b..dc89d13 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -167,7 +167,9 @@ my $defaultData = {
 	    default => "Unlimited for users with Datastore.Allocate privilege, 5 for other users",
 	},
 	shared => {
-	    description => "Mark storage as shared.",
+	    description => "Indicate that this is a single storage which is already accessible by "
+		."all nodes (or all listed in the 'nodes' option). Will not make a local storage "
+		."automatically accessible to other nodes!",
 	    type => 'boolean',
 	    optional => 1,
 	},
-- 
2.39.2

[pve-devel] [PATCH storage 2/2] plugin schema: improve description of 'nodes' property

[Fiona Ebner]

The default description doesn't make it clear what the property is
used for in the context of storages.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/Storage/Plugin.pm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
index dc89d13..953b785 100644
--- a/src/PVE/Storage/Plugin.pm
+++ b/src/PVE/Storage/Plugin.pm
@@ -138,7 +138,10 @@ my $defaultData = {
 	type => { description => "Storage type." },
 	storage => get_standard_option('pve-storage-id',
 	    { completion => \&PVE::Storage::complete_storage }),
-	nodes => get_standard_option('pve-node-list', { optional => 1 }),
+	nodes => get_standard_option('pve-node-list', {
+	    description => "List of nodes for which the storage configuration applies.",
+	    optional => 1,
+	}),
 	content => {
 	    description => "Allowed content types.\n\nNOTE: the value " .
 		"'rootdir' is used for Containers, and value 'images' for VMs.\n",
-- 
2.39.2

Re: [pve-devel] [PATCH storage 1/2] plugin schema: improve description of 'shared' property

[Fiona Ebner]

Am 04.10.23 um 09:35 schrieb Fiona Ebner:
> It's not clear to users what this property does otherwise. Latest
> report from the community forum:
> 
> https://forum.proxmox.com/threads/134393
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>

Ping

Re: [pve-devel] [PATCH storage 1/2] plugin schema: improve description of 'shared' property

[Fabian Grünbichler]

On October 4, 2023 9:35 am, Fiona Ebner wrote:
> It's not clear to users what this property does otherwise. Latest
> report from the community forum:
> 
> https://forum.proxmox.com/threads/134393
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/Storage/Plugin.pm | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index 815773b..dc89d13 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -167,7 +167,9 @@ my $defaultData = {
>  	    default => "Unlimited for users with Datastore.Allocate privilege, 5 for other users",
>  	},
>  	shared => {
> -	    description => "Mark storage as shared.",
> +	    description => "Indicate that this is a single storage which is already accessible by "
> +		."all nodes (or all listed in the 'nodes' option). Will not make a local storage "
> +		."automatically accessible to other nodes!",

I am not sure the 'already' helps here if you don't already (heh) know
what 'shared' means. it also doesn't include that it means the same
contents on all nodes - a local storage is also accessible on all nodes
(or those listed in the 'nodes' option) after all, but it's not
shared/has different contents on each node ;)

maybe something like

Indicate that this storage has the same content on all nodes that can
access it. It will not automatically make a local storage shared, it
just marks already shared storages as such!

?

there's also content in pve-docs that would need to be harmonized after
such a change, e.g. the "common storage properties", but maybe also
other parts of pvesm.adoc

>  	    type => 'boolean',
>  	    optional => 1,
>  	},
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

[pve-devel] applied: [PATCH storage 2/2] plugin schema: improve description of 'nodes' property

[Fabian Grünbichler]

applied this on already, thanks!

On October 4, 2023 9:35 am, Fiona Ebner wrote:
> The default description doesn't make it clear what the property is
> used for in the context of storages.
> 
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>  src/PVE/Storage/Plugin.pm | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Storage/Plugin.pm b/src/PVE/Storage/Plugin.pm
> index dc89d13..953b785 100644
> --- a/src/PVE/Storage/Plugin.pm
> +++ b/src/PVE/Storage/Plugin.pm
> @@ -138,7 +138,10 @@ my $defaultData = {
>  	type => { description => "Storage type." },
>  	storage => get_standard_option('pve-storage-id',
>  	    { completion => \&PVE::Storage::complete_storage }),
> -	nodes => get_standard_option('pve-node-list', { optional => 1 }),
> +	nodes => get_standard_option('pve-node-list', {
> +	    description => "List of nodes for which the storage configuration applies.",
> +	    optional => 1,
> +	}),
>  	content => {
>  	    description => "Allowed content types.\n\nNOTE: the value " .
>  		"'rootdir' is used for Containers, and value 'images' for VMs.\n",
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
>