Wim ten Have wrote:
> On Mon, 17 Apr 2017 12:04:53 -0600
> Jim Fehlig <[email protected]> wrote:
>
>> On 03/24/2017 03:02 PM, Wim Ten Have wrote:
>>> From: Wim ten Have <[email protected]>
>>> @@ -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
--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list
