On 28/06/2021 12:47, Jan Beulich wrote: > The hypervisor may not have enough memory to satisfy the request. > > Requested-by: Andrew Cooper <[email protected]> > Signed-off-by: Jan Beulich <[email protected]> > --- > Especially if the request was mostly fulfilled, guests may have done > fine despite the failure, so there is a risk of perceived regressions > here. But not checking the error at all was certainly wrong. > > --- a/tools/libs/light/libxl_x86.c > +++ b/tools/libs/light/libxl_x86.c > @@ -531,8 +531,18 @@ int libxl__arch_domain_create(libxl__gc > if (d_config->b_info.type != LIBXL_DOMAIN_TYPE_PV) { > unsigned long shadow = DIV_ROUNDUP(d_config->b_info.shadow_memkb, > 1024); > - xc_shadow_control(ctx->xch, domid, > XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION, > - NULL, 0, &shadow, 0, NULL); > + int rc = xc_shadow_control(ctx->xch, domid, > + XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION, > + NULL, 0, &shadow, 0, NULL); > + > + if (rc) { > + LOGED(ERROR, domid, > + "Failed to set %s allocation: %d (errno:%d)\n", > + libxl_defbool_val(d_config->c_info.hap) ? "HAP" : "shadow",
The error message wants to include the value of shadow, just in case the cause of the failure is because the value is stupidly high. Having traced the number through, the local variable wants to be named shadow_mb to try and make the units clearer. (Also - why on earth do we have a hypercall which takes integer units of MB, and a memkb field in the config file...) Otherwise, Reviewed-by: Andrew Cooper <[email protected]>
