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

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

Reply via email to