From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Egger Subject: Re: [PATCH] xl create: endless loop Date: Thu, 4 Nov 2010 16:40:12 +0100 Message-ID: <201011041640.12947.Christoph.Egger@amd.com> References: <201010181450.36050.Christoph.Egger@amd.com> <201011031712.48007.Christoph.Egger@amd.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="Boundary-00=_cPt0McvykPlTuLB" Return-path: In-Reply-To: <201011031712.48007.Christoph.Egger@amd.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com Cc: Jackson , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org --Boundary-00=_cPt0McvykPlTuLB Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline 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 --Boundary-00=_cPt0McvykPlTuLB Content-Type: text/x-diff; charset="iso 8859-15"; name="xen_xl.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="xen_xl.diff" Content-Description: xen_xl.diff diff -r 8f66a16efee0 tools/libxl/libxl.c --- a/tools/libxl/libxl.c Tue Nov 02 16:08:56 2010 +0100 +++ b/tools/libxl/libxl.c Thu Nov 04 16:30:27 2010 +0100 @@ -2787,17 +2787,15 @@ retry_transaction: rc = ERROR_FAIL; goto out; } - rc = 0; - goto out; } 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,10 +2814,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; } --Boundary-00=_cPt0McvykPlTuLB Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --Boundary-00=_cPt0McvykPlTuLB--