Re: [Xen-devel] [PATCH QEMU-XEN v4 9/9] xen: make it possible to build without the Xen PV domain builder

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

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?

@@ -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.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel