Re: [libvirt] [PATCH] qemu: Take all PHBs into account while calculating memlock limits
On 07/03/2017 09:51 AM, Andrea Bolognani wrote: > On Fri, 2017-06-30 at 13:56 +0530, Shivaprasad G Bhat wrote: >> -/* TODO: Detect at runtime once we start using more than just >> - * the default PCI Host Bridge */ >> -nPCIHostBridges = 1; >> +for (i = 0; i < def->ncontrollers; i++) { >> +if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI >> || >> +def->controllers[i]->model != >> VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) { >> +continue; >> +} >> +nPCIHostBridges++; >> +} > > Just to be on the safe side, we should probably make sure the > pci-root controller is actually a PHB by looking at modelName > as well, like: > > for (i = 0; i < def->ncontrollers; i++) { > virDomainControllerDefPtr cont = def->controllers[i]; > > if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI || > cont->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || > cont->opts.pciopts->modelName != > VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) { > continue; > } > > nPCIHostBridges++; > } > > Boy, that model name sure is a mouthful[1]. > > I think we might have enough occurrences of this pattern to > warrant the creation of a virDomainControllerIsPCIHostBridge() > helper function, which you could then use in your patch. > > That said, it might be smarter to do so in a follow-up cleanup > commit in order not to invalidate existing Reviewed-by tags. > Laine, what would be your preference? Either is fine with me. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Take all PHBs into account while calculating memlock limits
On Fri, 2017-06-30 at 13:56 +0530, Shivaprasad G Bhat wrote: > -/* TODO: Detect at runtime once we start using more than just > - * the default PCI Host Bridge */ > -nPCIHostBridges = 1; > +for (i = 0; i < def->ncontrollers; i++) { > +if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI > || > +def->controllers[i]->model != > VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) { > +continue; > +} > +nPCIHostBridges++; > +} Just to be on the safe side, we should probably make sure the pci-root controller is actually a PHB by looking at modelName as well, like: for (i = 0; i < def->ncontrollers; i++) { virDomainControllerDefPtr cont = def->controllers[i]; if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI || cont->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || cont->opts.pciopts->modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) { continue; } nPCIHostBridges++; } Boy, that model name sure is a mouthful[1]. I think we might have enough occurrences of this pattern to warrant the creation of a virDomainControllerIsPCIHostBridge() helper function, which you could then use in your patch. That said, it might be smarter to do so in a follow-up cleanup commit in order not to invalidate existing Reviewed-by tags. Laine, what would be your preference? [1] Except for fingers. Fingerful? -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Take all PHBs into account while calculating memlock limits
Now that the multi-phb support series is in, work on the TODO at qemuDomainGetMemLockLimitBytes() to arrive at the correct memlock limit value. Signed-off-by: Shivaprasad G Bhat --- This patch should be applied on top of Andrea's multi-phb support patchset. src/qemu/qemu_domain.c | 12 tests/qemumemlocktest.c |2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index a3ce10a..a8293b4 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6674,12 +6674,16 @@ qemuDomainGetMemLockLimitBytes(virDomainDefPtr def) unsigned long long memory; unsigned long long baseLimit; unsigned long long passthroughLimit; -size_t nPCIHostBridges; +size_t nPCIHostBridges = 0; bool usesVFIO = false; -/* TODO: Detect at runtime once we start using more than just - * the default PCI Host Bridge */ -nPCIHostBridges = 1; +for (i = 0; i < def->ncontrollers; i++) { +if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI || +def->controllers[i]->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) { +continue; +} +nPCIHostBridges++; +} for (i = 0; i < def->nhostdevs; i++) { virDomainHostdevDefPtr dev = def->hostdevs[i]; diff --git a/tests/qemumemlocktest.c b/tests/qemumemlocktest.c index c0f1dc3..268563d 100644 --- a/tests/qemumemlocktest.c +++ b/tests/qemumemlocktest.c @@ -131,7 +131,7 @@ mymain(void) DO_TEST("pseries-hardlimit", 2147483648); DO_TEST("pseries-locked", VIR_DOMAIN_MEMORY_PARAM_UNLIMITED); -DO_TEST("pseries-hostdev", 2168455168); +DO_TEST("pseries-hostdev", 4320133120); DO_TEST("pseries-hardlimit+locked", 2147483648); DO_TEST("pseries-hardlimit+hostdev", 2147483648); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list