@@ -1755,7 +1755,7 @@ static void domcreate_complete(libxl__egc *egc,
bool retain_domain = !rc || rc == ERROR_ABORTED;
if (retain_domain) {
- libxl__domain_userdata_lock *lock;
+ libxl__flock *lock;
/* Note that we hold CTX lock at this point so only need to
* take data store lock
@@ -1769,7 +1769,7 @@ static void domcreate_complete(libxl__egc *egc,
(gc, dcs->guest_domid, d_config_saved);
if (!rc)
rc = cfg_rc;
- libxl__unlock_domain_userdata(lock);
+ libxl__unlock_file(lock);
}
}
@@ -1850,7 +1850,7 @@ void libxl__device_add_async(libxl__egc *egc, uint32_t domid,
xs_transaction_t t = XBT_NULL;
libxl_domain_config d_config;
void *type_saved;
- libxl__domain_userdata_lock *lock = NULL;
+ libxl__flock *lock = NULL;
int rc;
libxl_domain_config_init(&d_config);
@@ -1946,7 +1946,7 @@ void libxl__device_add_async(libxl__egc *egc, uint32_t domid,
out:
libxl__xs_transaction_abort(gc, &t);
- if (lock) libxl__unlock_domain_userdata(lock);
+ if (lock) libxl__unlock_file(lock);
dt->dispose(type_saved);
libxl_domain_config_dispose(&d_config);
aodev->rc = rc;
@@ -245,7 +245,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
xs_transaction_t t = XBT_NULL;
libxl_domain_config d_config;
libxl_device_disk disk_saved;
- libxl__domain_userdata_lock *lock = NULL;
+ libxl__flock *lock = NULL;
libxl_domain_config_init(&d_config);
libxl_device_disk_init(&disk_saved);
@@ -436,7 +436,7 @@ static void device_disk_add(libxl__egc *egc, uint32_t domid,
out:
libxl__xs_transaction_abort(gc, &t);
- if (lock) libxl__unlock_domain_userdata(lock);
+ if (lock) libxl__unlock_file(lock);
libxl_device_disk_dispose(&disk_saved);
libxl_domain_config_dispose(&d_config);
aodev->rc = rc;
@@ -794,7 +794,7 @@ static void cdrom_insert_ejected(libxl__egc *egc,
{
EGC_GC;
libxl__cdrom_insert_state *cis = CONTAINER_OF(qmp, *cis, qmp);
- libxl__domain_userdata_lock *data_lock = NULL;
+ libxl__flock *data_lock = NULL;
libxl__device device;
const char *be_path, *libxl_path;
flexarray_t *empty = NULL;
@@ -896,7 +896,7 @@ static void cdrom_insert_ejected(libxl__egc *egc,
out:
libxl__xs_transaction_abort(gc, &t);
libxl_domain_config_dispose(&d_config);
- if (data_lock) libxl__unlock_domain_userdata(data_lock);
+ if (data_lock) libxl__unlock_file(data_lock);
if (rc) {
cdrom_insert_done(egc, cis, rc); /* must be last */
} else if (!has_callback) {
@@ -951,7 +951,7 @@ static void cdrom_insert_inserted(libxl__egc *egc,
{
EGC_GC;
libxl__cdrom_insert_state *cis = CONTAINER_OF(qmp, *cis, qmp);
- libxl__domain_userdata_lock *data_lock = NULL;
+ libxl__flock *data_lock = NULL;
libxl_domain_config d_config;
flexarray_t *insert = NULL;
xs_transaction_t t = XBT_NULL;
@@ -1029,7 +1029,7 @@ static void cdrom_insert_inserted(libxl__egc *egc,
out:
libxl__xs_transaction_abort(gc, &t);
libxl_domain_config_dispose(&d_config);
- if (data_lock) libxl__unlock_domain_userdata(data_lock);
+ if (data_lock) libxl__unlock_file(data_lock);
cdrom_insert_done(egc, cis, rc); /* must be last */
}
@@ -1520,7 +1520,7 @@ int libxl_userdata_store(libxl_ctx *ctx, uint32_t domid,
{
GC_INIT(ctx);
int rc;
- libxl__domain_userdata_lock *lock;
+ libxl__flock *lock;
CTX_LOCK;
lock = libxl__lock_domain_userdata(gc, domid);
@@ -1532,7 +1532,7 @@ int libxl_userdata_store(libxl_ctx *ctx, uint32_t domid,
rc = libxl__userdata_store(gc, domid, userdata_userid,
data, datalen);
- libxl__unlock_domain_userdata(lock);
+ libxl__unlock_file(lock);
out:
CTX_UNLOCK;
@@ -1581,7 +1581,7 @@ int libxl_userdata_retrieve(libxl_ctx *ctx, uint32_t domid,
{
GC_INIT(ctx);
int rc;
- libxl__domain_userdata_lock *lock;
+ libxl__flock *lock;
CTX_LOCK;
lock = libxl__lock_domain_userdata(gc, domid);
@@ -1594,7 +1594,7 @@ int libxl_userdata_retrieve(libxl_ctx *ctx, uint32_t domid,
data_r, datalen_r);
- libxl__unlock_domain_userdata(lock);
+ libxl__unlock_file(lock);
out:
CTX_UNLOCK;
GC_FREE;
@@ -1608,7 +1608,7 @@ int libxl_userdata_unlink(libxl_ctx *ctx, uint32_t domid,
CTX_LOCK;
int rc;
- libxl__domain_userdata_lock *lock = NULL;
+ libxl__flock *lock = NULL;
const char *filename;
lock = libxl__lock_domain_userdata(gc, domid);
@@ -1631,7 +1631,7 @@ int libxl_userdata_unlink(libxl_ctx *ctx, uint32_t domid,
rc = 0;
out:
if (lock)
- libxl__unlock_domain_userdata(lock);
+ libxl__unlock_file(lock);
CTX_UNLOCK;
GC_FREE;
return rc;
@@ -1278,7 +1278,7 @@ static void devices_destroy_cb(libxl__egc *egc,
uint32_t domid = dis->domid;
char *dom_path;
char *vm_path;
- libxl__domain_userdata_lock *lock;
+ libxl__flock *lock;
dom_path = libxl__xs_get_dompath(gc, domid);
if (!dom_path) {
@@ -1308,7 +1308,7 @@ static void devices_destroy_cb(libxl__egc *egc,
}
libxl__userdata_destroyall(gc, domid);
- libxl__unlock_domain_userdata(lock);
+ libxl__unlock_file(lock);
/* Clean up qemu-save and qemu-resume files. They are
* intermediate files created by libxc. Unfortunately they
@@ -1917,7 +1917,7 @@ static void retrieve_domain_configuration_lock_acquired(
retrieve_domain_configuration_state *rdcs =
CONTAINER_OF(devlock, *rdcs, devlock);
STATE_AO_GC(rdcs->qmp.ao);
- libxl__domain_userdata_lock *lock = NULL;
+ libxl__flock *lock = NULL;
bool has_callback = false;
/* Convenience aliases */
@@ -1939,7 +1939,7 @@ static void retrieve_domain_configuration_lock_acquired(
goto out;
}
- libxl__unlock_domain_userdata(lock);
+ libxl__unlock_file(lock);
lock = NULL;
/* We start by querying QEMU, if it is running, for its cpumap as this
@@ -1964,7 +1964,7 @@ static void retrieve_domain_configuration_lock_acquired(
}
out:
- if (lock) libxl__unlock_domain_userdata(lock);
+ if (lock) libxl__unlock_file(lock);
if (!has_callback)
retrieve_domain_configuration_end(egc, rdcs, rc);
}
@@ -1998,7 +1998,7 @@ static void retrieve_domain_configuration_end(libxl__egc *egc,
retrieve_domain_configuration_state *rdcs, int rc)
{
STATE_AO_GC(rdcs->qmp.ao);
- libxl__domain_userdata_lock *lock = NULL;
+ libxl__flock *lock = NULL;
/* Convenience aliases */
libxl_domain_config *const d_config = rdcs->d_config;
@@ -2205,7 +2205,7 @@ static void retrieve_domain_configuration_end(libxl__egc *egc,
out:
libxl__ev_slowlock_unlock(gc, &rdcs->devlock);
- if (lock) libxl__unlock_domain_userdata(lock);
+ if (lock) libxl__unlock_file(lock);
libxl_bitmap_dispose(&rdcs->qemuu_cpus);
libxl__ev_qmp_dispose(gc, &rdcs->qmp);
libxl__ev_time_deregister(gc, &rdcs->timeout);
@@ -400,26 +400,22 @@ int libxl__device_model_version_running(libxl__gc *gc, uint32_t domid)
/* Portability note: this lock utilises flock(2) so a proper implementation of
* flock(2) is required.
*/
-libxl__domain_userdata_lock *libxl__lock_domain_userdata(libxl__gc *gc,
- uint32_t domid)
+libxl__flock *libxl__lock_file(libxl__gc *gc, const char *lockfile)
{
- libxl__domain_userdata_lock *lock = NULL;
- const char *lockfile;
+ libxl__flock *lock;
int fd;
struct stat stab, fstab;
- lockfile = libxl__userdata_path(gc, domid, "domain-userdata-lock", "l");
- if (!lockfile) goto out;
-
- lock = libxl__zalloc(NOGC, sizeof(libxl__domain_userdata_lock));
+ lock = libxl__zalloc(NOGC, sizeof(libxl__flock));
lock->path = libxl__strdup(NOGC, lockfile);
while (true) {
libxl__carefd_begin();
fd = open(lockfile, O_RDWR|O_CREAT, 0666);
if (fd < 0)
- LOGED(ERROR, domid,
- "cannot open lockfile %s, errno=%d", lockfile, errno);
+ LOGE(ERROR,
+ "cannot open lockfile %s, errno=%d",
+ lockfile, errno);
lock->carefd = libxl__carefd_opened(CTX, fd);
if (fd < 0) goto out;
@@ -433,21 +429,21 @@ libxl__domain_userdata_lock *libxl__lock_domain_userdata(libxl__gc *gc,
continue;
default:
/* All other errno: EBADF, EINVAL, ENOLCK, EWOULDBLOCK */
- LOGED(ERROR, domid,
- "unexpected error while trying to lock %s, fd=%d, errno=%d",
+ LOGE(ERROR,
+ "unexpected error while trying to lock %s, fd=%d, errno=%d",
lockfile, fd, errno);
goto out;
}
}
if (fstat(fd, &fstab)) {
- LOGED(ERROR, domid, "cannot fstat %s, fd=%d, errno=%d",
+ LOGE(ERROR, "cannot fstat %s, fd=%d, errno=%d",
lockfile, fd, errno);
goto out;
}
if (stat(lockfile, &stab)) {
if (errno != ENOENT) {
- LOGED(ERROR, domid, "cannot stat %s, errno=%d", lockfile, errno);
+ LOGE(ERROR, "cannot stat %s, errno=%d", lockfile, errno);
goto out;
}
} else {
@@ -458,20 +454,14 @@ libxl__domain_userdata_lock *libxl__lock_domain_userdata(libxl__gc *gc,
libxl__carefd_close(lock->carefd);
}
- /* Check the domain is still there, if not we should release the
- * lock and clean up.
- */
- if (libxl_domain_info(CTX, NULL, domid))
- goto out;
-
return lock;
out:
- if (lock) libxl__unlock_domain_userdata(lock);
+ if (lock) libxl__unlock_file(lock);
return NULL;
}
-void libxl__unlock_domain_userdata(libxl__domain_userdata_lock *lock)
+void libxl__unlock_file(libxl__flock *lock)
{
/* It's important to unlink the file before closing fd to avoid
* the following race (if close before unlink):
@@ -493,6 +483,27 @@ void libxl__unlock_domain_userdata(libxl__domain_userdata_lock *lock)
free(lock);
}
+libxl__flock *libxl__lock_domain_userdata(libxl__gc *gc, uint32_t domid)
+{
+ const char *lockfile;
+ libxl__flock *lock;
+
+ lockfile = libxl__userdata_path(gc, domid, "domain-userdata-lock", "l");
+ if (!lockfile) return NULL;
+
+ lock = libxl__lock_file(gc, lockfile);
+
+ /* Check the domain is still there, if not we should release the
+ * lock and clean up.
+ */
+ if (libxl_domain_info(CTX, NULL, domid)) {
+ libxl__unlock_file(lock);
+ return NULL;
+ }
+
+ return lock;
+}
+
int libxl__get_domain_configuration(libxl__gc *gc, uint32_t domid,
libxl_domain_config *d_config)
{
@@ -4624,11 +4624,13 @@ int libxl__cpuid_policy_is_empty(libxl_cpuid_policy_list *pl);
typedef struct {
libxl__carefd *carefd;
char *path; /* path of the lock file itself */
-} libxl__domain_userdata_lock;
+} libxl__flock;
/* The CTX_LOCK must be held around uses of this lock */
-libxl__domain_userdata_lock *libxl__lock_domain_userdata(libxl__gc *gc,
- uint32_t domid);
-void libxl__unlock_domain_userdata(libxl__domain_userdata_lock *lock);
+
+libxl__flock *libxl__lock_file(libxl__gc *gc, const char *filename);
+void libxl__unlock_file(libxl__flock *lock);
+
+libxl__flock *libxl__lock_domain_userdata(libxl__gc *gc, uint32_t domid);
/*
* Retrieve / store domain configuration from / to libxl private
@@ -30,7 +30,7 @@ int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint64_t max_memkb)
uint64_t memorykb, size;
char *dompath = libxl__xs_get_dompath(gc, domid);
int rc = 1;
- libxl__domain_userdata_lock *lock = NULL;
+ libxl__flock *lock = NULL;
libxl_domain_config d_config;
libxl_domain_config_init(&d_config);
@@ -85,7 +85,7 @@ int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint64_t max_memkb)
rc = 0;
out:
libxl_domain_config_dispose(&d_config);
- if (lock) libxl__unlock_domain_userdata(lock);
+ if (lock) libxl__unlock_file(lock);
CTX_UNLOCK;
GC_FREE;
return rc;
@@ -184,7 +184,7 @@ int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid,
libxl_dominfo ptr;
char *uuid;
xs_transaction_t t;
- libxl__domain_userdata_lock *lock;
+ libxl__flock *lock;
libxl_domain_config d_config;
libxl_domain_config_init(&d_config);
@@ -338,7 +338,7 @@ out:
out_no_transaction:
libxl_domain_config_dispose(&d_config);
- if (lock) libxl__unlock_domain_userdata(lock);
+ if (lock) libxl__unlock_file(lock);
CTX_UNLOCK;
GC_FREE;
return rc;
@@ -125,7 +125,7 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc,
xs_transaction_t t = XBT_NULL;
int rc;
libxl_domain_config d_config;
- libxl__domain_userdata_lock *lock = NULL;
+ libxl__flock *lock = NULL;
bool is_stubdomain = libxl_is_stubdom(CTX, domid, NULL);
/* Stubdomain doesn't have own config. */
@@ -195,7 +195,7 @@ static int libxl__device_pci_add_xenstore(libxl__gc *gc,
out:
libxl__xs_transaction_abort(gc, &t);
- if (lock) libxl__unlock_domain_userdata(lock);
+ if (lock) libxl__unlock_file(lock);
if (!is_stubdomain)
libxl_domain_config_dispose(&d_config);
return rc;
@@ -202,7 +202,7 @@ static int libxl__device_usbctrl_add_xenstore(libxl__gc *gc, uint32_t domid,
int i, rc;
libxl_domain_config d_config;
libxl_device_usbctrl usbctrl_saved;
- libxl__domain_userdata_lock *lock = NULL;
+ libxl__flock *lock = NULL;
libxl_domain_config_init(&d_config);
libxl_device_usbctrl_init(&usbctrl_saved);
@@ -291,7 +291,7 @@ static int libxl__device_usbctrl_add_xenstore(libxl__gc *gc, uint32_t domid,
out:
libxl__xs_transaction_abort(gc, &t);
- if (lock) libxl__unlock_domain_userdata(lock);
+ if (lock) libxl__unlock_file(lock);
libxl_device_usbctrl_dispose(&usbctrl_saved);
libxl_domain_config_dispose(&d_config);
return rc;
@@ -1266,7 +1266,7 @@ static int libxl__device_usbdev_add_xenstore(libxl__gc *gc, uint32_t domid,
xs_transaction_t t = XBT_NULL;
libxl_domain_config d_config;
libxl_device_usbdev usbdev_saved;
- libxl__domain_userdata_lock *lock = NULL;
+ libxl__flock *lock = NULL;
libxl_domain_config_init(&d_config);
libxl_device_usbdev_init(&usbdev_saved);
@@ -1323,7 +1323,7 @@ static int libxl__device_usbdev_add_xenstore(libxl__gc *gc, uint32_t domid,
rc = 0;
out:
- if (lock) libxl__unlock_domain_userdata(lock);
+ if (lock) libxl__unlock_file(lock);
libxl_device_usbdev_dispose(&usbdev_saved);
libxl_domain_config_dispose(&d_config);
return rc;
This function implements a file-based lock with a file name generated from a domid. This patch splits it into two, generalising the core of the locking code into a new libxl__lock_file() function which operates on a specified file, leaving just the file name generation in libxl__domain_userdata_lock(). This patch also generalises libxl__unlock_domain_userdata() to libxl__unlock_file() and modifies all call-sites. Suggested-by: Ian Jackson <ian.jackson@eu.citrix.com> Signed-off-by: Paul Durrant <pdurrant@amazon.com> --- Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Wei Liu <wl@xen.org> Cc: Anthony PERARD <anthony.perard@citrix.com> v4: - New in v4. --- tools/libxl/libxl_create.c | 4 +-- tools/libxl/libxl_device.c | 4 +-- tools/libxl/libxl_disk.c | 12 ++++---- tools/libxl/libxl_dom.c | 12 ++++---- tools/libxl/libxl_domain.c | 14 ++++----- tools/libxl/libxl_internal.c | 55 +++++++++++++++++++++--------------- tools/libxl/libxl_internal.h | 10 ++++--- tools/libxl/libxl_mem.c | 8 +++--- tools/libxl/libxl_pci.c | 4 +-- tools/libxl/libxl_usb.c | 8 +++--- 10 files changed, 72 insertions(+), 59 deletions(-)