| Message ID | 1401333804-15456-2-git-send-email-cyliu@suse.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 2014-05-29 at 11:23 +0800, Chunyan Liu wrote: > [support xen HVM direct kernel boot] What is this? > xen side patch: support 'kernel', 'ramdisk', 'root', 'extra' > in HVM config file, parse config file, pass -kernel, -initrd > and -append parameters to qemu. Is this a completely new feature or is this adding parity with a xend feature? > [config example] > kernel="/mnt/vmlinuz-3.0.13-0.27-default" > ramdisk="/mnt/initrd-3.0.13-0.27-default" > root="/dev/hda2" > extra="console=tty0 console=ttyS0" > disk=[ 'file:/mnt/images/bjz_04_sles11_sp2/disk0.raw,hda,w', ] > > Signed-off-by: Chunyan Liu <cyliu@suse.com> > --- > tools/libxl/libxl_dm.c | 20 ++++++++++++++++++++ > tools/libxl/libxl_types.idl | 3 +++ > tools/libxl/xl_cmdimpl.c | 33 +++++++++++++++++++++++++++------ > 3 files changed, 50 insertions(+), 6 deletions(-) > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > index 51ab2bf..133bb56 100644 > --- a/tools/libxl/libxl_dm.c > +++ b/tools/libxl/libxl_dm.c > @@ -196,6 +196,16 @@ static char ** libxl__build_device_model_args_old(libxl__gc *gc, Does this work with old qemu+rombios? I question whether we should be adding this sort of new feature here anyway. > int nr_set_cpus = 0; > char *s; > > + if (b_info->u.hvm.kernel) { > + flexarray_vappend(dm_args, "-kernel", b_info->u.hvm.kernel, NULL); > + } libxl style would be to omit the {} for a single line if. > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index 52f1aa9..b8b973b 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -336,6 +336,9 @@ libxl_domain_build_info = Struct("domain_build_info",[ > ("event_channels", uint32), > ("u", KeyedUnion(None, libxl_domain_type, "type", > [("hvm", Struct(None, [("firmware", string), > + ("kernel", string), > + ("cmdline", string), > + ("ramdisk", string), Alignment. > ("bios", libxl_bios_type), > ("pae", libxl_defbool), > ("apic", libxl_defbool), > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 5195914..efd2474 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > + xlu_cfg_get_string (config, "root", &root, 0); > + xlu_cfg_get_string (config, "extra", &extra, 0); > + > + if (root) { > + if (asprintf(&cmdline, "root=%s %s", root, extra) == -1) > + cmdline = NULL; > + } else { > + cmdline = strdup(extra); > + } > + > + if ((root || extra) && !cmdline) { > + fprintf(stderr, "Failed to allocate memory for cmdline\n"); > + exit(1); > + } This all seems to be duplicating pv code, please refactor into a common helper or something. > + > + b_info->u.hvm.cmdline = cmdline; > + xlu_cfg_replace_string (config, "ramdisk", &b_info->u.hvm.ramdisk, 0); > > xlu_cfg_replace_string (config, "firmware_override", > &b_info->u.hvm.firmware, 0); > @@ -1061,9 +1085,6 @@ static void parse_config_data(const char *config_source, > break; > case LIBXL_DOMAIN_TYPE_PV: > { > - char *cmdline = NULL; > - const char *root = NULL, *extra = ""; > - > xlu_cfg_replace_string (config, "kernel", &b_info->u.pv.kernel, 0); > > xlu_cfg_get_string (config, "root", &root, 0);
>>> On 6/2/2014 at 11:24 PM, in message <1401722652.30097.9.camel@kazak.uk.xensource.com>, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Thu, 2014-05-29 at 11:23 +0800, Chunyan Liu wrote: > > [support xen HVM direct kernel boot] > > What is this? Just to indicate this is part of work to support xen HVM direct kernel boot. To make it work, there is another patch to qemu. > > > xen side patch: support 'kernel', 'ramdisk', 'root', 'extra' > > in HVM config file, parse config file, pass -kernel, -initrd > > and -append parameters to qemu. > > Is this a completely new feature or is this adding parity with a xend > feature? I think it's new. Xend doesn't support HVM direct kernel boot. But considering stubdom-dm, such interface might not work. I'll rework on that. > > > [config example] > > kernel="/mnt/vmlinuz-3.0.13-0.27-default" > > ramdisk="/mnt/initrd-3.0.13-0.27-default" > > root="/dev/hda2" > > extra="console=tty0 console=ttyS0" > > disk=[ 'file:/mnt/images/bjz_04_sles11_sp2/disk0.raw,hda,w', ] > > > > Signed-off-by: Chunyan Liu <cyliu@suse.com> > > --- > > tools/libxl/libxl_dm.c | 20 ++++++++++++++++++++ > > tools/libxl/libxl_types.idl | 3 +++ > > tools/libxl/xl_cmdimpl.c | 33 +++++++++++++++++++++++++++------ > > 3 files changed, 50 insertions(+), 6 deletions(-) > > > > diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c > > index 51ab2bf..133bb56 100644 > > --- a/tools/libxl/libxl_dm.c > > +++ b/tools/libxl/libxl_dm.c > > @@ -196,6 +196,16 @@ static char ** > libxl__build_device_model_args_old(libxl__gc *gc, > > Does this work with old qemu+rombios? Oh, no. I'll remove this piece. > > I question whether we should be adding this sort of new feature here > anyway. > > > int nr_set_cpus = 0; > > char *s; > > > > + if (b_info->u.hvm.kernel) { > > + flexarray_vappend(dm_args, "-kernel", b_info->u.hvm.kernel, > NULL); > > + } > > libxl style would be to omit the {} for a single line if. > > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > > index 52f1aa9..b8b973b 100644 > > --- a/tools/libxl/libxl_types.idl > > +++ b/tools/libxl/libxl_types.idl > > @@ -336,6 +336,9 @@ libxl_domain_build_info = Struct("domain_build_info",[ > > ("event_channels", uint32), > > ("u", KeyedUnion(None, libxl_domain_type, "type", > > [("hvm", Struct(None, [("firmware", string), > > + ("kernel", string), > > + ("cmdline", string), > > + ("ramdisk", string), > > Alignment. > > > ("bios", > libxl_bios_type), > > ("pae", > libxl_defbool), > > ("apic", > libxl_defbool), > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > > index 5195914..efd2474 100644 > > --- a/tools/libxl/xl_cmdimpl.c > > +++ b/tools/libxl/xl_cmdimpl.c > > > + xlu_cfg_get_string (config, "root", &root, 0); > > + xlu_cfg_get_string (config, "extra", &extra, 0); > > + > > + if (root) { > > + if (asprintf(&cmdline, "root=%s %s", root, extra) == -1) > > + cmdline = NULL; > > + } else { > > + cmdline = strdup(extra); > > + } > > + > > + if ((root || extra) && !cmdline) { > > + fprintf(stderr, "Failed to allocate memory for cmdline\n"); > > + exit(1); > > + } > > This all seems to be duplicating pv code, please refactor into a common > helper or something. > > > + > > + b_info->u.hvm.cmdline = cmdline; > > + xlu_cfg_replace_string (config, "ramdisk", &b_info->u.hvm.ramdisk, > 0); > > > > xlu_cfg_replace_string (config, "firmware_override", > > &b_info->u.hvm.firmware, 0); > > @@ -1061,9 +1085,6 @@ static void parse_config_data(const char > *config_source, > > break; > > case LIBXL_DOMAIN_TYPE_PV: > > { > > - char *cmdline = NULL; > > - const char *root = NULL, *extra = ""; > > - > > xlu_cfg_replace_string (config, "kernel", &b_info->u.pv.kernel, 0); > > > > xlu_cfg_get_string (config, "root", &root, 0); > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel > >
On Mon, 2014-06-02 at 22:49 -0600, Chun Yan Liu wrote: > > >>> On 6/2/2014 at 11:24 PM, in message > <1401722652.30097.9.camel@kazak.uk.xensource.com>, Ian Campbell > <Ian.Campbell@citrix.com> wrote: > > On Thu, 2014-05-29 at 11:23 +0800, Chunyan Liu wrote: > > > [support xen HVM direct kernel boot] > > > > What is this? > > Just to indicate this is part of work to support xen HVM direct kernel boot. > To make it work, there is another patch to qemu. Putting it there means that it will end up in the eventual commit message unless the committed takes some special steps. I'd recommend either adding it to the subject (i.e. "[RFC PATCH XEN 1/2]", "[RFC PATCH QEMU 2/2]") or after the --- which follows your Signed-off-by. > > > > > > xen side patch: support 'kernel', 'ramdisk', 'root', 'extra' > > > in HVM config file, parse config file, pass -kernel, -initrd > > > and -append parameters to qemu. > > > > Is this a completely new feature or is this adding parity with a xend > > feature? > > I think it's new. Xend doesn't support HVM direct kernel boot. OK, then you certainly don't want the old qemu support (which you were going to remove anyway) > > > @@ -196,6 +196,16 @@ static char ** > > libxl__build_device_model_args_old(libxl__gc *gc, > > > > Does this work with old qemu+rombios? > > Oh, no. I'll remove this piece. Thanks. Ian.
>>> On 6/3/2014 at 05:06 PM, in message <1401786392.8841.44.camel@kazak.uk.xensource.com>, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Mon, 2014-06-02 at 22:49 -0600, Chun Yan Liu wrote: > > > > >>> On 6/2/2014 at 11:24 PM, in message > > <1401722652.30097.9.camel@kazak.uk.xensource.com>, Ian Campbell > > <Ian.Campbell@citrix.com> wrote: > > > On Thu, 2014-05-29 at 11:23 +0800, Chunyan Liu wrote: > > > > [support xen HVM direct kernel boot] > > > > > > What is this? > > > > Just to indicate this is part of work to support xen HVM direct kernel > boot. > > To make it work, there is another patch to qemu. > > Putting it there means that it will end up in the eventual commit > message unless the committed takes some special steps. I'd recommend > either adding it to the subject (i.e. "[RFC PATCH XEN 1/2]", "[RFC PATCH > QEMU 2/2]") or after the --- which follows your Signed-off-by. Oh, yes :) Thanks for reminding. > > > > > > > > > > xen side patch: support 'kernel', 'ramdisk', 'root', 'extra' > > > > in HVM config file, parse config file, pass -kernel, -initrd > > > > and -append parameters to qemu. > > > > > > Is this a completely new feature or is this adding parity with a xend > > > feature? > > > > I think it's new. Xend doesn't support HVM direct kernel boot. > > OK, then you certainly don't want the old qemu support (which you were > going to remove anyway) > > > > > @@ -196,6 +196,16 @@ static char ** > > > libxl__build_device_model_args_old(libxl__gc *gc, > > > > > > Does this work with old qemu+rombios? > > > > Oh, no. I'll remove this piece. > > Thanks. > > Ian. > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel > >
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 51ab2bf..133bb56 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -196,6 +196,16 @@ static char ** libxl__build_device_model_args_old(libxl__gc *gc, int nr_set_cpus = 0; char *s; + if (b_info->u.hvm.kernel) { + flexarray_vappend(dm_args, "-kernel", b_info->u.hvm.kernel, NULL); + } + if (b_info->u.hvm.ramdisk) { + flexarray_vappend(dm_args, "-initrd", b_info->u.hvm.ramdisk, NULL); + } + if (b_info->u.hvm.cmdline) { + flexarray_vappend(dm_args, "-append", b_info->u.hvm.cmdline, NULL); + } + if (b_info->u.hvm.serial) { flexarray_vappend(dm_args, "-serial", b_info->u.hvm.serial, NULL); } @@ -479,6 +489,16 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) { int ioemu_nics = 0; + if (b_info->u.hvm.kernel) { + flexarray_vappend(dm_args, "-kernel", b_info->u.hvm.kernel, NULL); + } + if (b_info->u.hvm.ramdisk) { + flexarray_vappend(dm_args, "-initrd", b_info->u.hvm.ramdisk, NULL); + } + if (b_info->u.hvm.cmdline) { + flexarray_vappend(dm_args, "-append", b_info->u.hvm.cmdline, NULL); + } + if (b_info->u.hvm.serial) { flexarray_vappend(dm_args, "-serial", b_info->u.hvm.serial, NULL); } diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 52f1aa9..b8b973b 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -336,6 +336,9 @@ libxl_domain_build_info = Struct("domain_build_info",[ ("event_channels", uint32), ("u", KeyedUnion(None, libxl_domain_type, "type", [("hvm", Struct(None, [("firmware", string), + ("kernel", string), + ("cmdline", string), + ("ramdisk", string), ("bios", libxl_bios_type), ("pae", libxl_defbool), ("apic", libxl_defbool), diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 5195914..efd2474 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -743,6 +743,8 @@ static void parse_config_data(const char *config_source, int pci_permissive = 0; int pci_seize = 0; int i, e; + char *cmdline = NULL; + const char *root = NULL, *extra = ""; libxl_domain_create_info *c_info = &d_config->c_info; libxl_domain_build_info *b_info = &d_config->b_info; @@ -1007,9 +1009,31 @@ static void parse_config_data(const char *config_source, switch(b_info->type) { case LIBXL_DOMAIN_TYPE_HVM: - if (!xlu_cfg_get_string (config, "kernel", &buf, 0)) - fprintf(stderr, "WARNING: ignoring \"kernel\" directive for HVM guest. " - "Use \"firmware_override\" instead if you really want a non-default firmware\n"); + if (!xlu_cfg_get_string (config, "kernel", &buf, 0)) { + if (strstr(buf, "hvmloader")) + fprintf(stderr, "WARNING: ignoring \"kernel\" directive for HVM guest. " + "Use \"firmware_override\" instead if you really want a non-default firmware\n"); + else + b_info->u.hvm.kernel = strdup(buf); + } + + xlu_cfg_get_string (config, "root", &root, 0); + xlu_cfg_get_string (config, "extra", &extra, 0); + + if (root) { + if (asprintf(&cmdline, "root=%s %s", root, extra) == -1) + cmdline = NULL; + } else { + cmdline = strdup(extra); + } + + if ((root || extra) && !cmdline) { + fprintf(stderr, "Failed to allocate memory for cmdline\n"); + exit(1); + } + + b_info->u.hvm.cmdline = cmdline; + xlu_cfg_replace_string (config, "ramdisk", &b_info->u.hvm.ramdisk, 0); xlu_cfg_replace_string (config, "firmware_override", &b_info->u.hvm.firmware, 0); @@ -1061,9 +1085,6 @@ static void parse_config_data(const char *config_source, break; case LIBXL_DOMAIN_TYPE_PV: { - char *cmdline = NULL; - const char *root = NULL, *extra = ""; - xlu_cfg_replace_string (config, "kernel", &b_info->u.pv.kernel, 0); xlu_cfg_get_string (config, "root", &root, 0);
[support xen HVM direct kernel boot] xen side patch: support 'kernel', 'ramdisk', 'root', 'extra' in HVM config file, parse config file, pass -kernel, -initrd and -append parameters to qemu. [config example] kernel="/mnt/vmlinuz-3.0.13-0.27-default" ramdisk="/mnt/initrd-3.0.13-0.27-default" root="/dev/hda2" extra="console=tty0 console=ttyS0" disk=[ 'file:/mnt/images/bjz_04_sles11_sp2/disk0.raw,hda,w', ] Signed-off-by: Chunyan Liu <cyliu@suse.com> --- tools/libxl/libxl_dm.c | 20 ++++++++++++++++++++ tools/libxl/libxl_types.idl | 3 +++ tools/libxl/xl_cmdimpl.c | 33 +++++++++++++++++++++++++++------ 3 files changed, 50 insertions(+), 6 deletions(-)