From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Zhang, Yang Z" Subject: RE: [PATCH] xl create: endless loop Date: Mon, 8 Nov 2010 16:18:19 +0800 Message-ID: <749B9D3DBF0F054390025D9EAFF47F22300BA646@shsmsx501.ccr.corp.intel.com> References: <201010181450.36050.Christoph.Egger@amd.com> <201011031712.48007.Christoph.Egger@amd.com> <201011041640.12947.Christoph.Egger@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <201011041640.12947.Christoph.Egger@amd.com> Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Christoph Egger , "xen-devel@lists.xensource.com" Cc: Jackson , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org Did the patch apply? This issue still exist in xen-unstable(22364). best regards yang > -----Original Message----- > From: xen-devel-bounces@lists.xensource.com > [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Christoph Egg= er > 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 >=20 > 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 =3D libxl__xs_read(gc, XBT_NULL, > > > > > free_mem_slack_path); if (!free_mem_slack_s) { > > > > > rc =3D 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 =3D libxl__xs_read(gc, XBT_NULL, > > > free_mem_slack_path); if (!free_mem_slack_s) { > > > rc =3D libxl__fill_dom0_memory_info(gc, &target_memkb); > > > - if (rc < 0) > > > + if (rc <=3D 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 =3D libxl_domain_info(ctx, &info, 0); > > > if (rc < 0) > > > - return rc; > > > + goto out; > > > > > > rc =3D 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 =3D 0; > > > > > > out: > > > - if (!xs_transaction_end(ctx->xsh, t, 0)) > > > + if (!xs_transaction_end(ctx->xsh, t, 0)) { > > > if (errno =3D=3D EAGAIN) > > > goto retry_transaction; > > > + else > > > + rc =3D 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 =3D xs_transaction_start(ctx->xsh); > > > > target =3D libxl__xs_read(gc, t, target_path); > > if (target) { <-- target contains "5" > > *target_memkb =3D strtoul(target, &endptr, 10); > > if (*endptr !=3D '\0') { <-- *endptr contains '\0' > > LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, > > "invalid memory target %s from %s\n", target, > > target_path); > > rc =3D ERROR_FAIL; > > goto out; > > } > > rc =3D 0; > > goto out; <-- take this jump with rc being 0 > > } > > >=20 > A slightly modified version works. >=20 > Signed-off-by: Mark Langsdorf > Signed-off-by: Christoph Egger >=20 >=20 >=20 >=20 > -- > ---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