[PATCH 3/7] nbd-server: Add locking around transaction log writes
Bugfix for the previous patch:
nbd-server uses multiple processes and within each process multiple threads.
Thus: Locking is needed, to ensure that the data in the transaction log
is not corrupted.
Proposal: use sem_open():
sem_open() is probably the simplest solution.
Alternatives:
- shm_open() + a shared pthread_mutex
- fcntl() for cross process locking, and a ptrace_mutex for intra-process
locking.
Signed-off-by: Manfred Spraul <manfred.spraul@de.bosch.com>
---
nbd-server.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++---
nbdsrv.h | 3 +++
2 files changed, 57 insertions(+), 3 deletions(-)
diff --git a/nbd-server.c b/nbd-server.c
index 8c1fba0..5cd86b0 100644
--- a/nbd-server.c
+++ b/nbd-server.c
@@ -120,8 +120,6 @@
#include <pthread.h>
#endif
-#include <semaphore.h>
-
/* used in cliserv.h, so must come first */
#define MY_NAME "nbd_server"
#include "cliserv.h"
@@ -399,6 +397,48 @@ static inline void socket_closed_negotiate(CLIENT* client) {
err("Negotiation failed: %m");
}
+static void alloc_logsem(CLIENT *client) {
+ struct stat fdinfo;
+ int ret;
+
+ client->logsem = SEM_FAILED;
+ /* no transaction log -> no semaphore */
+ if (client->transactionlogfd == -1)
+ return;
+
+ ret = fstat(client->transactionlogfd, &fdinfo);
+ if (ret == -1) {
+ close(client->transactionlogfd);
+ client->transactionlogfd = -1;
+ return;
+ }
+ snprintf(client->semname, sizeof(client->semname), "/nbd-server-%llx-%llx",
+ (unsigned long long)fdinfo.st_dev,
+ (unsigned long long)fdinfo.st_ino);
+ client->logsem = sem_open(client->semname, O_CREAT, 0600, 1);
+ if (client->logsem == SEM_FAILED) {
+ close(client->transactionlogfd);
+ client->transactionlogfd = -1;
+ }
+}
+
+static void free_logsem(CLIENT *client) {
+ if (client->transactionlogfd == -1)
+ return;
+ if (client->logsem == SEM_FAILED)
+ return;
+ sem_close(client->logsem);
+ client->logsem = SEM_FAILED;
+ sem_unlink(client->semname);
+}
+
+static void lock_logsem(CLIENT *client) {
+ sem_wait(client->logsem);
+}
+static void unlock_logsem(CLIENT *client) {
+ sem_post(client->logsem);
+}
+
/**
* Run a command. This is used for the ``prerun'' and ``postrun'' config file
* options
@@ -422,6 +462,7 @@ static inline void finalize_client(CLIENT* client) {
g_thread_pool_free(tpool, FALSE, TRUE);
do_run(client->server->postrun, client->exportname);
if(client->transactionlogfd != -1) {
+ free_logsem(client);
close(client->transactionlogfd);
client->transactionlogfd = -1;
}
@@ -2061,6 +2102,8 @@ static bool commit_client(CLIENT* client, SERVER* server) {
-1) {
msg(LOG_INFO, "Could not open transactionlog %s, moving on without it",
client->server->transactionlog);
+ } else {
+ alloc_logsem(client);
}
}
@@ -2767,6 +2810,7 @@ end:
static int mainloop_threaded(CLIENT* client) {
struct nbd_request* req;
struct work_package* pkg;
+ int write_data = false;
DEBUG("Entering request loop\n");
while(1) {
@@ -2774,12 +2818,16 @@ static int mainloop_threaded(CLIENT* client) {
socket_read(client, req, sizeof(struct nbd_request));
if(client->transactionlogfd != -1) {
+ lock_logsem(client);
if(((ntohl(req->type) & NBD_CMD_MASK_COMMAND) == NBD_CMD_WRITE) &&
(client->server->flags & F_DATALOG) &&
!(client->server->flags & F_SPLICE)) {
req->type = htonl(ntohl(req->type)|NBD_CMD_FLAG_DATALOG);
+ write_data = true;
}
writeit(client->transactionlogfd, req, sizeof(struct nbd_request));
+ if (!write_data)
+ unlock_logsem(client);
}
req->from = ntohll(req->from);
@@ -2802,8 +2850,11 @@ static int mainloop_threaded(CLIENT* client) {
#endif
socket_read(client, pkg->data, req->len);
- if (req->type & NBD_CMD_FLAG_DATALOG)
+ if (write_data) {
writeit(client->transactionlogfd, pkg->data, req->len);
+ unlock_logsem(client);
+ write_data = false;
+ }
}
if(req->type == NBD_CMD_DISC) {
finalize_client(client);
diff --git a/nbdsrv.h b/nbdsrv.h
index b84e26a..85c6073 100644
--- a/nbdsrv.h
+++ b/nbdsrv.h
@@ -9,6 +9,7 @@
#include <sys/socket.h>
#include <sys/types.h>
+#include <semaphore.h>
#include "nbd.h"
/* Structures */
@@ -70,6 +71,8 @@ typedef struct _client {
uint32_t difffilelen; /**< number of pages in difffile */
uint32_t *difmap; /**< see comment on the global difmap for this one */
int transactionlogfd;/**< fd for transaction log */
+ char semname[100]; /**< name of the posix sem that protects access to the transaction log */
+ sem_t *logsem; /**< posix sem that protects access to the transaction log */
int clientfeats; /**< Features supported by this client */
pthread_mutex_t lock; /**< socket lock */
void *tls_session; /**< TLS session context. Is NULL unless STARTTLS
--
2.33.1
Reply to: