* [PATCH] libxl: check nesthvm and altp2m in libxl level
@ 2015-07-24 15:39 Wei Liu
2015-07-24 16:02 ` Sahita, Ravi
2015-07-27 12:34 ` Julien Grall
0 siblings, 2 replies; 4+ messages in thread
From: Wei Liu @ 2015-07-24 15:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Wei Liu, Sahita, Ravi, Ian Jackson, Ian Campbell, Ed White
In ea214001 ("x86/altp2m: add altp2mhvm HVM domain parameter"), a
check was added to ensure nestedhvm and altp2m cannot be enabled at
the same time. That check was added in xl, but in fact it should be in
libxl because it should be the entity that decides whether
the provided configuration is valid.
This patch moves the check to libxl. The code snippet is moved after
calling libxl__domain_build_info_setdefault so that we can:
1. remove libxl_defbool_is_default in `if()';
2. detect mistake in libxl__domain_build_info_setdefault.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ed White <edmund.h.white@intel.com>
Cc: "Sahita, Ravi" <ravi.sahita@intel.com>
I said I discovered an issue during review but also volunteered to fix
it after the series is merged. Here is the patch to do that.
---
tools/libxl/libxl_create.c | 6 ++++++
tools/libxl/xl_cmdimpl.c | 8 --------
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 855b42c..de536ba 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -883,6 +883,12 @@ static void initiate_domain_create(libxl__egc *egc,
goto error_out;
}
+ if (libxl_defbool_val(d_config->b_info.u.hvm.nested_hvm) &&
+ libxl_defbool_val(d_config->b_info.u.hvm.altp2m)) {
+ LOG(ERROR, "nestedhvm and altp2mhvm cannot be used together");
+ goto error_out;
+ }
+
for (i = 0; i < d_config->num_disks; i++) {
ret = libxl__device_disk_setdefault(gc, &d_config->disks[i]);
if (ret) {
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index d0bf0cb..9755d55 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1568,14 +1568,6 @@ static void parse_config_data(const char *config_source,
xlu_cfg_get_defbool(config, "altp2mhvm", &b_info->u.hvm.altp2m, 0);
- if (!libxl_defbool_is_default(b_info->u.hvm.nested_hvm) &&
- libxl_defbool_val(b_info->u.hvm.nested_hvm) &&
- !libxl_defbool_is_default(b_info->u.hvm.altp2m) &&
- libxl_defbool_val(b_info->u.hvm.altp2m)) {
- fprintf(stderr, "ERROR: nestedhvm and altp2mhvm cannot be used together\n");
- exit(1);
- }
-
xlu_cfg_replace_string(config, "smbios_firmware",
&b_info->u.hvm.smbios_firmware, 0);
xlu_cfg_replace_string(config, "acpi_firmware",
--
2.1.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] libxl: check nesthvm and altp2m in libxl level
2015-07-24 15:39 [PATCH] libxl: check nesthvm and altp2m in libxl level Wei Liu
@ 2015-07-24 16:02 ` Sahita, Ravi
2015-07-27 12:34 ` Julien Grall
1 sibling, 0 replies; 4+ messages in thread
From: Sahita, Ravi @ 2015-07-24 16:02 UTC (permalink / raw)
To: Wei Liu, Xen-devel; +Cc: Ian Jackson, Ian Campbell, White, Edmund H
>From: Wei Liu [mailto:wei.liu2@citrix.com]
>Sent: Friday, July 24, 2015 8:40 AM
>
>In ea214001 ("x86/altp2m: add altp2mhvm HVM domain parameter"), a check
>was added to ensure nestedhvm and altp2m cannot be enabled at the same
>time. That check was added in xl, but in fact it should be in libxl because it
>should be the entity that decides whether the provided configuration is valid.
>
>This patch moves the check to libxl. The code snippet is moved after calling
>libxl__domain_build_info_setdefault so that we can:
>1. remove libxl_defbool_is_default in `if()'; 2. detect mistake in
>libxl__domain_build_info_setdefault.
>
>Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>---
>Cc: Ed White <edmund.h.white@intel.com>
>Cc: "Sahita, Ravi" <ravi.sahita@intel.com>
>
>I said I discovered an issue during review but also volunteered to fix it after
>the series is merged. Here is the patch to do that.
>---
> tools/libxl/libxl_create.c | 6 ++++++
> tools/libxl/xl_cmdimpl.c | 8 --------
> 2 files changed, 6 insertions(+), 8 deletions(-)
>
>diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index
>855b42c..de536ba 100644
>--- a/tools/libxl/libxl_create.c
>+++ b/tools/libxl/libxl_create.c
>@@ -883,6 +883,12 @@ static void initiate_domain_create(libxl__egc *egc,
> goto error_out;
> }
>
>+ if (libxl_defbool_val(d_config->b_info.u.hvm.nested_hvm) &&
>+ libxl_defbool_val(d_config->b_info.u.hvm.altp2m)) {
>+ LOG(ERROR, "nestedhvm and altp2mhvm cannot be used together");
>+ goto error_out;
>+ }
>+
> for (i = 0; i < d_config->num_disks; i++) {
> ret = libxl__device_disk_setdefault(gc, &d_config->disks[i]);
> if (ret) {
>diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index
>d0bf0cb..9755d55 100644
>--- a/tools/libxl/xl_cmdimpl.c
>+++ b/tools/libxl/xl_cmdimpl.c
>@@ -1568,14 +1568,6 @@ static void parse_config_data(const char
>*config_source,
>
> xlu_cfg_get_defbool(config, "altp2mhvm", &b_info->u.hvm.altp2m, 0);
>
>- if (!libxl_defbool_is_default(b_info->u.hvm.nested_hvm) &&
>- libxl_defbool_val(b_info->u.hvm.nested_hvm) &&
>- !libxl_defbool_is_default(b_info->u.hvm.altp2m) &&
>- libxl_defbool_val(b_info->u.hvm.altp2m)) {
>- fprintf(stderr, "ERROR: nestedhvm and altp2mhvm cannot be used
>together\n");
>- exit(1);
>- }
>-
> xlu_cfg_replace_string(config, "smbios_firmware",
> &b_info->u.hvm.smbios_firmware, 0);
> xlu_cfg_replace_string(config, "acpi_firmware",
>--
>2.1.4
Thanks Wei
Ravi
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] libxl: check nesthvm and altp2m in libxl level
2015-07-24 15:39 [PATCH] libxl: check nesthvm and altp2m in libxl level Wei Liu
2015-07-24 16:02 ` Sahita, Ravi
@ 2015-07-27 12:34 ` Julien Grall
2015-07-27 13:26 ` Wei Liu
1 sibling, 1 reply; 4+ messages in thread
From: Julien Grall @ 2015-07-27 12:34 UTC (permalink / raw)
To: Wei Liu, Xen-devel; +Cc: Sahita, Ravi, Ian Jackson, Ian Campbell, Ed White
Hi Wei,
On 24/07/15 16:39, Wei Liu wrote:
> In ea214001 ("x86/altp2m: add altp2mhvm HVM domain parameter"), a
> check was added to ensure nestedhvm and altp2m cannot be enabled at
> the same time. That check was added in xl, but in fact it should be in
> libxl because it should be the entity that decides whether
> the provided configuration is valid.
>
> This patch moves the check to libxl. The code snippet is moved after
> calling libxl__domain_build_info_setdefault so that we can:
> 1. remove libxl_defbool_is_default in `if()';
> 2. detect mistake in libxl__domain_build_info_setdefault.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Ed White <edmund.h.white@intel.com>
> Cc: "Sahita, Ravi" <ravi.sahita@intel.com>
>
> I said I discovered an issue during review but also volunteered to fix
> it after the series is merged. Here is the patch to do that.
> ---
> tools/libxl/libxl_create.c | 6 ++++++
> tools/libxl/xl_cmdimpl.c | 8 --------
> 2 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 855b42c..de536ba 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -883,6 +883,12 @@ static void initiate_domain_create(libxl__egc *egc,
> goto error_out;
> }
>
> + if (libxl_defbool_val(d_config->b_info.u.hvm.nested_hvm) &&
> + libxl_defbool_val(d_config->b_info.u.hvm.altp2m)) {
> + LOG(ERROR, "nestedhvm and altp2mhvm cannot be used together");
> + goto error_out;
> + }
> +
The u.hvm.{nested_hvm,altp2m} can only be checked when the created
domain is an HVM.
But initiate_domain_create is called with either a PV or HVM domain. So
you may need to check if we are creating a HVM one.
Regards,
--
Julien Grall
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] libxl: check nesthvm and altp2m in libxl level
2015-07-27 12:34 ` Julien Grall
@ 2015-07-27 13:26 ` Wei Liu
0 siblings, 0 replies; 4+ messages in thread
From: Wei Liu @ 2015-07-27 13:26 UTC (permalink / raw)
To: Julien Grall
Cc: Sahita, Ravi, Wei Liu, Ian Campbell, Ian Jackson, Ed White, Xen-devel
On Mon, Jul 27, 2015 at 01:34:04PM +0100, Julien Grall wrote:
> Hi Wei,
>
> On 24/07/15 16:39, Wei Liu wrote:
> > In ea214001 ("x86/altp2m: add altp2mhvm HVM domain parameter"), a
> > check was added to ensure nestedhvm and altp2m cannot be enabled at
> > the same time. That check was added in xl, but in fact it should be in
> > libxl because it should be the entity that decides whether
> > the provided configuration is valid.
> >
> > This patch moves the check to libxl. The code snippet is moved after
> > calling libxl__domain_build_info_setdefault so that we can:
> > 1. remove libxl_defbool_is_default in `if()';
> > 2. detect mistake in libxl__domain_build_info_setdefault.
> >
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > Cc: Ed White <edmund.h.white@intel.com>
> > Cc: "Sahita, Ravi" <ravi.sahita@intel.com>
> >
> > I said I discovered an issue during review but also volunteered to fix
> > it after the series is merged. Here is the patch to do that.
> > ---
> > tools/libxl/libxl_create.c | 6 ++++++
> > tools/libxl/xl_cmdimpl.c | 8 --------
> > 2 files changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> > index 855b42c..de536ba 100644
> > --- a/tools/libxl/libxl_create.c
> > +++ b/tools/libxl/libxl_create.c
> > @@ -883,6 +883,12 @@ static void initiate_domain_create(libxl__egc *egc,
> > goto error_out;
> > }
> >
> > + if (libxl_defbool_val(d_config->b_info.u.hvm.nested_hvm) &&
> > + libxl_defbool_val(d_config->b_info.u.hvm.altp2m)) {
> > + LOG(ERROR, "nestedhvm and altp2mhvm cannot be used together");
> > + goto error_out;
> > + }
> > +
>
> The u.hvm.{nested_hvm,altp2m} can only be checked when the created
> domain is an HVM.
>
> But initiate_domain_create is called with either a PV or HVM domain. So
> you may need to check if we are creating a HVM one.
>
Good point. I will respin.
Wei.
> Regards,
>
> --
> Julien Grall
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-07-27 13:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-24 15:39 [PATCH] libxl: check nesthvm and altp2m in libxl level Wei Liu
2015-07-24 16:02 ` Sahita, Ravi
2015-07-27 12:34 ` Julien Grall
2015-07-27 13:26 ` Wei Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).