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

Re: [libvirt] [PATCH 08/12] vcpu: support maxvcpu in domain_conf



On Wed, Sep 29, 2010 at 06:02:12PM -0600, Eric Blake wrote:
> * src/conf/domain_conf.h (_virDomainDef): Adjust vcpus to unsigned
> short, to match virDomainGetInfo limit.  Add maxvcpus member.
[...]
> 
> Two tightly-related changes.  One: virDomainGetInfo implicitly limits
> vcpus to a 16-bit number; so there's no need to pretend otherwise
> through the rest of the code.

 well, yes and no, in a few years we may look ridiculous, but it would
 be good to have the new APIs cleared up from that limitation.

> Two: add a new maxvcpus member, but
> for now, ensure that all domains treat vcpus == maxvcpus at all
> times (domains that support hot-unplugging vcpus will be changed
> in later patches).
[...]
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4179,6 +4179,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>      int i, n;
>      long id = -1;
>      virDomainDefPtr def;
> +    unsigned long count;
> 
>      if (VIR_ALLOC(def) < 0) {
>          virReportOOMError();
> @@ -4244,8 +4245,27 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>      if (node)
>          def->hugepage_backed = 1;
> 
> -    if (virXPathULong("string(./vcpu[1])", ctxt, &def->vcpus) < 0)
> -        def->vcpus = 1;
> +    if (virXPathULong("string(./vcpu[1])", ctxt, &count) < 0)
> +        def->maxvcpus = 1;
> +    else {
> +        def->maxvcpus = count;
> +        if (def->maxvcpus != count || count == 0) {
> +            virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> +                                 _("invalid maxvcpus %lu"), count);
> +            goto error;
> +        }
> +    }

  Hum, virXPathULong will return -2 for an non ULong format, and we
discard the error by just setting up maxvcpus = 1 silently but on the
other hand we make a fuss about 0 being provided :-)
  If we start raising an error on invalid values maybe it should be
done for both (-2 need to be checked)
  but not a big deal.

> +    if (virXPathULong("string(./vcpu[1]/@current)", ctxt, &count) < 0)
> +        def->vcpus = def->maxvcpus;
> +    else {
> +        def->vcpus = count;
> +        if (def->vcpus != count || count == 0 || def->maxvcpus < count) {
> +            virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> +                                 _("invalid current vcpus %lu"), count);
> +            goto error;
> +        }
> +    }

  same here

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]