Re: [pve-devel] [PATCH manager v5 02/16] api: jobs: vzdump: pass job 'id' parameter

[Fiona Ebner <f.ebner@proxmox.com>]

Am 15.04.24 um 10:26 schrieb Lukas Wagner:
> This allows us to access us the backup job id in the send_notification
> function, where we can set it as metadata for the notification.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>  PVE/API2/VZDump.pm | 8 ++++++++
>  PVE/Jobs/VZDump.pm | 4 +++-
>  PVE/VZDump.pm      | 6 +++---
>  3 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/PVE/API2/VZDump.pm b/PVE/API2/VZDump.pm
> index f66fc740..6bc0b792 100644
> --- a/PVE/API2/VZDump.pm
> +++ b/PVE/API2/VZDump.pm
> @@ -52,6 +52,14 @@ __PACKAGE__->register_method ({
>      parameters => {
>  	additionalProperties => 0,
>  	properties => PVE::VZDump::Common::json_config_properties({
> +	    id => {
It's very generic and it's not the ID of the invocation, so maybe 'job-id'?

Hmm, ideally it would be internal-use only. Like this we have to
remember never to fully "trust" this 'id', because a user can lie when
issuing a vzdump API call.

> +		description => "The ID of the backup job. If set, the 'backup-job' metadata field"
> +		    . " of the backup notification will be set to this value.",
> +		type => 'string',
> +		format => 'pve-configid',
> +		maxLength => 64,

Hmm, where does that limit come from? Seems like we do not explicitly
limit it yet upon job creation, which we should too! For creation, it
will fail for long lengths with

500 unable to open file
'/var/lib/pve-manager/jobs/vzdump-backup-a01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789.json.tmp.200643'
- File name too long

Seems like the limit is 256, so a bit shorter for backup job IDs, but
IDs longer than 64 can exist right now. We can either choose 64, which
might break it for a (small) number of users or choose a limit closer to
256.

> +		optional => 1,
> +	    },
>  	    stdout => {
>  		type => 'boolean',
>  		description => "Write tar to stdout, not to a file.",


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