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>
Subject: Re: [pve-devel] [PATCH qemu-server v3 1/2] migration: secure and use source volume names for deactivation
Date: Tue, 19 Dec 2023 12:37:32 +0100	[thread overview]
Message-ID: <1702985173.wwf0cwrukx.astroid@yuna.none> (raw)
In-Reply-To: <20231219104337.100411-2-h.duerr@proxmox.com>

On December 19, 2023 11:43 am, Hannes Duerr wrote:
> During migration, the volume names may change if the name is already in
> use at the target location. We therefore want to save the original names
> before the migration so that we can deactivate the original volumes
> afterwards.

we already do that though, no need for a new key in $self ;)

> 
> Signed-off-by: Hannes Duerr <h.duerr@proxmox.com>
> ---
>  PVE/QemuMigrate.pm | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index b87e47a..ec4710d 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -744,6 +744,11 @@ sub phase1 {
>      $conf->{lock} = 'migrate';
>      PVE::QemuConfig->write_config($vmid, $conf);
>  
> +    PVE::QemuConfig->foreach_volume($conf, sub {
> +	my ($ds, $drive) = @_;
> +        push $self->{source_volumes}->@*, $drive->{file};
> +    });
> +
>      $self->scan_local_volumes($vmid);

scan_local_volumes already records the local volumes (and shared ones,
in case of remote migration), and any mappings.

>  
>      # fix disk sizes to match their actual size and write changes,
> @@ -1586,8 +1591,7 @@ sub phase3_cleanup {
>  
>      # always deactivate volumes - avoid lvm LVs to be active on several nodes
>      eval {
> -	my $vollist = PVE::QemuServer::get_vm_volumes($conf);
> -	PVE::Storage::deactivate_volumes($self->{storecfg}, $vollist);
> +	PVE::Storage::deactivate_volumes($self->{storecfg}, $self->{source_volumes});

one way to approach this here would be to re-use the information from
$self->{volume_map} and do a reverse lookup per volid. even easier
though would be to just get the vollist higher up in this sub, since the
local mapping takes place here as well, and we just need to call
get_vm_volumes with $conf before the volume_map replacements.

>      };
>      if (my $err = $@) {
>  	$self->log('err', $err);
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 




  reply	other threads:[~2023-12-19 11:38 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-19 10:43 [pve-devel] [PATCH qemu-server/storage v3 0/2] fix #1611: implement import of base-images for LVM-thin Storage Hannes Duerr
2023-12-19 10:43 ` [pve-devel] [PATCH qemu-server v3 1/2] migration: secure and use source volume names for deactivation Hannes Duerr
2023-12-19 11:37   ` Fabian Grünbichler [this message]
2023-12-19 10:43 ` [pve-devel] [PATCH pve-storage v3 2/2] fix #1611: implement import of base-images for LVM-thin Storage Hannes Duerr

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=1702985173.wwf0cwrukx.astroid@yuna.none \
    --to=f.gruenbichler@proxmox.com \
    --cc=pve-devel@lists.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