From patchwork Thu Feb 25 13:58:23 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Cole Robinson X-Patchwork-Id: 62890 Delivered-To: patch@linaro.org Received: by 10.112.199.169 with SMTP id jl9csp175519lbc; Thu, 25 Feb 2016 06:01:20 -0800 (PST) X-Received: by 10.31.139.132 with SMTP id n126mr37146168vkd.78.1456408880822; Thu, 25 Feb 2016 06:01:20 -0800 (PST) Return-Path: Received: from mx5-phx2.redhat.com (mx5-phx2.redhat.com. [209.132.183.37]) by mx.google.com with ESMTPS id 184si5177711vkj.191.2016.02.25.06.01.19 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 25 Feb 2016 06:01:20 -0800 (PST) Received-SPF: pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.37 as permitted sender) client-ip=209.132.183.37; Authentication-Results: mx.google.com; spf=pass (google.com: domain of libvir-list-bounces@redhat.com designates 209.132.183.37 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx5-phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1PDwPSJ025821; Thu, 25 Feb 2016 08:58:25 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id u1PDwN2N022492 for ; Thu, 25 Feb 2016 08:58:23 -0500 Received: from [10.16.196.225] (wlan-196-225.bos.redhat.com [10.16.196.225]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1PDwN3b012227; Thu, 25 Feb 2016 08:58:23 -0500 To: =?UTF-8?Q?J=c3=a1n_Tomko?= References: <56CEF75D.8040804@redhat.com> <20160225135036.GB8729@dnr.brq.redhat.com> From: Cole Robinson Message-ID: <56CF087F.9040404@redhat.com> Date: Thu, 25 Feb 2016 08:58:23 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <20160225135036.GB8729@dnr.brq.redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-loop: libvir-list@redhat.com Cc: libvir-list@redhat.com Subject: Re: [libvirt] [PATCH] Revert "Error out on missing machine type in machine configs" X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com 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: That should fix the original bug, and the lxc issue John reported, and seems safe for the next release IMO. Or am I missing something and that solution isn't sufficient? Thanks, Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list 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)))