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

[Alexandre DERUMIER]

>>considering some of the code looks like it's prepared for multiple
>>disks, I wonder if the remote side should send a mapping containing the
>>old + new names?

yes, I think it can prepare output for multiple disks, it'll be easier later.
Maybe simply send multiple lines, 1 by disk ?



> + PVE::QemuServer::qemu_drive_mirror($vmid, $self->{target_drive}, $nbd_uri, $vmid); 
> + #update config 

>>As far as I can see you have qemu running on the remote side already, 
>>since you use hmp/mon commnads to export the nbd devices, so it seems 
>>it would be a better choice to update this after the migration has 
>>completed, and change the cleanup code below to detach the nbd drive. 

The problem is that when drive_mirror is finished, the source vm write to the remote disk.
So I think it's better to update config, to have the new disk in config.
If source host die before end of livemigration, user only need to move the config file to destination host.


Alternativly, I was thinking to use pending to store new local disk path, and switch at the end of the live migration.
Maybe add a node=xxx option to drive during the migration, to known where the local disk is located exactly.
I'm not sure...








----- Mail original -----
De: "Wolfgang Bumiller" <w.bumiller at proxmox.com>
À: "aderumier" <aderumier at odiso.com>
Cc: "pve-devel" <pve-devel at pve.proxmox.com>
Envoyé: Lundi 17 Octobre 2016 15:16:39
Objet: Re: [pve-devel] [PATCH 1/3] add live storage migration with vm migration

