[libvirt PATCH v3 4/5] qemu: allow use of async teardown in domain

Boris Fiuczynski fiuczy at linux.ibm.com
Mon Jul 10 14:01:15 UTC 2023


On 7/10/23 1:30 PM, Michal Prívozník wrote:
> On 7/5/23 08:20, Boris Fiuczynski wrote:
>> Asynchronous teardown can be specified if the QEMU binary supports it by
>> adding in the domain XML
>>
>>    <features>
>>      ...
>>      <async-teardown enabled='yes|no'/>
>>      ...
>>    </features>
>>
>> By default this new feature is disabled.
>>
>> Signed-off-by: Boris Fiuczynski <fiuczy at linux.ibm.com>
>> Reviewed-by: Thomas Huth <thuth at redhat.com>
>> ---
>>   docs/formatdomain.rst                         |  6 +++
>>   src/conf/domain_conf.c                        | 22 ++++++++++
>>   src/conf/domain_conf.h                        |  1 +
>>   src/conf/schemas/domaincommon.rng             |  9 ++++
>>   src/qemu/qemu_command.c                       | 20 +++++++++
>>   src/qemu/qemu_validate.c                      |  9 ++++
>>   .../async-teardown.x86_64-latest.args         | 37 ++++++++++++++++
>>   tests/qemuxml2argvdata/async-teardown.xml     | 31 +++++++++++++
>>   ...0-async-teardown-disabled.s390x-6.0.0.args | 35 +++++++++++++++
>>   ...-async-teardown-disabled.s390x-latest.args | 36 +++++++++++++++
>>   .../s390-async-teardown-disabled.xml          | 24 ++++++++++
>>   ...async-teardown-no-attrib.s390x-latest.args | 36 +++++++++++++++
>>   .../s390-async-teardown-no-attrib.xml         | 24 ++++++++++
>>   .../s390-async-teardown.s390x-6.0.0.err       |  1 +
>>   .../s390-async-teardown.s390x-latest.args     | 36 +++++++++++++++
>>   .../qemuxml2argvdata/s390-async-teardown.xml  | 24 ++++++++++
>>   tests/qemuxml2argvtest.c                      |  7 +++
>>   .../async-teardown.x86_64-latest.xml          | 44 +++++++++++++++++++
>>   ...90-async-teardown-disabled.s390x-6.0.0.xml | 36 +++++++++++++++
>>   ...0-async-teardown-disabled.s390x-latest.xml | 36 +++++++++++++++
>>   ...-async-teardown-no-attrib.s390x-latest.xml | 36 +++++++++++++++
>>   .../s390-async-teardown.s390x-latest.xml      | 36 +++++++++++++++
>>   tests/qemuxml2xmltest.c                       |  6 +++
>>   23 files changed, 552 insertions(+)
>>   create mode 100644 tests/qemuxml2argvdata/async-teardown.x86_64-latest.args
>>   create mode 100644 tests/qemuxml2argvdata/async-teardown.xml
>>   create mode 100644 tests/qemuxml2argvdata/s390-async-teardown-disabled.s390x-6.0.0.args
>>   create mode 100644 tests/qemuxml2argvdata/s390-async-teardown-disabled.s390x-latest.args
>>   create mode 100644 tests/qemuxml2argvdata/s390-async-teardown-disabled.xml
>>   create mode 100644 tests/qemuxml2argvdata/s390-async-teardown-no-attrib.s390x-latest.args
>>   create mode 100644 tests/qemuxml2argvdata/s390-async-teardown-no-attrib.xml
>>   create mode 100644 tests/qemuxml2argvdata/s390-async-teardown.s390x-6.0.0.err
>>   create mode 100644 tests/qemuxml2argvdata/s390-async-teardown.s390x-latest.args
>>   create mode 100644 tests/qemuxml2argvdata/s390-async-teardown.xml
>>   create mode 100644 tests/qemuxml2xmloutdata/async-teardown.x86_64-latest.xml
>>   create mode 100644 tests/qemuxml2xmloutdata/s390-async-teardown-disabled.s390x-6.0.0.xml
>>   create mode 100644 tests/qemuxml2xmloutdata/s390-async-teardown-disabled.s390x-latest.xml
>>   create mode 100644 tests/qemuxml2xmloutdata/s390-async-teardown-no-attrib.s390x-latest.xml
>>   create mode 100644 tests/qemuxml2xmloutdata/s390-async-teardown.s390x-latest.xml
>>
>> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
>> index f29449f749..98273c87ad 100644
>> --- a/docs/formatdomain.rst
>> +++ b/docs/formatdomain.rst
>> @@ -2000,6 +2000,7 @@ Hypervisors may allow certain CPU / machine features to be toggled on/off.
>>        <tcg>
>>          <tb-cache unit='MiB'>128</tb-cache>
>>        </tcg>
>> +     <async-teardown enabled='yes'/>
>>      </features>
>>      ...
>>   
>> @@ -2230,6 +2231,11 @@ are:
>>      tb-cache    The size of translation block cache size       an integer (a multiple of MiB)                      :since:`8.0.0`
>>      =========== ============================================== =================================================== ==============
>>   
>> +``async-teardown``
>> +   Depending on the ``enabled`` attribute (values ``yes``, ``no``) enable or
>> +   disable QEMU asynchronous teardown to improve memory reclaiming on a guest.
>> +   :since:`Since 9.5.0` (QEMU only)
> 
> Unfortunately, this has missed 9.5.0 timeframe.

