diff mbox series

[v2,01/13] libxl/x86: check return value of SHADOW_OP_SET_ALLOCATION domctl

Message ID 4d1e5a4d-05cf-964a-bc4b-9eecf26e58bb@suse.com (mailing list archive)
State New
Headers show
Series x86: more or less log-dirty related improvements | expand

Commit Message

Jan Beulich July 5, 2021, 3:11 p.m. UTC
The hypervisor may not have enough memory to satisfy the request. While
there, make the unit of the value clear by renaming the local variable.

Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v2: Rename local variables. Also log requested value. Don't log errno
    explicitly in LOGED() (nor function return value). Integrate into
    series.
---
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.

Comments

Anthony PERARD July 16, 2021, 1:03 p.m. UTC | #1
On Mon, Jul 05, 2021 at 05:11:31PM +0200, Jan Beulich wrote:
> The hypervisor may not have enough memory to satisfy the request. While
> there, make the unit of the value clear by renaming the local variable.
> 
> Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> v2: Rename local variables. Also log requested value. Don't log errno
>     explicitly in LOGED() (nor function return value). Integrate into
>     series.

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

Thanks,
diff mbox series

Patch

--- a/tools/libs/light/libxl_x86.c
+++ b/tools/libs/light/libxl_x86.c
@@ -529,10 +529,20 @@  int libxl__arch_domain_create(libxl__gc
         xc_domain_set_time_offset(ctx->xch, domid, rtc_timeoffset);
 
     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);
+        unsigned long shadow_mb = DIV_ROUNDUP(d_config->b_info.shadow_memkb,
+                                              1024);
+        int r = xc_shadow_control(ctx->xch, domid,
+                                  XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION,
+                                  NULL, 0, &shadow_mb, 0, NULL);
+
+        if (r) {
+            LOGED(ERROR, domid,
+                  "Failed to set %lu MiB %s allocation",
+                  shadow_mb,
+                  libxl_defbool_val(d_config->c_info.hap) ? "HAP" : "shadow");
+            ret = ERROR_FAIL;
+            goto out;
+        }
     }
 
     if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV &&