Re: [libvirt-go PATCH 2/2] Add support for AMD SEV platform info

[Daniel P. Berrangé <berrange@xxxxxxxxxx>]

On Thu, Jun 14, 2018 at 04:30:01PM +0200, Erik Skultety wrote:
> Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx>
> ---
>  connect.go        | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  connect_compat.go | 12 +++++++++++
>  connect_compat.h  |  7 +++++++
>  3 files changed, 78 insertions(+)
> 
> diff --git a/connect.go b/connect.go
> index e3e643e..8bb5fe6 100644
> --- a/connect.go
> +++ b/connect.go
> @@ -2765,3 +2765,62 @@ func (c *Connect) GetAllDomainStats(doms []*Domain, statsTypes DomainStatsTypes,
>  
>  	return stats, nil
>  }
> +
> +type NodeSEVParameters struct {
> +	PdhSet             bool
> +	Pdh                string

Lets s/Pdh/PDH/ since its an acronym

> +	CertChainSet       bool
> +	CertChain          string
> +	CbitposSet         bool
> +	Cbitpos            uint

and s/Cbitpos/CBitPos/

> +	ReducedPhysBitsSet bool
> +	ReducedPhysBits    uint
> +}
> +
> +func getNodeSEVFieldInfo(params *NodeSEVParameters) map[string]typedParamsFieldInfo {
> +	return map[string]typedParamsFieldInfo{
> +		C.VIR_NODE_SEV_PDH: typedParamsFieldInfo{
> +			set: &params.PdhSet,
> +			s:   &params.Pdh,
> +		},
> +		C.VIR_NODE_SEV_CERT_CHAIN: typedParamsFieldInfo{
> +			set: &params.CertChainSet,
> +			s:   &params.CertChain,
> +		},
> +		C.VIR_NODE_SEV_CBITPOS: typedParamsFieldInfo{
> +			set: &params.CbitposSet,
> +			ui:   &params.Cbitpos,
> +		},
> +		C.VIR_NODE_SEV_REDUCED_PHYS_BITS: typedParamsFieldInfo{
> +			set: &params.ReducedPhysBitsSet,
> +			ui:   &params.ReducedPhysBits,
> +		},

Miinor nitpick on indentation  - just run gofmt over it



> diff --git a/connect_compat.go b/connect_compat.go
> index 617bc4a..544def2 100644
> --- a/connect_compat.go
> +++ b/connect_compat.go
> @@ -157,5 +157,17 @@ int virConnectCompareHypervisorCPUCompat(virConnectPtr conn,
>  #endif
>  }
>  
> +int virNodeGetSEVInfoCompat(virConnectPtr conn,
> +			    virTypedParameterPtr *params,
> +			    int *nparams,
> +			    unsigned int flags)

Indent here too, though gofmt probably won't fix this since it is C code layer

> +{
> +#if LIBVIR_VERSION_NUMBER < 4005000
> +    assert(0); // Caller should have checked version
> +#else
> +    return virNodeGetSEVInfo(conn, params, nparams, flags);
> +#endif
> +}
> +
>  */
>  import "C"

With the few nitpicks fixed

Reviewed-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list