[pve-devel] [PATCH v2 qemu-server 2/2] remote-migration: add target-cpu param

[DERUMIER, Alexandre]

Le mercredi 26 avril 2023 à 15:14 +0200, Fabian Grünbichler a écrit :
> On April 25, 2023 6:52 pm, Alexandre Derumier wrote:
> > This patch add support for remote migration when target
> > cpu model is different.
> > 
> > The target vm is restart after the migration
> 
> so this effectively introduces a new "hybrid" migration mode ;) the
> changes are a bit smaller than I expected (in part thanks to patch
> #1),
> which is good.
> 
> there are semi-frequent requests for another variant (also applicable
> to
> containers) in the form of a two phase migration
> - storage migrate
> - stop guest
> - incremental storage migrate
> - start guest on target
> 
> given that it might make sense to save-guard this implementation
> here,
> and maybe switch to a new "mode" parameter?
> 

I have implemented in v3 a working switch to remote nbd.

so, after the disk migration, we do a block-job-complete,
source vm is still running and now is running over nbd through the
target-vm.
Then the source vm is shutdown, flushing last pending writes through
nbd.
then the target vm is restarted



> online => switching CPU not allowed
> offline or however-we-call-this-new-mode (or in the future, two-
> phase-restart) => switching CPU allowed
> 
> > 
Still unsure about it, I have added an extra flag  in v3 "-target-
reboot"

- online : check if source vm is online
- target-cpu: change the targetcpu.  (only change value on targetvm)
- target-reboot: skip live migration, do shutdown of source vm and
restart of target vm.



> > Signed-off-by: Alexandre Derumier <aderumier at odiso.com>
> > ---
> >  PVE/API2/Qemu.pm   | 18 ++++++++++++++++++
> >  PVE/CLI/qm.pm      |  6 ++++++
> >  PVE/QemuMigrate.pm | 25 +++++++++++++++++++++++++
> >  3 files changed, 49 insertions(+)
> > 
> > diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> > index 587bb22..6703c87 100644
> > --- a/PVE/API2/Qemu.pm
> > +++ b/PVE/API2/Qemu.pm
> > @@ -4460,6 +4460,12 @@ __PACKAGE__->register_method({
> >                 optional => 1,
> >                 default => 0,
> >             },
> > +           'target-cpu' => {
> > +               optional => 1,
> > +               description => "Target Emulated CPU model. For
> > online migration, the storage is live migrate, but the memory
> > migration is skipped and the target vm is restarted.",
> > +               type => 'string',
> > +               format => 'pve-vm-cpu-conf',
> > +           },
> >             'target-storage' => get_standard_option('pve-
> > targetstorage', {
> >                 completion =>
> > \&PVE::QemuServer::complete_migration_storage,
> >                 optional => 0,
> > @@ -4557,11 +4563,14 @@ __PACKAGE__->register_method({
> >         raise_param_exc({ 'target-bridge' => "failed to parse
> > bridge map: $@" })
> >             if $@;
> >  
> > +       my $target_cpu = extract_param($param, 'target-cpu');
> 
> this is okay
> 
> > +
> >         die "remote migration requires explicit storage mapping!\n"
> >             if $storagemap->{identity};
> >  
> >         $param->{storagemap} = $storagemap;
> >         $param->{bridgemap} = $bridgemap;
> > +       $param->{targetcpu} = $target_cpu;
> 
> but this is a bit confusing with the variable/hash key naming ;)
> 
Fixed in the v4

...
> >  
> > +    $remote_conf->{cpu} = $self->{opts}->{targetcpu};
> 
> do we need permission checks here (or better, somewhere early on, for
> doing this here)
> 
> 
> 
fixed in v4: do not override cpuconfig is targetcpu is empty.

About permission, I'm not sure but we don't have specific permission
for cpu.  Do we need to check perm on vm.config ? 
Because Anyway,we should already a have permission to create a vm on
target cluster.