Stefano Stabellini
2010-Aug-27 11:19 UTC
[Xen-devel] [PATCH 7 of 8] xl: free memory before building a domain
xl: free memory before building a domain Free the needed amount of memory before proceeding with the domain build. Use a filelock to prevent other xl instances from conflicting during this operation. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> diff -r b80a1688b5c9 tools/examples/xl.conf --- a/tools/examples/xl.conf Thu Aug 26 18:03:20 2010 +0100 +++ b/tools/examples/xl.conf Thu Aug 26 18:26:04 2010 +0100 @@ -3,3 +3,6 @@ # automatically balloon down dom0 when xen doesn''t have enough free # memory to create a domain autoballon=1 + +# full path of the lockfile used by xl during domain creation +lockfile="/var/lock/xl" diff -r b80a1688b5c9 tools/libxl/xl.c --- a/tools/libxl/xl.c Thu Aug 26 18:03:20 2010 +0100 +++ b/tools/libxl/xl.c Thu Aug 26 18:26:04 2010 +0100 @@ -34,6 +34,7 @@ xentoollog_logger_stdiostream *logger; int autoballoon = 1; +char *lockfile = "/var/lock/xl"; static xentoollog_level minmsglevel = XTL_PROGRESS; @@ -44,6 +45,7 @@ static void parse_global_config(const ch long l; XLU_Config *config; int e; + const char *buf; config = xlu_cfg_init(stderr, configfile); if (!config) { @@ -60,6 +62,9 @@ static void parse_global_config(const ch if (!xlu_cfg_get_long (config, "autoballoon", &l)) autoballoon = l; + if (!xlu_cfg_get_string (config, "lockfile", &buf)) + lockfile = strdup(buf); + xlu_cfg_destroy(config); } diff -r b80a1688b5c9 tools/libxl/xl.h --- a/tools/libxl/xl.h Thu Aug 26 18:03:20 2010 +0100 +++ b/tools/libxl/xl.h Thu Aug 26 18:26:04 2010 +0100 @@ -92,5 +92,6 @@ extern xentoollog_logger_stdiostream *lo /* global options */ extern int autoballoon; +extern char *lockfile; #endif /* XL_H */ diff -r b80a1688b5c9 tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Thu Aug 26 18:03:20 2010 +0100 +++ b/tools/libxl/xl_cmdimpl.c Thu Aug 26 18:26:04 2010 +0100 @@ -68,6 +68,7 @@ libxl_ctx ctx; /* when we operate on a domain, it is this one: */ static uint32_t domid; static const char *common_domname; +static int fd_lock = -1; static const char savefileheader_magic[32]@@ -234,6 +235,65 @@ static void find_domain(const char *p) common_domname = was_name ? p : libxl_domid_to_name(&ctx, domid); } +static int acquire_lock(void) +{ + int rc; + struct flock fl; + + /* lock already acquired */ + if (fd_lock >= 0) + return ERROR_INVAL; + + fl.l_type = F_WRLCK; + fl.l_whence = SEEK_SET; + fl.l_start = 0; + fl.l_len = 0; + fd_lock = open(lockfile, O_WRONLY|O_CREAT, S_IWUSR); + if (fd_lock < 0) { + fprintf(stderr, "cannot open the lockfile %s errno=%d\n", lockfile, errno); + return ERROR_FAIL; + } +get_lock: + rc = fcntl(fd_lock, F_SETLKW, &fl); + if (rc < 0 && errno == EINTR) + goto get_lock; + if (rc < 0) { + fprintf(stderr, "cannot acquire lock %s errno=%d\n", lockfile, errno); + rc = ERROR_FAIL; + } else + rc = 0; + return rc; +} + +static int release_lock(void) +{ + int rc; + struct flock fl; + + /* lock not acquired */ + if (fd_lock < 0) + return ERROR_INVAL; + +release_lock: + fl.l_type = F_UNLCK; + fl.l_whence = SEEK_SET; + fl.l_start = 0; + fl.l_len = 0; + + rc = fcntl(fd_lock, F_SETLKW, &fl); + if (rc < 0 && errno == EINTR) + goto release_lock; + if (rc < 0) { + fprintf(stderr, "cannot release lock %s, errno=%d\n", lockfile, errno); + rc = ERROR_FAIL; + } else + rc = 0; + close(fd_lock); + fd_lock = -1; + + return rc; +} + #define LOG(_f, _a...) dolog(__FILE__, __LINE__, __func__, _f "\n", ##_a) static void dolog(const char *file, int line, const char *func, char *fmt, ...) @@ -1209,6 +1269,48 @@ struct domain_create { char **migration_domname_r; }; +static int freemem(libxl_domain_build_info *b_info, libxl_device_model_info *dm_info) +{ + int rc, retries = 3; + uint32_t need_memkb, free_memkb; + + if (!autoballoon) + return 0; + + rc = libxl_domain_need_memory(&ctx, b_info, dm_info, &need_memkb); + if (rc < 0) + return rc; + + do { + rc = libxl_get_free_memory(&ctx, &free_memkb); + if (rc < 0) + return rc; + + if (free_memkb >= need_memkb) + return 0; + + rc = libxl_set_relative_memory_target(&ctx, 0, free_memkb - need_memkb, 0); + if (rc < 0) + return rc; + + rc = libxl_wait_for_free_memory(&ctx, domid, need_memkb, 10); + if (!rc) + return 0; + else if (rc != ERROR_NOMEM) + return rc; + + /* the memory target has been reached but the free memory is still + * not enough: loop over again */ + rc = libxl_wait_for_memory_target(&ctx, 0, 1); + if (rc < 0) + return rc; + + retries--; + } while (retries > 0); + + return ERROR_NOMEM; +} + static int create_domain(struct domain_create *dom_info) { struct domain_config d_config; @@ -1367,6 +1469,17 @@ static int create_domain(struct domain_c start: domid = 0; + rc = acquire_lock(); + if (rc < 0) + goto error_out; + + ret = freemem(&d_config.b_info, &dm_info); + if (ret < 0) { + fprintf(stderr, "failed to free memory for the domain\n"); + ret = ERROR_FAIL; + goto error_out; + } + ret = libxl_domain_make(&ctx, &d_config.c_info, &domid); if (ret) { fprintf(stderr, "cannot make domain: %d\n", ret); @@ -1475,6 +1588,8 @@ start: goto error_out; } + release_lock(); + if (!paused) libxl_domain_unpause(&ctx, domid); @@ -1612,6 +1727,7 @@ start: } error_out: + release_lock(); if (domid) libxl_domain_destroy(&ctx, domid, 0); _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Christoph Egger
2010-Aug-27 11:37 UTC
Re: [Xen-devel] [PATCH 7 of 8] xl: free memory before building a domain
On Friday 27 August 2010 13:19:19 Stefano Stabellini wrote:> xl: free memory before building a domain > > Free the needed amount of memory before proceeding with the domain > build. > Use a filelock to prevent other xl instances from conflicting during > this operation. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > diff -r b80a1688b5c9 tools/examples/xl.conf > --- a/tools/examples/xl.conf Thu Aug 26 18:03:20 2010 +0100 > +++ b/tools/examples/xl.conf Thu Aug 26 18:26:04 2010 +0100 > @@ -3,3 +3,6 @@ > # automatically balloon down dom0 when xen doesn''t have enough free > # memory to create a domain > autoballon=1 > + > +# full path of the lockfile used by xl during domain creation > +lockfile="/var/lock/xl" > diff -r b80a1688b5c9 tools/libxl/xl.c > --- a/tools/libxl/xl.c Thu Aug 26 18:03:20 2010 +0100 > +++ b/tools/libxl/xl.c Thu Aug 26 18:26:04 2010 +0100 > @@ -34,6 +34,7 @@ > > xentoollog_logger_stdiostream *logger; > int autoballoon = 1; > +char *lockfile = "/var/lock/xl";NetBSD doesn''t have /var/lock. Is it ok to use /var/lib ? Christoph> > static xentoollog_level minmsglevel = XTL_PROGRESS; > > @@ -44,6 +45,7 @@ static void parse_global_config(const ch > long l; > XLU_Config *config; > int e; > + const char *buf; > > config = xlu_cfg_init(stderr, configfile); > if (!config) { > @@ -60,6 +62,9 @@ static void parse_global_config(const ch > if (!xlu_cfg_get_long (config, "autoballoon", &l)) > autoballoon = l; > > + if (!xlu_cfg_get_string (config, "lockfile", &buf)) > + lockfile = strdup(buf); > + > xlu_cfg_destroy(config); > } > > diff -r b80a1688b5c9 tools/libxl/xl.h > --- a/tools/libxl/xl.h Thu Aug 26 18:03:20 2010 +0100 > +++ b/tools/libxl/xl.h Thu Aug 26 18:26:04 2010 +0100 > @@ -92,5 +92,6 @@ extern xentoollog_logger_stdiostream *lo > > /* global options */ > extern int autoballoon; > +extern char *lockfile; > > #endif /* XL_H */ > diff -r b80a1688b5c9 tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Thu Aug 26 18:03:20 2010 +0100 > +++ b/tools/libxl/xl_cmdimpl.c Thu Aug 26 18:26:04 2010 +0100 > @@ -68,6 +68,7 @@ libxl_ctx ctx; > /* when we operate on a domain, it is this one: */ > static uint32_t domid; > static const char *common_domname; > +static int fd_lock = -1; > > > static const char savefileheader_magic[32]> @@ -234,6 +235,65 @@ static void find_domain(const char *p) > common_domname = was_name ? p : libxl_domid_to_name(&ctx, domid); > } > > +static int acquire_lock(void) > +{ > + int rc; > + struct flock fl; > + > + /* lock already acquired */ > + if (fd_lock >= 0) > + return ERROR_INVAL; > + > + fl.l_type = F_WRLCK; > + fl.l_whence = SEEK_SET; > + fl.l_start = 0; > + fl.l_len = 0; > + fd_lock = open(lockfile, O_WRONLY|O_CREAT, S_IWUSR); > + if (fd_lock < 0) { > + fprintf(stderr, "cannot open the lockfile %s errno=%d\n", > lockfile, errno); + return ERROR_FAIL; > + } > +get_lock: > + rc = fcntl(fd_lock, F_SETLKW, &fl); > + if (rc < 0 && errno == EINTR) > + goto get_lock; > + if (rc < 0) { > + fprintf(stderr, "cannot acquire lock %s errno=%d\n", lockfile, > errno); + rc = ERROR_FAIL; > + } else > + rc = 0; > + return rc; > +} > + > +static int release_lock(void) > +{ > + int rc; > + struct flock fl; > + > + /* lock not acquired */ > + if (fd_lock < 0) > + return ERROR_INVAL; > + > +release_lock: > + fl.l_type = F_UNLCK; > + fl.l_whence = SEEK_SET; > + fl.l_start = 0; > + fl.l_len = 0; > + > + rc = fcntl(fd_lock, F_SETLKW, &fl); > + if (rc < 0 && errno == EINTR) > + goto release_lock; > + if (rc < 0) { > + fprintf(stderr, "cannot release lock %s, errno=%d\n", lockfile, > errno); + rc = ERROR_FAIL; > + } else > + rc = 0; > + close(fd_lock); > + fd_lock = -1; > + > + return rc; > +} > + > #define LOG(_f, _a...) dolog(__FILE__, __LINE__, __func__, _f "\n", > ##_a) > > static void dolog(const char *file, int line, const char *func, char *fmt, > ...) @@ -1209,6 +1269,48 @@ struct domain_create { > char **migration_domname_r; > }; > > +static int freemem(libxl_domain_build_info *b_info, > libxl_device_model_info *dm_info) +{ > + int rc, retries = 3; > + uint32_t need_memkb, free_memkb; > + > + if (!autoballoon) > + return 0; > + > + rc = libxl_domain_need_memory(&ctx, b_info, dm_info, &need_memkb); > + if (rc < 0) > + return rc; > + > + do { > + rc = libxl_get_free_memory(&ctx, &free_memkb); > + if (rc < 0) > + return rc; > + > + if (free_memkb >= need_memkb) > + return 0; > + > + rc = libxl_set_relative_memory_target(&ctx, 0, free_memkb - > need_memkb, 0); + if (rc < 0) > + return rc; > + > + rc = libxl_wait_for_free_memory(&ctx, domid, need_memkb, 10); > + if (!rc) > + return 0; > + else if (rc != ERROR_NOMEM) > + return rc; > + > + /* the memory target has been reached but the free memory is still > + * not enough: loop over again */ > + rc = libxl_wait_for_memory_target(&ctx, 0, 1); > + if (rc < 0) > + return rc; > + > + retries--; > + } while (retries > 0); > + > + return ERROR_NOMEM; > +} > + > static int create_domain(struct domain_create *dom_info) > { > struct domain_config d_config; > @@ -1367,6 +1469,17 @@ static int create_domain(struct domain_c > start: > domid = 0; > > + rc = acquire_lock(); > + if (rc < 0) > + goto error_out; > + > + ret = freemem(&d_config.b_info, &dm_info); > + if (ret < 0) { > + fprintf(stderr, "failed to free memory for the domain\n"); > + ret = ERROR_FAIL; > + goto error_out; > + } > + > ret = libxl_domain_make(&ctx, &d_config.c_info, &domid); > if (ret) { > fprintf(stderr, "cannot make domain: %d\n", ret); > @@ -1475,6 +1588,8 @@ start: > goto error_out; > } > > + release_lock(); > + > if (!paused) > libxl_domain_unpause(&ctx, domid); > > @@ -1612,6 +1727,7 @@ start: > } > > error_out: > + release_lock(); > if (domid) > libxl_domain_destroy(&ctx, domid, 0); > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel-- ---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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Aug-27 11:41 UTC
Re: [Xen-devel] [PATCH 7 of 8] xl: free memory before building a domain
On Fri, 27 Aug 2010, Christoph Egger wrote:> On Friday 27 August 2010 13:19:19 Stefano Stabellini wrote: > > xl: free memory before building a domain > > > > Free the needed amount of memory before proceeding with the domain > > build. > > Use a filelock to prevent other xl instances from conflicting during > > this operation. > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > > > diff -r b80a1688b5c9 tools/examples/xl.conf > > --- a/tools/examples/xl.conf Thu Aug 26 18:03:20 2010 +0100 > > +++ b/tools/examples/xl.conf Thu Aug 26 18:26:04 2010 +0100 > > @@ -3,3 +3,6 @@ > > # automatically balloon down dom0 when xen doesn''t have enough free > > # memory to create a domain > > autoballon=1 > > + > > +# full path of the lockfile used by xl during domain creation > > +lockfile="/var/lock/xl" > > diff -r b80a1688b5c9 tools/libxl/xl.c > > --- a/tools/libxl/xl.c Thu Aug 26 18:03:20 2010 +0100 > > +++ b/tools/libxl/xl.c Thu Aug 26 18:26:04 2010 +0100 > > @@ -34,6 +34,7 @@ > > > > xentoollog_logger_stdiostream *logger; > > int autoballoon = 1; > > +char *lockfile = "/var/lock/xl"; > > NetBSD doesn''t have /var/lock. Is it ok to use /var/lib ? >you can set it in the global xl config file to /var/lib or anything you want. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2010-Aug-31 17:37 UTC
Re: [Xen-devel] [PATCH 7 of 8] xl: free memory before building a domain
Stefano Stabellini writes ("[Xen-devel] [PATCH 7 of 8] xl: free memory before building a domain"): ...> +# full path of the lockfile used by xl during domain creation > +lockfile="/var/lock/xl"I think the default config file should contain commented-out copies of the compiled-in defaults, not uncommented versions.> +char *lockfile = "/var/lock/xl";And the value should be from libx_paths.h et al surely ? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2010-Aug-31 17:38 UTC
Re: [Xen-devel] [PATCH 7 of 8] xl: free memory before building a domain
Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 7 of 8] xl: free memory before building a domain"):> you can set it in the global xl config file to /var/lib or anything you > want.The compiled-in default should be sensible for the build platform. We shouldn''t be having people fiddle with config files to make the software work. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Sep-01 11:46 UTC
Re: [Xen-devel] [PATCH 7 of 8] xl: free memory before building a domain
On Tue, 31 Aug 2010, Ian Jackson wrote:> Stefano Stabellini writes ("Re: [Xen-devel] [PATCH 7 of 8] xl: free memory before building a domain"): > > you can set it in the global xl config file to /var/lib or anything you > > want. > > The compiled-in default should be sensible for the build platform. We > shouldn''t be having people fiddle with config files to make the > software work.Yes, I had already repented and sent an update with this fix (it contains one more patch called "introduce XEN_LOCK_DIR"). _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel