Re: [Qemu-devel] [PATCH 1/7] xen: groundwork for xen support

[Anthony Liguori <anthony@codemonkey.ws>]

Gerd Hoffmann wrote:
> - configure script and build system changes.
> - wind up new machine type.
> - add -domid command line option.
> - allow xenpv machines run without disk and kernel specified.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  Makefile.target       |    8 +++++
>  configure             |   27 ++++++++++++++++
>  hw/boards.h           |    4 ++
>  hw/xen-machine.c      |   83 +++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/xen.h              |   12 +++++++
>  target-i386/machine.c |    3 ++
>  vl.c                  |   25 +++++++++++++--
>  7 files changed, 159 insertions(+), 3 deletions(-)
>  create mode 100644 hw/xen-machine.c
>  create mode 100644 hw/xen.h
>
> diff --git a/Makefile.target b/Makefile.target
> index ff105c1..4f42582 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -515,6 +515,14 @@ CPPFLAGS += $(CONFIG_VNC_TLS_CFLAGS)
>  LIBS += $(CONFIG_VNC_TLS_LIBS)
>  endif
>  
> +# xen backend driver support
> +XEN_OBJS := xen-machine.o
> +ifeq ($(CONFIG_XEN), yes)
> +  OBJS += $(XEN_OBJS)
> +  LIBS += $(XEN_LIBS)
> +  $(XEN_OBJS) : CFLAGS += -Wall -Wmissing-prototypes -Wstrict-prototypes
> +endif
> +
>  # SCSI layer
>  OBJS+= lsi53c895a.o esp.o
>  
> diff --git a/configure b/configure
> index fd04766..34516b9 100755
> --- a/configure
> +++ b/configure
> @@ -108,6 +108,7 @@ uname_release=""
>  curses="yes"
>  nptl="yes"
>  mixemu="no"
> +xen="no"
>  
>  # OS specific
>  targetos=`uname -s`
> @@ -202,6 +203,7 @@ linux="yes"
>  linux_user="yes"
>  if [ "$cpu" = "i386" -o "$cpu" = "x86_64" ] ; then
>      kqemu="yes"
> +    xen="yes"
>      audio_possible_drivers="$audio_possible_drivers fmod"
>  fi
>  ;;
> @@ -285,6 +287,8 @@ for opt do
>    ;;
>    --disable-kqemu) kqemu="no"
>    ;;
> +  --disable-xen) xen="no"
> +  ;;
>    --disable-brlapi) brlapi="no"
>    ;;
>    --enable-profiler) profiler="yes"
> @@ -421,6 +425,7 @@ echo "                           Available drivers: $audio_possible_drivers"
>  echo "  --audio-card-list=LIST   set list of additional emulated audio cards"
>  echo "                           Available cards: ac97 adlib cs4231a gus"
>  echo "  --enable-mixemu          enable mixer emulation"
> +echo "  --disable-xen            disable xen backend driver support"
>  echo "  --disable-brlapi         disable BrlAPI"
>  echo "  --disable-vnc-tls        disable TLS encryption for VNC server"
>  echo "  --disable-curses         disable curses output"
> @@ -681,6 +686,22 @@ else
>  fi
>  
>  ##########################################
> +# xen probe
> +
> +if test "$xen" = "yes" ; then
> +cat > $TMPC <<EOF
> +#include <xs.h>
> +#include <xenctrl.h>
> +int main(void) { xs_daemon_open; xc_interface_open; }
> +EOF
> +   if $cc $ARCH_CFLAGS -c -o $TMPO $TMPC -lxenstore -lxenctrl 2> /dev/null ; then
> +      :
> +   else
> +      xen="no"
> +   fi
> +fi
> +
> +##########################################
>  # SDL probe
>  
>  sdl_too_old=no
> @@ -916,6 +937,7 @@ if test -n "$sparc_cpu"; then
>      echo "Target Sparc Arch $sparc_cpu"
>  fi
>  echo "kqemu support     $kqemu"
> +echo "xen support       $xen"
>  echo "brlapi support    $brlapi"
>  echo "Documentation     $build_docs"
>  [ ! -z "$uname_release" ] && \
> @@ -1167,6 +1189,11 @@ if test "$brlapi" = "yes" ; then
>    echo "#define CONFIG_BRLAPI 1" >> $config_h
>    echo "BRLAPI_LIBS=-lbrlapi" >> $config_mak
>  fi
> +if test "$xen" = "yes" ; then
> +  echo "CONFIG_XEN=yes" >> $config_mak
> +  echo "#define CONFIG_XEN 1" >> $config_h
> +  echo "XEN_LIBS=-lxenstore -lxenctrl" >> $config_mak
> +fi
>  
>  # XXX: suppress that
>  if [ "$bsd" = "yes" ] ; then
> diff --git a/hw/boards.h b/hw/boards.h
> index 22ac332..5931720 100644
> --- a/hw/boards.h
> +++ b/hw/boards.h
> @@ -29,6 +29,10 @@ extern QEMUMachine bareetraxfs_machine;
>  extern QEMUMachine pc_machine;
>  extern QEMUMachine isapc_machine;
>  
> +/* xen_machine.c */
> +extern QEMUMachine xenpv_machine;
> +extern QEMUMachine xenfv_machine;
>   

