|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v1 02/10] libxl_internal: functions to lock / unlock domain configuration
On Thu, 2014-07-10 at 15:32 +0100, Wei Liu wrote:
> Simple file lock taken from xl to serialise access to "libxl-json" file.
Do you mean libxl here?
> If a thread cannot get hold of the lock it waits due to F_SETLKW.
How is deadlock avoided here?
I think this sort of locking is safe against processes crashing with the
lock held (it gets released), so assuming I'm right its ok from that
PoV.
> In order to generate lock file name, rename userdata_path to
> libxl__userdata_path and declare it in libxl_internal.h
Given that you don't appear to be locking the userdata file itself that
seems like an odd place for a lock to me, why not XEN_LOCK_DIR?
Did you consider locking the actual file though? That would avoid this
lock being a bottleneck...
I'm going to leave all the fctrl wrangling to Ian to look at, much more
his area of expertise.
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
> tools/libxl/libxl_dom.c | 14 ++++----
> tools/libxl/libxl_internal.c | 76
> ++++++++++++++++++++++++++++++++++++++++++
> tools/libxl/libxl_internal.h | 11 ++++++
> 3 files changed, 94 insertions(+), 7 deletions(-)
>
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 83eb29a..a41e8fd 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -1762,9 +1762,9 @@ char *libxl__uuid2string(libxl__gc *gc, const
> libxl_uuid uuid)
> return GCSPRINTF(LIBXL_UUID_FMT, LIBXL_UUID_BYTES(uuid));
> }
>
> -static const char *userdata_path(libxl__gc *gc, uint32_t domid,
> - const char *userdata_userid,
> - const char *wh)
> +const char *libxl__userdata_path(libxl__gc *gc, uint32_t domid,
> + const char *userdata_userid,
> + const char *wh)
> {
> libxl_ctx *ctx = libxl__gc_owner(gc);
> char *uuid_string;
> @@ -1799,7 +1799,7 @@ void libxl__userdata_destroyall(libxl__gc *gc, uint32_t
> domid)
> glob_t gl;
> int r, i;
>
> - pattern = userdata_path(gc, domid, "*", "?");
> + pattern = libxl__userdata_path(gc, domid, "*", "?");
> if (!pattern)
> goto out;
>
> @@ -1830,7 +1830,7 @@ int libxl_userdata_store(libxl_ctx *ctx, uint32_t domid,
> int e, rc;
> int fd = -1;
>
> - filename = userdata_path(gc, domid, userdata_userid, "d");
> + filename = libxl__userdata_path(gc, domid, userdata_userid, "d");
> if (!filename) {
> rc = ERROR_NOMEM;
> goto out;
> @@ -1841,7 +1841,7 @@ int libxl_userdata_store(libxl_ctx *ctx, uint32_t domid,
> goto out;
> }
>
> - newfilename = userdata_path(gc, domid, userdata_userid, "n");
> + newfilename = libxl__userdata_path(gc, domid, userdata_userid, "n");
> if (!newfilename) {
> rc = ERROR_NOMEM;
> goto out;
> @@ -1892,7 +1892,7 @@ int libxl_userdata_retrieve(libxl_ctx *ctx, uint32_t
> domid,
> int datalen = 0;
> void *data = 0;
>
> - filename = userdata_path(gc, domid, userdata_userid, "d");
> + filename = libxl__userdata_path(gc, domid, userdata_userid, "d");
> if (!filename) {
> rc = ERROR_NOMEM;
> goto out;
> diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> index dc47177..9cc60e8 100644
> --- a/tools/libxl/libxl_internal.c
> +++ b/tools/libxl/libxl_internal.c
> @@ -436,6 +436,82 @@ out:
> return rc;
> }
>
> +int libxl__lock_domain_configuration(libxl__gc *gc, uint32_t domid,
> + int *fd_lock)
> +{
> + int rc;
> + struct flock fl;
> + const char *lockfile;
> +
> + if (*fd_lock >= 0)
> + return ERROR_INVAL;
> +
> + lockfile = libxl__userdata_path(gc, domid, "libxl-json.lock", "d");
> + if (!lockfile)
> + return ERROR_FAIL;
> +
> + *fd_lock = open(lockfile, O_WRONLY|O_CREAT, S_IWUSR);
> + if (*fd_lock < 0) {
> + LOGE(ERROR, "cannot open lockfile %s errno=%d\n", lockfile, errno);
> + return ERROR_FAIL;
> + }
> +
> + if (fcntl(*fd_lock, F_SETFD, FD_CLOEXEC) < 0) {
> + close(*fd_lock);
> + LOGE(ERROR, "cannot set cloexec to lockfile %s errno=%d\n",
> + lockfile, errno);
> + return ERROR_FAIL;
> + }
> +
> +get_lock:
> + fl.l_type = F_WRLCK;
> + 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 get_lock;
> +
> + if (rc < 0) {
> + LOGE(ERROR, "cannot acquire lock %s errno=%d\n", lockfile, errno);
> + rc = ERROR_FAIL;
> + } else
> + rc = 0;
> +
> + return rc;
> +}
> +
> +int libxl__unlock_domain_configuration(libxl__gc *gc, uint32_t domid,
> + int *fd_lock)
> +{
> + int rc;
> + struct flock fl;
> +
> + 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) {
> + const char *lockfile;
> + lockfile = libxl__userdata_path(gc, domid, "libxl-json.lock", "d");
> + LOGE(ERROR, "cannot release lock %s, errno=%d\n", lockfile, errno);
> + rc = ERROR_FAIL;
> + } else
> + rc = 0;
> +
> + close(*fd_lock);
> + *fd_lock = -1;
> +
> + return rc;
> +}
>
> /*
> * Local variables:
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index ef2111b..e9d93ac 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -991,6 +991,9 @@ _hidden int libxl__toolstack_restore(uint32_t domid,
> const uint8_t *buf,
> uint32_t size, void *data);
> _hidden int libxl__domain_resume_device_model(libxl__gc *gc, uint32_t domid);
>
> +_hidden const char *libxl__userdata_path(libxl__gc *gc, uint32_t domid,
> + const char *userdata_userid,
> + const char *wh);
> _hidden void libxl__userdata_destroyall(libxl__gc *gc, uint32_t domid);
>
> _hidden int libxl__domain_resume(libxl__gc *gc, uint32_t domid,
> @@ -3228,6 +3231,14 @@ int libxl__get_domain_configuration(libxl__gc *gc,
> uint32_t domid,
> libxl_domain_config *d_config);
> int libxl__set_domain_configuration(libxl__gc *gc, uint32_t domid,
> libxl_domain_config *d_config);
> +/*
> + * Lock / unlock domain configuration in libxl private data store.
> + * fd_lock contains the file descriptor pointing to the lock file.
> + */
> +int libxl__lock_domain_configuration(libxl__gc *gc, uint32_t domid,
> + int *fd_lock);
> +int libxl__unlock_domain_configuration(libxl__gc *gc, uint32_t domid,
> + int *fd_lock);
>
> #endif
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |