Re: [pve-devel] [PATCH qemu-server] api: snapshot create/delete/rollback: print log line that mentions snapshot name

[Dominik Csapak <d.csapak@proxmox.com>]

On 5/29/24 12:09, Fabian Grünbichler wrote:
> On May 29, 2024 11:20 am, Dominik Csapak wrote:
>> so that an admin can see from the task logs which snapshot was rolled
>> back/created/removed without the need to look into the journal (to which
>> the admin might not have access to).
> 
> good idea in general, but wouldn't it be better to log it before
> attempting the operation, so that the context is in the task log
> irrespective of how a potential error message looks like?

i'd probably log both

> 
> (side note, we might want to think about making the whole snapshot
> process a bit more verbose? i.e., log the individual parts as well?)

i noticed that i missed doing that for containers .
also for some storages there is already output (e.g. ceph logs the
snapshot progress)

For a v2 i'd do the logging in 'snapshot_create' etc. in AbstractConfig,
that should handle both vms and ct, and there we can print between
phases too

does that sound ok?

> 
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> noticed while trying to find out, to which snapshot a vm was rolled back
>> to on a host where i don't have access to the syslog ;)
>>
>>   PVE/API2/Qemu.pm | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
>> index 2a1d4d79..903c60a4 100644
>> --- a/PVE/API2/Qemu.pm
>> +++ b/PVE/API2/Qemu.pm
>> @@ -5196,6 +5196,7 @@ __PACKAGE__->register_method({
>>   	    PVE::Cluster::log_msg('info', $authuser, "snapshot VM $vmid: $snapname");
>>   	    PVE::QemuConfig->snapshot_create($vmid, $snapname, $param->{vmstate},
>>   					     $param->{description});
>> +	    print "Created snapshot '$snapname'.\n";
>>   	};
>>   
>>   	return $rpcenv->fork_worker('qmsnapshot', $vmid, $authuser, $realcmd);
>> @@ -5376,6 +5377,7 @@ __PACKAGE__->register_method({
>>   	my $realcmd = sub {
>>   	    PVE::Cluster::log_msg('info', $authuser, "rollback snapshot VM $vmid: $snapname");
>>   	    PVE::QemuConfig->snapshot_rollback($vmid, $snapname);
>> +	    print "Rolled back to snapshot '$snapname'.\n";
>>   
>>   	    if ($param->{start} && !PVE::QemuServer::Helpers::vm_running_locally($vmid)) {
>>   		PVE::API2::Qemu->vm_start({ vmid => $vmid, node => $node });
>> @@ -5435,6 +5437,7 @@ __PACKAGE__->register_method({
>>   	    $lock_obtained = 1;
>>   	    PVE::Cluster::log_msg('info', $authuser, "delete snapshot VM $vmid: $snapname");
>>   	    PVE::QemuConfig->snapshot_delete($vmid, $snapname, $param->{force});
>> +	    print "Removed snapshot '$snapname'.\n";
>>   	};
>>   
>>   	my $realcmd = sub {
>> -- 
>> 2.39.2
>>
>>
>>
>> _______________________________________________
>> pve-devel mailing list
>> pve-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>>
>>
>>
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 



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