From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54358) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpbNE-0004Xy-Kk for qemu-devel@nongnu.org; Fri, 23 Oct 2015 08:23:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZpbNA-0001rD-Mf for qemu-devel@nongnu.org; Fri, 23 Oct 2015 08:23:08 -0400 Received: from smtp02.citrix.com ([66.165.176.63]:32347) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpbNA-0001qE-HA for qemu-devel@nongnu.org; Fri, 23 Oct 2015 08:23:04 -0400 Message-ID: <1445602981.2374.137.camel@citrix.com> From: Ian Campbell Date: Fri, 23 Oct 2015 13:23:01 +0100 In-Reply-To: References: <1445440941.9563.163.camel@citrix.com> <1445441038-25903-1-git-send-email-ian.campbell@citrix.com> <1445441038-25903-10-git-send-email-ian.campbell@citrix.com> <1445599197.2374.127.camel@citrix.com> Content-Type: text/plain; charset="UTF-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH QEMU-XEN v4 9/9] xen: make it possible to build without the Xen PV domain builder List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefano Stabellini Cc: wei.liu2@citrix.com, ian.jackson@eu.citrix.com, qemu-devel@nongnu.org, xen-devel@lists.xen.org On Fri, 2015-10-23 at 12:35 +0100, Stefano Stabellini wrote: > On Fri, 23 Oct 2015, Ian Campbell wrote: > > On Fri, 2015-10-23 at 12:12 +0100, Stefano Stabellini wrote: > > > @@ -2113,6 +2117,15 @@ if test "$xen_pci_passthrough" != "no"; then > > > > fi > > > > fi > > > > > > > > +if test "$xen_pv_domain_build" != "no"; then > > > > + if test "$xen_pv_domain_build" = "yes" && > > > > + test "$xen" != "yes"; then > > > > + error_exit "User requested Xen PV domain builder support" \ > > > > + "which requires Xen support." > > > > + fi > > > > + xen_pv_domain_build=no > > > > +fi > > > > > > Can we simplify this to: > > > > > > if test "$xen_pv_domain_build" = "yes" && > > > test "$xen" != "yes"; then > > > error_exit "User requested Xen PV domain builder support" \ > > > "which requires Xen support." > > > fi > > > fi > > > > > > and move xen_pv_domain_build=no at the beginning of the file? > > > > I think so, I hadn't noticed the precedent for doing so further up in > > the > > file. > > > > Just to check, is this still your preference after my earlier reply-to > > -self > > explaining why the code above is utter rubbish and proposing a > > different > > simplified version? > > The simplified check is OK, but I would still prefer if you moved > xen_pv_domain_build=no at the beginning of the file. Ack, I'll do that then. > > > > @@ -46,13 +43,22 @@ static void xen_init_pv(MachineState *machine) > > > > case XEN_ATTACH: > > > > /* nothing to do, xend handles everything */ > > > > break; > > > > - case XEN_CREATE: > > > > + case XEN_CREATE: { > > > > +#ifdef CONFIG_XEN_PV_DOMAIN_BUILD > > > > + const char *kernel_filename = machine->kernel_filename; > > > > + const char *kernel_cmdline = machine->kernel_cmdline; > > > > + const char *initrd_filename = machine->initrd_filename; > > > > if (xen_domain_build_pv(kernel_filename, initrd_filename, > > > > kernel_cmdline) < 0) { > > > > fprintf(stderr, "xen pv domain creation failed\n"); > > > > exit(1); > > > > } > > > > +#else > > > > + fprintf(stderr, "xen pv domain creation not supported\n"); > > > > + exit(1); > > > > +#endif > > > > break; > > > > + } > > > > case XEN_EMULATE: > > > > fprintf(stderr, "xen emulation not implemented (yet)\n"); > > > > exit(1); > > > > > > Please add a default case with an error and place the XEN_CREATE > > > entirely within the ifdef CONFIG_XEN_PV_DOMAIN_BUILD. > > > > Will do. > >