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

[Thomas Lamprecht]

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.