Re: [PATCH 2/5] numad: Copy 'placement' of <numatune> to <vcpu> if its placement is not specified

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

 



On 05/08/2012 10:04 AM, Osier Yang wrote:
> With this patch, one can also fully drive numad by:
> 
>   <vcpu>2</vcpu>
>   <numatune>
>     <memory placement='auto'/>
>   </numatune>
> 
> New tests are added, and s/"/'/ on tests (*.xml) added in previous patch.

That feels like churn; I might float those hunks to 1/5.

> +++ b/docs/formatdomain.html.in
> @@ -362,14 +362,14 @@
>          0.9.11 (QEMU and KVM only)</span>, the optional attribute
>          <code>placement</code> can be used to indicate the CPU placement
>          mode for domain process, its value can be either "static" or
> -        "auto", defaults to "static" if <code>cpuset</code> is specified,
> -        "auto" indicates the domain process will be pinned to the advisory
> -        nodeset from querying numad, and the value of attribute
> -        <code>cpuset</code> will be ignored if it's specified. If both
> -        <code>cpuset</code> and <code>placement</code> are not specified,
> -        or if <code>placement</code> is "static", but no <code>cpuset</code>
> -        is specified, the domain process will be pinned to all the
> -        available physical CPUs.
> +        "auto", defaults to <code>placement</code> of <code>numatune</code>,
> +         or "static" if <code>cpuset</code> is specified. "auto" indicates
> +        the domain process will be pinned to the advisory nodeset from querying
> +        numad, and the value of attribute <code>cpuset</code> will be ignored
> +        if it's specified. If both <code>cpuset</code> and <code>placement</code>

s/both...and...are not/neither...nor...is/

> +        are not specified, or if <code>placement</code> is "static", but no
> +        <code>cpuset</code> is specified, the domain process will be pinned to
> +        all the available physical CPUs.

Nice addition; I'll probably also float this to 1/5.

>        </dd>
>      </dl>
>  
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index e9c9db7..14b94d3 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7958,9 +7958,9 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>              if (virDomainCpuSetParse(set, 0, def->cpumask,
>                                       def->cpumasklen) < 0)
>                  goto error;
> -            VIR_FREE(tmp);
>              if (def->placement_mode == VIR_DOMAIN_CPU_PLACEMENT_MODE_DEFAULT)
>                  def->placement_mode = VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC;
> +            VIR_FREE(tmp);
>          }
>      }

Churn.

Everything else looks okay.

ACK; again, I'll probably push this once I finish the series review.

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]