[libvirt] [PATCH V2] libxl: support hvm direct kernel boot

[Jim Fehlig]

Chunyan Liu wrote:
> Xen libxl can support Xen HVM direct kernel boot now. To support
> Xen HVM direct kernel boot in libvirt, it still needs some changes
> to libvirt code: accept HVM direct kernel boot related configuration
> in xml, parse those info into virDomainDefPtr, and fill in related
> parts of libxl_domain_build_info, so that libxl can handle. This
> patch is just to do this.
>
> Signed-off-by: Chunyan Liu <cyliu at suse.com>
> ---
> Changes:
>   - fix Jim's comments
>
>  src/libxl/libxl_conf.c     | 18 +++++++++++++++
>  src/xenconfig/xen_common.c | 57 ++++++++++++++++++++++++++++++++++------------
>  2 files changed, 60 insertions(+), 15 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index acba69c..a5bda64 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -704,6 +704,15 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>          if (VIR_STRDUP(b_info->u.hvm.boot, bootorder) < 0)
>              goto error;
>  
> +#ifdef LIBXL_HAVE_BUILDINFO_KERNEL
> +        if (VIR_STRDUP(b_info->cmdline, def->os.cmdline) < 0)
> +            goto error;
> +        if (VIR_STRDUP(b_info->kernel, def->os.kernel) < 0)
> +            goto error;
> +        if (VIR_STRDUP(b_info->ramdisk, def->os.initrd) < 0)
> +            goto error;
> +#endif
> +
>          if (def->nserials) {
>              if (def->nserials > 1) {
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -748,6 +757,14 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>                    virStringSplit(def->os.bootloaderArgs, " \t\n", 0)))
>                  goto error;
>          }
> +#ifdef LIBXL_HAVE_BUILDINFO_KERNEL
> +        if (VIR_STRDUP(b_info->cmdline, def->os.cmdline) < 0)
> +            goto error;
> +        if (VIR_STRDUP(b_info->kernel, def->os.kernel) < 0)
> +            goto error;
> +        if (VIR_STRDUP(b_info->ramdisk, def->os.initrd) < 0)
> +            goto error;
> +#else
>          if (VIR_STRDUP(b_info->u.pv.cmdline, def->os.cmdline) < 0)
>              goto error;
>          if (def->os.kernel) {
> @@ -758,6 +775,7 @@ libxlMakeDomBuildInfo(virDomainDefPtr def,
>          }
>          if (VIR_STRDUP(b_info->u.pv.ramdisk, def->os.initrd) < 0)
>              goto error;
> +#endif
>   

This part looks good.

>      }
>  
>      return 0;
> diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
> index 32954f3..3cf7553 100644
> --- a/src/xenconfig/xen_common.c
> +++ b/src/xenconfig/xen_common.c
> @@ -39,6 +39,9 @@
>  #include "virstring.h"
>  #include "xen_common.h"
>  
> +#if WITH_LIBXL
> +# include <libxl.h>
> +#endif
>  
>  /*
>   * Convenience method to grab a long int from the config file object
> @@ -1053,6 +1056,27 @@ xenParseGeneralMeta(virConfPtr conf, virDomainDefPtr def, virCapsPtr caps)
>      return 0;
>  }
>  
> +static int
> +xenParseCmdline(virConfPtr conf, virDomainDefPtr def)
> +{
> +    const char *extra, *root;
> +
> +    if (xenConfigGetString(conf, "extra", &extra, NULL) < 0)
> +        return -1;
> +
> +    if (xenConfigGetString(conf, "root", &root, NULL) < 0)
> +        return -1;
> +
> +    if (root) {
> +        if (virAsprintf(&def->os.cmdline, "root=%s %s", root, extra) < 0)
> +            return -1;
> +    } else {
> +        if (VIR_STRDUP(def->os.cmdline, extra) < 0)
> +            return -1;
> +    }
> +
> +    return 0;
> +}
>  
>  static int
>  xenParseOS(virConfPtr conf, virDomainDefPtr def)
> @@ -1065,9 +1089,25 @@ xenParseOS(virConfPtr conf, virDomainDefPtr def)
>      if (STREQ(def->os.type, "hvm")) {
>          const char *boot;
>  
> +#ifdef LIBXL_HAVE_BUILDINFO_KERNEL
>   

Sorry I didn't realize this when reviewing V1, but this file should not
contain libxl specific stuff.  The name xen_common.c should be a hint
:-).  An ideal place for this code would be xen_xl.c, introduced by
David's series "Xen-xl parser" in patch 2/4

https://www.redhat.com/archives/libvir-list/2014-September/msg00589.html

But that series is blocked on getting flex properly integrated.  You can
read the thread for details, and any ideas are kindly welcomed.  Eric
mentioned that coreutils uses flex with autotools, but I couldn't find
it in the code at git://git.sv.gnu.org/coreutils.

An alternative would be to introduce xen_xl.[ch] with this work, and
rebase David's disk parser after resolving the flex and autotools issue.

Regards,
Jim