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@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list