Re: [libvirt] [PATCH 0/9] Couple of migration fixes
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.
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.
Jirka