[pve-devel] [PATCH 1/3] add live storage migration with vm migration

[Wolfgang Bumiller]

On Wed, Oct 12, 2016 at 09:44:25AM +0200, Thomas Lamprecht wrote:
> >  	    die "can't migrate VM which uses local devices\n";
> >  	} else {
> >  	    $self->log('info', "migrating VM which uses local devices");
> >@@ -182,12 +184,16 @@ sub prepare {
> >      my $vollist = PVE::QemuServer::get_vm_volumes($conf);
> >      my $need_activate = [];
> >+    my $unsharedcount = 0;
> >      foreach my $volid (@$vollist) {
> >  	my ($sid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
> >  	# check if storage is available on both nodes
> >  	my $scfg = PVE::Storage::storage_check_node($self->{storecfg}, $sid);
> >-	PVE::Storage::storage_check_node($self->{storecfg}, $sid, $self->{node});
> >+	my $targetsid = $sid;
> >+	$targetsid = $self->{opts}->{targetstorage} if $self->{opts}->{targetstorage};
> >+
> we often use
> 
> my $targetsid = $self->{opts}->{targetstorage} || $sid;

Or assign the long one first and use
 $targetsid = $sid if !$targetsid;
as second line (which is btw. what our new style guide suggests ;-)
(as a preference))

> >+        elsif ($line =~ m/^storage migration listens on nbd:(localhost|[\d\.]+|\[[\d\.:a-fA-F]+\]):(\d+):exportname=(\S+) volume:(\S+)$/) {
> we have a regex for valid IP v4/v6 addresses, maybe use that here

That's fine, this part of the regex is copied from existing code and
handles domains/hostnames in addition to addresses; wouldn't get any
less complex when testing them separately.