[pve-devel] [RFC qemu-server 6/6] template: start VM for VMA backup

[Fabian Grünbichler]

On June 7, 2021 11:29 am, Stefan Reiter wrote:
> On 6/4/21 11:47 AM, Fabian Grünbichler wrote:
>> since using 'vma create ..' no longer works with immutable/read-only
>> base volumes.
> 
> Why that? It shouldn't matter to vma if the base is read-only? Though I 
> do see that in vma.c we don't tell QEMU that it should open the block 
> backend read-only...

I have not checked in-depth on the Qemu side what changed, but 
alternatively we could fix the vma binary to mark as read-only like the 
rest of the series and past patches did for the regular startup for 
backup purposes.

> Anyway, I do personally think that just starting a VM is also a good way 
> of doing this, though as you say, for templates it would make a lot of 
> sense to restrict the VM to use less resources.

yeah, I am on the fence (hence the RFC) - on the one hand, it would be 
more consistent with PBSp-backups to just start the VM, OTOH we'd save 
on memory usage/.. if we fix vma.

> 
>> 
>> first hunk drops a leftover variable from when we did the same change to
>> the PBS code path.
>> 
>> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
>> ---
>> this mimics what we already did for PBS, we still might want to think
>> about limiting memory for the started VM for template backups which
>> can't be resumed into operation anyway..
>> 
>>   PVE/VZDump/QemuServer.pm | 20 ++++----------------
>>   1 file changed, 4 insertions(+), 16 deletions(-)
>> 
>> diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
>> index 44b705f..9ac77c1 100644
>> --- a/PVE/VZDump/QemuServer.pm
>> +++ b/PVE/VZDump/QemuServer.pm
>> @@ -446,7 +446,6 @@ sub archive_pbs {
>>       # proxmox-backup-client can only handle raw files and block devs
>>       # only use it (directly) for disk-less VMs
>>       if (!$diskcount) {
>> -	my @pathlist;
>>   	$self->loginfo("backup contains no disks");
>>   
>>   	local $ENV{PBS_PASSWORD} = $password;
>> @@ -621,19 +620,8 @@ sub archive_vma {
>>       }
>>   
>>       my $diskcount = scalar(@{$task->{disks}});
>> -    if (PVE::QemuConfig->is_template($self->{vmlist}->{$vmid}) || !$diskcount) {
>> -	my @pathlist;
>> -	foreach my $di (@{$task->{disks}}) {
>> -	    if ($di->{type} eq 'block' || $di->{type} eq 'file') {
>> -		push @pathlist, "$di->{qmdevice}=$di->{path}";
>> -	    } else {
>> -		die "implement me";
>> -	    }
>> -	}
>> -
>> -	if (!$diskcount) {
>> -	    $self->loginfo("backup contains no disks");
>> -	}
>> +    if (!$diskcount) {
>> +	$self->loginfo("backup doesn't contain any disks");
> 
> nit: I'd leave the "backup contains no disks" message the same
> 
>>   
>>   	my $outcmd;
>>   	if ($comp) {
>> @@ -646,9 +634,9 @@ sub archive_vma {
>>   
>>   	my $cmd = ['/usr/bin/vma', 'create', '-v', '-c', $conffile];
>>   	push @$cmd, '-c', $firewall if -e $firewall;
>> -	push @$cmd, $outcmd, @pathlist;
>> +	push @$cmd, $outcmd;
>>   
>> -	$self->loginfo("starting template backup");
>> +	$self->loginfo("starting backup");
>>   	$self->loginfo(join(' ', @$cmd));
>>   
>>   	if ($opts->{stdout}) {
>> 
>