Re: [PATCH v2 1/3] libxl: set nestedhvm for mode host-passthrough

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux