diff mbox series

[XEN,for-4.13,2/3] libxl: libxl__spawn_stub_dm: Call domain_config_setdefault

Message ID 20191028152948.11900-3-ian.jackson@eu.citrix.com (mailing list archive)
State New, archived
Headers show
Series Fix stub dm since pci pt overhaul | expand

Commit Message

Ian Jackson Oct. 28, 2019, 3:29 p.m. UTC
Previously, defaulting and checking of some aspects of the domain
config was skipped for stub dms.  This has been the case forever.

In ad011ad08843 "libxl/xl: Overhaul passthrough setting logic" some
defaulting that was needed for stub dms was moved from
libxl__domain_create_info_setdefault to .._config_setdefault with the
result that for stub dms, libxl__domain_make fails with this
assertion:
  xl: libxl_create.c:582: libxl__domain_make: Assertion
  `info->passthrough != LIBXL_PASSTHROUGH_DEFAULT' failed.

Fix this by properly doing all defaulting and all checking for stub
dms.  This is more correct, but (especially at this stage of the
release) it is necessary to more closely evaluate the effects by
reviewing the body of _config_setdefault.  The changes are as follows:

One actual functional change:

* The new passthrough defaulting is properly done.  This is what we
  are trying to actually fix here.

And a lot of things that make no difference:

* shadow_memkb would now be set.  Whether this would be correct is not
  entirely clear.  It seems better to make this patch (whose purpose
  is to fix the passthrough defaulting) *not* include that semantic
  change, so here I have included a hunk to explicitly override this.

* FLASK ssid_label is processed.  But the actual ssidref is copied
  from the guest domain by spawn_stub_dm, and ssid_label is set to
  NULL.  So no change.

* We set iommu_memkb.  But to 0 since passthrough is disabled.

* cpuid pool_name is processed.  But this is not set by
  spawn_stub_dm.  (Arguably this is a bug: stub dms should inherit the
  parent cpupool.)  The effect is to leave poolid set to 0 and call
  libxl_cpupoolid_is_valid but that always succeeds for 0.  So no
  change.

* Various extra checks are done: reject PCI passthrough for HVM with
  POD (stub dm is PV); reject pod + vnuma, or PV + vnuma (stub dm has
  no vnuma); reject nested HVM or pod, with alt2pm-hvm (again, stub dm
  is PV).  So these checks will always pass.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
CC: Juergen Gross <jgross@suse.com>
---
 tools/libxl/libxl_dm.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

Comments

Anthony PERARD Oct. 28, 2019, 5:49 p.m. UTC | #1
On Mon, Oct 28, 2019 at 03:29:47PM +0000, Ian Jackson wrote:
> Previously, defaulting and checking of some aspects of the domain
> config was skipped for stub dms.  This has been the case forever.
> 
> In ad011ad08843 "libxl/xl: Overhaul passthrough setting logic" some
> defaulting that was needed for stub dms was moved from
> libxl__domain_create_info_setdefault to .._config_setdefault with the
> result that for stub dms, libxl__domain_make fails with this
> assertion:
>   xl: libxl_create.c:582: libxl__domain_make: Assertion
>   `info->passthrough != LIBXL_PASSTHROUGH_DEFAULT' failed.
> 
> Fix this by properly doing all defaulting and all checking for stub
> dms.  This is more correct, but (especially at this stage of the
> release) it is necessary to more closely evaluate the effects by
> reviewing the body of _config_setdefault.  The changes are as follows:
> 
> One actual functional change:
> 
> * The new passthrough defaulting is properly done.  This is what we
>   are trying to actually fix here.
> 
> And a lot of things that make no difference:
> 
> * shadow_memkb would now be set.  Whether this would be correct is not
>   entirely clear.  It seems better to make this patch (whose purpose
>   is to fix the passthrough defaulting) *not* include that semantic
>   change, so here I have included a hunk to explicitly override this.
> 
> * FLASK ssid_label is processed.  But the actual ssidref is copied
>   from the guest domain by spawn_stub_dm, and ssid_label is set to
>   NULL.  So no change.
> 
> * We set iommu_memkb.  But to 0 since passthrough is disabled.
> 
> * cpuid pool_name is processed.  But this is not set by
>   spawn_stub_dm.  (Arguably this is a bug: stub dms should inherit the
>   parent cpupool.)  The effect is to leave poolid set to 0 and call
>   libxl_cpupoolid_is_valid but that always succeeds for 0.  So no
>   change.
> 
> * Various extra checks are done: reject PCI passthrough for HVM with
>   POD (stub dm is PV); reject pod + vnuma, or PV + vnuma (stub dm has
>   no vnuma); reject nested HVM or pod, with alt2pm-hvm (again, stub dm
>   is PV).  So these checks will always pass.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> CC: Juergen Gross <jgross@suse.com>

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,
diff mbox series

Patch

diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index e6a48974f8..7e52f09731 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -2141,6 +2141,7 @@  void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
     libxl_domain_build_info_init(&dm_config->b_info);
     libxl_domain_build_info_init_type(&dm_config->b_info, LIBXL_DOMAIN_TYPE_PV);
 
+    dm_config->b_info.shadow_memkb = 0;
     dm_config->b_info.max_vcpus = 1;
     dm_config->b_info.max_memkb = 28 * 1024 +
         guest_config->b_info.video_memkb;
@@ -2167,14 +2168,7 @@  void libxl__spawn_stub_dm(libxl__egc *egc, libxl__stub_dm_spawn_state *sdss)
     dm_config->c_info.run_hotplug_scripts =
         guest_config->c_info.run_hotplug_scripts;
 
-    libxl_physinfo physinfo;
-    ret = libxl_get_physinfo(CTX, &physinfo);
-    if (ret) goto out;
-
-    ret = libxl__domain_create_info_setdefault(gc, &dm_config->c_info,
-                                               &physinfo);
-    if (ret) goto out;
-    ret = libxl__domain_build_info_setdefault(gc, &dm_config->b_info);
+    ret = libxl__domain_config_setdefault(gc, dm_config, guest_domid);
     if (ret) goto out;
 
     if (libxl_defbool_val(guest_config->b_info.u.hvm.vnc.enable)