On Wed, Aug 23, 2017 at 10:59:55AM +0200, Wolfgang Link wrote:
> ---
>  PVE/CLI/pvesm.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/CLI/pvesm.pm b/PVE/CLI/pvesm.pm
> index 9455595..6b37c05 100755
> --- a/PVE/CLI/pvesm.pm
> +++ b/PVE/CLI/pvesm.pm
> @@ -183,7 +183,7 @@ __PACKAGE__->register_method ({
>           base => {
>               description => "Snapshot to start an incremental stream from",
>               type => 'string',
> -             pattern => qr/[a-z0-9_\-]{1,40}/,
> +             pattern => qr/[a-zA-Z0-9_\-]{1,40}/,

wouldn't it make more sense to define a new standard option for this
instead of defining the same pattern and length in multiple places?

we already have the option 'pve-snapshot-name', defined in QemuServer.pm
as up to 40 character long string with a format 'pve-configid', which is
defined in JSONSchema.pm as m/^[a-z][a-z0-9_]+$/i . IIRC storage
migration and replication have their own naming scheme starting with
'__' and including '-', but only pvesm export/import expose this over
the API (since pvesr works around this by having the sync timestamp as
parameter, and recreating the snapshot name based on this and other
metadata?).

IMHO, we should define the new option to be a combination of the
existing pve-snapshot-name, and the specific naming scheme(s) used by our
code which deviates from pve-snapshot-name on purpose to avoid naming
conflicts.

>               maxLength => 40,
>               optional => 1,
>           },
> @@ -258,7 +258,7 @@ __PACKAGE__->register_method ({
>           base => {
>               description => "Base snapshot of an incremental stream",
>               type => 'string',
> -             pattern => qr/[a-z0-9_\-]{1,40}/,
> +             pattern => qr/[a-zA-Z0-9_\-]{1,40}/,

see above ;)

>               maxLength => 40,
>               optional => 1,
>           },
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to