Re: [pve-devel] [PATCH guest-common 1/1] vzdump: schema: add 'notes' and 'protected' properties

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 17.03.22 08:57, Fabian Ebner wrote:
> Am 16.03.22 um 19:25 schrieb Thomas Lamprecht:
>> On 16.03.22 12:04, Fabian Ebner wrote:
>>> Am 16.12.21 um 13:12 schrieb Fabian Ebner:
>>>
>>> Fabian G.:
>>> we could offer something like a simple template system that allows
>>> substitution of certain variables (like name, or source node
>>> hostname/clustername, ..). or just a boolean switch for setting VM/CT
>>> $HOSTNAME from $CLUSTER/$NODENAME (or an enum, with
>>> [job-comment,hostname,long,none] where long is that, and hostname is
>>> just the guest hostname, and job-comment is the comment of the vzdump
>>> job if one is set)
>>>
>>> Me:
>>> The template variant would be the most flexible one and would avoid the
>>> need for a second vzdump option besides --notes. Ideally, support for it
>>> would be there from the beginning though, as otherwise it will stop
>>> working for a user wanting to literally set $HOSTNAME when we add it ;)
>>> The downside is that it doesn't match the volume-level --notes option,
>>> but I don't think that should be a big deal.
>>>
>>> Fabian G.:
>>> well it could just be called notes-template for vzdump to disambiguate?
>>
>>
>> fwiw, I believe I commented that approach in the internal chat a while ago,
>> but as its search functions are abysmal I don't find it anymore.
>>
>> IIRC, just extend what we have now and allow a fixed set of {VARS} (vmid,
>> guest name, host name, job-id, ..?).
> 
> I might be misunderstanding, but we don't have anything right now,
> because this patch would be the one introducing the option?
> 

yeah in this vzdump CLI context I was confusing, I talked more from the
job POV.

>>
>> While extending one has a slight chance of changing an existing setup I find
>> this very unlikely in this specific case, as we had no such feature whatsoever
>> and it makes not sense in any practical example to use such special strings
>> for a backup comment.
> 
> Yes, I'd simply document the list of currently valid variables, and that
> it might be extended in the future.
> 

k.

a subset of the VM config could be possibly interesting too (e.g., ostype, first
line of description) but as the config is already in the backup it may not be worth
it - so maybe better to wait for the non-obvious variables for users to actually
requesting them.

>>
>> That said, if one can whip up another reason besides backward compat for
>> having a separate flag to turn this on/off then feel free to comment.
>>
>> I mean, for the backup jobs itself it could have some value to differ
>> between the comment about the job itself and a comment template for the
>> resulting backups.
> 
> Yes, I think it'd be better to not mix the job's comment (which is part
> of the generic job properties) and the vzdump-specific notes{-template}
> which this patch (or rather a future version of it) will introduce.

Agree. So, to summarize, vzdump does the interpreting for a plain, new `--notes`
CLI which it also prints (with variables already resolved) in the task log and
sets that also as note for the (created) backup.

The job config would get a new notes-template config that allows to add such
a dynamically interpreted string to each backup created by this job by bassing
it as --notes to vzdump. That way we avoid vzdump having two different, clashing,
CLI options but keep it cleanly separated for the job definitions.