* xl create: endless loop
@ 2010-10-18 12:50 Christoph Egger
2010-10-19 8:31 ` [PATCH] " Christoph Egger
0 siblings, 1 reply; 21+ messages in thread
From: Christoph Egger @ 2010-10-18 12:50 UTC (permalink / raw)
To: xen-devel
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.
Christoph
--
---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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH] xl create: endless loop 2010-10-18 12:50 xl create: endless loop Christoph Egger @ 2010-10-19 8:31 ` Christoph Egger 2010-10-21 14:11 ` Stefano Stabellini 0 siblings, 1 reply; 21+ messages in thread From: Christoph Egger @ 2010-10-19 8:31 UTC (permalink / raw) To: xen-devel, Ian Jackson [-- Attachment #1: Type: text/plain, Size: 963 bytes --] 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 <Christoph.Egger@amd.com> -- ---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 [-- Attachment #2: xen_xl.diff --] [-- Type: text/x-diff, Size: 454 bytes --] 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 { [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] xl create: endless loop 2010-10-19 8:31 ` [PATCH] " Christoph Egger @ 2010-10-21 14:11 ` Stefano Stabellini 2010-11-03 16:12 ` Christoph Egger 0 siblings, 1 reply; 21+ messages in thread From: Stefano Stabellini @ 2010-10-21 14:11 UTC (permalink / raw) To: Christoph Egger; +Cc: Ian, xen-devel, Jackson 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 <Christoph.Egger@amd.com> 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; ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] xl create: endless loop 2010-10-21 14:11 ` Stefano Stabellini @ 2010-11-03 16:12 ` Christoph Egger 2010-11-04 15:40 ` Christoph Egger 2010-11-09 18:08 ` Stefano Stabellini 0 siblings, 2 replies; 21+ messages in thread From: Christoph Egger @ 2010-11-03 16:12 UTC (permalink / raw) To: Stefano Stabellini; +Cc: Ian, xen-devel, Jackson 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 <Christoph.Egger@amd.com> > > 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 } Christoph -- ---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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] xl create: endless loop 2010-11-03 16:12 ` Christoph Egger @ 2010-11-04 15:40 ` Christoph Egger 2010-11-08 8:18 ` Zhang, Yang Z 2010-11-09 18:08 ` Stefano Stabellini 1 sibling, 1 reply; 21+ messages in thread From: Christoph Egger @ 2010-11-04 15:40 UTC (permalink / raw) To: xen-devel; +Cc: Jackson, Stefano Stabellini [-- Attachment #1: Type: text/plain, Size: 4044 bytes --] 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 <Christoph.Egger@amd.com> > > > > 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 <Mark.Langsdorf@amd.com> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> -- ---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 [-- Attachment #2: xen_xl.diff --] [-- Type: text/x-diff, Size: 909 bytes --] 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; } [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] xl create: endless loop 2010-11-04 15:40 ` Christoph Egger @ 2010-11-08 8:18 ` Zhang, Yang Z 2010-11-09 13:27 ` Christoph Egger 0 siblings, 1 reply; 21+ messages in thread From: Zhang, Yang Z @ 2010-11-08 8:18 UTC (permalink / raw) To: Christoph Egger, xen-devel; +Cc: Jackson, Stefano Stabellini 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 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 <Christoph.Egger@amd.com> > > > > > > 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 <Mark.Langsdorf@amd.com> > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> > > > > > -- > ---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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] xl create: endless loop 2010-11-08 8:18 ` Zhang, Yang Z @ 2010-11-09 13:27 ` Christoph Egger 0 siblings, 0 replies; 21+ messages in thread From: Christoph Egger @ 2010-11-09 13:27 UTC (permalink / raw) To: Zhang, Yang Z; +Cc: xen-devel, Jackson, Stefano Stabellini 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 <Christoph.Egger@amd.com> > > > > > > > > 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 <Mark.Langsdorf@amd.com> > > Signed-off-by: Christoph Egger <Christoph.Egger@amd.com> -- ---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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] xl create: endless loop 2010-11-03 16:12 ` Christoph Egger 2010-11-04 15:40 ` Christoph Egger @ 2010-11-09 18:08 ` Stefano Stabellini 2010-11-10 10:34 ` Christoph Egger 1 sibling, 1 reply; 21+ messages in thread From: Stefano Stabellini @ 2010-11-09 18:08 UTC (permalink / raw) To: Christoph Egger; +Cc: xen-devel, Ian Jackson, Stefano Stabellini On Wed, 3 Nov 2010, Christoph Egger wrote: > 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 > } > > The problem you are having is that somebody in your system is setting a target for dom0 without setting freemem-slack. Are you still running xend at boot? Currently libxl__fill_dom0_memory_info assumes that both values are set initially at the same time (by libxl__fill_dom0_memory_info). This patch should fix the issue, I would appreciate if you could test it. --- libxl: do not assume target and freemem-slack are written at the same time Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> diff -r 7188d1e4b0e1 tools/libxl/libxl.c --- a/tools/libxl/libxl.c Tue Nov 09 12:00:05 2010 +0000 +++ b/tools/libxl/libxl.c Tue Nov 09 18:05:52 2010 +0000 @@ -2779,18 +2779,25 @@ static int libxl__fill_dom0_memory_info( int rc; libxl_dominfo info; libxl_physinfo physinfo; - char *target = NULL, *endptr = NULL; + char *target = NULL, *staticmax = NULL, *freememslack = NULL, *endptr = NULL; char *target_path = "/local/domain/0/memory/target"; char *max_path = "/local/domain/0/memory/static-max"; char *free_mem_slack_path = "/local/domain/0/memory/freemem-slack"; xs_transaction_t t; libxl_ctx *ctx = libxl__gc_owner(gc); - uint32_t free_mem_slack = 0; + uint32_t free_mem_slack_kb = 0; retry_transaction: t = xs_transaction_start(ctx->xsh); target = libxl__xs_read(gc, t, target_path); + staticmax = libxl__xs_read(gc, t, target_path); + freememslack = libxl__xs_read(gc, t, target_path); + if (target && staticmax && freememslack) { + rc = 0; + goto out; + } + if (target) { *target_memkb = strtoul(target, &endptr, 10); if (*endptr != '\0') { @@ -2799,38 +2806,43 @@ 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; - - libxl__xs_write(gc, t, target_path, "%"PRIu32, - (uint32_t) info.current_memkb); - libxl__xs_write(gc, t, max_path, "%"PRIu32, - (uint32_t) info.max_memkb); - - free_mem_slack = (uint32_t) (PAGE_TO_MEMKB(physinfo.total_pages) - - info.current_memkb); - /* From empirical measurements the free_mem_slack shouldn't be more - * than 15% of the total memory present on the system. */ - if (free_mem_slack > PAGE_TO_MEMKB(physinfo.total_pages) * 0.15) - free_mem_slack = PAGE_TO_MEMKB(physinfo.total_pages) * 0.15; - libxl__xs_write(gc, t, free_mem_slack_path, "%"PRIu32, free_mem_slack); - - *target_memkb = (uint32_t) info.current_memkb; + goto out; + + if (target == NULL) { + libxl__xs_write(gc, t, target_path, "%"PRIu32, + (uint32_t) info.current_memkb); + *target_memkb = (uint32_t) info.current_memkb; + } + if (staticmax == NULL) + libxl__xs_write(gc, t, max_path, "%"PRIu32, + (uint32_t) info.max_memkb); + + if (freememslack == NULL) { + free_mem_slack_kb = (uint32_t) (PAGE_TO_MEMKB(physinfo.total_pages) - + info.current_memkb); + /* From empirical measurements the free_mem_slack shouldn't be more + * than 15% of the total memory present on the system. */ + if (free_mem_slack_kb > PAGE_TO_MEMKB(physinfo.total_pages) * 0.15) + free_mem_slack_kb = PAGE_TO_MEMKB(physinfo.total_pages) * 0.15; + libxl__xs_write(gc, t, free_mem_slack_path, "%"PRIu32, free_mem_slack_kb); + } 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; ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] xl create: endless loop 2010-11-09 18:08 ` Stefano Stabellini @ 2010-11-10 10:34 ` Christoph Egger 2010-11-10 11:52 ` Stefano Stabellini 0 siblings, 1 reply; 21+ messages in thread From: Christoph Egger @ 2010-11-10 10:34 UTC (permalink / raw) To: Stefano Stabellini; +Cc: Ian, xen-devel, Jackson On Tuesday 09 November 2010 19:08:14 Stefano Stabellini wrote: > On Wed, 3 Nov 2010, Christoph Egger wrote: > > 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 > > } > > The problem you are having is that somebody in your system is setting a > target for dom0 without setting freemem-slack. Are you still running > xend at boot? Yes, I do. Xen is booted with dom0_mem. Dom0 has autoballooning disabled in the kernel. > Currently libxl__fill_dom0_memory_info assumes that both values are set > initially at the same time (by libxl__fill_dom0_memory_info). > This patch should fix the issue, I would appreciate if you could test > it. > > --- > > > libxl: do not assume target and freemem-slack are written at the same time > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > diff -r 7188d1e4b0e1 tools/libxl/libxl.c > --- a/tools/libxl/libxl.c Tue Nov 09 12:00:05 2010 +0000 > +++ b/tools/libxl/libxl.c Tue Nov 09 18:05:52 2010 +0000 > @@ -2779,18 +2779,25 @@ static int libxl__fill_dom0_memory_info( > int rc; > libxl_dominfo info; > libxl_physinfo physinfo; > - char *target = NULL, *endptr = NULL; > + char *target = NULL, *staticmax = NULL, *freememslack = NULL, *endptr > = NULL; char *target_path = "/local/domain/0/memory/target"; > char *max_path = "/local/domain/0/memory/static-max"; > char *free_mem_slack_path = "/local/domain/0/memory/freemem-slack"; > xs_transaction_t t; > libxl_ctx *ctx = libxl__gc_owner(gc); > - uint32_t free_mem_slack = 0; > + uint32_t free_mem_slack_kb = 0; > > retry_transaction: > t = xs_transaction_start(ctx->xsh); > > target = libxl__xs_read(gc, t, target_path); > + staticmax = libxl__xs_read(gc, t, target_path); > + freememslack = libxl__xs_read(gc, t, target_path); > + if (target && staticmax && freememslack) { > + rc = 0; > + goto out; > + } *target, *staticmax and *freememslack contain the value "5". So with this patch, rc always returns 0 from there. Christoph > + > if (target) { > *target_memkb = strtoul(target, &endptr, 10); > if (*endptr != '\0') { > @@ -2799,38 +2806,43 @@ 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; > - > - libxl__xs_write(gc, t, target_path, "%"PRIu32, > - (uint32_t) info.current_memkb); > - libxl__xs_write(gc, t, max_path, "%"PRIu32, > - (uint32_t) info.max_memkb); > - > - free_mem_slack = (uint32_t) (PAGE_TO_MEMKB(physinfo.total_pages) - > - info.current_memkb); > - /* From empirical measurements the free_mem_slack shouldn't be more > - * than 15% of the total memory present on the system. */ > - if (free_mem_slack > PAGE_TO_MEMKB(physinfo.total_pages) * 0.15) > - free_mem_slack = PAGE_TO_MEMKB(physinfo.total_pages) * 0.15; > - libxl__xs_write(gc, t, free_mem_slack_path, "%"PRIu32, > free_mem_slack); - > - *target_memkb = (uint32_t) info.current_memkb; > + goto out; > + > + if (target == NULL) { > + libxl__xs_write(gc, t, target_path, "%"PRIu32, > + (uint32_t) info.current_memkb); > + *target_memkb = (uint32_t) info.current_memkb; > + } > + if (staticmax == NULL) > + libxl__xs_write(gc, t, max_path, "%"PRIu32, > + (uint32_t) info.max_memkb); > + > + if (freememslack == NULL) { > + free_mem_slack_kb = (uint32_t) > (PAGE_TO_MEMKB(physinfo.total_pages) - + > info.current_memkb); > + /* From empirical measurements the free_mem_slack shouldn't be > more + * than 15% of the total memory present on the system. */ + > if (free_mem_slack_kb > PAGE_TO_MEMKB(physinfo.total_pages) * 0.15) + > free_mem_slack_kb = PAGE_TO_MEMKB(physinfo.total_pages) * 0.15; + > libxl__xs_write(gc, t, free_mem_slack_path, "%"PRIu32, > free_mem_slack_kb); + } > 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; -- ---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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] xl create: endless loop 2010-11-10 10:34 ` Christoph Egger @ 2010-11-10 11:52 ` Stefano Stabellini 2010-11-16 13:29 ` Zhang, Yang Z 2010-12-02 13:43 ` Christoph Egger 0 siblings, 2 replies; 21+ messages in thread From: Stefano Stabellini @ 2010-11-10 11:52 UTC (permalink / raw) To: Christoph Egger; +Cc: xen-devel, Ian Jackson, Stefano Stabellini On Wed, 10 Nov 2010, Christoph Egger wrote: > On Tuesday 09 November 2010 19:08:14 Stefano Stabellini wrote: > > On Wed, 3 Nov 2010, Christoph Egger wrote: > > > 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 > > > } > > > > The problem you are having is that somebody in your system is setting a > > target for dom0 without setting freemem-slack. Are you still running > > xend at boot? > > Yes, I do. running xend alongside xl is not recommended, it could cause bugs, especially if you don't disable autoballooning > Xen is booted with dom0_mem. > Dom0 has autoballooning disabled in the kernel. > You still need to disable autoballooning in xl, setting autoballooning to 0 in /etc/xen/xl.conf > > Currently libxl__fill_dom0_memory_info assumes that both values are set > > initially at the same time (by libxl__fill_dom0_memory_info). > > This patch should fix the issue, I would appreciate if you could test > > it. > > > > --- > > > > > > libxl: do not assume target and freemem-slack are written at the same time > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > diff -r 7188d1e4b0e1 tools/libxl/libxl.c > > --- a/tools/libxl/libxl.c Tue Nov 09 12:00:05 2010 +0000 > > +++ b/tools/libxl/libxl.c Tue Nov 09 18:05:52 2010 +0000 > > @@ -2779,18 +2779,25 @@ static int libxl__fill_dom0_memory_info( > > int rc; > > libxl_dominfo info; > > libxl_physinfo physinfo; > > - char *target = NULL, *endptr = NULL; > > + char *target = NULL, *staticmax = NULL, *freememslack = NULL, *endptr > > = NULL; char *target_path = "/local/domain/0/memory/target"; > > char *max_path = "/local/domain/0/memory/static-max"; > > char *free_mem_slack_path = "/local/domain/0/memory/freemem-slack"; > > xs_transaction_t t; > > libxl_ctx *ctx = libxl__gc_owner(gc); > > - uint32_t free_mem_slack = 0; > > + uint32_t free_mem_slack_kb = 0; > > > > retry_transaction: > > t = xs_transaction_start(ctx->xsh); > > > > target = libxl__xs_read(gc, t, target_path); > > + staticmax = libxl__xs_read(gc, t, target_path); > > + freememslack = libxl__xs_read(gc, t, target_path); > > + if (target && staticmax && freememslack) { > > + rc = 0; > > + goto out; > > + } > > *target, *staticmax and *freememslack contain the value "5". > So with this patch, rc always returns 0 from there. > that is correct: the values are there, so there is no need to write them to xenstore again, and the caller should be able to read the target correctly ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] xl create: endless loop 2010-11-10 11:52 ` Stefano Stabellini @ 2010-11-16 13:29 ` Zhang, Yang Z 2010-11-16 14:07 ` Stefano Stabellini 2010-12-02 13:43 ` Christoph Egger 1 sibling, 1 reply; 21+ messages in thread From: Zhang, Yang Z @ 2010-11-16 13:29 UTC (permalink / raw) To: Stefano Stabellini, Christoph Egger; +Cc: Ian, xen-devel, Jackson This issue still exist in my side with latest xen unstable. Did the fixed patch work for you? best regards yang > -----Original Message----- > From: xen-devel-bounces@lists.xensource.com > [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Stefano > Stabellini > Sent: Wednesday, November 10, 2010 7:53 PM > To: Christoph Egger > Cc: xen-devel@lists.xensource.com; Ian Jackson; Stefano Stabellini > Subject: Re: [Xen-devel][PATCH] xl create: endless loop > > On Wed, 10 Nov 2010, Christoph Egger wrote: > > On Tuesday 09 November 2010 19:08:14 Stefano Stabellini wrote: > > > On Wed, 3 Nov 2010, Christoph Egger wrote: > > > > 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 > > > > } > > > > > > The problem you are having is that somebody in your system is setting a > > > target for dom0 without setting freemem-slack. Are you still running > > > xend at boot? > > > > Yes, I do. > > running xend alongside xl is not recommended, it could cause bugs, > especially if you don't disable autoballooning > > > > Xen is booted with dom0_mem. > > Dom0 has autoballooning disabled in the kernel. > > > > You still need to disable autoballooning in xl, setting autoballooning > to 0 in /etc/xen/xl.conf > > > > > Currently libxl__fill_dom0_memory_info assumes that both values are set > > > initially at the same time (by libxl__fill_dom0_memory_info). > > > This patch should fix the issue, I would appreciate if you could test > > > it. > > > > > > --- > > > > > > > > > libxl: do not assume target and freemem-slack are written at the same time > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > > > diff -r 7188d1e4b0e1 tools/libxl/libxl.c > > > --- a/tools/libxl/libxl.c Tue Nov 09 12:00:05 2010 +0000 > > > +++ b/tools/libxl/libxl.c Tue Nov 09 18:05:52 2010 +0000 > > > @@ -2779,18 +2779,25 @@ static int libxl__fill_dom0_memory_info( > > > int rc; > > > libxl_dominfo info; > > > libxl_physinfo physinfo; > > > - char *target = NULL, *endptr = NULL; > > > + char *target = NULL, *staticmax = NULL, *freememslack = NULL, > *endptr > > > = NULL; char *target_path = "/local/domain/0/memory/target"; > > > char *max_path = "/local/domain/0/memory/static-max"; > > > char *free_mem_slack_path = > "/local/domain/0/memory/freemem-slack"; > > > xs_transaction_t t; > > > libxl_ctx *ctx = libxl__gc_owner(gc); > > > - uint32_t free_mem_slack = 0; > > > + uint32_t free_mem_slack_kb = 0; > > > > > > retry_transaction: > > > t = xs_transaction_start(ctx->xsh); > > > > > > target = libxl__xs_read(gc, t, target_path); > > > + staticmax = libxl__xs_read(gc, t, target_path); > > > + freememslack = libxl__xs_read(gc, t, target_path); > > > + if (target && staticmax && freememslack) { > > > + rc = 0; > > > + goto out; > > > + } > > > > *target, *staticmax and *freememslack contain the value "5". > > So with this patch, rc always returns 0 from there. > > > > that is correct: the values are there, so there is no need to write them > to xenstore again, and the caller should be able to read the target > correctly > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] xl create: endless loop 2010-11-16 13:29 ` Zhang, Yang Z @ 2010-11-16 14:07 ` Stefano Stabellini 2010-11-16 14:29 ` Zhang, Yang Z 0 siblings, 1 reply; 21+ messages in thread From: Stefano Stabellini @ 2010-11-16 14:07 UTC (permalink / raw) To: Zhang, Yang Z; +Cc: Christoph Egger, xen-devel, Ian Jackson, Stefano Stabellini On Tue, 16 Nov 2010, Zhang, Yang Z wrote: > This issue still exist in my side with latest xen unstable. Did the fixed patch work for you? > Do you have autoballoon (in /etc/xen/xl.conf) set to 0 or 1? Do you pass dom0_mem as Xen command line option? If you pass dom0_mem to Xen, then autoballoon should be set to 0. If that's not the case, how much memory do you have on your testbox? How big is the VM you are trying to start? ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] xl create: endless loop 2010-11-16 14:07 ` Stefano Stabellini @ 2010-11-16 14:29 ` Zhang, Yang Z 2010-11-16 14:40 ` Stefano Stabellini 0 siblings, 1 reply; 21+ messages in thread From: Zhang, Yang Z @ 2010-11-16 14:29 UTC (permalink / raw) To: Stefano Stabellini; +Cc: Christoph Egger, xen-devel, Ian Jackson Yes I have the dom0_mem in xen command line. But I think the autoballoon is disable by default since there had an annotate before it, if it not I think we should add some comments like "autoballoon default is enable" as we see in the xmexample.hvm. And the another question is why I should to disable the autoballoon manually in the xl.conf? why don't you to add a check in the xl to see whether the autoballon is enable or disable? best regards yang > -----Original Message----- > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > Sent: Tuesday, November 16, 2010 10:07 PM > To: Zhang, Yang Z > Cc: Stefano Stabellini; Christoph Egger; xen-devel@lists.xensource.com; Ian > Jackson > Subject: RE: [Xen-devel][PATCH] xl create: endless loop > > On Tue, 16 Nov 2010, Zhang, Yang Z wrote: > > This issue still exist in my side with latest xen unstable. Did the fixed patch > work for you? > > > > Do you have autoballoon (in /etc/xen/xl.conf) set to 0 or 1? > Do you pass dom0_mem as Xen command line option? > > If you pass dom0_mem to Xen, then autoballoon should be set to 0. > If that's not the case, how much memory do you have on your testbox? > How big is the VM you are trying to start? ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] xl create: endless loop 2010-11-16 14:29 ` Zhang, Yang Z @ 2010-11-16 14:40 ` Stefano Stabellini 2010-11-16 14:48 ` Zhang, Yang Z 0 siblings, 1 reply; 21+ messages in thread From: Stefano Stabellini @ 2010-11-16 14:40 UTC (permalink / raw) To: Zhang, Yang Z; +Cc: Christoph Egger, xen-devel, Ian Jackson, Stefano Stabellini On Tue, 16 Nov 2010, Zhang, Yang Z wrote: > Yes I have the dom0_mem in xen command line. But I think the autoballoon is disable by default since there had an annotate before it, if it not I think we should add some comments like "autoballoon default is enable" as we see in the xmexample.hvm. > Autoballoon is enabled by default, a comment would probably be good > And the another question is why I should to disable the autoballoon manually in the xl.conf? why don't you to add a check in the xl to see whether the autoballon is enable or disable? > Good question. The problem is: how should I write this check? AFAIK there is no clear way to know if autoballooning should be enabled or disabled, apart from parsing the xen command line options, but it seems really ugly to me. In any case, could you please try disabling autoballooning and let me know if you still have this issue? ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] xl create: endless loop 2010-11-16 14:40 ` Stefano Stabellini @ 2010-11-16 14:48 ` Zhang, Yang Z 2010-11-16 15:57 ` Stefano Stabellini 0 siblings, 1 reply; 21+ messages in thread From: Zhang, Yang Z @ 2010-11-16 14:48 UTC (permalink / raw) To: Stefano Stabellini; +Cc: Christoph Egger, xen-devel, Ian Jackson > -----Original Message----- > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > Sent: Tuesday, November 16, 2010 10:40 PM > To: Zhang, Yang Z > Cc: Stefano Stabellini; Christoph Egger; xen-devel@lists.xensource.com; Ian > Jackson > Subject: RE: [Xen-devel][PATCH] xl create: endless loop > > On Tue, 16 Nov 2010, Zhang, Yang Z wrote: > > Yes I have the dom0_mem in xen command line. But I think the autoballoon is > disable by default since there had an annotate before it, if it not I think we > should add some comments like "autoballoon default is enable" as we see in > the xmexample.hvm. > > > > Autoballoon is enabled by default, a comment would probably be good > > > > And the another question is why I should to disable the autoballoon manually > in the xl.conf? why don't you to add a check in the xl to see whether the > autoballon is enable or disable? > > > > Good question. The problem is: how should I write this check? > AFAIK there is no clear way to know if autoballooning should be enabled > or disabled, apart from parsing the xen command line options, but it > seems really ugly to me. > > In any case, could you please try disabling autoballooning and let me > know if you still have this issue? It works with disable the autoballoon. Thanks for you help. And a question with qcow issue I reported in another mail. Since our automation test is based in qcow images, because copy raw images take a long times(we have 60+ case), so will anyone to take a look at this issue? it will block our automation test with xl. best regards yang ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] xl create: endless loop 2010-11-16 14:48 ` Zhang, Yang Z @ 2010-11-16 15:57 ` Stefano Stabellini 2010-11-17 2:50 ` Daniel Stodden 0 siblings, 1 reply; 21+ messages in thread From: Stefano Stabellini @ 2010-11-16 15:57 UTC (permalink / raw) To: Zhang, Yang Z Cc: Daniel Stodden, Christoph Egger, xen-devel, Ian Jackson, Stefano Stabellini On Tue, 16 Nov 2010, Zhang, Yang Z wrote: > > -----Original Message----- > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > Sent: Tuesday, November 16, 2010 10:40 PM > > To: Zhang, Yang Z > > Cc: Stefano Stabellini; Christoph Egger; xen-devel@lists.xensource.com; Ian > > Jackson > > Subject: RE: [Xen-devel][PATCH] xl create: endless loop > > > > On Tue, 16 Nov 2010, Zhang, Yang Z wrote: > > > Yes I have the dom0_mem in xen command line. But I think the autoballoon is > > disable by default since there had an annotate before it, if it not I think we > > should add some comments like "autoballoon default is enable" as we see in > > the xmexample.hvm. > > > > > > > Autoballoon is enabled by default, a comment would probably be good > > > > > > > And the another question is why I should to disable the autoballoon manually > > in the xl.conf? why don't you to add a check in the xl to see whether the > > autoballon is enable or disable? > > > > > > > Good question. The problem is: how should I write this check? > > AFAIK there is no clear way to know if autoballooning should be enabled > > or disabled, apart from parsing the xen command line options, but it > > seems really ugly to me. > > > > In any case, could you please try disabling autoballooning and let me > > know if you still have this issue? > It works with disable the autoballoon. Thanks for you help. > > And a question with qcow issue I reported in another mail. Since our automation test is based in qcow images, because copy raw images take a long times(we have 60+ case), so will anyone to take a look at this issue? it will block our automation test with xl. > Daniel, do you have any ideas? ^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH] xl create: endless loop 2010-11-16 15:57 ` Stefano Stabellini @ 2010-11-17 2:50 ` Daniel Stodden 2010-11-18 10:43 ` Christoph Egger 0 siblings, 1 reply; 21+ messages in thread From: Daniel Stodden @ 2010-11-17 2:50 UTC (permalink / raw) To: Stefano Stabellini; +Cc: Zhang, Yang Z, Christoph Egger, xen-devel, Ian Jackson On Tue, 2010-11-16 at 10:57 -0500, Stefano Stabellini wrote: > On Tue, 16 Nov 2010, Zhang, Yang Z wrote: > > > -----Original Message----- > > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > > Sent: Tuesday, November 16, 2010 10:40 PM > > > To: Zhang, Yang Z > > > Cc: Stefano Stabellini; Christoph Egger; xen-devel@lists.xensource.com; Ian > > > Jackson > > > Subject: RE: [Xen-devel][PATCH] xl create: endless loop > > > > > > On Tue, 16 Nov 2010, Zhang, Yang Z wrote: > > > > Yes I have the dom0_mem in xen command line. But I think the autoballoon is > > > disable by default since there had an annotate before it, if it not I think we > > > should add some comments like "autoballoon default is enable" as we see in > > > the xmexample.hvm. > > > > > > > > > > Autoballoon is enabled by default, a comment would probably be good > > > > > > > > > > And the another question is why I should to disable the autoballoon manually > > > in the xl.conf? why don't you to add a check in the xl to see whether the > > > autoballon is enable or disable? > > > > > > > > > > Good question. The problem is: how should I write this check? > > > AFAIK there is no clear way to know if autoballooning should be enabled > > > or disabled, apart from parsing the xen command line options, but it > > > seems really ugly to me. > > > > > > In any case, could you please try disabling autoballooning and let me > > > know if you still have this issue? > > It works with disable the autoballoon. Thanks for you help. > > > > And a question with qcow issue I reported in another mail. Since our automation test is based in qcow images, because copy raw images take a long times(we have 60+ case), so will anyone to take a look at this issue? it will block our automation test with xl. > > > > Daniel, do you have any ideas? Not the slightest. Just proof that: Are we talking about blktap qcow or qemu? Still trying to find that mail. Daniel ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] xl create: endless loop 2010-11-17 2:50 ` Daniel Stodden @ 2010-11-18 10:43 ` Christoph Egger 2010-11-18 12:21 ` Stefano Stabellini 0 siblings, 1 reply; 21+ messages in thread From: Christoph Egger @ 2010-11-18 10:43 UTC (permalink / raw) To: Daniel Stodden; +Cc: Zhang, Yang Z, xen-devel, Ian Jackson, Stefano Stabellini On Wednesday 17 November 2010 03:50:51 Daniel Stodden wrote: > On Tue, 2010-11-16 at 10:57 -0500, Stefano Stabellini wrote: > > On Tue, 16 Nov 2010, Zhang, Yang Z wrote: > > > > -----Original Message----- > > > > From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] > > > > Sent: Tuesday, November 16, 2010 10:40 PM > > > > To: Zhang, Yang Z > > > > Cc: Stefano Stabellini; Christoph Egger; > > > > xen-devel@lists.xensource.com; Ian Jackson > > > > Subject: RE: [Xen-devel][PATCH] xl create: endless loop > > > > > > > > On Tue, 16 Nov 2010, Zhang, Yang Z wrote: > > > > > Yes I have the dom0_mem in xen command line. But I think the > > > > > autoballoon is > > > > > > > > disable by default since there had an annotate before it, if it not I > > > > think we should add some comments like "autoballoon default is > > > > enable" as we see in the xmexample.hvm. > > > > > > > > > > > > Autoballoon is enabled by default, a comment would probably be good > > > > > > > > > And the another question is why I should to disable the autoballoon > > > > > manually > > > > > > > > in the xl.conf? why don't you to add a check in the xl to see whether > > > > the autoballon is enable or disable? > > > > > > > > > > > > Good question. The problem is: how should I write this check? > > > > AFAIK there is no clear way to know if autoballooning should be > > > > enabled or disabled, apart from parsing the xen command line options, > > > > but it seems really ugly to me. > > > > > > > > In any case, could you please try disabling autoballooning and let me > > > > know if you still have this issue? > > > > > > It works with disable the autoballoon. Thanks for you help. > > > > > > And a question with qcow issue I reported in another mail. Since our > > > automation test is based in qcow images, because copy raw images take a > > > long times(we have 60+ case), so will anyone to take a look at this > > > issue? it will block our automation test with xl. > > > > Daniel, do you have any ideas? > > Not the slightest. Just proof that: Are we talking about blktap qcow or > qemu? Still trying to find that mail. xl links against blktap2, qemu links against blktap. I think, this may cause some random behaviour. qemu should move to blktap2 and drop blktap from the tools then. Christoph -- ---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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] xl create: endless loop 2010-11-18 10:43 ` Christoph Egger @ 2010-11-18 12:21 ` Stefano Stabellini 0 siblings, 0 replies; 21+ messages in thread From: Stefano Stabellini @ 2010-11-18 12:21 UTC (permalink / raw) To: Christoph Egger Cc: Zhang, Yang Z, Stefano Stabellini, Ian Jackson, xen-devel, Daniel Stodden On Thu, 18 Nov 2010, Christoph Egger wrote: > > Not the slightest. Just proof that: Are we talking about blktap qcow or > > qemu? Still trying to find that mail. > > xl links against blktap2, qemu links against blktap. I think, this may cause > some random behaviour. > qemu should move to blktap2 and drop blktap from the tools then. > Is blktap support in qemu even used anywhere? Maybe with xend and the XCP kernel because xl and new pvops kernels don't even work with blktap. I think we should probably remove it. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] xl create: endless loop 2010-11-10 11:52 ` Stefano Stabellini 2010-11-16 13:29 ` Zhang, Yang Z @ 2010-12-02 13:43 ` Christoph Egger 2010-12-14 19:04 ` [PATCH] xl create: endless loop [and 1 more messages] Ian Jackson 1 sibling, 1 reply; 21+ messages in thread From: Christoph Egger @ 2010-12-02 13:43 UTC (permalink / raw) To: Stefano Stabellini; +Cc: Ian, xen-devel, Jackson On Wednesday 10 November 2010 12:52:42 Stefano Stabellini wrote: > On Wed, 10 Nov 2010, Christoph Egger wrote: > > On Tuesday 09 November 2010 19:08:14 Stefano Stabellini wrote: > > > On Wed, 3 Nov 2010, Christoph Egger wrote: > > > > 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 > > > > } > > > > > > The problem you are having is that somebody in your system is setting a > > > target for dom0 without setting freemem-slack. Are you still running > > > xend at boot? > > > > Yes, I do. > > running xend alongside xl is not recommended, it could cause bugs, > especially if you don't disable autoballooning > > > > > Currently libxl__fill_dom0_memory_info assumes that both values are set > > > initially at the same time (by libxl__fill_dom0_memory_info). > > > This patch should fix the issue, I would appreciate if you could test > > > it. > > > > > > --- > > > > > > > > > libxl: do not assume target and freemem-slack are written at the same > > > time > > > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > > > diff -r 7188d1e4b0e1 tools/libxl/libxl.c > > > --- a/tools/libxl/libxl.c Tue Nov 09 12:00:05 2010 +0000 > > > +++ b/tools/libxl/libxl.c Tue Nov 09 18:05:52 2010 +0000 > > > @@ -2779,18 +2779,25 @@ static int libxl__fill_dom0_memory_info( > > > int rc; > > > libxl_dominfo info; > > > libxl_physinfo physinfo; > > > - char *target = NULL, *endptr = NULL; > > > + char *target = NULL, *staticmax = NULL, *freememslack = NULL, > > > *endptr = NULL; char *target_path = "/local/domain/0/memory/target"; > > > char *max_path = "/local/domain/0/memory/static-max"; > > > char *free_mem_slack_path = > > > "/local/domain/0/memory/freemem-slack"; xs_transaction_t t; > > > libxl_ctx *ctx = libxl__gc_owner(gc); > > > - uint32_t free_mem_slack = 0; > > > + uint32_t free_mem_slack_kb = 0; > > > > > > retry_transaction: > > > t = xs_transaction_start(ctx->xsh); > > > > > > target = libxl__xs_read(gc, t, target_path); > > > + staticmax = libxl__xs_read(gc, t, target_path); > > > + freememslack = libxl__xs_read(gc, t, target_path); > > > + if (target && staticmax && freememslack) { > > > + rc = 0; > > > + goto out; > > > + } > > > > *target, *staticmax and *freememslack contain the value "5". > > So with this patch, rc always returns 0 from there. > > that is correct: the values are there, so there is no need to write them > to xenstore again, and the caller should be able to read the target > correctly > > Xen is booted with dom0_mem. > > Dom0 has autoballooning disabled in the kernel. > > You still need to disable autoballooning in xl, setting autoballooning > to 0 in /etc/xen/xl.conf Yes, that makes the patch work for me. I have success in booting the guest the first time with 'xl create'. Please commit the patch. Christoph -- ---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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] xl create: endless loop [and 1 more messages] 2010-12-02 13:43 ` Christoph Egger @ 2010-12-14 19:04 ` Ian Jackson 0 siblings, 0 replies; 21+ messages in thread From: Ian Jackson @ 2010-12-14 19:04 UTC (permalink / raw) To: Stefano Stabellini, Christoph Egger; +Cc: xen-devel Stefano Stabellini writes ("Re: [Xen-devel][PATCH] xl create: endless loop"): > libxl: do not assume target and freemem-slack are written at the same time I have applied this patch, thanks. Ian. ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2010-12-14 19:04 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-10-18 12:50 xl create: endless loop Christoph Egger 2010-10-19 8:31 ` [PATCH] " Christoph Egger 2010-10-21 14:11 ` Stefano Stabellini 2010-11-03 16:12 ` Christoph Egger 2010-11-04 15:40 ` Christoph Egger 2010-11-08 8:18 ` Zhang, Yang Z 2010-11-09 13:27 ` Christoph Egger 2010-11-09 18:08 ` Stefano Stabellini 2010-11-10 10:34 ` Christoph Egger 2010-11-10 11:52 ` Stefano Stabellini 2010-11-16 13:29 ` Zhang, Yang Z 2010-11-16 14:07 ` Stefano Stabellini 2010-11-16 14:29 ` Zhang, Yang Z 2010-11-16 14:40 ` Stefano Stabellini 2010-11-16 14:48 ` Zhang, Yang Z 2010-11-16 15:57 ` Stefano Stabellini 2010-11-17 2:50 ` Daniel Stodden 2010-11-18 10:43 ` Christoph Egger 2010-11-18 12:21 ` Stefano Stabellini 2010-12-02 13:43 ` Christoph Egger 2010-12-14 19:04 ` [PATCH] xl create: endless loop [and 1 more messages] Ian Jackson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.