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

Re: [libvirt] [PATCH v5 3/4] libxl: vnuma support



On Thu, 26 Oct 2017 17:40:37 -0600
Jim Fehlig <jfehlig suse com> wrote:

> On 10/12/2017 01:31 PM, Wim Ten Have wrote:
> > From: Wim ten Have <wim ten have oracle com>
> > 
> > This patch generates a NUMA distance-aware libxl description from the
> > information extracted from a NUMA distance-aware libvirt XML file.
	...
> > +        /* pnode */
> > +        p->pnode = simulate ? 0 : i;  
> 
> So any time the number of vnuma nodes exceeds the number of physical nodes, all 
> vnuma nodes are confined to physical node 0?

  Yes.
>                                              Does xl behave this way too?

  Yes.

> > +
> > +        /* memory size */
> > +        p->memkb = virDomainNumaGetNodeMemorySize(numa, i);
> > +
> > +        /* vcpus */
> > +        bitmap = virDomainNumaGetNodeCpumask(numa, i);
> > +        if (bitmap == NULL) {
> > +            virReportError(VIR_ERR_INTERNAL_ERROR,
> > +                           _("vnuma sibling %zu missing vcpus set"), i);
> > +            goto cleanup;
> > +        }
> > +
> > +        if ((cpu = virBitmapNextSetBit(bitmap, -1)) < 0)
> > +            goto cleanup;
> > +
> > +        libxl_bitmap_init(&vcpu_bitmap);
> > +        if (libxl_cpu_bitmap_alloc(ctx, &vcpu_bitmap, b_info->max_vcpus)) {
> > +            virReportOOMError();
> > +            goto cleanup;
> > +        }
> > +
> > +        do {
> > +            libxl_bitmap_set(&vcpu_bitmap, cpu);
> > +        } while ((cpu = virBitmapNextSetBit(bitmap, cpu)) >= 0);
> > +
> > +        libxl_bitmap_copy_alloc(ctx, &p->vcpus, &vcpu_bitmap);
> > +        libxl_bitmap_dispose(&vcpu_bitmap);
> > +
> > +        /* vdistances */
> > +        if (VIR_ALLOC_N(p->distances, num_vnuma) < 0)
> > +            goto cleanup;
> > +        p->num_distances = num_vnuma;
> > +
> > +        for (j = 0; j < num_vnuma; j++)
> > +            p->distances[j] = virDomainNumaGetNodeDistance(numa, i, j);
> > +    }
> > +
> > +    b_info->vnuma_nodes = vnuma_nodes;
> > +    b_info->num_vnuma_nodes = num_vnuma;
> > +
> > +    ret = 0;
> > +
> > + cleanup:
> > +    if (ret) {
> > +        for (i = 0; i < num_vnuma; i++) {
> > +            libxl_vnode_info *p = &vnuma_nodes[i];
> > +
> > +            VIR_FREE(p->distances);
> > +        }
> > +        VIR_FREE(vnuma_nodes);
> > +    }
> > +
> > +    return ret;
> > +}
> > +#endif
> > +
> >   static int
> >   libxlDiskSetDiscard(libxl_device_disk *x_disk, int discard)
> >   {
> > @@ -2209,6 +2325,11 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
> >       if (libxlMakeDomBuildInfo(def, ctx, caps, d_config) < 0)
> >           return -1;
> >   
> > +#ifdef LIBXL_HAVE_VNUMA
> > +    if (libxlMakeVnumaList(def, ctx, d_config) < 0)
> > +        return -1;
> > +#endif
> > +
> >       if (libxlMakeDiskList(def, d_config) < 0)
> >           return -1;
> >   
> > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> > index 8483d6ecf..656f4a82d 100644
> > --- a/src/libxl/libxl_driver.c
> > +++ b/src/libxl/libxl_driver.c
> > @@ -2788,7 +2788,8 @@ libxlDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag
> >       virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL);
> >   
> >       if (flags & VIR_DOMAIN_DEFINE_VALIDATE)
> > -        parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA;
> > +        parse_flags |= (VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA |
> > +                        VIR_DOMAIN_DEF_PARSE_ABI_UPDATE);  
> 
> Why is this change needed? In its current form, the patch doesn't touch any code 
> in the domain def post parse callback.

  I remove this.  I added this to forcibly recompute memory size in case total_memory
  held a value not matching the numa cells setting. (!= 0)
  See virDomainDefPostParseMemory()
  
    /* Attempt to infer the initial memory size from the sum NUMA memory sizes
     * in case ABI updates are allowed or the <memory> element wasn't specified */
    if (def->mem.total_memory == 0 ||
        parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE ||
        parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE_MIGRATION)
        numaMemory = virDomainNumaGetMemorySize(def->numa);

  Will address all other brought comments under v6.

Regards,
- Wim.


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