Re: [Xen-devel] [PATCH] xen: fix booting ballooned down hvm guest

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 10/24/2017 04:10 AM, Juergen Gross wrote:
> Commit 96edd61dcf44362d3ef0bed1a5361e0ac7886a63 ("xen/balloon: don't
> online new memory initially") introduced a regression when booting a
> HVM domain with memory less than mem-max: instead of ballooning down
> immediately the system would try to use the memory up to mem-max
> resulting in Xen crashing the domain.
>
> For HVM domains the current size will be reflected in Xenstore node
> memory/static-max instead of memory/target.
>
> Additionally we have to trigger the ballooning process at once.
>
> Cc: <stable@xxxxxxxxxxxxxxx> # 4.13
> Fixes: 96edd61dcf44362d3ef0bed1a5361e0ac7886a63 ("xen/balloon: don't
>        online new memory initially")
>
> Suggested-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

Reported-by: HW42 <hw42@xxxxxxxxx>

> ---
>  drivers/xen/xen-balloon.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/xen/xen-balloon.c b/drivers/xen/xen-balloon.c
> index e89136ab851e..3745748d9644 100644
> --- a/drivers/xen/xen-balloon.c
> +++ b/drivers/xen/xen-balloon.c
> @@ -57,7 +57,7 @@ static int register_balloon(struct device *dev);
>  static void watch_target(struct xenbus_watch *watch,
>                        const char *path, const char *token)
>  {
> -     unsigned long long new_target;
> +     unsigned long long new_target, static_max;
>       int err;
>       static bool watch_fired;
>       static long target_diff;
> @@ -72,13 +72,19 @@ static void watch_target(struct xenbus_watch *watch,
>        * pages. PAGE_SHIFT converts bytes to pages, hence PAGE_SHIFT - 10.
>        */
>       new_target >>= PAGE_SHIFT - 10;
> -     if (watch_fired) {
> -             balloon_set_new_target(new_target - target_diff);
> -             return;
> +
> +     if (!watch_fired) {
> +             watch_fired = true;
> +             err = xenbus_scanf(XBT_NIL, "memory", "static-max", "%llu",
> +                                &static_max);
> +             if (err != 1)
> +                     static_max = new_target;
> +             static_max >>= PAGE_SHIFT - 10;

if you set static_max to new_target you've already done the shift,
haven't you?


> +             target_diff = xen_pv_domain() ? 0

Why do we special-case PV?

-boris

> +                             : static_max - balloon_stats.target_pages;
>       }
>  
> -     watch_fired = true;
> -     target_diff = new_target - balloon_stats.target_pages;
> +     balloon_set_new_target(new_target - target_diff);
>  }
>  static struct xenbus_watch target_watch = {
>       .node = "memory/target",


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel