Wim ten Have wrote:
On Mon, 17 Apr 2017 12:04:53 -0600
Jim Fehlig <jfehlig(a)suse.com> wrote:
> On 03/24/2017 03:02 PM, Wim Ten Have wrote:
>> From: Wim ten Have <wim.ten.have(a)oracle.com>
>> @@ -2087,15 +2124,15 @@ int
>> libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
>> virDomainDefPtr def,
>> const char *channelDir LIBXL_ATTR_UNUSED,
>> - libxl_ctx *ctx,
>> + libxlDriverConfigPtr cfg,
>> libxl_domain_config *d_config)
> Long ago, in commits 5da28f24 and a6abdbf6, we changed this function signature
> to make it easier to unit test. Unfortunately, the subsequent unit tests were
> never ACKed and committed, but I haven't given up on that effort. Latest attempt
> was sent to the list in February
>
>
https://www.redhat.com/archives/libvir-list/2017-February/msg01477.html
>
> Looks like we just need cfg->caps in the call chain. Can we pass the virCapsPtr
> to libxlBuildDomainConfig instead of the libxlDriverConfig object?
Perhaps I am missing what you try to tell me here. We need cfg->ctx for
libxl allocation requirements. Eliminating by solely switching to virCapsPtr
won't work.
Right, we need ctx *and* caps in this function. I'm suggesting changing the
signature to
libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
virDomainDefPtr def,
const char *channelDir LIBXL_ATTR_UNUSED,
libxl_ctx *ctx,
virCapsPtr caps,
libxl_domain_config *d_config)
That would make it easier for code testing this function. Most test code already
creates the virCaps object for other purposes, so it is already available to
pass into this function. E.g. see how I test this function in the following patch
https://www.redhat.com/archives/libvir-list/2017-February/msg01478.html
Refreshing the test patch after such a change would be as simple as including
'xencaps' in the call to libxlBuildDomainConfig.
Is it good to skip this for now? There's more coming forth soon so i'll keep
this in mind and try to give it a good approach near future if possible.
I'd rather add the virCapsPtr parameter to libxlBuildDomainConfig.
Regards,
Jim