I missed that before sending v3.
Thanks for catching and fixing it.

> 
>> +
>>   Time keeping
>>   ------------
>>   
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 4121b6a054..5ac5c0b771 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -181,6 +181,7 @@ VIR_ENUM_IMPL(virDomainFeature,
>>                 "sbbc",
>>                 "ibs",
>>                 "tcg",
>> +              "async-teardown",
>>   );
>>   
>>   VIR_ENUM_IMPL(virDomainCapabilitiesPolicy,
>> @@ -16689,6 +16690,20 @@ virDomainFeaturesDefParse(virDomainDef *def,
>>                   return -1;
>>               break;
>>   
>> +        case VIR_DOMAIN_FEATURE_ASYNC_TEARDOWN: {
>> +            virTristateBool enabled;
>> +
>> +            if (virXMLPropTristateBool(nodes[i], "enabled",
>> +                                       VIR_XML_PROP_NONE, &enabled) < 0)
>> +                return -1;
>> +
>> +            if (enabled == VIR_TRISTATE_BOOL_ABSENT)
>> +                enabled = VIR_TRISTATE_BOOL_YES;
>> +
>> +            def->features[val] = enabled;
>> +            break;
>> +        }
>> +
>>           case VIR_DOMAIN_FEATURE_LAST:
>>               break;
>>           }
>> @@ -20628,6 +20643,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src,
>>   
>>           case VIR_DOMAIN_FEATURE_MSRS:
>>           case VIR_DOMAIN_FEATURE_TCG:
>> +        case VIR_DOMAIN_FEATURE_ASYNC_TEARDOWN:
>>           case VIR_DOMAIN_FEATURE_LAST:
>>               break;
>>           }
>> @@ -27340,6 +27356,12 @@ virDomainDefFormatFeatures(virBuffer *buf,
>>               virDomainFeatureTCGFormat(&childBuf, def);
>>               break;
>>   
>> +        case VIR_DOMAIN_FEATURE_ASYNC_TEARDOWN:
>> +            if (def->features[i] != VIR_TRISTATE_SWITCH_ABSENT)
>> +                virBufferAsprintf(&childBuf, "<async-teardown enabled='%s'/>\n",
>> +                                  virTristateBoolTypeToString(def->features[i]));
>> +            break;
>> +
>>           case VIR_DOMAIN_FEATURE_LAST:
>>               break;
>>           }
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index cddaa3824d..c857ba556f 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -2170,6 +2170,7 @@ typedef enum {
>>       VIR_DOMAIN_FEATURE_SBBC,
>>       VIR_DOMAIN_FEATURE_IBS,
>>       VIR_DOMAIN_FEATURE_TCG,
>> +    VIR_DOMAIN_FEATURE_ASYNC_TEARDOWN,
>>   
>>       VIR_DOMAIN_FEATURE_LAST
>>   } virDomainFeature;
>> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
>> index fcf9e00600..c2f56b0490 100644
>> --- a/src/conf/schemas/domaincommon.rng
>> +++ b/src/conf/schemas/domaincommon.rng
>> @@ -6660,6 +6660,15 @@
>>             <optional>
>>               <ref name="tcgfeatures"/>
>>             </optional>
>> +          <optional>
>> +            <element name="async-teardown">
>> +              <optional>
>> +                <attribute name="enabled">
>> +                  <ref name="virYesNo"/>
>> +                </attribute>
>> +              </optional>
>> +            </element>
>> +          </optional>
>>           </interleave>
>>         </element>
>>       </optional>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index cde6ab4dde..3d386e1738 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -10175,6 +10175,23 @@ qemuBuildCryptoCommandLine(virCommand *cmd,
>>   }
>>   
>>   
>> +static int
>> +qemuBuildAsyncTeardownCommandLine(virCommand *cmd,
>> +                                  const virDomainDef *def,
>> +                                  virQEMUCaps *qemuCaps)
>> +{
>> +    g_autofree char *async = NULL;
>> +
>> +    if (def->features[VIR_DOMAIN_FEATURE_ASYNC_TEARDOWN] != VIR_TRISTATE_BOOL_ABSENT &&
> 
> For this ^^
> 
>> +        virQEMUCapsGet(qemuCaps, QEMU_CAPS_RUN_WITH_ASYNC_TEARDOWN)) {
>> +        async = g_strdup_printf("async-teardown=%s",
>> +                                virTristateSwitchTypeToString(def->features[VIR_DOMAIN_FEATURE_ASYNC_TEARDOWN]));
> 
> and this ^^ let me use a variable. It's going to be more readable that way.

Ok, that shortens the lines a bit. Thanks.

> 
>> +        virCommandAddArgList(cmd, "-run-with", async, NULL);
>> +    }
>> +    return 0;
>> +}
> 
> 
> Michal
> 

Thanks, Michal


-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294



More information about the libvir-list mailing list