[pve-devel] [PATCH qemu-server 2/2] update disk size before local disk migration

[Fabian Grünbichler]

On December 5, 2019 4:11 pm, Stefan Reiter wrote:
> Split out 'update_disksize' from the renamed 'update_disk_config' to
> allow code reuse in QemuMigrate.
> 
> Remove dots after messages to keep style consistent for migration log.
> 
> After updating in sync_disks (phase1) of migration, write out updated
> config. This means that even if migration fails or is aborted in later
> stages, we keep the fixed config - this is not an issue, as it would
> have been fixed on the next attempt anyway, and it can't hurt to have
> the correct size instead of a wrong one either way.
> 
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
> 
> @Thomas: Is this what you had in mind with the local disk migrate bug?
> 
> Also, I'm not 100% sure about that last paragraph, if it really doesn't
> matter...
> 
>  PVE/QemuMigrate.pm | 13 +++++++++++++
>  PVE/QemuServer.pm  | 37 +++++++++++++++++++++++++++++--------
>  2 files changed, 42 insertions(+), 8 deletions(-)
> 
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm
> index 27c5b7a..37624b3 100644
> --- a/PVE/QemuMigrate.pm
> +++ b/PVE/QemuMigrate.pm
> @@ -447,6 +447,16 @@ sub sync_disks {
>  	       'PVE::QemuConfig', $jobcfg, $start_time, $start_time, $logfunc);
>  	}
>  
> +	# sizes in config have to be accurate for remote node to correctly
> +	# allocate disks, rescan to be sure
> +	my $volid_hash = PVE::QemuServer::scan_volids($self->{storecfg}, $vmid);
> +	PVE::QemuServer::foreach_drive($conf, sub {
> +	    my ($key, $drive) = @_;
> +	    my $updated = PVE::QemuServer::update_disksize($drive, $volid_hash,
> +		sub { $self->log('info', $_[0]) });
> +	    $conf->{$key} = PVE::QemuServer::print_drive($updated) if defined($updated);
> +	});
> +
>  	$self->log('info', "copying local disk images") if scalar(%$local_volumes);
>  
>  	foreach my $volid (keys %$local_volumes) {
> @@ -510,6 +520,9 @@ sub phase1 {
>  
>      sync_disks($self, $vmid);
>  
> +    # sync_disks fixes disk sizes to match their actual size, write changes so
> +    # target allocates correct volumes
> +    PVE::QemuConfig->write_config($vmid, $conf);
>  };
>  
>  sub phase1_cleanup {
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index c568d37..49419a4 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -6059,6 +6059,27 @@ sub is_volume_in_use {
>  }
>  
>  sub update_disksize {
> +    my ($drive, $volid_hash, $log_fn) = @_;

this is not a very nice interface.. maybe the logging/printing could be 
moved to the call site(s), by returning 

wantarray ? ($drive, $old, $new) : $drive;

?

> +
> +    my $volid = $drive->{file};
> +    return undef if !defined($volid);
> +
> +    my $oldsize = $drive->{size};
> +    my $newsize = $volid_hash->{$volid}->{size};
> +
> +    if (defined($newsize) && defined($oldsize) && $newsize != $oldsize) {
> +	my $old_fmt = PVE::JSONSchema::format_size($oldsize);
> +	my $new_fmt = PVE::JSONSchema::format_size($newsize);
> +	$log_fn->("size of disk '$volid' updated from $old_fmt to $new_fmt\n")
> +	    if $log_fn;
> +	$drive->{size} = $newsize;
> +	return $drive;
> +    }
> +
> +    return undef;
> +}
> +
> +sub update_disk_config {
>      my ($vmid, $conf, $volid_hash) = @_;
>  
>      my $changes;
> @@ -6080,6 +6101,7 @@ sub update_disksize {
>  	    my $volid = $drive->{file};
>  	    next if !$volid;
>  
> +	    # mark volid as "in-use" for next step
>  	    $referenced->{$volid} = 1;
>  	    if ($volid_hash->{$volid} &&
>  		(my $path = $volid_hash->{$volid}->{path})) {
> @@ -6089,12 +6111,11 @@ sub update_disksize {
>  	    next if drive_is_cdrom($drive);
>  	    next if !$volid_hash->{$volid};
>  
> -	    $drive->{size} = $volid_hash->{$volid}->{size};
> -	    my $new = print_drive($drive);
> -	    if ($new ne $conf->{$opt}) {
> +	    my $updated = update_disksize($drive, $volid_hash,
> +		sub { print "$prefix $_[0]" });
> +	    if (defined($updated)) {
>  		$changes = 1;
> -		$conf->{$opt} = $new;
> -		print "$prefix update disk '$opt' information.\n";
> +		$conf->{$opt} = print_drive($updated);
>  	    }
>  	}
>      }
> @@ -6105,7 +6126,7 @@ sub update_disksize {
>  	my $volid = $conf->{$opt};
>  	my $path = $volid_hash->{$volid}->{path} if $volid_hash->{$volid};
>  	if ($referenced->{$volid} || ($path && $referencedpath->{$path})) {
> -	    print "$prefix remove entry '$opt', its volume '$volid' is in use.\n";
> +	    print "$prefix remove entry '$opt', its volume '$volid' is in use\n";
>  	    $changes = 1;
>  	    delete $conf->{$opt};
>  	}
> @@ -6122,7 +6143,7 @@ sub update_disksize {
>  	next if $referencedpath->{$path};
>  	$changes = 1;
>  	my $key = PVE::QemuConfig->add_unused_volume($conf, $volid);
> -	print "$prefix add unreferenced volume '$volid' as '$key' to config.\n";
> +	print "$prefix add unreferenced volume '$volid' as '$key' to config\n";
>  	$referencedpath->{$path} = 1; # avoid to add more than once (aliases)
>      }
>  
> @@ -6156,7 +6177,7 @@ sub rescan {
>  	    $vm_volids->{$volid} = $info if $info->{vmid} && $info->{vmid} == $vmid;
>  	}
>  
> -	my $changes = update_disksize($vmid, $conf, $vm_volids);
> +	my $changes = update_disk_config($vmid, $conf, $vm_volids);
>  
>  	PVE::QemuConfig->write_config($vmid, $conf) if $changes && !$dryrun;
>      };
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
>