[libvirt] [PATCH v2 5/9] libvirt: add new public API to get launch security info

[Brijesh Singh]

On 03/12/2018 07:01 AM, Daniel P. Berrangé wrote:
> On Thu, Mar 08, 2018 at 11:12:04AM -0600, Brijesh Singh wrote:
>> The API can be used outside the libvirt to get the launch security
>> information. When SEV is enabled, the API can be used to get the
>> measurement of the launch process.
>>
>> Signed-off-by: Brijesh Singh <brijesh.singh at amd.com>
>> ---
>>   include/libvirt/libvirt-domain.h | 17 ++++++++++++++
>>   src/driver-hypervisor.h          |  7 ++++++
>>   src/libvirt-domain.c             | 50 ++++++++++++++++++++++++++++++++++++++++
>>   src/libvirt_public.syms          |  5 ++++
>>   4 files changed, 79 insertions(+)
>>
>> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
>> index 4048acf38aaf..11c3fec92bfa 100644
>> --- a/include/libvirt/libvirt-domain.h
>> +++ b/include/libvirt/libvirt-domain.h
>> @@ -4756,4 +4756,21 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
>>                                   unsigned int action,
>>                                   unsigned int flags);
>>   
>> +/**
>> + * Launch Security API
>> + */
>> +
>> +/**
>> + * VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT:
>> + *
>> + * Macro represents the launch measurement of the SEV guest,
>> + * as VIR_TYPED_PARAM_STRING.
>> + */
>> +#define VIR_DOMAIN_LAUNCH_SECURITY_SEV_MEASUREMENT "sev-measurement"
>> +
>> +int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
>> +                                   virTypedParameterPtr params,
>> +                                   int *nparams,
>> +                                   unsigned int flags);
> 
> These days we prefer new APIs to use
> 
>         virTypedParameterPtr *params,
> 
> and have the API implementation allocate the right number of
> elements for the array, so the caller doesn't have to allocate
> anything itself - only free the returned memory.
> 
> See virDomainGetJobStats for an example.
> 

Ah okay, I will take a look and change the API to allocate the array 
elements instead of forcing caller to allocate the arrays.



>> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
>> index ce0e2b252552..dc4873a8ad1c 100644
>> --- a/src/driver-hypervisor.h
>> +++ b/src/driver-hypervisor.h
>> @@ -1283,6 +1283,12 @@ typedef int
>>                                     unsigned int action,
>>                                     unsigned int flags);
>>   
>> +typedef int
>> +(*virDrvDomainGetLaunchSecurityInfo)(virDomainPtr domain,
>> +                                     virTypedParameterPtr params,
>> +                                     int *nparams,
>> +                                     unsigned int flags);
>> +
>>   
>>   typedef struct _virHypervisorDriver virHypervisorDriver;
>>   typedef virHypervisorDriver *virHypervisorDriverPtr;
>> @@ -1528,6 +1534,7 @@ struct _virHypervisorDriver {
>>       virDrvDomainSetVcpu domainSetVcpu;
>>       virDrvDomainSetBlockThreshold domainSetBlockThreshold;
>>       virDrvDomainSetLifecycleAction domainSetLifecycleAction;
>> +    virDrvDomainGetLaunchSecurityInfo domainGetLaunchSecurityInfo;
>>   };
>>   
>>   
>> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
>> index eaec0979ad49..21356bb92894 100644
>> --- a/src/libvirt-domain.c
>> +++ b/src/libvirt-domain.c
>> @@ -12095,3 +12095,53 @@ int virDomainSetLifecycleAction(virDomainPtr domain,
>>       virDispatchError(domain->conn);
>>       return -1;
>>   }
>> +
>> +/**
>> + * virDomainGetLaunchSecurityInfo:
>> + * @domain: a domain object
>> + * @params: where to store security info
>> + * @nparams: number of items in @params
>> + * @flags: bitwise-OR of virDomainModificationImpact
> 
> This API doesn't use virDomainModificationImpact. So just say
> 
>     @flags currently used, set to 0
> 

Noted. thanks


>> + *
>> + * Get the launch security info. In case of the SEV guest, this will
>> + * return the launch measurement.
>> + *
>> + * Returns -1 in case of failure, 0 in case of success.
>> + */
>> +int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
>> +                                   virTypedParameterPtr params,
>> +                                   int *nparams,
>> +                                   unsigned int flags)
>> +{
>> +    virConnectPtr conn;
>> +
>> +    VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%p flags=0x%x",
>> +                     params, nparams, flags);
>> +
>> +    virResetLastError();
>> +
>> +    virCheckDomainReturn(domain, -1);
>> +    virCheckNonNegativeArgGoto(*nparams, error);
>> +    if (*nparams != 0)
>> +        virCheckNonNullArgGoto(params, error);
> 
> We should require both args to be non-null, when we do allocation
> ourselves.
> 
> I think we probably want to forbid this method for read-only
> users, so add
> 
>    virCheckReadOnlyGoto(conn->flags, error);
> 
> 
> 

Okay, got it.


>> +
>> +    if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
>> +                                 VIR_DRV_FEATURE_TYPED_PARAM_STRING))
>> +        flags |= VIR_TYPED_PARAM_STRING_OKAY;
>> +
>> +    conn = domain->conn;
>> +
>> +    if (conn->driver->domainGetLaunchSecurityInfo) {
>> +        int ret;
>> +        ret = conn->driver->domainGetLaunchSecurityInfo(domain, params,
>> +                                                        nparams, flags);
>> +        if (ret < 0)
>> +            goto error;
>> +        return ret;
>> +    }
>> +    virReportUnsupportedError();
>> +
>> + error:
>> +    virDispatchError(domain->conn);
>> +    return -1;
>> +}
>> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
>> index 95df3a0dbc7b..caba2862d371 100644
>> --- a/src/libvirt_public.syms
>> +++ b/src/libvirt_public.syms
>> @@ -785,4 +785,9 @@ LIBVIRT_4.1.0 {
>>           virStoragePoolLookupByTargetPath;
>>   } LIBVIRT_3.9.0;
>>   
>> +LIBVIRT_4.2.0 {
>> +    global:
>> +        virDomainGetLaunchSecurityInfo;
>> +} LIBVIRT_4.1.0;
>> +
>>   # .... define new API here using predicted next version number ....
>> -- 
>> 2.14.3
>>
> 
> Regards,
> Daniel
>