On Thu, Jun 25, 2015 at 18:13:02 +0200, Michal Privoznik wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1196644 > > So this function is called to construct the guest NUMA part of > the qemu command line. At the beginning, the configured hugepages This function constructs the backend (host facing) part of the memory device. > are searched to find the best match for given guest NUMA node. > Configured hugepages can have a @nodeset attribute to specify on > which guest NUMA nodes should be the hugepages backing used. > There is, however, one 'corner case'. Users may just tell 'use > hugepages to back all the nodes'. In other words: > > <memoryBacking> > <hugepages/> > </memoryBacking> > > <cpu> > <numa> > <cell id='0' cpus='0-1' memory='1024000' unit='KiB'/> > </numa> > </cpu> > > Our code fails in this case. Well, since there's no @nodeset (nor > any <page/> child element to <hugepages/>) we fail to lookup the > default hugepage size to use. > > Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> > --- > src/qemu/qemu_command.c | 41 ++++---- > .../qemuxml2argv-hugepages-numa.args | 46 +++++++++ > .../qemuxml2argv-hugepages-numa.xml | 107 +++++++++++++++++++++ > tests/qemuxml2argvtest.c | 8 ++ > 4 files changed, 183 insertions(+), 19 deletions(-) > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.args > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-numa.xml > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 99755f1..cb31fac 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -4760,9 +4760,6 @@ qemuBuildMemoryBackendStr(unsigned long long size, > hugepage = master_hugepage; > } > > - if (hugepage) > - pagesize = hugepage->size; > - This hunk is a bit awkward here since you add it back in the next patch. A rebase mistake perhaps? > if (hugepage && hugepage->size == system_page_size) { > /* However, if user specified to use "huge" page > * of regular system page size, it's as if they > @@ -4778,23 +4775,29 @@ qemuBuildMemoryBackendStr(unsigned long long size, > goto cleanup; > } > > - /* Now lets see, if the huge page we want to use is even mounted > - * and ready to use */ > - for (i = 0; i < cfg->nhugetlbfs; i++) { > - if (cfg->hugetlbfs[i].size == hugepage->size) > - break; > - } > - > - if (i == cfg->nhugetlbfs) { > - virReportError(VIR_ERR_INTERNAL_ERROR, > - _("Unable to find any usable hugetlbfs mount for %llu KiB"), > - pagesize); > - goto cleanup; > - } > - > VIR_FREE(mem_path); mem_path is NULL prior to this free statement, and it is not in a loop so the VIR_FREE can be removed here. > - if (!(mem_path = qemuGetHugepagePath(&cfg->hugetlbfs[i]))) > - goto cleanup; > + if (hugepage->size) { > + /* Now lets see, if the huge page we want to use is even mounted > + * and ready to use */ > + for (i = 0; i < cfg->nhugetlbfs; i++) { > + if (cfg->hugetlbfs[i].size == hugepage->size) > + break; > + } > + > + if (i == cfg->nhugetlbfs) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("Unable to find any usable hugetlbfs mount for %llu KiB"), > + hugepage->size); > + goto cleanup; > + } > + > + if (!(mem_path = qemuGetHugepagePath(&cfg->hugetlbfs[i]))) > + goto cleanup; > + } else { > + if (!(mem_path = qemuGetDefaultHugepath(cfg->hugetlbfs, > + cfg->nhugetlbfs))) > + goto cleanup; > + } > > *backendType = "memory-backend-file"; > ACK, Peter
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list