Re: [libvirt] [PATCH] qemu: Take all PHBs into account while calculating memlock limits

[Laine Stump]

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

[Andrea Bolognani]

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

[Shivaprasad G Bhat]

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