Why does xenfv need its own machine type?

>  /* ppc.c */
>  extern QEMUMachine prep_machine;
>  extern QEMUMachine core99_machine;
> diff --git a/hw/xen-machine.c b/hw/xen-machine.c
> new file mode 100644
> index 0000000..88f0f6e
> --- /dev/null
> +++ b/hw/xen-machine.c
> @@ -0,0 +1,83 @@
> +/*
> + * QEMU Xen PV Machine
> + *
> + * Copyright (c) 2007,08 Red Hat
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "hw.h"
> +#include "boards.h"
> +
> +#include "xen.h"
> +
> +/* -------------------------------------------------------------------- */
> +/* variables                                                            */
> +
> +int xen_domid;
> +
> +/* -------------------------------------------------------------------- */
> +/* paravirtualized xen guests                                           */
> +
> +static void xenpv_init(ram_addr_t ram_size, int vga_ram_size,
> +		       const char *boot_device, DisplayState *ds,
> +		       const char *kernel_filename,
> +		       const char *kernel_cmdline,
> +		       const char *initrd_filename,
> +		       const char *cpu_model)
> +{
> +    CPUState *env;
> +
> +    /* create dummy cpu, halted */
> +    if (cpu_model == NULL) {
> +#ifdef TARGET_X86_64
> +        cpu_model = "qemu64";
> +#else
> +        cpu_model = "qemu32";
> +#endif
> +    }
> +    env = cpu_init(cpu_model);
> +    env->halted = 1;
> +}
> +
> +QEMUMachine xenpv_machine = {
> +    "xenpv",
> +    "paravirtualized Xen machine",
> +    xenpv_init,
> +};
> +
> +/* -------------------------------------------------------------------- */
> +/* fully virtualized xen guests                                         */
> +
> +static void xenfv_init(ram_addr_t ram_size, int vga_ram_size,
> +		       const char *boot_device, DisplayState *ds,
> +		       const char *kernel_filename,
> +		       const char *kernel_cmdline,
> +		       const char *initrd_filename,
> +		       const char *cpu_model)
> +{
> +    /* to be done */
> +    fprintf(stderr, "%s: not implemented yet\n", __FUNCTION__);
> +}
> +
> +QEMUMachine xenfv_machine = {
> +    "xenfv",
> +    "fully virtualized Xen machine",
> +    xenfv_init,
> +};
> diff --git a/hw/xen.h b/hw/xen.h
> new file mode 100644
> index 0000000..19349b5
> --- /dev/null
> +++ b/hw/xen.h
> @@ -0,0 +1,12 @@
> +/*
> + * public xen header
> + *   stuff needed outside xen-*.c, i.e. interfaces to qemu.
> + *   should not depend on any xen headers being present in
> + *   /usr/include/xen, so it can be included unconditionally.
> + *
> + * internal bits for the xen backend drivers are in xen-backend.h
> + */
> +
> +/* xen-machine.c */
> +extern int xen_domid;
> +
> diff --git a/target-i386/machine.c b/target-i386/machine.c
> index 91dbd55..98ece17 100644
> --- a/target-i386/machine.c
> +++ b/target-i386/machine.c
> @@ -9,6 +9,9 @@ void register_machines(void)
>  {
>      qemu_register_machine(&pc_machine);
>      qemu_register_machine(&isapc_machine);
> +#ifdef CONFIG_XEN
> +    qemu_register_machine(&xenpv_machine);
> +#endif
>  }
>  
>  static void cpu_put_seg(QEMUFile *f, SegmentCache *dt)
> diff --git a/vl.c b/vl.c
> index 8801615..8aef3bd 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -29,6 +29,7 @@
>  #include "hw/audiodev.h"
>  #include "hw/isa.h"
>  #include "hw/baum.h"
> +#include "hw/xen.h"
>  #include "net.h"
>  #include "console.h"
>  #include "sysemu.h"
> @@ -7737,6 +7738,9 @@ static void help(int exitcode)
>             "-startdate      select initial date of the clock\n"
>             "-icount [N|auto]\n"
>             "                Enable virtual instruction counter with 2^N clock ticks per instruction\n"
> +#ifdef CONFIG_XEN
> +	   "-domid          specify xen guest domain id\n"
> +#endif
>             "\n"
>             "During emulation, the following keys are useful:\n"
>             "ctrl-alt-f      toggle full screen\n"
> @@ -7842,6 +7846,9 @@ enum {
>      QEMU_OPTION_startdate,
>      QEMU_OPTION_tb_size,
>      QEMU_OPTION_icount,
> +#ifdef CONFIG_XEN
> +    QEMU_OPTION_domid,
> +#endif
>  };
>  
>  typedef struct QEMUOption {
> @@ -7930,6 +7937,9 @@ const QEMUOption qemu_options[] = {
>  #ifdef CONFIG_CURSES
>      { "curses", 0, QEMU_OPTION_curses },
>  #endif
> +#ifdef CONFIG_XEN
> +    { "domid", HAS_ARG, QEMU_OPTION_domid },
> +#endif
>  
>      /* temporary options */
>      { "usb", 0, QEMU_OPTION_usb },
> @@ -8150,7 +8160,7 @@ int main(int argc, char **argv)
>  #endif
>      uint32_t boot_devices_bitmap = 0;
>      int i;
> -    int snapshot, linux_boot, net_boot;
> +    int snapshot, linux_boot, net_boot, nodisk_ok;
>      const char *initrd_filename;
>      const char *kernel_filename, *kernel_cmdline;
>      const char *boot_devices = "";
> @@ -8787,6 +8797,11 @@ int main(int argc, char **argv)
>                      icount_time_shift = strtol(optarg, NULL, 0);
>                  }
>                  break;
> +#ifdef CONFIG_XEN
> +            case QEMU_OPTION_domid:
> +                xen_domid = atoi(optarg);
> +                break;
> +#endif
>              }
>          }
>      }
> @@ -8852,9 +8867,13 @@ int main(int argc, char **argv)
>      linux_boot = (kernel_filename != NULL);
>      net_boot = (boot_devices_bitmap >> ('n' - 'a')) & 0xF;
>  
> -    /* XXX: this should not be: some embedded targets just have flash */
> +    /* need a disk for this machine to boot ? */
> +    /* XXX: add embedded targets which just have flash */
> +    nodisk_ok = 0;
> +    if (0 == strcmp(machine->name, "xenpv"))
> +	nodisk_ok = 1;
>      if (!linux_boot && net_boot == 0 &&
> -        nb_drives_opt == 0)
> +        !nodisk_ok && nb_drives_opt == 0)
>          help(1);

This patch is pretty clean with the exception of this bit.  I think a 
cleaner way to do this would be to let the machine types specify whether 
a disk is needed or not.

Regards,

Anthony LIguori

>  
>      if (!linux_boot && *kernel_cmdline != '\0') {
>