public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Stefan Reiter <s.reiter@proxmox.com>
Subject: Re: [pve-devel] [RFC qemu-server 6/6] template: start VM for VMA backup
Date: Mon, 07 Jun 2021 12:22:43 +0200	[thread overview]
Message-ID: <1623061163.4mjzwxxy9c.astroid@nora.none> (raw)
In-Reply-To: <a9ba9653-c1c8-d0fc-c8db-c5f4c7614c8f@proxmox.com>

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@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}) {
>> 
> 




  reply	other threads:[~2021-06-07 10:23 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-04  9:47 [pve-devel] [PATCH qemu-server 0/6] fix #2862: more template backup fixes Fabian Grünbichler
2021-06-04  9:47 ` [pve-devel] [PATCH qemu-server 1/6] test: unbreak restore_config_test Fabian Grünbichler
2021-06-04  9:47 ` [pve-devel] [PATCH qemu-server 2/6] drive: factor out read-only helper Fabian Grünbichler
2021-06-07  9:29   ` Stefan Reiter
2021-06-07 10:23     ` Fabian Grünbichler
2021-06-07 10:35       ` Stefan Reiter
2021-06-04  9:47 ` [pve-devel] [PATCH qemu-server 3/6] template: mark efidisk as read-only Fabian Grünbichler
2021-06-07  9:29   ` Stefan Reiter
2021-06-07 10:23     ` Fabian Grünbichler
2021-06-04  9:47 ` [pve-devel] [PATCH qemu-server 4/6] test: add template drive read-only tests Fabian Grünbichler
2021-06-04  9:47 ` [pve-devel] [PATCH qemu-server 5/6] template: add -snapshot to KVM command Fabian Grünbichler
2021-06-04  9:47 ` [pve-devel] [RFC qemu-server 6/6] template: start VM for VMA backup Fabian Grünbichler
2021-06-07  9:29   ` Stefan Reiter
2021-06-07 10:22     ` Fabian Grünbichler [this message]
2021-06-23 10:50 ` [pve-devel] partially-applied: [PATCH qemu-server 0/6] fix #2862: more template backup fixes Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1623061163.4mjzwxxy9c.astroid@nora.none \
    --to=f.gruenbichler@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    --cc=s.reiter@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal