On Thu, Feb 25, 2016 at 08:58:23AM -0500, Cole Robinson wrote: > On 02/25/2016 08:50 AM, Ján Tomko wrote: > > On Thu, Feb 25, 2016 at 07:45:17AM -0500, Cole Robinson wrote: > >> On 02/25/2016 07:24 AM, Ján Tomko wrote: > >>> Revert commit 55e6d8cd9eac7eb2aaa4d221585e9402cf7269d5. > >>> > >>> It unconditionally required a machine type for all machine types > >>> even though qemu is the only emulator using them. > >>> > >>> Reverting this re-introduces the crash when someone fiddles with > >>> libvirt's machine configs with /etc/, but fixes persistent domains > >>> for other drivers. > >>> > >>> Breaks https://bugzilla.redhat.com/show_bug.cgi?id=1267256 again. > >>> --- > >>> A proper fix would be too invasive for the freeze, we would either > >>> * have to revert commit f1a89a8 which relaxed the requirement > >>> to execute the emulator > >>> * or (partially) revert commits a8b628e and 3d92a00 which reintroduced > >>> them to post parse callbacks and keep track of which parser > >>> requires the machine type in virDomainDefParserConfig. > >>> > >>> src/conf/domain_conf.c | 6 ------ > >>> 1 file changed, 6 deletions(-) > >>> > >>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > >>> index 3b15cb4..79758d4 100644 > >>> --- a/src/conf/domain_conf.c > >>> +++ b/src/conf/domain_conf.c > >>> @@ -14851,12 +14851,6 @@ virDomainDefParseXML(xmlDocPtr xml, > >>> goto error; > >>> } > >>> VIR_FREE(capsdata); > >>> - } else { > >>> - if (!def->os.machine) { > >>> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > >>> - _("Missing machine type")); > >>> - goto error; > >>> - } > >>> } > >>> > >>> /* Extract domain name */ > >>> > >> > >> Why not just move that check to the beginning of the qemu specific > >> qemuDomainDefPostParse? And add a test case that hits the error > >> > > > > That is basically the second option I suggested and this patch is a > > prerequisite for that. > > > > I don't follow... that option mentions reverting patches and you called it > invasive. I'm thinking this patch plus: Right, VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS not being useful for qemu driver is an unrelated issue. > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index c56f9f1..610ffdd 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -1307,6 +1307,12 @@ qemuDomainDefPostParse(virDomainDefPtr def, > return ret; > } > > + if (!def->os.machine) { > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("Missing machine type")); > + goto error; > + } > + > /* check for emulator and create a default one if needed */ > if (!def->emulator && > !(def->emulator = virDomainDefGetDefaultEmulator(def, caps))) > > > That should fix the original bug, and the lxc issue John reported, and seems > safe for the next release IMO. I'll send v2 series with tests. Jan -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list