Re: [PATCH qemu-server 5/5] drive: rescan drives after migrating to different storage

[Fiona Ebner <f.ebner@proxmox.com>]

Am 19.10.26 um 11:54 AM schrieb Thomas Ellmenreich:
> In some cases, migrating to a different storage can change the size of
> disks due to the underlying granularity of the storage. In the case of a
> storage with 64MiB granularity, this can increase a 4MiB TPM disk to
> 64MiB. To avoid out of sync ui, rescan all disks of the migrated vm after
> such a storage change.
> 
> The rescan not only adjusts the disk size, but also shows all unused
> disks that may not have been shown before.
> 

This is not specific to TPM or this series and warrants more discussion,
so I would suggest leaving it out.

I don't think we should do a full rescan:

1. It can help to find orphaned volumes, but it would be nicer to have a
proper view/panel for all orphaned volumes in the UI.
2. It will iterate all storages with images and thus migration can
suddenly produce an error (not a bad one, everything is safe on the
target already, but still), because of an unrelated storage with issues.

If we go for this, I feel like it could be a tunnel command, that does
an update for all volumes in the configuration. See also the FIXME in
the beginning of phase3_cleanup() and the (git) history behind it.

For remote migration, it doesn't need a special command, it could be
part of the 'quit'. We could also introduce a 'quit' for intra-cluster
migration and do it as part of that. Then we can add other such
operations there without requiring a new command in the future.

> Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
> ---
>  src/PVE/QemuMigrate.pm | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
> index 3845fb41..fe5f422a 100644
> --- a/src/PVE/QemuMigrate.pm
> +++ b/src/PVE/QemuMigrate.pm
> @@ -1905,6 +1905,18 @@ sub phase3_cleanup {
>          $self->cmd_logerr($cmd, errmsg => "failed to clear migrate lock");
>      }
>  
> +    if (!$self->{opts}->{remote} && scalar(keys %{ $self->{volume_map} })) {
> +        my $cmd = [@{ $self->{rem_ssh} }, 'qm', 'disk', 'rescan', '--vmid', $vmid];
> +        eval {
> +            PVE::Tools::run_command(

Nit: please use PVE::Cmd::run() for new calls

> +                $cmd,
> +                outfunc => sub { },

Nit: the resulting stdout could also be logged as info for completeness

> +                errfunc => sub { $self->log('warn', shift) },
> +            );
> +        };
> +        $self->log('warn', "failed to rescan disks after migration - $@") if $@;
> +    }
> +
>      if ($self->{opts}->{remote} && $self->{opts}->{delete}) {
>          eval { PVE::QemuServer::destroy_vm($self->{storecfg}, $vmid, 1, undef, 0) };
>          warn "Failed to remove source VM - $@\n" if $@;