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
