From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Egger Subject: Re: [PATCH] xl create: endless loop Date: Tue, 9 Nov 2010 14:27:34 +0100 Message-ID: <201011091427.35580.Christoph.Egger@amd.com> References: <201010181450.36050.Christoph.Egger@amd.com> <201011041640.12947.Christoph.Egger@amd.com> <749B9D3DBF0F054390025D9EAFF47F22300BA646@shsmsx501.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <749B9D3DBF0F054390025D9EAFF47F22300BA646@shsmsx501.ccr.corp.intel.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: "Zhang, Yang Z" Cc: "xen-devel@lists.xensource.com" , Jackson , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On Monday 08 November 2010 09:18:19 Zhang, Yang Z wrote: > Did the patch apply? This issue still exist in xen-unstable(22364). Yes. > > best regards > yang > > > -----Original Message----- > > From: xen-devel-bounces@lists.xensource.com > > [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Christoph > > Egger Sent: Thursday, November 04, 2010 11:40 PM > > To: xen-devel@lists.xensource.com > > Cc: Jackson; Stefano Stabellini > > Subject: Re: [Xen-devel][PATCH] xl create: endless loop > > > > On Wednesday 03 November 2010 17:12:47 Christoph Egger wrote: > > > On Thursday 21 October 2010 16:11:55 Stefano Stabellini wrote: > > > > On Tue, 19 Oct 2010, Christoph Egger wrote: > > > > > On Monday 18 October 2010 14:50:35 Christoph Egger wrote: > > > > > > Hi! > > > > > > > > > > > > I cannot start a guest with 'xl create' due to an endless loop > > > > > > in libxl.c, function libxl__get_free_memory_slack(): > > > > > > > > > > > > There is this code snippet: > > > > > > > > > > > > retry: > > > > > > free_mem_slack_s = libxl__xs_read(gc, XBT_NULL, > > > > > > free_mem_slack_path); if (!free_mem_slack_s) { > > > > > > rc = libxl__fill_dom0_memory_info(gc, &target_memkb); > > > > > > if (rc < 0) > > > > > > return rc; > > > > > > goto retry; > > > > > > } else { > > > > > > > > > > > > > > > > > > libxl__xs_read() returns 0 and libxl__fill_dom0_memory_info() > > > > > > also returns 0. So there's a loop of retries. > > > > > > > > > > Attached patch fixes the endless loop. > > > > > > > > > > Signed-off-by: Christoph Egger > > > > > > > > thanks for the patch but this cannot possibly be the right fix: > > > > > > > > > > > > diff -r 9d8d6b93114e tools/libxl/libxl.c > > > > --- a/tools/libxl/libxl.c Mon Oct 18 15:24:39 2010 +0200 > > > > +++ b/tools/libxl/libxl.c Tue Oct 19 10:29:00 2010 +0200 > > > > @@ -2836,7 +2836,7 @@ retry: > > > > free_mem_slack_s = libxl__xs_read(gc, XBT_NULL, > > > > free_mem_slack_path); if (!free_mem_slack_s) { > > > > rc = libxl__fill_dom0_memory_info(gc, &target_memkb); > > > > - if (rc < 0) > > > > + if (rc <= 0) > > > > return rc; > > > > goto retry; > > > > } else { > > > > > > > > the idea is that libxl__fill_dom0_memory_info should fill the > > > > missing informations in xenstore so that we can go ahead and try to > > > > read them again and the second time should be successful. > > > > Libxl__fill_dom0_memory_info returns 0 on success, so it is correct > > > > to goto retry in that case. > > > > > > > > The bug must be in libxl__fill_dom0_memory_info that doesn't return > > > > error when it should. > > > > Does the appended patch works for you? > > > > > > > > --- > > > > > > > > diff -r 00b92112b055 tools/libxl/libxl.c > > > > --- a/tools/libxl/libxl.c Wed Oct 20 17:26:51 2010 +0100 > > > > +++ b/tools/libxl/libxl.c Thu Oct 21 15:08:29 2010 +0100 > > > > @@ -2793,11 +2793,11 @@ retry_transaction: > > > > > > > > rc = libxl_domain_info(ctx, &info, 0); > > > > if (rc < 0) > > > > - return rc; > > > > + goto out; > > > > > > > > rc = libxl_get_physinfo(ctx, &physinfo); > > > > if (rc < 0) > > > > - return rc; > > > > + goto out; > > > > > > > > libxl__xs_write(gc, t, target_path, "%"PRIu32, > > > > (uint32_t) info.current_memkb); @@ -2816,9 +2816,12 > > > > @@ > > > > > > retry_transaction: > > > > rc = 0; > > > > > > > > out: > > > > - if (!xs_transaction_end(ctx->xsh, t, 0)) > > > > + if (!xs_transaction_end(ctx->xsh, t, 0)) { > > > > if (errno == EAGAIN) > > > > goto retry_transaction; > > > > + else > > > > + rc = ERROR_FAIL; > > > > + } > > > > > > > > > > > > return rc; > > > > > > No, this patch has no effect for me. > > > In libxl__fill_dom0_memory_info(), the code path goes that way: > > > > > > t = xs_transaction_start(ctx->xsh); > > > > > > target = libxl__xs_read(gc, t, target_path); > > > if (target) { <-- target contains "5" > > > *target_memkb = strtoul(target, &endptr, 10); > > > if (*endptr != '\0') { <-- *endptr contains '\0' > > > LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, > > > "invalid memory target %s from %s\n", target, > > > target_path); > > > rc = ERROR_FAIL; > > > goto out; > > > } > > > rc = 0; > > > goto out; <-- take this jump with rc being 0 > > > } > > > > A slightly modified version works. > > > > Signed-off-by: Mark Langsdorf > > Signed-off-by: Christoph Egger -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632