[v5,3/7] libxl: generalise libxl__domain_userdata_lock()

Message ID	20200131150149.2008-4-pdurrant@amazon.com (mailing list archive)
State	Superseded
Headers	show Return-Path: <SRS0=nADe=3U=lists.xenproject.org=xen-devel-bounces@kernel.org> DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1559F20705 IronPort-SDR: kDhRdpRa5NsJp0UnFeaat6oaEqAI3vTjAZoaom7bEIZyAkaiq7Ay6TGsjXVDKQN8OMMNua/2vj a/biF73OSNZQ== From: Paul Durrant <pdurrant@amazon.com> To: <xen-devel@lists.xenproject.org> Date: Fri, 31 Jan 2020 15:01:45 +0000 Message-ID: <20200131150149.2008-4-pdurrant@amazon.com> In-Reply-To: <20200131150149.2008-1-pdurrant@amazon.com> References: <20200131150149.2008-1-pdurrant@amazon.com> MIME-Version: 1.0 Precedence: Bulk Subject: [Xen-devel] [PATCH v5 3/7] libxl: generalise libxl__domain_userdata_lock() Cc: Anthony PERARD <anthony.perard@citrix.com>, Paul Durrant <pdurrant@amazon.com>, Ian Jackson <ian.jackson@eu.citrix.com>, Wei Liu <wl@xen.org> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" <xen-devel-bounces@lists.xenproject.org>
Series	xl/libxl: domid allocation/preservation changes \| expand [v5,0/7] xl/libxl: domid allocation/preservation changes [v5,1/7] libxl: add definition of INVALID_DOMID to the API [v5,2/7] libxl_create: make 'soft reset' explicit [v5,3/7] libxl: generalise libxl__domain_userdata_lock() [v5,4/7] libxl: add infrastructure to track and query 'recent' domids [v5,5/7] libxl: allow creation of domains with a specified or random domid [v5,6/7] xl.conf: introduce 'domid_policy' [v5,7/7] xl: allow domid to be preserved on save/restore or migrate

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 1835a5502c..3a7364e2ac 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -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); } } diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c index 9d05d2fd13..0381c5d509 100644 --- a/tools/libxl/libxl_device.c +++ b/tools/libxl/libxl_device.c @@ -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; diff --git a/tools/libxl/libxl_disk.c b/tools/libxl/libxl_disk.c index a463334130..ddc1eec176 100644 --- a/tools/libxl/libxl_disk.c +++ b/tools/libxl/libxl_disk.c @@ -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 */ } diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index 573c63692b..d9ada8a422 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -1347,7 +1347,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); @@ -1359,7 +1359,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; @@ -1408,7 +1408,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); @@ -1421,7 +1421,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; @@ -1435,7 +1435,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); @@ -1458,7 +1458,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; diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c index b59cc65750..973fc1434d 100644 --- a/tools/libxl/libxl_domain.c +++ b/tools/libxl/libxl_domain.c @@ -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); diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c index ba5637358e..211236dc99 100644 --- a/tools/libxl/libxl_internal.c +++ b/tools/libxl/libxl_internal.c @@ -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) { diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index f2efdedfba..dd3c08bc14 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -4651,11 +4651,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 diff --git a/tools/libxl/libxl_mem.c b/tools/libxl/libxl_mem.c index 7c01fac7e5..bc7b95aa74 100644 --- a/tools/libxl/libxl_mem.c +++ b/tools/libxl/libxl_mem.c @@ -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; diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c index 664d74c478..f91bce07ec 100644 --- a/tools/libxl/libxl_pci.c +++ b/tools/libxl/libxl_pci.c @@ -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; diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c index da5e3708e6..171bb04439 100644 --- a/tools/libxl/libxl_usb.c +++ b/tools/libxl/libxl_usb.c @@ -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;

[v5,3/7] libxl: generalise libxl__domain_userdata_lock()

Commit Message

Patch