lockwait: Pass all locking information on commandline to lockwait helper
authorAmitay Isaacs <amitay@gmail.com>
Wed, 13 Mar 2013 06:05:00 +0000 (17:05 +1100)
committerAmitay Isaacs <amitay@gmail.com>
Mon, 25 Mar 2013 06:52:41 +0000 (17:52 +1100)
Simplify lockwait code by getting rid of the communication between ctdbd
and ctdb lockwait helper child by passing all the locking information
on command line.

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Makefile.in
server/ctdb_lockwait.c
server/ctdb_lockwait_helper.c

index 9c4f55522ee1e260f733bd0a7ccb8f1b6aaee085..10323491d8e173b0bfbebf57a2aa7504116d231b 100755 (executable)
@@ -112,7 +112,7 @@ bin/ctdbd: $(CTDB_SERVER_OBJ)
        @echo Linking $@
        @$(CC) $(CFLAGS) -o $@ $(CTDB_SERVER_OBJ) $(LIB_FLAGS)
 
-bin/ctdb_lockwait_helper: server/ctdb_lockwait_helper.o @TDB_OBJ@
+bin/ctdb_lockwait_helper: server/ctdb_lockwait_helper.o lib/util/util_file.o @TDB_OBJ@ @TALLOC_OBJ@
        @echo Linking $@
        @$(CC) $(CFLAGS) -o $@ $^ $(LIB_FLAGS)
 
index cdc1af0cc27a7641506b1ffeb83ea1ce106bb0ff..3680e3314530c97fd1425ff91494b9011d621ce6 100644 (file)
@@ -157,11 +157,8 @@ struct lockwait_handle *ctdb_lockwait(struct ctdb_db_context *ctdb_db,
 {
        struct lockwait_handle *result, *i;
        int ret;
-       int datafd[2];
-       TDB_DATA tmp_key, tmp_data;
-       struct ctdb_marshall_buffer *m;
        const char *prog = BINDIR "/ctdb_lockwait_helper";
-       char arg0[128], arg1[8], arg2[8], arg3[8];
+       char *arg0, *arg1, *arg2, *arg3;
 
        CTDB_INCREMENT_STAT(ctdb_db->ctdb, lockwait_calls);
        CTDB_INCREMENT_STAT(ctdb_db->ctdb, pending_lockwait_calls);
@@ -198,51 +195,6 @@ struct lockwait_handle *ctdb_lockwait(struct ctdb_db_context *ctdb_db,
                return result;
        }
 
-       /* Create data for the child process */
-       m = talloc_zero(result, struct ctdb_marshall_buffer);
-       if (m == NULL) {
-               talloc_free(result);
-               CTDB_DECREMENT_STAT(ctdb_db->ctdb, pending_lockwait_calls);
-               return NULL;
-       }
-
-       tmp_key.dptr = (uint8_t *)talloc_strdup(result, "dbpath");
-       if (tmp_key.dptr == NULL) {
-               talloc_free(result);
-               CTDB_DECREMENT_STAT(ctdb_db->ctdb, pending_lockwait_calls);
-               return NULL;
-       }
-       tmp_key.dsize = strlen((char *)tmp_key.dptr);
-
-       tmp_data.dptr = discard_const_p(uint8_t, ctdb_db->db_path);
-       tmp_data.dsize = strlen((char *)tmp_data.dptr);
-
-       m = ctdb_marshall_add(result, m, ctdb_db->db_id, 0, tmp_key, NULL, tmp_data);
-       if (m == NULL) {
-               talloc_free(result);
-               CTDB_DECREMENT_STAT(ctdb_db->ctdb, pending_lockwait_calls);
-               return NULL;
-       }
-       talloc_free(tmp_key.dptr);
-
-       tmp_key.dptr = (uint8_t *)talloc_strdup(result, "dbkey");
-       if (tmp_key.dptr == NULL) {
-               talloc_free(result);
-               CTDB_DECREMENT_STAT(ctdb_db->ctdb, pending_lockwait_calls);
-               return NULL;
-       }
-       tmp_key.dsize = strlen((char *)tmp_key.dptr);
-
-       m = ctdb_marshall_add(result, m, ctdb_db->db_id, 0, tmp_key, NULL, key);
-       if (m == NULL) {
-               talloc_free(result);
-               CTDB_DECREMENT_STAT(ctdb_db->ctdb, pending_lockwait_calls);
-               return NULL;
-       }
-       talloc_free(tmp_key.dptr);
-
-       tmp_data = ctdb_marshall_finish(m);
-
        ret = pipe(result->fd);
        if (ret != 0) {
                talloc_free(result);
@@ -250,8 +202,13 @@ struct lockwait_handle *ctdb_lockwait(struct ctdb_db_context *ctdb_db,
                return NULL;
        }
 
-       ret = pipe(datafd);
-       if (ret != 0) {
+       /* Create data for the child process */
+       arg0 = talloc_asprintf(result, "ctdb_lock-%s", ctdb_db->db_name);
+       arg1 = talloc_asprintf(result, "%d", result->fd[1]);
+       arg2 = talloc_strdup(result, ctdb_db->db_path);
+       arg3 = hex_encode_talloc(result, key.dptr, key.dsize);
+
+       if (!arg0 || !arg1 || !arg2 || !arg3) {
                close(result->fd[0]);
                close(result->fd[1]);
                talloc_free(result);
@@ -267,27 +224,23 @@ struct lockwait_handle *ctdb_lockwait(struct ctdb_db_context *ctdb_db,
        if (result->child == (pid_t)-1) {
                close(result->fd[0]);
                close(result->fd[1]);
-               close(datafd[0]);
-               close(datafd[1]);
                talloc_free(result);
                CTDB_DECREMENT_STAT(ctdb_db->ctdb, pending_lockwait_calls);
                return NULL;
        }
 
        if (result->child == 0) {
-               sprintf(arg0, "ctdb-lock-%s", ctdb_db->db_name);
-               sprintf(arg1, "%d", datafd[0]);
-               sprintf(arg2, "%d", result->fd[1]);
-               sprintf(arg3, "%d", ctdb_db->ctdb->tunable.database_hash_size);
-
                close(result->fd[0]);
-               close(datafd[1]);
 
                execl(prog, arg0, arg1, arg2, arg3, NULL);
                _exit(1);
        }
 
-       close(datafd[0]);
+       talloc_free(arg0);
+       talloc_free(arg1);
+       talloc_free(arg2);
+       talloc_free(arg3);
+
        close(result->fd[1]);
 
        /* This is an active lockwait child process */
@@ -298,25 +251,6 @@ struct lockwait_handle *ctdb_lockwait(struct ctdb_db_context *ctdb_db,
        ctdb_db->pending_requests++;
        talloc_set_destructor(result, lockwait_destructor);
 
-       set_nonblocking(datafd[1]);
-       ret = write(datafd[1], &tmp_data.dsize, sizeof(tmp_data.dsize));
-       if (ret < 0) {
-               close(datafd[1]);
-               close(result->fd[0]);
-               talloc_free(result);
-               CTDB_DECREMENT_STAT(ctdb_db->ctdb, pending_lockwait_calls);
-               return NULL;
-       }
-       ret = write(datafd[1], tmp_data.dptr, tmp_data.dsize);
-       if (ret < 0) {
-               close(datafd[1]);
-               close(result->fd[0]);
-               talloc_free(result);
-               CTDB_DECREMENT_STAT(ctdb_db->ctdb, pending_lockwait_calls);
-               return NULL;
-       }
-       close(datafd[1]);
-
        result->fde = event_add_fd(ctdb_db->ctdb->ev, result, result->fd[0],
                                   EVENT_FD_READ, lockwait_handler,
                                   (void *)result);
index 67f2994a67e330862ecf1e2424265439015a0519..b300fe434d22a2116288e5ef507e2faefa58a48b 100644 (file)
 #include "system/filesys.h"
 #include "../include/ctdb_private.h"
 
-/*
- * Use stdin to read information about record and database
- *
- * 1. Database full path
- * 2. Key
- */
-
-struct lock_info {
-       char *dbpath;
-       TDB_DATA dbkey;
-};
-
-static bool read_data(int fd, TDB_DATA *data)
-{
-       int len;
-
-       if (read(fd, &len, sizeof(len)) != sizeof(len)) {
-               return false;
-       }
-
-       data->dptr = malloc(len);
-       if (data->dptr == NULL) {
-               return false;
-       }
-       data->dsize = len;
-
-       if (read(fd, data->dptr, len) != len) {
-               free(data->dptr);
-               return false;
-       }
-
-       return true;
-}
-
-/* This is a copy of ctdb_marshall_loop_next() */
-static struct ctdb_rec_data *extract_key_data(struct ctdb_marshall_buffer *m,
-                                             struct ctdb_rec_data *r,
-                                             TDB_DATA *key, TDB_DATA *data)
-{
-       if (r == NULL) {
-               r = (struct ctdb_rec_data *)&m->data[0];
-       } else {
-               r = (struct ctdb_rec_data *)(r->length + (uint8_t *)r);
-       }
-
-       key->dptr = &r->data[0];
-       key->dsize = r->keylen;
-
-       data->dptr = &r->data[r->keylen];
-       data->dsize = r->datalen;
-
-       return r;
-}
-
-
-static bool parse_data(TDB_DATA buffer, struct lock_info *lock)
-{
-       struct ctdb_marshall_buffer *m = (struct ctdb_marshall_buffer *)buffer.dptr;
-       struct ctdb_rec_data *r;
-       TDB_DATA key, data;
-
-       /* Database name */
-       r = extract_key_data(m, NULL, &key, &data);
-       if (key.dsize == 0 || data.dsize == 0) {
-               return false;
-       }
-       if (strncmp((char *)key.dptr, "dbpath", 6) != 0) {
-               return false;
-       }
-       lock->dbpath = strndup((char *)data.dptr, data.dsize);
-       if (lock->dbpath == NULL) {
-               return false;
-       }
-
-       /* Database key */
-       if (r == NULL) {
-               return false;
-       }
-       r = extract_key_data(m, r, &key, &data);
-       if (key.dsize == 0 || data.dsize == 0) {
-               return false;
-       }
-
-       if (strncmp((char *)key.dptr, "dbkey", 5) != 0) {
-               return false;
-       }
-       lock->dbkey.dptr = malloc(data.dsize);
-       if (lock->dbkey.dptr == NULL) {
-               return false;
-       }
-       lock->dbkey.dsize = data.dsize;
-       memcpy(lock->dbkey.dptr, data.dptr, data.dsize);
-
-       return true;
-}
-
-
 static void send_result(int fd, char result)
 {
        write(fd, &result, 1);
@@ -130,39 +33,33 @@ static void send_result(int fd, char result)
 
 int main(int argc, char *argv[])
 {
-       TDB_DATA data;
-       struct lock_info lock;
+       TDB_DATA key;
+       const char *dbpath, *dbkey;
+       int write_fd;
        struct tdb_context *tdb;
        char result = 1;
-       int read_fd, write_fd;
-       int hash_size;
        int ppid;
 
        if (argc != 4) {
-               fprintf(stderr, "Usage: %s <read-fd> <write-fd> <db-hashsize>\n", argv[0]);
+               fprintf(stderr, "Usage: %s <output-fd> <db-path> <db-key>\n", argv[0]);
                exit(1);
        }
 
-       read_fd = atoi(argv[1]);
-       write_fd = atoi(argv[2]);
-       hash_size = atoi(argv[3]);
+       write_fd = atoi(argv[1]);
+       dbpath = argv[2];
+       dbkey = argv[3];
 
-       if (!read_data(read_fd, &data)) {
-               send_result(write_fd, result);
-       }
-
-       if (!parse_data(data, &lock)) {
-               send_result(write_fd, result);
-       }
+       /* Convert hex key to key */
+       key.dptr = hex_decode_talloc(NULL, dbkey, &key.dsize);
 
-       tdb = tdb_open(lock.dbpath, hash_size, TDB_DEFAULT, O_RDWR, 0600);
+       tdb = tdb_open(dbpath, 0, TDB_DEFAULT, O_RDWR, 0600);
        if (tdb == NULL) {
-               fprintf(stderr, "Error opening database %s\n", lock.dbpath);
+               fprintf(stderr, "Lockwait: Error opening database %s\n", dbpath);
                send_result(write_fd, result);
        }
 
-       if (tdb_chainlock(tdb, lock.dbkey) < 0) {
-               fprintf(stderr, "Error locking (%s)\n", tdb_errorstr(tdb));
+       if (tdb_chainlock(tdb, key) < 0) {
+               fprintf(stderr, "Lockwait: Error locking (%s)\n", tdb_errorstr(tdb));
                send_result(write_fd, result);
        }