Re: [Xen-devel] [PATCH 1/1] xc_domain_restore: Allow QEMU to increase memory

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Apr 13, 2015 at 12:09:13PM -0400, Don Slutz wrote:
> If QEMU has called on xc_domain_setmaxmem to add more memory for
> option ROMs, domain restore needs to also increase the memory.
> 
> Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>
> ---
> To see the hvmloader loader issue:
> 
>   xl cre -p e1000x8.xfg
>   xl save e1000x8 e1000x8.save
>   xl restore e1000x8.save
> 
> With e1000x8.xfg:
> -------------------------------------------------------------------------------
> builder = "hvm"
> device_model_args_hvm = [
>  "-monitor",
>  "pty",
>  "-boot",
>  "menu=on",
> ]
> device_model_version = "qemu-xen"
> disk = [
>  "/dev/etherd/e500.1,,xvda",
>  "/dev/etherd/e500.2,,xvdb",
>  "/dev/etherd/e500.3,,xvde",
>  "/local-isos/iso/centos/x86_64/CentOS-6.3-x86_64-bin-DVD1.iso,,hdc,cdrom",
>  "/local-isos/iso/centos/x86_64/CentOS-6.3-x86_64-bin-DVD2.iso,,hdd,cdrom",
> ]
> maxmem = "8192"
> memory = "8192"
> name = "e1000x8"
> serial = "pty"
> tsc_mode = "native_paravirt"
> uuid = "e5000105-3d83-c962-07ae-2bc46c3644a0"
> videoram = "16"
> vif = [
>  "model=e1000,bridge=xenbr0,mac=00:0c:29:86:44:a0",
>  "model=e1000,bridge=xenbr0,mac=00:0c:29:86:44:aa",
>  "model=e1000,bridge=xenbr0,mac=00:0c:29:86:44:b4",
>  "model=e1000,bridge=xenbr0,mac=00:0c:29:86:44:be",
>  "model=e1000,bridge=xenbr0,mac=00:0c:29:86:44:c8",
>  "model=e1000,bridge=xenbr0,mac=00:0c:29:86:44:d2",
>  "model=e1000,bridge=xenbr0,mac=00:0c:29:86:44:dc",
>  "model=e1000,bridge=xenbr0,mac=00:0c:29:86:44:e6",
>  "model=e1000,bridge=xenbr0,mac=00:0c:29:86:44:f0",
>  "model=e1000,bridge=xenbr0,mac=00:0c:29:86:44:fa",
> ]
> viridian = 0
> xen_platform_pci = 1
> mmio_hole = 2048
> vcpus = 1
> maxvcpus = 6
> on_poweroff = "preserve"
> on_reboot = "preserve"
> on_watchdog = "preserve"
> on_crash = "preserve"
> -------------------------------------------------------------------------------
> 
> 
>  tools/libxc/xc_domain_restore.c | 75 
> +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 73 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
> index 2ab9f46..28b4fa6 100644
> --- a/tools/libxc/xc_domain_restore.c
> +++ b/tools/libxc/xc_domain_restore.c
> @@ -47,6 +47,13 @@
>  #include <xen/hvm/ioreq.h>
>  #include <xen/hvm/params.h>
>  
> +/* Leave some slack so that hvmloader does not complain about lack of
> + * memory at boot time ("Could not allocate order=0 extent").
> + * Once hvmloader is modified to cope with that situation without
> + * printing warning messages, QEMU_SPARE_PAGES can be removed.

Yes, please properly modify hvmloader to do this instead of trying to
workaround this over and over again.

But didn't Stefano make some changes to libxl too to handle the extra
ram needed?

> + */
> +#define QEMU_SPARE_PAGES 16
> +

This is still arbitrary and prone to breakage.

>  struct restore_ctx {
>      unsigned long max_mfn; /* max mfn of the current host machine */
>      unsigned long hvirt_start; /* virtual starting address of the hypervisor 
> */
> @@ -209,12 +216,44 @@ static int uncanonicalize_pagetable(
>          if (!ctx->hvm && ctx->superpages)
>              rc = alloc_superpage_mfns(xch, dom, ctx, nr_mfns);
>          else
> +        {
> +            xc_domaininfo_t info;
> +            unsigned long free_pages;
> +
> +            if ((xc_domain_getinfolist(xch, dom, 1, &info) != 1) ||
> +                (info.domain != dom)) {
> +                ERROR("Failed xc_domain_getinfolist for batch 
> (uncanonicalize_pagetable)\n");
> +                errno = ENOMEM;
> +                return 0;
> +            }
> +            free_pages = info.max_pages - info.tot_pages;
> +            if (free_pages > QEMU_SPARE_PAGES) {
> +                free_pages -= QEMU_SPARE_PAGES;
> +            } else {
> +                free_pages = 0;
> +            }
> +            if (free_pages < nr_mfns)
> +            {
> +                DPRINTF("Adjust memory for batch (uncanonicalize_pagetable): 
> free_pages=%lu nr_mfns=%d max_pages=%lu tot_pages=%lu max_mfn=%lu\n",
> +                        free_pages, nr_mfns, (unsigned long)info.max_pages,
> +                        (unsigned long)info.tot_pages, ctx->max_mfn);
> +                if (xc_domain_setmaxmem(xch, dom,
> +                                        ((info.max_pages + nr_mfns - 
> free_pages)
> +                                         << (XC_PAGE_SHIFT - 10))) < 0)
> +                {
> +                    ERROR("Failed xc_domain_setmaxmem for batch 
> (uncanonicalize_pagetable)\n");
> +                    errno = ENOMEM;
> +                    return 0;
> +                }
> +            }

This is not maintainable. Trying to do smart things inside libxc
without libxl knowing it is prone to breakage.

Wei.

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