Re: [PATCH 2/2] qemu: Check for guest NUMA nodes properly when validating hugepages

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

 



On Wed, Jul 04, 2018 at 01:51:57PM +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1448149

This BZ is not related to the patch, it describes different issue.

There is different BZ that tracks the issue:

https://bugzilla.redhat.com/show_bug.cgi?id=1591235

> In fa6bdf6afa878 and ec982f6d929f3c23 I've tried to forbid
> configuring <hugepages/> for non-existent guest NUMA nodes.
> However, the check was not fine tuned enough as it failed when no
> guest NUMA was configured but <hugepages/> were configured for
> guest NUMA node #0.
> 
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
>  src/qemu/qemu_command.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 04c5c28438..15caf392aa 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7421,7 +7421,7 @@ qemuBuildMemPathStr(virQEMUDriverConfigPtr cfg,
>  
>      if (def->mem.hugepages[0].nodemask) {
>          ssize_t next_bit = virBitmapNextSetBit(def->mem.hugepages[0].nodemask, -1);
> -        if (next_bit >= 0) {
> +        if (next_bit > 0) {
>              virReportError(VIR_ERR_XML_DETAIL,
>                             _("hugepages: node %zd not found"),
>                             next_bit);

This code is weird, it checks only the first hugepage element but 
the XML in this case can look like this:

  <memoryBacking>
    <hugepages>
      <page size='2048' unit='KiB' nodeset='0'/>
      <page size='1048576' unit='KiB'/>
    </hugepages>
    <locked/>
  </memoryBacking>

which is obviously wrong and would be accepted and 2048KiB pages would
be used.

> @@ -7577,7 +7577,7 @@ qemuBuildNumaArgStr(virQEMUDriverConfigPtr cfg,
>          }
>  
>          next_bit = virBitmapNextSetBit(def->mem.hugepages[i].nodemask, pos);
> -        if (next_bit >= 0) {
> +        if (next_bit > 0) {
>              virReportError(VIR_ERR_XML_DETAIL,
>                             _("hugepages: node %zd not found"),
>                             next_bit);

The fact that we have this check in two places makes me thing that it's
wrong.  Actually this should be moved from qemu build command into qemu
validate code or event into generic validate code.

I'm working on some patches to fix the hugepages.

Pavel

Attachment: signature.asc
Description: PGP 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]

  Powered by Linux