[libvirt] qemuParseCommandLine and virDomainDefPostParse (and virDomaniDefAddImplicitControllers)
While playing with the idea of forcing explicit USB controller models, I ended up with the qemuargv2xml test failing; it ended up that this was because changes I had made to qemuDomainDefPostParse() were affecting the XML produced by qemuParseCommandline(). The reason - after constructing a virDomainDef object by parsing a qemu commandline, qemuParseCommandline() calls two functions that are supposed to be called after parsing domain XML - virDomainDefPostParse() (which calls qemuDomainDefPostParse()). In my opinion, qemuParseCommandLine() shouldn't be calling virDomainDefPostParse() (or virDomainefAddImplicitControllers(), which it calls in the wrong order relative to virDomainDefPostParse() BTW). The reasons are: 1) this is causing the argv-to-xml conversion to include things in the XML that were not on the original commandline, in particular "default" devices like PCI and USB controllers (added in qemuDomainDefPostParse() based on machinetype) as well as disk, smartcard, virtio-serial, and hostdev-scsi controllers in virDomainDefAddImplicitControllers() (why the duality there, anyway?) 2) If the output of argv-to-xml is used for a virDomainDefine, those post-parse functions will be called then, and the implicit/auto devices will be added at that time anyway, so in practice nothing is gained by adding them during argv-to-xml. Does anyone else have an opinion about this?
On 18.11.2015 21:19, Laine Stump wrote:
While playing with the idea of forcing explicit USB controller models, I ended up with the qemuargv2xml test failing; it ended up that this was because changes I had made to qemuDomainDefPostParse() were affecting the XML produced by qemuParseCommandline(). The reason - after constructing a virDomainDef object by parsing a qemu commandline, qemuParseCommandline() calls two functions that are supposed to be called after parsing domain XML - virDomainDefPostParse() (which calls qemuDomainDefPostParse()).
I'm afraid you are touching area of the code that is not maintained. Brace yourself :)
In my opinion, qemuParseCommandLine() shouldn't be calling virDomainDefPostParse() (or virDomainefAddImplicitControllers(), which it calls in the wrong order relative to virDomainDefPostParse() BTW). The reasons are:
1) this is causing the argv-to-xml conversion to include things in the XML that were not on the original commandline, in particular "default" devices like PCI and USB controllers (added in qemuDomainDefPostParse() based on machinetype) as well as disk, smartcard, virtio-serial, and hostdev-scsi controllers in virDomainDefAddImplicitControllers() (why the duality there, anyway?)
Hold on, so they are not even visible in the guest? I mean, if I run qemu by hand, don't I always get virtio-serial? If not, then we should not add them during cmd line parse.
2) If the output of argv-to-xml is used for a virDomainDefine, those post-parse functions will be called then, and the implicit/auto devices will be added at that time anyway, so in practice nothing is gained by adding them during argv-to-xml.
Correct.
Does anyone else have an opinion about this?
I believe there's not much gain in calling post parse callbacks during cmd line parsing. Michal
On Thu, Nov 19, 2015 at 11:12:29 +0100, Michal Privoznik wrote:
On 18.11.2015 21:19, Laine Stump wrote:
[...]
Does anyone else have an opinion about this?
I believe there's not much gain in calling post parse callbacks during cmd line parsing.
Well, then you have to extract all the code that is setting the hypervisor-specific defaults that can be omitted on the commandline but need to be present in the definition (e.g. default network card model) into the command line parser, otherwise certain parts of the code will crash (e.g. network card model formatter). Peter
On 11/19/2015 07:50 AM, Peter Krempa wrote:
On Thu, Nov 19, 2015 at 11:12:29 +0100, Michal Privoznik wrote:
On 18.11.2015 21:19, Laine Stump wrote: [...]
Does anyone else have an opinion about this?
I believe there's not much gain in calling post parse callbacks during cmd line parsing. Well, then you have to extract all the code that is setting the hypervisor-specific defaults that can be omitted on the commandline but need to be present in the definition (e.g. default network card model) into the command line parser, otherwise certain parts of the code will crash (e.g. network card model formatter).
Ugh. Right - once you've got the XML out it will get all the defaults when it is fed back in to the parser with a virDomainDefine. But the very first time it is formatted, right at the end of the argv-to-xml, that could cause a problem. I would say though that, rather than fixing up the object created from parsing a qemu commandline by calling functions that are supposed to be called to post-process the results of parsing XML to get it ready for turning into a qemu commandline, we should instead make the formatter more robust, so that it is able to format whatever it is fed without a crash (I know that's a tall order!). What a can of worms...
On Thu, Nov 19, 2015 at 10:15:25 -0500, Laine Stump wrote:
On 11/19/2015 07:50 AM, Peter Krempa wrote:
On Thu, Nov 19, 2015 at 11:12:29 +0100, Michal Privoznik wrote:
On 18.11.2015 21:19, Laine Stump wrote: [...]
Does anyone else have an opinion about this?
I believe there's not much gain in calling post parse callbacks during cmd line parsing. Well, then you have to extract all the code that is setting the hypervisor-specific defaults that can be omitted on the commandline but need to be present in the definition (e.g. default network card model) into the command line parser, otherwise certain parts of the code will crash (e.g. network card model formatter).
Ugh. Right - once you've got the XML out it will get all the defaults when it is fed back in to the parser with a virDomainDefine. But the very first time it is formatted, right at the end of the argv-to-xml, that could cause a problem.
That is the same case as when you try to attach to a existing process. It's very unlikely to actually work, but there is a possibility to reuse the object created from argv parsing directly. In that case we probably do need to have the definiton working. Peter
On 11/19/2015 10:29 AM, Peter Krempa wrote:
On Thu, Nov 19, 2015 at 11:12:29 +0100, Michal Privoznik wrote:
On 18.11.2015 21:19, Laine Stump wrote: [...]
Does anyone else have an opinion about this?
I believe there's not much gain in calling post parse callbacks during cmd line parsing. Well, then you have to extract all the code that is setting the hypervisor-specific defaults that can be omitted on the commandline but need to be present in the definition (e.g. default network card model) into the command line parser, otherwise certain parts of the code will crash (e.g. network card model formatter). Ugh. Right - once you've got the XML out it will get all the defaults when it is fed back in to the parser with a virDomainDefine. But the very first time it is formatted, right at the end of the argv-to-xml,
On 11/19/2015 07:50 AM, Peter Krempa wrote: that could cause a problem. That is the same case as when you try to attach to a existing process. It's very unlikely to actually work, but there is a possibility to reuse
On Thu, Nov 19, 2015 at 10:15:25 -0500, Laine Stump wrote: the object created from argv parsing directly. In that case we probably do need to have the definiton working.
That could be handled by either adding the post-parse stuff into qemuDomainAttach(), or alternately by doing a format-parse cycle on the object (again, in qemuDomainAttach()) rather than using it directly (which I actually think could give better results, as long as qemuParseCommandLine() itself is doing the right stuff). Note that in the case where the original commandline doesn't include the PCI addresses of devices, we're very likely to end up with incorrect address info in the XML, so hopefully nobody relies on that...
participants (3)
-
Laine Stump -
Michal Privoznik -
Peter Krempa