Re: [libvirt] RFC: add <currentVcpu> element

On 09/27/2010 10:25 AM, Daniel Veillard wrote: >>using these flags: >> >>VIR_SET_VCPU_MAXIMUM = 1 >>VIR_SET_VCPU_PERSISTENT = 2 >> >>such that >> >>virDomainSetVcpusFlags(dom,1,0) - same as existing virDomainSetVcpus >>virDomainSetVcpusFlags(dom,1,VIR_SET_VCPU_MAXIMUM) - error; can't >>change max on active domain >>virDomainSetVcpusFlags(dom,1,VIR_SET_VCPU_MAXIMUM|VIR_SET_VCPU_PERSISTENT) >>- sets<vcpu> xml element for next boot >>virDomainSetVcpusFlags(dom,1,VIR_SET_VCPU_PERSISTENT) - sets >><currentVcpu> xml element for next boot >> > > Yes I suggest to get 2 functions one for set and one for get >allowing to do the full set of operations with the use of flags. OK, given your feedback, the proposal is now: XML layer - still debating on <currentVcpu> vs. <vcpu current=n> (see other email), but that is relatively trivial to switch between styles API layer - given your desire to make changes to an active domain also affect persistent state in one call, we need three flags instead of two. My current thoughts: add one new enum and two new functions: // flags for both virDomainSetVcpusFlags and virDomainGetVcpusFlags enum virDomainVcpuFlags { // whether to affect active state or next boot state VIR_DOMAIN_VCPU_ACTIVE = 1, VIR_DOMAIN_VCPU_PERSISTENT = 2, // whether to affect maximum rather than current VIR_DOMAIN_VCPU_MAXIMUM = 4, }; At least one of VIR_DOMAIN_VCPU_ACTIVE and VIR_DOMAIN_VCPU_PERSISTENT must be set. Using VIR_DOMAIN_VCPU_ACTIVE requires an active domain, while VIR_DOMAIN_VCPU_PERSISTENT works for active and inactive domains. For setting the count, both flags may be set (although setting both + VIR_DOMAIN_VCPU_MAXIMUM will fail); for getting, exactly one must be set. For setting, VIR_DOMAIN_VCPU_MAXIMUM must be paired with VIR_DOMAIN_VCPU_PERSISTENT; for getting, it can be paired with either flag.

// returns -1 on failure, 0 on success // virDomainSetVcpus maps to virDomainSetVcpusFlags(,VIR_DOMAIN_VCPU_ACTIVE) int virDomainSetVcpusFlags(virDomainPtr, unsigned int nvcpus, unsigned int flags); // returns -1 on failure, count on success // virDomainGetVcpus remains more complex regarding pinning info // virDomainGetMaxVcpus maps to virDomainGetVcpusFlags(,VIR_DOMAIN_VCPU_ACTIVE|VIR_DOMAIN_VCPU_MAXIMUM) int virDomainGetVcpusFlags(virDomainPtr, unsigned int flags); No change to existing API semantics, although the implementation can wrap old APIs to call the new ones with appropriate flags where appropriate to minimize code duplication.

> virDomainSetVcpusFlags could be used to set the maximum vcpus >of the persistant domain definition with a 3rd flag. Maybe we can >find a better name for that function though the Flags suffix is in >line with other API functions extensions. >What we really want is have convenient functions to get > - max vcpus on stopped guests virDomainGetVcpusFlags(,VIR_DOMAIN_VCPU_PERSISTENT|VIR_DOMAIN_VCPU_MAXIMUM) virDomainGetXMLDesc(,VIR_DOMAIN_XML_INACTIVE) + XML parsing > - max vcpus on running guests virDomainGetVcpusFlags(,VIR_DOMAIN_VCPU_ACTIVE|VIR_DOMAIN_VCPU_MAXIMUM) virDomainGetMaxVcpus() virDomainGetXMLDesc(,0) + XML parsing > - current vcpu on stopped guests virDomainGetVcpusFlags(,VIR_DOMAIN_VCPU_PERSISTENT) [virDomainGetXMLDesc + parsing if XML update goes in] > - current vcpu on running guests virDomainGetVcpusFlags(,VIR_DOMAIN_VCPU_ACTIVE) virDomainGetVcpus() + parsing pinning info >and set > - max vcpus on stopped guests virDomainSetVcpusFlags(,VIR_DOMAIN_VCPU_PERSISTENT|VIR_DOMAIN_VCPU_MAXIMUM) virDomainGetXMLDesc + XML mod + virDomainDefineXML > - max vcpu persistent on running guests virDomainSetVcpusFlags(,VIR_DOMAIN_VCPU_PERSISTENT|VIR_DOMAIN_VCPU_MAXIMUM) virDomainGetXMLDesc + XML mod + virDomainDefineXML > - current vcpu on stopped guests virDomainSetVcpusFlags(,VIR_DOMAIN_VCPU_PERSISTENT) [virDomainGetXMLDesc + XML mod + virDomainDefineXML if XML update goes in] > - current vcpu on running guests virDomainSetVcpusFlags(,VIR_DOMAIN_VCPU_ACTIVE) virDomainSetVcpus() > Another thing is that when setting the current vcpu count on a >running guest we should also save this to the persistant data so >that on domain restart one get an expected state. virDomainSetVcpusFlags(,VIR_DOMAIN_VCPU_ACTIVE|VIR_DOMAIN_VCPU_PERSISTENT) [combination of virDomainSetVcpus() and virDomainGetXMLDesc + XML mod + virDomainDefineXML if XML update goes in] So I think my latest proposal with three enum flags fits all these needs.

virsh layer: vcpuinfo unchanged, tracks pinning info setvcpus learns --max, --persistent, and --active flags mapping quite nicely to the three enum values at the API; omitting both --persistent and --active calls old API (which in turn implies --active)

new vcpucount command, I'm debating whether it is easier to provide all possible information without needing boolean options, or whether to provide --max, --persistent, and --active to make the user more closely match the API

> Another question I had, is there a way in QEmu to specifiy a different >cpu count from the -smp indicating the startup count ? I wish I knew off-hand, as it would make it easier for me to implement when I get to that part of the patch series :) But even if there isn't, I think that starting with the maximum via -smp and immediately after hot-unplugging to the current count is better than nothing.