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.

--
Eric Blake   ebl...@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to