[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH QEMU-XEN v4 9/9] xen: make it possible to build
From: |
Stefano Stabellini |
Subject: |
Re: [Qemu-devel] [PATCH QEMU-XEN v4 9/9] xen: make it possible to build without the Xen PV domain builder |
Date: |
Fri, 23 Oct 2015 12:35:50 +0100 |
User-agent: |
Alpine 2.02 (DEB 1266 2009-07-14) |
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.
> @@ -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.
>
- Re: [Qemu-devel] [PATCH QEMU-XEN v4 3/9] xen: Switch to libxengnttab interface for compat shims., (continued)
[Qemu-devel] [PATCH QEMU-XEN v4 8/9] xen: domainbuild: reopen libxenctrl interface after forking for domain watcher., Ian Campbell, 2015/10/21
[Qemu-devel] [PATCH QEMU-XEN v4 6/9] xen: Switch uses of xc_map_foreign_bulk to use libxenforeignmemory API., Ian Campbell, 2015/10/21
[Qemu-devel] [PATCH QEMU-XEN v4 9/9] xen: make it possible to build without the Xen PV domain builder, Ian Campbell, 2015/10/21
[Qemu-devel] [PATCH QEMU-XEN v4 7/9] xen: Use stable library interfaces when they are available., Ian Campbell, 2015/10/21