From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: Re: [PATCH 1/1] xc_domain_restore: Allow QEMU to increase memory Date: Mon, 13 Apr 2015 17:20:07 +0100 Message-ID: <20150413162007.GJ17670@zion.uk.xensource.com> References: <1428941353-18673-1-git-send-email-dslutz@verizon.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1428941353-18673-1-git-send-email-dslutz@verizon.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Don Slutz Cc: Wei Liu , Ian Campbell , Stefano Stabellini , Ian Jackson , xen-devel@lists.xen.org, Shriram Rajagopalan , Yang Hongyang List-Id: xen-devel@lists.xenproject.org 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 > --- > 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 > #include > > +/* 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.