On 09/26/2016 03:21 PM, Jim Fehlig wrote:
> On 09/25/2016 01:13 PM, Joao Martins wrote:
>> On 09/25/2016 07:55 PM, Joao Martins wrote:
>>> On 09/24/2016 12:15 AM, Joao Martins wrote:
>>>> On September 23, 2016 11:12:00 PM GMT+01:00, Jim Fehlig <jfehlig@xxxxxxxx> wrote:
>>>>> On 09/22/2016 01:53 PM, Joao Martins wrote:
>>>>>> [snip]
>>>>>> int
>>>>>> -libxlBuildDomainConfig(virPortAllocatorPtr graphicsports,
>>>>>> +libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
>>>>>> virDomainDefPtr def,
>>>>>> libxl_ctx *ctx,
>>>>>> libxl_domain_config *d_config)
>>>>>> {
>>>>>> + virPortAllocatorPtr graphicsports =
>>>>> driver->reservedGraphicsPorts;
>>>>>> +
>>>>> Spurious change?
>>>> This (and the other two below) was intended, as I needed
>>>> channelDir. and instead of having yet another argument, I
>>>> passed driver instead as graphics port was using it too.
>>>>
>>>> But I could use the macro directly, or add another argument if you prefer.
>>> Hmm, I can just avoid passing driver and have cfg->channelDir added as an
>>> argument. I just noticed that I am unnecessarily doing libxlDriverConfigGet
>>> twice and perhaps if a third argument is added in the future then probably
>>> consider having driver be passed as an argument?
>> Or even better have cfg as the function argument instead to allow also removing
>> "ctx" argument. Both channelDir and ctx are stored in cfg. This way we reduce
>> the number of arguments plus allow future usage on other functions called from
>> libxlBuildDomainConfig.
>
> Yep, I think that is fine. We primarily want to avoid making
> libxlBuildDomainConfig difficult to call from the unit tests. I realize we don't
> currently do that, but the eventual plan is to test the conversion of
> virDomainDef to libxl_domain_config. danpb did some initial work on that quite
> some time ago, see commit 5da28f24.
Ah nice to know, I wasn't aware of that work. This cover letter
(https://www.redhat.com/archives/libvir-list/2014-May/msg01102.html) explains it
all too. What I am in this patch is clearly opposite the effort of commit
5da28f24 and a6abdbf. But now I am not sure if what I proposed in my earlier
paragrah is any different: we can probably get away with a mock of
libxlDriverConfig but not sure. In the end sounds like just adding channelDir to
the function arguments might end up being better?
Joao
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list