On 22.09.2016 15:39, Jiri Denemark wrote:
> On Tue, Sep 20, 2016 at 15:54:54 +0200, Michal Privoznik wrote:
>> Looks like nobody tried migrations lately.
> 
> This statement sounds a bit too strong for such a narrow use case.
> 
>> I just did and found
>> interesting bug. On the destination qemu binary was at different
>> path. So I've dumped domain XML I was trying ti migrate and just
>> changed the emulator path. All of a sudden I observed plenty of
>> errors. Problem is that whilst parsing the user provided
>> migration XML (and doing other operations on it), because of
>> parse callbacks we need qemuCaps for the binary. However, we just
>> take the def->emulator and expect it to exist. Well, it doesn't.
> 
> I think you're just the first one who tried migrating to a host where
> QEMU is installed in a different path. Or at least the first one who
> didn't just make a symlink to make QEMU binary reachable with the same
> path :-)
> 
>> Michal Privoznik (9):
>>   conf: Introduce virDomainDefPostParseOpaque
>>   conf: Introduce virDomainDefParseStringOpaque
>>   qemuDomainDefPostParse: Fetch qemuCaps from domain object
>>   conf: Extend virDomainDeviceDefPostParse for parseOpaque
>>   qemuDomainDeviceDefPostParse: Fetch caps from domain object
>>   conf: Extend virDomainDefAssignAddressesCallback for parseOpaque
>>   qemuDomainDefAssignAddresses: Fetch caps from domain object
>>   domain_conf: Introduce VIR_DOMAIN_DEF_PARSE_SKIP_POST_PARSE
>>   conf: Skip post parse callbacks when creating copy
> 
> Overall, the series looks OK, except for the new *Opaque functions. They
> are all internal APIs and I think just adding an extra parameter to the
> existing functions is much better than introducing new ones with even
> longer and uglier names. And git grep doesn't even show a lot of callers
> so the change to the existing prototypes won't be too invasive.

Ah, okay, I can stick with that approach then. I thought that this is
going to be more acceptable to the upstream. Don't really know why now.

> 
> And do you plan to actually use vm pointers anywhere in the post-parse
> callbacks in the future? If it's not the case, I think passing
> priv->qemuCaps directly would be a bit better.

Well, okay. I think domain objects are more versatile, but if somebody
needs them, we can always change that.

So let me respin version 2.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to