On 09/26/2016 05:27 PM, Jim Fehlig wrote:
> On 09/26/2016 09:30 AM, Joao Martins wrote:
>> 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.
>
> Some of those patches were pushed. I did some followup work
>
> https://www.redhat.com/archives/libvir-list/2014-September/msg00180.html
>
> which also resulted in some patches being pushed. But the meaty parts of the
> series that actually provide the conversion tests were never committed. The last
> attempt to revive the work also stalled
>
> https://www.redhat.com/archives/libvir-list/2015-January/msg00924.html
>
<nods>
> Thinking about it again, I seems the best way forward is something along the
> lines of option 3 described in that thread
>
> "make use of new functionality in Xen 4.5 such as
> libxl_domain_config_from_json() and libxl_domain_config_compare()"
Interesting, the latter though (libxl_domain_config_compare) doesn't appear to
be implemented on 4.7 (or upcoming 4.8) and sounds like even if implemented that
it would rule out most of the versions :( Probably worked out with a shim for
older versions that implement equivalent of libxl_domain_config_compare().
>> 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.
>
> I suppose mocking it would be easier if libxl_ctx_alloc supported a dummy mode.
> I.e. if this bug was fixed
>
> https://bugs.xenproject.org/xen/bug/41
>
>> In the end sounds like just adding channelDir to
>> the function arguments might end up being better?
>
> IMO that is probably the best approach until we have the conversion tests
> figured out.
Cool, thanks for the feedback! Let me submit v3 with these fixed.
>
> BTW, can channels be hot (un)plugged? If so, it's another argument for just
> passing the channelDir. Future external callers of libxlMakeChannel() would have
> access to the libxlDriverConfig object, and hence channelDir.
AFAICT there's no API for hotplugging channels, very much like serials/consoles
can't be too hotplugged.
Joao
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list