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

Re: [libvirt] [PATCH 09/12] vcpu: add virsh support



On Wed, Sep 29, 2010 at 06:02:13PM -0600, Eric Blake wrote:
> * tools/virsh.c (cmdSetvcpus): Add new flags.  Let invalid
> commands through to driver, to ease testing of hypervisor argument
> validation.
> (cmdVcpucount): New command.
> (commands): Add new command.
> * tools/virsh.pod (setvcpus, vcpucount): Document new behavior.
> ---
> 
> I know - the typical API addition sequence adds driver support
> first and then virsh support.  I can rearrange the patch order
> if desired.

  Nahh, fine, actually the entry points are there, so really there
is no problem !

[...]
>  /*
> + * "vcpucount" command
> + */
> +static const vshCmdInfo info_vcpucount[] = {
> +    {"help", N_("domain vcpu counts")},
> +    {"desc", N_("Returns the number of domain virtual CPUs.")},

  Ouch "Returns the number of virtual CPUs used by the domain."  would
be clearer I think

[...]
> +    if (maximum + current + persistent + active == 1) {
> +        vshError(ctl,
> +                 _("when using --%s, either --%s or --%s must be specified"),
> +                 maximum ? "maximum" : current ? "current"
> +                 : persistent ? "persistent" : "active",
> +                 maximum + current ? "persistent" : "maximum",
> +                 maximum + current ? "active" : "current");

  Ouch, headache :-) but but looks right


Okay, code ended up being a bit more complex than I expected but that's
due to the various options and the fallback to old APIs, fine,

ACK,

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]