On Tue, Oct 11, 2016 at 04:45:19PM +0200, Alexandre Derumier wrote: 
> This allow to migrate a local storage (only 1 for now) to a remote node storage. 
> 
> When the target node start, a new volume is created and exposed through qemu embedded nbd server. 
> 
> qemu drive-mirror is done on source vm with nbd server as target. 
> 
> when drive-mirror is done, the source vm is running the disk though nbd. 
> 
> Then we live migration the vm to destination node. 
> 
> Signed-off-by: Alexandre Derumier <aderumier at odiso.com> 
> --- 
> PVE/API2/Qemu.pm | 7 +++++ 
> PVE/QemuMigrate.pm | 91 +++++++++++++++++++++++++++++++++++++++++++++++++++--- 
> 2 files changed, 93 insertions(+), 5 deletions(-) 
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm 
> index 21fbebb..acb1412 100644 
> --- a/PVE/API2/Qemu.pm 
> +++ b/PVE/API2/Qemu.pm 
> @@ -2648,6 +2648,10 @@ __PACKAGE__->register_method({ 
> description => "Allow to migrate VMs which use local devices. Only root may use this option.", 
> optional => 1, 
> }, 
> + targetstorage => get_standard_option('pve-storage-id', { 
> + description => "Target storage.", 
> + optional => 1, 
> + }), 
> }, 
> }, 
> returns => { 
> @@ -2674,6 +2678,9 @@ __PACKAGE__->register_method({ 
> 
> my $vmid = extract_param($param, 'vmid'); 
> 
> + raise_param_exc({ targetstorage => "Live Storage migration can only be done online" }) 
> + if !$param->{online} && $param->{targetstorage}; 
> + 
> raise_param_exc({ force => "Only root may use this option." }) 
> if $param->{force} && $authuser ne 'root at pam'; 
> 
> diff --git a/PVE/QemuMigrate.pm b/PVE/QemuMigrate.pm 
> index 22a49ef..6e90296 100644 
> --- a/PVE/QemuMigrate.pm 
> +++ b/PVE/QemuMigrate.pm 
> @@ -170,9 +170,11 @@ sub prepare { 
> $self->{forcemachine} = PVE::QemuServer::qemu_machine_pxe($vmid, $conf); 
> 
> } 
> - 
> if (my $loc_res = PVE::QemuServer::check_local_resources($conf, 1)) { 
> - if ($self->{running} || !$self->{opts}->{force}) { 
> + if ($self->{running} && $self->{opts}->{targetstorage}){ 
> + $self->log('info', "migrating VM with online storage migration"); 
> + } 
> + elsif ($self->{running} || !$self->{opts}->{force} ) { 
> 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}; 
> + 
> + PVE::Storage::storage_check_node($self->{storecfg}, $targetsid, $self->{node}); 
> 
> if ($scfg->{shared}) { 
> # PVE::Storage::activate_storage checks this for non-shared storages 
> @@ -197,9 +203,12 @@ sub prepare { 
> } else { 
> # only activate if not shared 
> push @$need_activate, $volid; 
> + $unsharedcount++; 
> } 
> } 
> 
> + die "online storage migration don't support more than 1 local disk currently" if $unsharedcount > 1; 
> + 
> # activate volumes 
> PVE::Storage::activate_volumes($self->{storecfg}, $need_activate); 
> 
> @@ -407,7 +416,7 @@ sub phase1 { 
> $conf->{lock} = 'migrate'; 
> PVE::QemuConfig->write_config($vmid, $conf); 
> 
> - sync_disks($self, $vmid); 
> + sync_disks($self, $vmid) if !$self->{opts}->{targetstorage}; 
> 
> }; 
> 
> @@ -452,7 +461,7 @@ sub phase2 { 
> $spice_ticket = $res->{ticket}; 
> } 
> 
> - push @$cmd , 'qm', 'start', $vmid, '--skiplock', '--migratedfrom', $nodename; 
> + push @$cmd , 'qm', 'start', $vmid, '--skiplock', '--migratedfrom', $nodename, '--targetstorage', $self->{opts}->{targetstorage}; 
> 
> # we use TCP only for unsecure migrations as TCP ssh forward tunnels often 
> # did appeared to late (they are hard, if not impossible, to check for) 
> @@ -472,6 +481,7 @@ sub phase2 { 
> } 
> 
> my $spice_port; 
> + my $nbd_uri; 
> 
> # Note: We try to keep $spice_ticket secret (do not pass via command line parameter) 
> # instead we pipe it through STDIN 
> @@ -496,6 +506,13 @@ sub phase2 { 
> elsif ($line =~ m/^spice listens on port (\d+)$/) { 
> $spice_port = int($1); 
> } 
> + elsif ($line =~ m/^storage migration listens on nbd:(localhost|[\d\.]+|\[[\d\.:a-fA-F]+\]):(\d+):exportname=(\S+) volume:(\S+)$/) { 

considering some of the code looks like it's prepared for multiple 
disks, I wonder if the remote side should send a mapping containing the 
old + new names? 

> + $nbd_uri = "nbd:$1:$2:exportname=$3"; 
> + $self->{target_volid} = $4; 
> + $self->{target_drive} = $3; 
> + $self->{target_drive} =~ s/drive-//g; 
> + 
> + } 
> }, errfunc => sub { 
> my $line = shift; 
> $self->log('info', $line); 
> @@ -540,6 +557,18 @@ sub phase2 { 
> } 
> 
> my $start = time(); 
> + 
> + if ($self->{opts}->{targetstorage}) { 
> + $self->log('info', "starting storage migration on $nbd_uri"); 
> + $self->{storage_migration} = 'running'; 
> + PVE::QemuServer::qemu_drive_mirror($vmid, $self->{target_drive}, $nbd_uri, $vmid); 
> + #update config 

As far as I can see you have qemu running on the remote side already, 
since you use hmp/mon commnads to export the nbd devices, so it seems 
it would be a better choice to update this after the migration has 
completed, and change the cleanup code below to detach the nbd drive. 

> + $self->{storage_migration} = 'finish'; 
> + my $drive = PVE::QemuServer::parse_drive($self->{target_drive}, $self->{target_volid}); 
> + $conf->{$self->{target_drive}} = PVE::QemuServer::print_drive($vmid, $drive); 
> + PVE::QemuConfig->write_config($vmid, $conf); 
> + } 
> + 
> $self->log('info', "starting online/live migration on $ruri"); 
> $self->{livemigration} = 1; 
> 
> @@ -748,6 +777,48 @@ sub phase2_cleanup { 
> $self->{errors} = 1; 
> } 
> 
> + if($self->{storage_migration} && $self->{storage_migration} eq 'running' && $self->{target_volid}) { 
> + 
> + my $drive = PVE::QemuServer::parse_drive($self->{target_drive}, $self->{target_volid}); 
> + my ($storeid, $volname) = PVE::Storage::parse_volume_id($drive->{file}); 
> + 
> + my $cmd = [@{$self->{rem_ssh}}, 'pvesm', 'free', "$storeid:$volname"]; 
> + 
> + eval{ PVE::Tools::run_command($cmd, outfunc => sub {}, errfunc => sub {}) }; 
> + if (my $err = $@) { 
> + $self->log('err', $err); 
> + $self->{errors} = 1; 
> + } 
> + } 
> + 
> + #if storage migration is already done, but vm migration crash, we need to move the vm config 
> + if($self->{storage_migration} && $self->{storage_migration} eq 'finish' && $self->{target_volid}) { 

Detach nbd device here. (If you do choose to have the config updated at 
this point already, restore the original, but that might be less 
convenient.) 

> + 
> + # stop local VM 
> + eval { PVE::QemuServer::vm_stop($self->{storecfg}, $vmid, 1, 1); }; 
> + if (my $err = $@) { 
> + $self->log('err', "stopping vm failed - $err"); 
> + $self->{errors} = 1; 
> + } 
> + 
> + # 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); 
> + }; 
> + if (my $err = $@) { 
> + $self->log('err', $err); 
> + $self->{errors} = 1; 
> + } 
> + 
> + # move config to remote node 
> + my $conffile = PVE::QemuConfig->config_file($vmid); 
> + my $newconffile = PVE::QemuConfig->config_file($vmid, $self->{node}); 
> + 
> + die "Failed to move config to node '$self->{node}' - rename failed: $!\n" 
> + if !rename($conffile, $newconffile); 
> + } 
> + 
> if ($self->{tunnel}) { 
> eval { finish_tunnel($self, $self->{tunnel}); }; 
> if (my $err = $@) { 
> @@ -755,6 +826,9 @@ sub phase2_cleanup { 
> $self->{errors} = 1; 
> } 
> } 
> + 
> + 
> + 
> } 
> 
> sub phase3 { 
> @@ -834,6 +908,13 @@ sub phase3_cleanup { 
> $self->{errors} = 1; 
> } 
> 
> + #improve me 
> + if($self->{storage_migration}) { 
> + #delete source volid ? 
> + 
> + #stop nbd server to remote vm 
> + } 
> + 
> # clear migrate lock 
> my $cmd = [ @{$self->{rem_ssh}}, 'qm', 'unlock', $vmid ]; 
> $self->cmd_logerr($cmd, errmsg => "failed to clear migrate lock"); 
> -- 
> 2.1.4