[PATCH qemu-server] fix #7786: migrate: deactivate shared storage volumes of offline VMs

[PATCH qemu-server] fix #7786: migrate: deactivate shared storage volumes of offline VMs

[Daniel Herzig]

If non-running VMs with active LVs on shared storage get migrated to
another node, these volumes currently stay activated on the source
node. This can cause issues on the source node when resizing the
volume on the target node [0] after migration.

This patch addresses this issue by ensuring volume deactivation on the
source node in such cases.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=7786

Signed-off-by: Daniel Herzig <d.herzig@proxmox.com>
---
 src/PVE/QemuMigrate.pm | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
index 8da6f15d..bf55b73d 100644
--- a/src/PVE/QemuMigrate.pm
+++ b/src/PVE/QemuMigrate.pm
@@ -710,6 +710,26 @@ sub filter_local_volumes {
     return @filtered_volids;
 }
 
+# deactivate shared-storage volumes on source node.
+# NOTE: only meant to be called for non-running VMs!
+# Active volumes may be present for non-running VMs
+# eg after cloning, disk move, or user-intervention.
+# For running VMs deactivation is handled separately.
+sub deactivate_volumes_on_shared_storage {
+    my ($self) = @_;
+    my $storecfg = $self->{storecfg};
+    my $conf = $self->{vmconf};
+    my $vollist = PVE::QemuServer::get_vm_volumes($conf);
+    for my $volid (@$vollist) {
+        my ($sid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
+        my $scfg = PVE::Storage::storage_check_enabled($storecfg, $sid);
+        eval { PVE::Storage::deactivate_volumes($storecfg, [$volid]); } if $scfg->{shared};
+        if (my $err = $@) {
+            $self->log('warn',$err);
+        }
+    }
+}
+
 sub sync_offline_local_volumes {
     my ($self) = @_;
 
@@ -846,6 +866,13 @@ sub phase1 {
     $self->handle_replication($vmid);
 
     $self->sync_offline_local_volumes();
+
+    # make sure that volumes of non-running VMs on shared storage
+    # are deactivated.
+    if (! $self->{running}) {
+        $self->deactivate_volumes_on_shared_storage();
+    }
+
     $self->phase1_remote($vmid) if $self->{opts}->{remote};
 }
 
-- 
2.47.3

Re: [PATCH qemu-server] fix #7786: migrate: deactivate shared storage volumes of offline VMs

[Daniel Kral]

On Wed Jul 8, 2026 at 9:31 AM CEST, Daniel Herzig wrote:
> If non-running VMs with active LVs on shared storage get migrated to
> another node, these volumes currently stay activated on the source
> node. This can cause issues on the source node when resizing the
> volume on the target node [0] after migration.
>
> This patch addresses this issue by ensuring volume deactivation on the
> source node in such cases.
>
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=7786

Thanks for tackling this and the thorough reproducer in the BZ entry!

>
> Signed-off-by: Daniel Herzig <d.herzig@proxmox.com>
> ---
>  src/PVE/QemuMigrate.pm | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
> index 8da6f15d..bf55b73d 100644
> --- a/src/PVE/QemuMigrate.pm
> +++ b/src/PVE/QemuMigrate.pm
> @@ -710,6 +710,26 @@ sub filter_local_volumes {
>      return @filtered_volids;
>  }
>  
> +# deactivate shared-storage volumes on source node.
> +# NOTE: only meant to be called for non-running VMs!
> +# Active volumes may be present for non-running VMs
> +# eg after cloning, disk move, or user-intervention.
> +# For running VMs deactivation is handled separately.
> +sub deactivate_volumes_on_shared_storage {
> +    my ($self) = @_;
> +    my $storecfg = $self->{storecfg};
> +    my $conf = $self->{vmconf};
> +    my $vollist = PVE::QemuServer::get_vm_volumes($conf);
> +    for my $volid (@$vollist) {
> +        my ($sid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
> +        my $scfg = PVE::Storage::storage_check_enabled($storecfg, $sid);
> +        eval { PVE::Storage::deactivate_volumes($storecfg, [$volid]); } if $scfg->{shared};
> +        if (my $err = $@) {
> +            $self->log('warn',$err);
> +        }
> +    }
> +}
> +
>  sub sync_offline_local_volumes {
>      my ($self) = @_;
>  
> @@ -846,6 +866,13 @@ sub phase1 {
>      $self->handle_replication($vmid);
>  
>      $self->sync_offline_local_volumes();
> +
> +    # make sure that volumes of non-running VMs on shared storage
> +    # are deactivated.
> +    if (! $self->{running}) {
> +        $self->deactivate_volumes_on_shared_storage();
> +    }

As discussed off-list, I wonder whether this can be a

    if (!$self->{running}) {
        my $vollist = PVE::QemuServer::get_vm_volumes($conf);
        PVE::Storage::deactivate_volumes($self->{storecfg}, $vollist);
    }

Just to be sure that there are no active volumes in the case of a
offline migration.

I wonder whether there are any assumptions in later migration steps
where this might break something that I'm not aware of?

> +
>      $self->phase1_remote($vmid) if $self->{opts}->{remote};
>  }
>  

Re: [PATCH qemu-server] fix #7786: migrate: deactivate shared storage volumes of offline VMs

[Daniel Herzig]

Thanks for looking into this!

"Daniel Kral" <d.kral@proxmox.com> writes:

>> @@ -846,6 +866,13 @@ sub phase1 {
>>      $self->handle_replication($vmid);
>>  
>>      $self->sync_offline_local_volumes();
>> +
>> +    # make sure that volumes of non-running VMs on shared storage
>> +    # are deactivated.
>> +    if (! $self->{running}) {
>> +        $self->deactivate_volumes_on_shared_storage();
>> +    }
>
> As discussed off-list, I wonder whether this can be a
>
>     if (!$self->{running}) {
>         my $vollist = PVE::QemuServer::get_vm_volumes($conf);
>         PVE::Storage::deactivate_volumes($self->{storecfg}, $vollist);
>     }
>
> Just to be sure that there are no active volumes in the case of a
> offline migration.
>
I guess a call to `deactivate_volumes` without restricting to shared
storage (as in `deactivate_volumes_on_shared_storage()`) would also
do. But in such a case we won't have complications anyway, as
`sync_offline_local_volumes` is taking care of these with its own
call to `deactivate_volumes` in the end.

I'm not against shortening down the code (will see what I can
do) -- but I'd like to keep the `eval->error->log` sequence and not
bloat `phase1` at the same time, as it's such a lean sub.

> I wonder whether there are any assumptions in later migration steps
> where this might break something that I'm not aware of?

I'll double-check, but for now I think the issue is, that we
currently don't do anything with the activation state in case of an
offline migration on shared storage. Clearly the `if (!self->{running})`
check is important, as it would cause issues with online-migrations,
where volume deactivation is handled in a dedicated step.

>
>> +
>>      $self->phase1_remote($vmid) if $self->{opts}->{remote};
>>  }
>>  

Re: [PATCH qemu-server] fix #7786: migrate: deactivate shared storage volumes of offline VMs

[Daniel Kral]

On Thu Jul 9, 2026 at 3:30 PM CEST, Daniel Herzig wrote:
> Thanks for looking into this!
>
> "Daniel Kral" <d.kral@proxmox.com> writes:
>
>>> @@ -846,6 +866,13 @@ sub phase1 {
>>>      $self->handle_replication($vmid);
>>>  
>>>      $self->sync_offline_local_volumes();
>>> +
>>> +    # make sure that volumes of non-running VMs on shared storage
>>> +    # are deactivated.
>>> +    if (! $self->{running}) {
>>> +        $self->deactivate_volumes_on_shared_storage();
>>> +    }
>>
>> As discussed off-list, I wonder whether this can be a
>>
>>     if (!$self->{running}) {
>>         my $vollist = PVE::QemuServer::get_vm_volumes($conf);
>>         PVE::Storage::deactivate_volumes($self->{storecfg}, $vollist);
>>     }
>>
>> Just to be sure that there are no active volumes in the case of a
>> offline migration.
>>
> I guess a call to `deactivate_volumes` without restricting to shared
> storage (as in `deactivate_volumes_on_shared_storage()`) would also
> do. But in such a case we won't have complications anyway, as
> `sync_offline_local_volumes` is taking care of these with its own
> call to `deactivate_volumes` in the end.
>
> I'm not against shortening down the code (will see what I can
> do) -- but I'd like to keep the `eval->error->log` sequence and not
> bloat `phase1` at the same time, as it's such a lean sub.
>

No hard feelings from my side, just thought that it might not hurt here
as with an offline migration it seems natural that the _local_ volumes
on the source node are deactivated with or without shared storage.

But you're right, sync_offline_local_volumes() already does handle that
for some volumes.

>> I wonder whether there are any assumptions in later migration steps
>> where this might break something that I'm not aware of?
>
> I'll double-check, but for now I think the issue is, that we
> currently don't do anything with the activation state in case of an
> offline migration on shared storage. Clearly the `if (!self->{running})`
> check is important, as it would cause issues with online-migrations,
> where volume deactivation is handled in a dedicated step.
>

Yeah, it might be worth to investigate if we can reduce the amount of
time volumes are active when these are needed, e.g. after the disk
resize, clone, and disk move without deleting the original volume.

>>
>>> +
>>>      $self->phase1_remote($vmid) if $self->{opts}->{remote};
>>>  }
>>>  

Re: [PATCH qemu-server] fix #7786: migrate: deactivate shared storage volumes of offline VMs

[Daniel Herzig]

"Daniel Kral" <d.kral@proxmox.com> writes:

>>>
>>> As discussed off-list, I wonder whether this can be a
>>>
>>>     if (!$self->{running}) {
>>>         my $vollist = PVE::QemuServer::get_vm_volumes($conf);
>>>         PVE::Storage::deactivate_volumes($self->{storecfg}, $vollist);
>>>     }
>>>
>>> Just to be sure that there are no active volumes in the case of a
>>> offline migration.
>>>
>> I guess a call to `deactivate_volumes` without restricting to shared
>> storage (as in `deactivate_volumes_on_shared_storage()`) would also
>> do. But in such a case we won't have complications anyway, as
>> `sync_offline_local_volumes` is taking care of these with its own
>> call to `deactivate_volumes` in the end.
>>
>> I'm not against shortening down the code (will see what I can
>> do) -- but I'd like to keep the `eval->error->log` sequence and not
>> bloat `phase1` at the same time, as it's such a lean sub.
>>
>
> No hard feelings from my side, just thought that it might not hurt here
> as with an offline migration it seems natural that the _local_ volumes
> on the source node are deactivated with or without shared storage.
>
> But you're right, sync_offline_local_volumes() already does handle that
> for some volumes.
>
>>> I wonder whether there are any assumptions in later migration steps
>>> where this might break something that I'm not aware of?
>>
>> I'll double-check, but for now I think the issue is, that we
>> currently don't do anything with the activation state in case of an
>> offline migration on shared storage. Clearly the `if (!self->{running})`
>> check is important, as it would cause issues with online-migrations,
>> where volume deactivation is handled in a dedicated step.
>>
>
> Yeah, it might be worth to investigate if we can reduce the amount of
> time volumes are active when these are needed, e.g. after the disk
> resize, clone, and disk move without deleting the original volume.

I second on that. A definite deactivation of volumes on shared storage
in the end of a migration should be in place for both offline and online
migrations in any case, as we cannot exclude user-activated volumes
either (which would cause the same pattern). We're actually already
doing this 'sanity-check' for containers, but I guess there we already
had more focus on it, as everything's about offline migrations with
containers.

>
>>>
>>>> +
>>>>      $self->phase1_remote($vmid) if $self->{opts}->{remote};
>>>>  }
>>>>