From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH] libxc: fix claim mode when creating HVM guest Date: Mon, 27 Jan 2014 19:09:45 +0000 Message-ID: <52E6AEF9.9050406@eu.citrix.com> References: <1390845218-823-1-git-send-email-wei.liu2@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1390845218-823-1-git-send-email-wei.liu2@citrix.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: Wei Liu , xen-devel@lists.xen.org Cc: Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org On 01/27/2014 05:53 PM, Wei Liu wrote: > The original code is wrong because: > * claim mode wants to know the total number of pages needed while > original code provides the additional number of pages needed. > * if pod is enabled memory will already be allocated by the time we try > to claim memory. > > So the fix would be: > * move claim mode before actual memory allocation. > * pass the right number of pages to hypervisor. > > The "right number of pages" should be number of pages of target memory > minus VGA_HOLE_SIZE, regardless of whether PoD is enabled. > > This fixes bug #32. > > Signed-off-by: Wei Liu > Cc: Konrad Wilk > Cc: George Dunlap > Cc: Ian Campbell > Cc: Ian Jackson Reviewed-by: George Dunlap > --- > WRT 4.4 release: this patch should be accpeted, otherwise PoD + claim > mode is complete broken. If this patch is deemed too complicated, we > should flip the switch to disable claim mode by default for 4.4. I think a more reasonable mitigation strategy would simply be to ignore claim mode when constructing a domain that uses PoD. I'm inclined to take this one. Since claim mode is on by default, the currently-working path should get exercised well before the release to shake out any bugs. The other path doesn't work at all currently (AFAICT) unless you disable claim mode -- which is still available as a work-around, even if there is a bug in this patch. I'll wait a day or two for others to speak up before giving it a formal ack, just in case. -George