From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pve-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id 95C451FF380
	for <inbox@lore.proxmox.com>; Fri, 19 Apr 2024 13:53:50 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id A89C977F5;
	Fri, 19 Apr 2024 13:53:50 +0200 (CEST)
Message-ID: <4c53ae4f-9f21-4860-99ba-a7f7ce597285@proxmox.com>
Date: Fri, 19 Apr 2024 13:53:16 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
 Lukas Wagner <l.wagner@proxmox.com>
References: <20240415082614.2515677-1-l.wagner@proxmox.com>
 <20240415082614.2515677-3-l.wagner@proxmox.com>
Content-Language: en-US
From: Fiona Ebner <f.ebner@proxmox.com>
In-Reply-To: <20240415082614.2515677-3-l.wagner@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.194 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 KAM_LOTSOFHASH           0.25 Emails with lots of hash-like gibberish
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pve-devel] [PATCH manager v5 02/16] api: jobs: vzdump: pass
 job 'id' parameter
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pve-devel-bounces@lists.proxmox.com
Sender: "pve-devel" <pve-devel-bounces@lists.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