On 24.09.24 22:17, David Hildenbrand wrote:
On 24.09.24 18:22, Nina Schoetterl-Glausch wrote:
On Tue, 2024-09-10 at 19:58 +0200, David Hildenbrand wrote:
We actually want to check the available RAM, not the maximum RAM size.

Signed-off-by: David Hildenbrand <da...@redhat.com>

Reviewed-by: Nina Schoetterl-Glausch <n...@linux.ibm.com>
Nit below.
---
   target/s390x/kvm/pv.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/kvm/pv.c b/target/s390x/kvm/pv.c
index dde836d21a..424cce75ca 100644
--- a/target/s390x/kvm/pv.c
+++ b/target/s390x/kvm/pv.c
@@ -133,7 +133,7 @@ bool s390_pv_vm_try_disable_async(S390CcwMachineState *ms)
        * If the feature is not present or if the VM is not larger than 2 GiB,
        * KVM_PV_ASYNC_CLEANUP_PREPARE fill fail; no point in attempting it.
        */
-    if ((MACHINE(ms)->maxram_size <= 2 * GiB) ||
+    if ((MACHINE(ms)->ram_size <= 2 * GiB) ||
           !kvm_check_extension(kvm_state, 
KVM_CAP_S390_PROTECTED_ASYNC_DISABLE)) {
           return false;
       }

If I understood the kernel code right, the decision is made wrt
the size of the gmap address space, which is the same as the
limit set for the VM. So using s390_get_memory_limit would be
semantically cleaner.

I wonder if we should just drop the RAM size check. Not convinced the
slightly faster reboot for such small VMs is really relevant? Makes the
code more complicated than really necessary.

Thinking about it,

diff --git a/target/s390x/kvm/pv.c b/target/s390x/kvm/pv.c
index 424cce75ca..bbb2108546 100644
--- a/target/s390x/kvm/pv.c
+++ b/target/s390x/kvm/pv.c
@@ -133,7 +133,7 @@ bool s390_pv_vm_try_disable_async(S390CcwMachineState *ms)
      * If the feature is not present or if the VM is not larger than 2 GiB,
      * KVM_PV_ASYNC_CLEANUP_PREPARE fill fail; no point in attempting it.
      */
-    if ((MACHINE(ms)->ram_size <= 2 * GiB) ||
+    if (s390_get_memory_limit(ms) < 2 * GiB ||
         !kvm_check_extension(kvm_state, KVM_CAP_S390_PROTECTED_ASYNC_DISABLE)) 
{
         return false;
     }

Is probably the easiest change, thanks!


--
Cheers,

David / dhildenb


Reply via email to