On Wed, Oct 12, 2016 at 05:02:09PM +0200, Alexandre DERUMIER wrote:
> > +    my $machine_type = PVE::QemuServer::get_current_qemu_machine($vmid);
> > +    my $kvmver = kvm_user_version();
> 
> >>Note that this calls out to the qemu binary rather than querying the
> >>currently running qemu process, which is probably what you meant to
> >>query instead.
> 
> >>We could add an alternative using the query-version qmp command for this 
> >>purpose. 
> 
> 
> are you sure ?
> 
> qmp query-machines ( get_current_qemu_machine) should return the current 
> machine version running in qemu
> qmp query-version should return the qemu binary version.
> 
> 
> qemu_machine_feature_enabled() return current qemu process or if machine 
> version is forced (live migration for example),
> it's return the machine version.
> 
> So I think it's fine here. (Only use new cpu hotplug framework if qemu 
> process or machine version is >= 2.7)
> 

Ah sorry, I got the machine/version part mixed up. Should be fine then.

> 
> ----- Mail original -----
> De: "Wolfgang Bumiller" <w.bumil...@proxmox.com>
> À: "aderumier" <aderum...@odiso.com>
> Cc: "pve-devel" <pve-devel@pve.proxmox.com>
> Envoyé: Mercredi 12 Octobre 2016 16:36:09
> Objet: Re: [pve-devel] [PATCH 3/4] cpu hotplug : add cpu hot-unplug support
> 
> On Tue, Sep 06, 2016 at 12:41:55PM +0200, Alexandre Derumier wrote: 
> > Signed-off-by: Alexandre Derumier <aderum...@odiso.com> 
> > --- 
> > PVE/QemuServer.pm | 31 +++++++++++++++++++++++++++++-- 
> > 1 file changed, 29 insertions(+), 2 deletions(-) 
> > 
> > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm 
> > index d3092ba..8e840f5 100644 
> > --- a/PVE/QemuServer.pm 
> > +++ b/PVE/QemuServer.pm 
> > @@ -3683,6 +3683,9 @@ sub qemu_usb_hotplug { 
> > sub qemu_cpu_hotplug { 
> > my ($vmid, $conf, $vcpus) = @_; 
> > 
> > + my $machine_type = PVE::QemuServer::get_current_qemu_machine($vmid); 
> > + my $kvmver = kvm_user_version(); 
> 
> Note that this calls out to the qemu binary rather than querying the 
> currently running qemu process, which is probably what you meant to 
> query instead. 
> 
> We could add an alternative using the query-version qmp command for this 
> purpose. 
> 
> AFAIC the other (and I hope *all* other) calls to 
> qemu_machine_feature_enabled() using this variable are in 
> config_to_command() or functions reached only through that, where the 
> current behavior is fine. 
> 
> > + 
> > my $sockets = 1; 
> > $sockets = $conf->{smp} if $conf->{smp}; # old style - no longer iused 
> > $sockets = $conf->{sockets} if $conf->{sockets}; 
> > @@ -3695,8 +3698,32 @@ sub qemu_cpu_hotplug { 
> > if $vcpus > $maxcpus; 
> > 
> > my $currentvcpus = $conf->{vcpus} || $maxcpus; 
> > - die "online cpu unplug is not yet possible\n" 
> > - if $vcpus < $currentvcpus; 
> > + 
> > + if ($vcpus < $currentvcpus) { 
> > + 
> > + if (qemu_machine_feature_enabled ($machine_type, $kvmver, 2, 7)) { 
> > + 
> > + for (my $i = $currentvcpus; $i > $vcpus; $i--) { 
> > + qemu_devicedel($vmid, "cpu$i"); 
> > + my $retry = 0; 
> > + my $currentrunningvcpus = undef; 
> > + while (1) { 
> > + $currentrunningvcpus = vm_mon_cmd($vmid, "query-cpus"); 
> > + last if scalar(@{$currentrunningvcpus}) == $i-1; 
> > + raise_param_exc({ "cpu unplug" => "error unplug cpu$i" }) if $retry > 5; 
> > + $retry++; 
> > + sleep 1; 
> > + } 
> > + #update conf after each succesfull cpu unplug 
> > + $conf->{vcpus} = scalar(@{$currentrunningvcpus}); 
> > + PVE::QemuConfig->write_config($vmid, $conf); 
> > + } 
> > + } else { 
> > + die "online cpu unplug is only possible since qemu 2.7\n" 
> > + } 
> > + 
> > + return; 
> > + } 
> > 
> > my $currentrunningvcpus = vm_mon_cmd($vmid, "query-cpus"); 
> > die "vcpus in running vm is different than configuration\n" 
> > -- 
> > 2.1.4 
> > 
> > _______________________________________________ 
> > pve-devel mailing list 
> > pve-devel@pve.proxmox.com 
> > http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 

_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to