[v4] QEMU: support QEMU built without TCG

[libvirt PATCH v4 3/3] add virQEMUCapsProbeQMPTCGState function to set QEMU_CAPS_TCG_DISABLED

Posted by Tobin Feldman-Fitzthum 5 years, 9 months ago

Add virQEMUCapsProbeQMPTCGState to set TCG_DISABLED cap if version 
is > 2.10, KVM is enabled, and tcg-accel is not present in 
qom-list-types result.

Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.vnet.ibm.com>
---
 src/qemu/qemu_capabilities.c | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index e7179ea048..4a3170fc5c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2576,7 +2576,35 @@ virQEMUCapsProbeQMPGenericProps(virQEMUCapsPtr qemuCaps,
 }
 
 static int
-virQEMUCapsProbeQMPDevices(virQEMUCapsPtr qemuCaps,
+virQEMUCapsProbeQMPTCGState(virQEMUCapsPtr qemuCaps,
+                            char **values,
+                            int nvalues)
+{
+    size_t i;
+    bool found = false;
+    /*
+     * As of version 2.10, QEMU can be built without the TCG.
+     * */
+    if (qemuCaps->version < 2010000)
+        return 0;
+    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
+        return 0;
+
+    for (i = 0; i < nvalues; i++) {
+        if (STREQ(values[i], "tcg-accel")) {
+            found = true;
+            break;
+        }
+    }
+
+    if (!found)
+        virQEMUCapsSet(qemuCaps, QEMU_CAPS_TCG_DISABLED);
+
+    return 0;
+}
+
+static int
+virQEMUCapsProbeQMPTypes(virQEMUCapsPtr qemuCaps,
                            qemuMonitorPtr mon)
 {
     int nvalues;
@@ -2584,6 +2612,8 @@ virQEMUCapsProbeQMPDevices(virQEMUCapsPtr qemuCaps,
 
     if ((nvalues = qemuMonitorGetObjectTypes(mon, &values)) < 0)
         return -1;
+
+    virQEMUCapsProbeQMPTCGState(qemuCaps, values, nvalues);
     virQEMUCapsProcessStringFlags(qemuCaps,
                                   G_N_ELEMENTS(virQEMUCapsObjectTypes),
                                   virQEMUCapsObjectTypes,
@@ -5047,7 +5077,7 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
 
     if (virQEMUCapsProbeQMPEvents(qemuCaps, mon) < 0)
         return -1;
-    if (virQEMUCapsProbeQMPDevices(qemuCaps, mon) < 0)
+    if (virQEMUCapsProbeQMPTypes(qemuCaps, mon) < 0)
         return -1;
     if (virQEMUCapsProbeQMPMachineTypes(qemuCaps, type, mon) < 0)
         return -1;
-- 
2.20.1 (Apple Git-117)

Re: [libvirt PATCH v4 3/3] add virQEMUCapsProbeQMPTCGState function to set QEMU_CAPS_TCG_DISABLED

Posted by Peter Krempa 5 years, 9 months ago

On Wed, Apr 22, 2020 at 17:50:44 -0400, Tobin Feldman-Fitzthum wrote:
> Add virQEMUCapsProbeQMPTCGState to set TCG_DISABLED cap if version 
> is > 2.10, KVM is enabled, and tcg-accel is not present in 
> qom-list-types result.
> 
> Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.vnet.ibm.com>
> ---
>  src/qemu/qemu_capabilities.c | 34 ++++++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)

I don't feel comfortable reviewing the other two patches which deal with
the actual use of the capability, so I'll leave those for somebody else.

> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index e7179ea048..4a3170fc5c 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -2576,7 +2576,35 @@ virQEMUCapsProbeQMPGenericProps(virQEMUCapsPtr qemuCaps,
>  }
>  
>  static int
> -virQEMUCapsProbeQMPDevices(virQEMUCapsPtr qemuCaps,
> +virQEMUCapsProbeQMPTCGState(virQEMUCapsPtr qemuCaps,

Returned value of the new function is not used, so please define it as
'void'

> +                            char **values,
> +                            int nvalues)
> +{
> +    size_t i;
> +    bool found = false;
> +    /*
> +     * As of version 2.10, QEMU can be built without the TCG.
> +     * */
> +    if (qemuCaps->version < 2010000)
> +        return 0;
> +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
> +        return 0;
> +
> +    for (i = 0; i < nvalues; i++) {
> +        if (STREQ(values[i], "tcg-accel")) {
> +            found = true;

You can set the capability here without intermediate variable and just
return.

> +            break;
> +        }
> +    }
> +
> +    if (!found)
> +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_TCG_DISABLED);
> +
> +    return 0;
> +}
> +
> +static int
> +virQEMUCapsProbeQMPTypes(virQEMUCapsPtr qemuCaps,
>                             qemuMonitorPtr mon)
>  {
>      int nvalues;

With the above resolved:

Reviewed-by: Peter Krempa <pkrempa@redhat.com>

Re: [libvirt PATCH v4 3/3] add virQEMUCapsProbeQMPTCGState function to set QEMU_CAPS_TCG_DISABLED

Posted by Michal Privoznik 5 years, 9 months ago

On 4/24/20 9:58 AM, Peter Krempa wrote:
> On Wed, Apr 22, 2020 at 17:50:44 -0400, Tobin Feldman-Fitzthum wrote:
>> Add virQEMUCapsProbeQMPTCGState to set TCG_DISABLED cap if version
>> is > 2.10, KVM is enabled, and tcg-accel is not present in
>> qom-list-types result.
>>
>> Signed-off-by: Tobin Feldman-Fitzthum <tobin@linux.vnet.ibm.com>
>> ---
>>   src/qemu/qemu_capabilities.c | 34 ++++++++++++++++++++++++++++++++--
>>   1 file changed, 32 insertions(+), 2 deletions(-)
> 
> I don't feel comfortable reviewing the other two patches which deal with
> the actual use of the capability, so I'll leave those for somebody else.
> 
>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>> index e7179ea048..4a3170fc5c 100644
>> --- a/src/qemu/qemu_capabilities.c
>> +++ b/src/qemu/qemu_capabilities.c
>> @@ -2576,7 +2576,35 @@ virQEMUCapsProbeQMPGenericProps(virQEMUCapsPtr qemuCaps,
>>   }
>>   
>>   static int
>> -virQEMUCapsProbeQMPDevices(virQEMUCapsPtr qemuCaps,
>> +virQEMUCapsProbeQMPTCGState(virQEMUCapsPtr qemuCaps,
> 
> Returned value of the new function is not used, so please define it as
> 'void'
> 
>> +                            char **values,
>> +                            int nvalues)
>> +{
>> +    size_t i;
>> +    bool found = false;
>> +    /*
>> +     * As of version 2.10, QEMU can be built without the TCG.
>> +     * */
>> +    if (qemuCaps->version < 2010000)
>> +        return 0;
>> +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM))
>> +        return 0;
>> +
>> +    for (i = 0; i < nvalues; i++) {
>> +        if (STREQ(values[i], "tcg-accel")) {
>> +            found = true;
> 
> You can set the capability here without intermediate variable and just
> return.

I'm not quite sure what you mean. The capability tracks lack of 
tcg-accel. So what can be done here is return and drop the check before 
setting the capability below.

> 
>> +            break;
>> +        }
>> +    }
>> +
>> +    if (!found)
>> +        virQEMUCapsSet(qemuCaps, QEMU_CAPS_TCG_DISABLED);
>> +
>> +    return 0;
>> +}
>> +
>> +static int
>> +virQEMUCapsProbeQMPTypes(virQEMUCapsPtr qemuCaps,
>>                              qemuMonitorPtr mon)
>>   {
>>       int nvalues;
> 
> With the above resolved:
> 
> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
> 

Michal