Change in vdsm[master]: [RFE] set the # of vcpus for a VM (hot plug/unplug)

danken at redhat.com danken at redhat.com
Mon Jan 6 18:50:47 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: [RFE] set the # of vcpus for a VM (hot plug/unplug)
......................................................................


Patch Set 18: Code-Review-1

(7 comments)

....................................................
Commit Message
Line 20: 
Line 21: snip from a domxml:
Line 22:  <vcpu placement='static' current='3'>160</vcpu>
Line 23:  <cpu mode='custom' match='exact'>
Line 24:    <topology sockets='160' cores='1' threads='1'/>
please add a word about backward compatibility: if macVCpus is not supplied, there must be no change to cpu topology.
Line 25:  </cpu>
Line 26: 
Line 27: this means there is a total of 160 cpus and the current num is 3
Line 28: 


Line 33: Notes:
Line 34: * guest OS support and behavior may(and probably will) vary on
Line 35:   trying to hot plug/unplug.
Line 36: 
Line 37: * Upgrade: UP VMs not be able to hotplug CPUs as its XML doesn't
Engilsh: VMs that have been started on hosts before this patch, will not be able to hotplug...
Line 38:   the vcpu set as the above mentioned.
Line 39: 
Line 40: Change-Id: Ief35e1d335737cd98d21a5413ac9f8ab9d824c3e
Line 41: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1036492


....................................................
File vdsm/hooks.py
Line 295:     return _runHooksDir(domxml, 'after_disk_hotunplug', vmconf=vmconf,
Line 296:                         params=params, raiseError=False)
Line 297: 
Line 298: 
Line 299: def before_set_num_of_cpus(vmconf={}, params={}):
I love hooks, but I personally find it hard to imagine a usage for these. Do you have a use case ready?
Line 300:     return _runHooksDir(None, 'before_set_num_of_cpus', vmconf=vmconf,
Line 301:                         params=params, raiseError=True)
Line 302: 
Line 303: 


....................................................
File vdsm/vm.py
Line 898:         self.dom.appendChildWithArgs('currentMemory', text=memSizeKB)
Line 899:         smp = self.conf.get('smp', 1)
Line 900:         vcpu = self.dom.appendChildWithArgs(
Line 901:             'vcpu', text=self.conf.get('maxVCpus', smp))
Line 902:         # hotplug cpu requires a maximum number which is higher than the static
the comment seems misplaced here. Instead, macVCpus should be documented in vdsmapi-schema.json.
Line 903:         # assignment. 160 is the max number of cpu's for a guest
Line 904:         # see http://www.ovirt.org/Hot_plug_cpu
Line 905:         vcpu.setAttrs(**{'current': smp, 'placement': 'static'})
Line 906: 


Line 901:             'vcpu', text=self.conf.get('maxVCpus', smp))
Line 902:         # hotplug cpu requires a maximum number which is higher than the static
Line 903:         # assignment. 160 is the max number of cpu's for a guest
Line 904:         # see http://www.ovirt.org/Hot_plug_cpu
Line 905:         vcpu.setAttrs(**{'current': smp, 'placement': 'static'})
from what I read in http://libvirt.org/formatdomain.html#elementsCPUAllocation placement="static" does not seem related to this feature. If so, have it on another patch.
Line 906: 
Line 907:         memSizeGuaranteedKB = str(1024 * int(
Line 908:             self.conf.get('memGuaranteedSize', '0')
Line 909:         ))


Line 1056:                 cpu.appendChildWithArgs('feature', **featureAttrs)
Line 1057: 
Line 1058:         if ('smpCoresPerSocket' in self.conf or
Line 1059:                 'smpThreadsPerCore' in self.conf):
Line 1060:             maxVCpus = int(self.conf.get('maxVCpus', '160'))
maxVcpu is currently computed in two places (here and line 901), giving potentially two different values. This one is not backward compatible for old hosts. Better calculate once, or have a helper function.
Line 1061:             cores = int(self.conf.get('smpCoresPerSocket', '1'))
Line 1062:             threads = int(self.conf.get('smpThreadsPerCore', '1'))
Line 1063:             cpu.appendChildWithArgs('topology',
Line 1064:                                     sockets=str(maxVCpus / cores / threads),


Line 3386:         except libvirt.libvirtError as e:
Line 3387:             self.log.error("setNumberOfCpus failed", exc_info=True)
Line 3388:             if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
Line 3389:                 return errCode['noVM']
Line 3390:             return {'status': {'code': errCode['setNumberOfCpusErr /']
typo in name of error.
Line 3391:                     ['status']['code'], 'message': e.message}}
Line 3392: 
Line 3393:         self.saveState()
Line 3394:         hooks.after_set_num_of_cpus()


-- 
To view, visit http://gerrit.ovirt.org/21789
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief35e1d335737cd98d21a5413ac9f8ab9d824c3e
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <rgolan at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Roy Golan <rgolan at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list