| Message ID | 1490915175-28692-1-git-send-email-boris.ostrovsky@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Mar 30, 2017 at 07:06:15PM -0400, Boris Ostrovsky wrote: > In addition to 'device_model_version="none"' config option users can > also use 'pvh=1' in xl configuration file when creating a PVH guest. I'm not sure, but I think the plan was to remove device_model_version="none" and instead just use pvh=1, instead of keeping both. I don't have a strong opinion here, so I will leave that to the xl maintainers. I'm also not sure, but if you use device_model_version="none" can you use QDISK for PVH disk backend? (and pygrub). > We can skip parsing options related to device model once we establish > that we are building PVH guest. > > Also process 'device_model_version="none"' for HVM guests only since > it is not a valid model for PV guests. > > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> > --- > docs/man/xl.cfg.pod.5.in | 7 ++++++- > tools/xl/xl_parse.c | 12 +++++++++++- > 2 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in > index 206d33e..5833987 100644 > --- a/docs/man/xl.cfg.pod.5.in > +++ b/docs/man/xl.cfg.pod.5.in > @@ -1201,6 +1201,11 @@ expose unexpected bugs in the guest, or find bugs in Xen, so it is > possible to disable this feature. Use of out of sync page tables, > when Xen thinks it appropriate, is the default. > > +=item B<pvh=BOOLEAN> > + > +Don't use any device model. This requires a kernel capable of booting > +without emulated devices. Default is 0. > + > =item B<shadow_memory=MBYTES> > > Number of megabytes to set aside for shadowing guest pagetable pages > @@ -1966,7 +1971,7 @@ This device-model is still the default for NetBSD dom0. > =item B<none> > > Don't use any device model. This requires a kernel capable of booting > -without emulated devices. > +without emulated devices. This is a synonym for L</"pvh"> option above. > > =back > > diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c > index 66327dc..aa591cd 100644 > --- a/tools/xl/xl_parse.c > +++ b/tools/xl/xl_parse.c > @@ -1817,6 +1817,12 @@ skip_usbdev: > break; > } > > + if (c_info->type == LIBXL_DOMAIN_TYPE_HVM && Hm, this will mean that the user needs to specify: builder="hvm" pvh=1 Or else the option is not going to be parsed. > + !xlu_cfg_get_long(config, "pvh", &l, 0) && l) { > + b_info->device_model_version = LIBXL_DEVICE_MODEL_VERSION_NONE; > + goto skip_device_model; > + } > + Roger.
On Fri, Mar 31, 2017 at 11:23:04AM +0100, Roger Pau Monné wrote: > On Thu, Mar 30, 2017 at 07:06:15PM -0400, Boris Ostrovsky wrote: > > In addition to 'device_model_version="none"' config option users can > > also use 'pvh=1' in xl configuration file when creating a PVH guest. > > I'm not sure, but I think the plan was to remove device_model_version="none" > and instead just use pvh=1, instead of keeping both. I don't have a strong > opinion here, so I will leave that to the xl maintainers. > Yes, that's the plan.
On 03/31/2017 06:23 AM, Roger Pau Monné wrote: > On Thu, Mar 30, 2017 at 07:06:15PM -0400, Boris Ostrovsky wrote: >> In addition to 'device_model_version="none"' config option users can >> also use 'pvh=1' in xl configuration file when creating a PVH guest. > I'm not sure, but I think the plan was to remove device_model_version="none" > and instead just use pvh=1, instead of keeping both. I don't have a strong > opinion here, so I will leave that to the xl maintainers. I thought we had PVHv2 in 4.8 but apparently not. I will remove it then. > > I'm also not sure, but if you use device_model_version="none" can you use QDISK > for PVH disk backend? (and pygrub). We are not starting qemu so I don't think I see how we can use qdisk (unless you are suggesting that we should do that for qdisk specifically). Not sure what you are asking for pygrub. > >> We can skip parsing options related to device model once we establish >> that we are building PVH guest. >> >> Also process 'device_model_version="none"' for HVM guests only since >> it is not a valid model for PV guests. >> >> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> >> --- >> docs/man/xl.cfg.pod.5.in | 7 ++++++- >> tools/xl/xl_parse.c | 12 +++++++++++- >> 2 files changed, 17 insertions(+), 2 deletions(-) >> >> diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in >> index 206d33e..5833987 100644 >> --- a/docs/man/xl.cfg.pod.5.in >> +++ b/docs/man/xl.cfg.pod.5.in >> @@ -1201,6 +1201,11 @@ expose unexpected bugs in the guest, or find bugs in Xen, so it is >> possible to disable this feature. Use of out of sync page tables, >> when Xen thinks it appropriate, is the default. >> >> +=item B<pvh=BOOLEAN> >> + >> +Don't use any device model. This requires a kernel capable of booting >> +without emulated devices. Default is 0. >> + >> =item B<shadow_memory=MBYTES> >> >> Number of megabytes to set aside for shadowing guest pagetable pages >> @@ -1966,7 +1971,7 @@ This device-model is still the default for NetBSD dom0. >> =item B<none> >> >> Don't use any device model. This requires a kernel capable of booting >> -without emulated devices. >> +without emulated devices. This is a synonym for L</"pvh"> option above. >> >> =back >> >> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c >> index 66327dc..aa591cd 100644 >> --- a/tools/xl/xl_parse.c >> +++ b/tools/xl/xl_parse.c >> @@ -1817,6 +1817,12 @@ skip_usbdev: >> break; >> } >> >> + if (c_info->type == LIBXL_DOMAIN_TYPE_HVM && > Hm, this will mean that the user needs to specify: > > builder="hvm" > pvh=1 > > Or else the option is not going to be parsed. Right. I've always had builder directive in my config file so I didn't think of that. I'll remove the check. -boris > >> + !xlu_cfg_get_long(config, "pvh", &l, 0) && l) { >> + b_info->device_model_version = LIBXL_DEVICE_MODEL_VERSION_NONE; >> + goto skip_device_model; >> + } >> + > Roger.
On Fri, Mar 31, 2017 at 09:43:07AM -0400, Boris Ostrovsky wrote: > On 03/31/2017 06:23 AM, Roger Pau Monné wrote: > > On Thu, Mar 30, 2017 at 07:06:15PM -0400, Boris Ostrovsky wrote: > >> In addition to 'device_model_version="none"' config option users can > >> also use 'pvh=1' in xl configuration file when creating a PVH guest. > > I'm not sure, but I think the plan was to remove device_model_version="none" > > and instead just use pvh=1, instead of keeping both. I don't have a strong > > opinion here, so I will leave that to the xl maintainers. > > I thought we had PVHv2 in 4.8 but apparently not. I will remove it then. > You can't remove the DM version none just yet. I think Ian has ask for adding a PVH guest type and pvh=1 sets that. It is a bit more work than this patch. Wei.
On 03/31/2017 09:49 AM, Wei Liu wrote: > On Fri, Mar 31, 2017 at 09:43:07AM -0400, Boris Ostrovsky wrote: >> On 03/31/2017 06:23 AM, Roger Pau Monné wrote: >>> On Thu, Mar 30, 2017 at 07:06:15PM -0400, Boris Ostrovsky wrote: >>>> In addition to 'device_model_version="none"' config option users can >>>> also use 'pvh=1' in xl configuration file when creating a PVH guest. >>> I'm not sure, but I think the plan was to remove device_model_version="none" >>> and instead just use pvh=1, instead of keeping both. I don't have a strong >>> opinion here, so I will leave that to the xl maintainers. >> I thought we had PVHv2 in 4.8 but apparently not. I will remove it then. >> > You can't remove the DM version none just yet. > > I think Ian has ask for adding a PVH guest type and pvh=1 sets that. It > is a bit more work than this patch. That would make the toolstack have three guest types while the hypervisor has two. (Maybe this was already covered but I don't remember seeing this discussion). -boris
On Fri, Mar 31, 2017 at 09:59:09AM -0400, Boris Ostrovsky wrote: > On 03/31/2017 09:49 AM, Wei Liu wrote: > > On Fri, Mar 31, 2017 at 09:43:07AM -0400, Boris Ostrovsky wrote: > >> On 03/31/2017 06:23 AM, Roger Pau Monné wrote: > >>> On Thu, Mar 30, 2017 at 07:06:15PM -0400, Boris Ostrovsky wrote: > >>>> In addition to 'device_model_version="none"' config option users can > >>>> also use 'pvh=1' in xl configuration file when creating a PVH guest. > >>> I'm not sure, but I think the plan was to remove device_model_version="none" > >>> and instead just use pvh=1, instead of keeping both. I don't have a strong > >>> opinion here, so I will leave that to the xl maintainers. > >> I thought we had PVHv2 in 4.8 but apparently not. I will remove it then. > >> > > You can't remove the DM version none just yet. > > > > I think Ian has ask for adding a PVH guest type and pvh=1 sets that. It > > is a bit more work than this patch. > > That would make the toolstack have three guest types while the > hypervisor has two. > Correct. > (Maybe this was already covered but I don't remember seeing this > discussion). > See: <22709.46867.434595.470032@mariner.uk.xensource.com> > -boris >
On Fri, Mar 31, 2017 at 09:59:09AM -0400, Boris Ostrovsky wrote: > On 03/31/2017 09:49 AM, Wei Liu wrote: > > On Fri, Mar 31, 2017 at 09:43:07AM -0400, Boris Ostrovsky wrote: > >> On 03/31/2017 06:23 AM, Roger Pau Monné wrote: > >>> On Thu, Mar 30, 2017 at 07:06:15PM -0400, Boris Ostrovsky wrote: > >>>> In addition to 'device_model_version="none"' config option users can > >>>> also use 'pvh=1' in xl configuration file when creating a PVH guest. > >>> I'm not sure, but I think the plan was to remove device_model_version="none" > >>> and instead just use pvh=1, instead of keeping both. I don't have a strong > >>> opinion here, so I will leave that to the xl maintainers. > >> I thought we had PVHv2 in 4.8 but apparently not. I will remove it then. > >> > > You can't remove the DM version none just yet. > > > > I think Ian has ask for adding a PVH guest type and pvh=1 sets that. It > > is a bit more work than this patch. > > That would make the toolstack have three guest types while the > hypervisor has two. > > (Maybe this was already covered but I don't remember seeing this > discussion). This was discussed on the PVHv1 removal patches. I sent a PVHv1 removal that partially covered this (by adding a PVH guest type to libxl), but IMHO I find it quite hard to add a guest type to libxl only (when it's not on the hypervisor). It can certainly be done, but it's painful due to libxl having to filter the output from Xen in order to properly detect guest types, since it cannot longer use Xen values blindly. Roger.
Roger Pau Monné writes ("Re: [PATCH] xl: Add 'pvh' config option"):
> On Fri, Mar 31, 2017 at 09:59:09AM -0400, Boris Ostrovsky wrote:
> > That would make the toolstack have three guest types while the
> > hypervisor has two.
Is that a problem, conceptually ? It seems to me to be accurate here.
> This was discussed on the PVHv1 removal patches. I sent a PVHv1 removal that
> partially covered this (by adding a PVH guest type to libxl), but IMHO I find
> it quite hard to add a guest type to libxl only (when it's not on the
> hypervisor).
>
> It can certainly be done, but it's painful due to libxl having to filter the
> output from Xen in order to properly detect guest types, since it cannot longer
> use Xen values blindly.
How many places in libxl have to change ?
Ian.
On 03/31/2017 10:33 AM, Ian Jackson wrote: > Roger Pau Monné writes ("Re: [PATCH] xl: Add 'pvh' config option"): >> On Fri, Mar 31, 2017 at 09:59:09AM -0400, Boris Ostrovsky wrote: >>> That would make the toolstack have three guest types while the >>> hypervisor has two. > Is that a problem, conceptually ? It seems to me to be accurate here. To my eyes --- yes. Guest type is a basic concept and having toolstack and the hypervisor disagree on how many guest types exist doesn't sound right. This doesn't mean that there shouldn't indeed be 3 types --- I agree that from admin perspective PVH is nothing like HVM. There is also another question, BTW: since PVH(v2) will first appear in 4.9 it would be nice to provide user-visible config options (i.e. 'pvh' instead of or in addition to 'device_model_version=none') before release, even if (proper) implementation is still under discussion. But perhaps the train has left the station already. -boris > >> This was discussed on the PVHv1 removal patches. I sent a PVHv1 removal that >> partially covered this (by adding a PVH guest type to libxl), but IMHO I find >> it quite hard to add a guest type to libxl only (when it's not on the >> hypervisor). >> >> It can certainly be done, but it's painful due to libxl having to filter the >> output from Xen in order to properly detect guest types, since it cannot longer >> use Xen values blindly. > How many places in libxl have to change ? >
diff --git a/docs/man/xl.cfg.pod.5.in b/docs/man/xl.cfg.pod.5.in index 206d33e..5833987 100644 --- a/docs/man/xl.cfg.pod.5.in +++ b/docs/man/xl.cfg.pod.5.in @@ -1201,6 +1201,11 @@ expose unexpected bugs in the guest, or find bugs in Xen, so it is possible to disable this feature. Use of out of sync page tables, when Xen thinks it appropriate, is the default. +=item B<pvh=BOOLEAN> + +Don't use any device model. This requires a kernel capable of booting +without emulated devices. Default is 0. + =item B<shadow_memory=MBYTES> Number of megabytes to set aside for shadowing guest pagetable pages @@ -1966,7 +1971,7 @@ This device-model is still the default for NetBSD dom0. =item B<none> Don't use any device model. This requires a kernel capable of booting -without emulated devices. +without emulated devices. This is a synonym for L</"pvh"> option above. =back diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c index 66327dc..aa591cd 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -1817,6 +1817,12 @@ skip_usbdev: break; } + if (c_info->type == LIBXL_DOMAIN_TYPE_HVM && + !xlu_cfg_get_long(config, "pvh", &l, 0) && l) { + b_info->device_model_version = LIBXL_DEVICE_MODEL_VERSION_NONE; + goto skip_device_model; + } + /* parse device model arguments, this works for pv, hvm and stubdom */ if (!xlu_cfg_get_string (config, "device_model", &buf, 0)) { fprintf(stderr, @@ -1845,8 +1851,10 @@ skip_usbdev: } else if (!strcmp(buf, "qemu-xen")) { b_info->device_model_version = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN; - } else if (!strcmp(buf, "none")) { + } else if (c_info->type == LIBXL_DOMAIN_TYPE_HVM && + !strcmp(buf, "none")) { b_info->device_model_version = LIBXL_DEVICE_MODEL_VERSION_NONE; + goto skip_device_model; } else { fprintf(stderr, "Unknown device_model_version \"%s\" specified\n", buf); @@ -1884,6 +1892,8 @@ skip_usbdev: #undef parse_extra_args +skip_device_model: + /* If we've already got vfb=[] for PV guest then ignore top level * VNC config. */ if (c_info->type == LIBXL_DOMAIN_TYPE_PV && !d_config->num_vfbs) {
In addition to 'device_model_version="none"' config option users can also use 'pvh=1' in xl configuration file when creating a PVH guest. We can skip parsing options related to device model once we establish that we are building PVH guest. Also process 'device_model_version="none"' for HVM guests only since it is not a valid model for PV guests. Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> --- docs/man/xl.cfg.pod.5.in | 7 ++++++- tools/xl/xl_parse.c | 12 +++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-)