[pve-devel] [PATCH v6 qemu-server 2/3] add QemuMigrateExternal.pm

[Alexandre DERUMIER]

>>I do not like the completel full duplication of QemuMigrate, how about using
>>the exisitng "PVE::QemuMigrate" migrate as base for this and changing only really
>>those methods which need to be changed? Maybe we even can split it up a little bit
>>more so that you only need to overwrite a handful of rather small methods (no full
>>phases)? 
>>
>>Looking at a diff between existing QemuMigrate and your External one, maybe:
>>
>>* one to get the ssh command
>>* one to get the target vmid (for the common case it would just return $vmid)
>>* one to get the remote start command
>>* ...?

Well, First versions of this patch was based on QemuMigrate, but David asked me 
to separate it. (and maybe merge it later).

I can rework it again, on current master QemuMigrate if you want.




----- Mail original -----
De: "Thomas Lamprecht" <t.lamprecht at proxmox.com>
À: "pve-devel" <pve-devel at pve.proxmox.com>, "aderumier" <aderumier at odiso.com>
Envoyé: Samedi 30 Mars 2019 17:27:09
Objet: Re: [pve-devel] [PATCH v6 qemu-server 2/3] add QemuMigrateExternal.pm

On 2/20/19 1:22 AM, Alexandre Derumier wrote: 
> --- 
> PVE/Makefile | 1 + 
> PVE/QemuMigrateExternal.pm | 872 +++++++++++++++++++++++++++++++++++++++++++++ 
> 2 files changed, 873 insertions(+) 
> create mode 100644 PVE/QemuMigrateExternal.pm 
> 
> diff --git a/PVE/Makefile b/PVE/Makefile 
> index 2c800f6..0494cfb 100644 
> --- a/PVE/Makefile 
> +++ b/PVE/Makefile 
> @@ -1,6 +1,7 @@ 
> PERLSOURCE = \ 
> QemuServer.pm \ 
> QemuMigrate.pm \ 
> + QemuMigrateExternal.pm \ 
> QMPClient.pm \ 
> QemuConfig.pm 
> 
> diff --git a/PVE/QemuMigrateExternal.pm b/PVE/QemuMigrateExternal.pm 
> new file mode 100644 
> index 0000000..dd6a1d9 
> --- /dev/null 
> +++ b/PVE/QemuMigrateExternal.pm 
> @@ -0,0 +1,872 @@ 
> +package PVE::QemuMigrateExternal; 
> + 
> +use strict; 
> +use warnings; 
> +use PVE::AbstractMigrate; 
> +use IO::File; 
> +use IPC::Open2; 
> +use POSIX qw( WNOHANG ); 
> +use PVE::INotify; 
> +use PVE::Tools; 
> +use PVE::Cluster; 
> +use PVE::Storage; 
> +use PVE::QemuServer; 
> +use Time::HiRes qw( usleep ); 
> +use PVE::RPCEnvironment; 
> +use PVE::ReplicationConfig; 
> +use PVE::ReplicationState; 
> +use PVE::Replication; 
> +use Storable qw(dclone); 
> + 
> +use base qw(PVE::AbstractMigrate); 

I do not like the completel full duplication of QemuMigrate, how about using 
the exisitng "PVE::QemuMigrate" migrate as base for this and changing only really 
those methods which need to be changed? Maybe we even can split it up a little bit 
more so that you only need to overwrite a handful of rather small methods (no full 
phases)? 

Looking at a diff between existing QemuMigrate and your External one, maybe: 

* one to get the ssh command 
* one to get the target vmid (for the common case it would just return $vmid) 
* one to get the remote start command 
* ...? 

Do you think that could be dooable and make sense? We can also expand AbstractMigrate 
if we can formulate the stuff we need for this in more general ways, i.e., I'd like 
to not put to much "for ext. migrate only" methods there as long as this is seen 
experimental.