[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Mon, Sep 27, 2010 at 11:20:42AM -0600, Eric Blake wrote:
> 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.

  yup looks fine to me :-)

> // 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.

  right

> >  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.

  yes that looks like a complete set, and we still have some room at the
flag level !

> 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)

  yes

> 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

  in general virsh commands follow really closely the APIs unless in
cases where we know on API isn't really used in isolation. We need to
keep in mind that the output may be reused in further scripting, which
is why I would tend to favor distinct flags

> >   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.

right :-)

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]