>-----Original Message-----
>From: Peter Krempa <pkrempa@redhat.com>
>Subject: Re: [PATCH v2 11/21] qemu: Add command line for TDX Quote
>Generation Service(QGS)
>
>On Wed, Jun 25, 2025 at 17:45:23 +0800, Zhenzhong Duan wrote:
>> 'tdx-guest' object supports a "quote-generation-socket" property for
>> attestation purpose. When "quote-generation-socket" is configured in
>> guest xml, libvirt generates unix socket format cmdline for QEMU.
>>
>> 'Path' element can be omitted, default path "/var/run/tdx-qgs/qgs.socket"
>> is used in this case.
>>
>> QEMU command line example:
>> qemu-system-x86_64 \
>> -object
>'{"qom-type":"tdx-guest","id":"lsec0","mrconfigid":"xxx","mrowner":"xxx","m
>rownerconfig":"xxx","quote-generation-socket":{"type":"unix","path":"/var/r
>un/tdx-qgs/qgs.socket"},"attributes":268435457}' \
>> -machine pc-q35-6.0,confidential-guest-support=lsec0
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>> src/conf/domain_conf.h | 3 +++
>> src/qemu/qemu_command.c | 25 +++++++++++++++++++++++++
>> 2 files changed, 28 insertions(+)
>>
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index cf5437f642..b89162df42 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -2999,6 +2999,9 @@ struct _virDomainSEVSNPDef {
>> };
>>
>>
>> +/* Copied from QGS source code */
>> +#define QGS_UNIX_SOCKET_FILE "/var/run/tdx-qgs/qgs.socket"
>> +
>> struct _virDomainTDXDef {
>> bool havePolicy;
>> unsigned long long policy;
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 3081636c24..4cd40d43e9 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -9945,18 +9945,43 @@ qemuBuildPVCommandLine(virCommand
>*cmd)
>> }
>>
>>
>> +static virJSONValue *
>> +qemuBuildTDXQGSCommandLine(virDomainTDXDef *tdx)
>> +{
>> + g_autoptr(virJSONValue) addr = NULL;
>> + const char *path;
>> +
>> + if (!tdx->haveQGS)
>> + return NULL;
>
>Returning NULL on "nothing to do..."
>
>> +
>> + path = tdx->qgs_unix_path ? : QGS_UNIX_SOCKET_FILE;
>
>Avoid the use of the ternary operator. Especially we don't use the
>variant where the first term is empty.
>
>You can initialize path to the constant and overwrite it if
>qgs_unix_path is set via an if statement.
Will do.
>
>> +
>> + if (virJSONValueObjectAdd(&addr,
>> + "s:type", "unix",
>> + "s:path", path,
>> + NULL) < 0)
>> + return NULL;
>
>... and on failure prevents you from actually reporting this failure ..
Good finding, will fix it by open coding qemuBuildTDXQGSCommandLine().
Thanks
Zhenzhong
>
>> +
>> + return g_steal_pointer(&addr);
>> +}
>> +
>> +
>> static int
>> qemuBuildTDXCommandLine(virCommand *cmd, virDomainTDXDef *tdx)
>> {
>> + g_autoptr(virJSONValue) addr = NULL;
>> g_autoptr(virJSONValue) props = NULL;
>>
>> if (tdx->havePolicy)
>> VIR_DEBUG("policy=0x%llx", tdx->policy);
>>
>> + addr = qemuBuildTDXQGSCommandLine(tdx);
>
>... here.
>
>> +
>> if (qemuMonitorCreateObjectProps(&props, "tdx-guest", "lsec0",
>> "S:mrconfigid",
>tdx->mrconfigid,
>> "S:mrowner",
>tdx->mrowner,
>> "S:mrownerconfig",
>tdx->mrownerconfig,
>> + "A:quote-generation-socket",
>&addr,
>> NULL) < 0)
>> return -1;
>>
>> --
>> 2.34.1
>>