diff options
| -rw-r--r-- | drm-lease-manager/lease-manager.c | 6 | ||||
| -rw-r--r-- | drm-lease-manager/lease-server.c | 117 | ||||
| -rw-r--r-- | drm-lease-manager/lease-server.h | 8 | ||||
| -rw-r--r-- | drm-lease-manager/main.c | 8 | ||||
| -rw-r--r-- | drm-lease-manager/test/lease-server-test.c | 141 |
5 files changed, 144 insertions, 136 deletions
diff --git a/drm-lease-manager/lease-manager.c b/drm-lease-manager/lease-manager.c index 5cfc5de..016f318 100644 --- a/drm-lease-manager/lease-manager.c +++ b/drm-lease-manager/lease-manager.c @@ -354,8 +354,10 @@ int lm_lease_grant(struct lm *lm, struct lease_handle *handle) assert(handle); struct lease *lease = (struct lease *)handle; - if (lease->is_granted) - return lease->lease_fd; + if (lease->is_granted) { + /* Lease is already claimed */ + return -1; + } lease->lease_fd = drmModeCreateLease(lm->drm_fd, lease->object_ids, diff --git a/drm-lease-manager/lease-server.c b/drm-lease-manager/lease-server.c index e05e8e4..c57316e 100644 --- a/drm-lease-manager/lease-server.c +++ b/drm-lease-manager/lease-server.c @@ -32,9 +32,31 @@ #define SOCK_LOCK_SUFFIX ".lock" +/* ACTIVE_CLIENTS + * An 'active' client is one that either + * - owns a lease, or + * - is requesting ownership of a lease (which will + * disconnect the current owner if granted) + * + * There can only be at most one of each kind of client at the same + * time. Any other client connections are queued in the + * listen() backlog, waiting to be accept()'ed. + */ +#define ACTIVE_CLIENTS 2 + struct ls_socket { int fd; + bool is_server; + union { + struct ls_server *server; + struct ls_client *client; + }; +}; + +struct ls_client { + struct ls_socket socket; struct ls_server *serv; + bool is_connected; }; struct ls_server { @@ -43,9 +65,7 @@ struct ls_server { int server_socket_lock; struct ls_socket listen; - struct ls_socket client; - - bool is_client_connected; + struct ls_client clients[ACTIVE_CLIENTS]; }; struct ls { @@ -55,37 +75,42 @@ struct ls { int nservers; }; -static bool client_connect(struct ls *ls, struct ls_server *serv) +static struct ls_client *client_connect(struct ls *ls, struct ls_server *serv) { int cfd = accept(serv->listen.fd, NULL, NULL); if (cfd < 0) { DEBUG_LOG("accept failed on %s: %s\n", serv->address.sun_path, strerror(errno)); - return false; + return NULL; } - if (serv->is_client_connected) { - WARN_LOG("Client already connected on %s\n", - serv->address.sun_path); + struct ls_client *client = NULL; + + for (int i = 0; i < ACTIVE_CLIENTS; i++) { + if (!serv->clients[i].is_connected) { + client = &serv->clients[i]; + break; + } + } + if (!client) { close(cfd); - return false; + return NULL; } - serv->client.fd = cfd; - serv->client.serv = serv; + client->socket.fd = cfd; struct epoll_event ev = { .events = POLLHUP, - .data.ptr = &serv->client, + .data.ptr = &client->socket, }; if (epoll_ctl(ls->epoll_fd, EPOLL_CTL_ADD, cfd, &ev)) { DEBUG_LOG("epoll_ctl add failed: %s\n", strerror(errno)); close(cfd); - return false; + return NULL; } - serv->is_client_connected = true; - return true; + client->is_connected = true; + return client; } static int create_socket_lock(struct sockaddr_un *addr) @@ -161,11 +186,18 @@ static bool server_setup(struct ls *ls, struct ls_server *serv, return false; } - serv->is_client_connected = false; + for (int i = 0; i < ACTIVE_CLIENTS; i++) { + struct ls_client *client = &serv->clients[i]; + client->serv = serv; + client->socket.client = client; + } + serv->lease_handle = lease_handle; serv->server_socket_lock = socket_lock; + serv->listen.fd = server_socket; - serv->listen.serv = serv; + serv->listen.server = serv; + serv->listen.is_server = true; struct epoll_event ev = { .events = POLLIN, @@ -193,7 +225,10 @@ static void server_shutdown(struct ls *ls, struct ls_server *serv) epoll_ctl(ls->epoll_fd, EPOLL_CTL_DEL, serv->listen.fd, NULL); close(serv->listen.fd); - ls_disconnect_client(ls, serv); + + for (int i = 0; i < ACTIVE_CLIENTS; i++) + ls_disconnect_client(ls, &serv->clients[i]); + close(serv->server_socket_lock); } @@ -261,32 +296,37 @@ bool ls_get_request(struct ls *ls, struct ls_req *req) struct ls_socket *sock = ev.data.ptr; assert(sock); - struct ls_server *server = sock->serv; - req->lease_handle = server->lease_handle; - req->server = server; + struct ls_server *server; + struct ls_client *client; - if (sock == &server->listen) { + if (sock->is_server) { if (!(ev.events & POLLIN)) continue; - if (client_connect(ls, server)) + + server = sock->server; + client = client_connect(ls, server); + if (client) request = LS_REQ_GET_LEASE; - } else if (sock == &server->client) { + } else { if (!(ev.events & POLLHUP)) continue; + + client = sock->client; + server = client->serv; request = LS_REQ_RELEASE_LEASE; - } else { - ERROR_LOG("Internal error: Invalid socket context\n"); - return false; } + + req->lease_handle = server->lease_handle; + req->client = client; + req->type = request; } - req->type = request; return true; } -bool ls_send_fd(struct ls *ls, struct ls_server *serv, int fd) +bool ls_send_fd(struct ls *ls, struct ls_client *client, int fd) { assert(ls); - assert(serv); + assert(client); if (fd < 0) return false; @@ -312,7 +352,9 @@ bool ls_send_fd(struct ls *ls, struct ls_server *serv, int fd) cmsg->cmsg_len = CMSG_LEN(sizeof(int)); *((int *)CMSG_DATA(cmsg)) = fd; - if (sendmsg(serv->client.fd, &msg, 0) < 0) { + struct ls_server *serv = client->serv; + + if (sendmsg(client->socket.fd, &msg, 0) < 0) { DEBUG_LOG("sendmsg failed on %s: %s\n", serv->address.sun_path, strerror(errno)); return false; @@ -322,14 +364,15 @@ bool ls_send_fd(struct ls *ls, struct ls_server *serv, int fd) return true; } -void ls_disconnect_client(struct ls *ls, struct ls_server *serv) +void ls_disconnect_client(struct ls *ls, struct ls_client *client) { assert(ls); - assert(serv); - if (!serv->is_client_connected) + assert(client); + + if (!client->is_connected) return; - epoll_ctl(ls->epoll_fd, EPOLL_CTL_DEL, serv->client.fd, NULL); - close(serv->client.fd); - serv->is_client_connected = false; + epoll_ctl(ls->epoll_fd, EPOLL_CTL_DEL, client->socket.fd, NULL); + close(client->socket.fd); + client->is_connected = false; } diff --git a/drm-lease-manager/lease-server.h b/drm-lease-manager/lease-server.h index c87b834..1aaad30 100644 --- a/drm-lease-manager/lease-server.h +++ b/drm-lease-manager/lease-server.h @@ -20,7 +20,7 @@ #include "drm-lease.h" struct ls; -struct ls_server; +struct ls_client; enum ls_req_type { LS_REQ_GET_LEASE, LS_REQ_RELEASE_LEASE, @@ -28,7 +28,7 @@ enum ls_req_type { struct ls_req { struct lease_handle *lease_handle; - struct ls_server *server; + struct ls_client *client; enum ls_req_type type; }; @@ -36,7 +36,7 @@ struct ls *ls_create(struct lease_handle **lease_handles, int count); void ls_destroy(struct ls *ls); bool ls_get_request(struct ls *ls, struct ls_req *req); -bool ls_send_fd(struct ls *ls, struct ls_server *server, int fd); +bool ls_send_fd(struct ls *ls, struct ls_client *client, int fd); -void ls_disconnect_client(struct ls *ls, struct ls_server *server); +void ls_disconnect_client(struct ls *ls, struct ls_client *client); #endif diff --git a/drm-lease-manager/main.c b/drm-lease-manager/main.c index c3a933f..491c80c 100644 --- a/drm-lease-manager/main.c +++ b/drm-lease-manager/main.c @@ -91,21 +91,21 @@ int main(int argc, char **argv) ERROR_LOG( "Can't fulfill lease request: lease=%s\n", req.lease_handle->name); - ls_disconnect_client(ls, req.server); + ls_disconnect_client(ls, req.client); break; } - if (!ls_send_fd(ls, req.server, fd)) { + if (!ls_send_fd(ls, req.client, fd)) { ERROR_LOG( "Client communication error: lease=%s\n", req.lease_handle->name); - ls_disconnect_client(ls, req.server); + ls_disconnect_client(ls, req.client); lm_lease_revoke(lm, req.lease_handle); } break; } case LS_REQ_RELEASE_LEASE: - ls_disconnect_client(ls, req.server); + ls_disconnect_client(ls, req.client); lm_lease_revoke(lm, req.lease_handle); break; default: diff --git a/drm-lease-manager/test/lease-server-test.c b/drm-lease-manager/test/lease-server-test.c index 2672820..6a4d26c 100644 --- a/drm-lease-manager/test/lease-server-test.c +++ b/drm-lease-manager/test/lease-server-test.c @@ -28,6 +28,8 @@ #define SOCKETDIR "/tmp" +#define ARRAY_LENGTH(x) sizeof(x) / sizeof(x[0]) + /************** Test fixutre functions *************************/ struct test_config default_test_config; @@ -128,59 +130,6 @@ static void get_and_check_request(struct ls *ls, check_request(&req, expected_lease, expected_type); } -/* Asynchronous version of the above. Has the extra overhead of - * spawning a new thread, so should be used sparingly. */ -struct async_req { - pthread_t tid; - struct ls *ls; - - bool req_valid; - struct ls_req expected; - struct ls_req actual; -}; - -static void *get_request_thread(void *arg) -{ - struct async_req *async_req = arg; - async_req->req_valid = - ls_get_request(async_req->ls, &async_req->actual); - - return NULL; -} - -static struct async_req * -get_and_check_request_async(struct ls *ls, struct lease_handle *expected_lease, - enum ls_req_type expected_type) - -{ - struct async_req *req = malloc(sizeof(struct async_req)); - ck_assert_ptr_ne(req, NULL); - - *req = (struct async_req){ - .ls = ls, - .expected = - { - .lease_handle = expected_lease, - .type = expected_type, - }, - }; - - int ret = pthread_create(&req->tid, NULL, get_request_thread, req); - ck_assert_int_eq(ret, 0); - - return req; -} - -static void check_async_req_result(struct async_req *req) -{ - - pthread_join(req->tid, NULL); - ck_assert_int_eq(req->req_valid, true); - check_request(&req->actual, req->expected.lease_handle, - req->expected.type); - free(req); -} - /* issue_lease_request_and_release * * Test details: Generate a lease request and lease release command from @@ -225,52 +174,66 @@ END_TEST * * Test details: Generate multiple lease requests to the same lease server from * multiple clients at the same time - * Expected results: One get lease and one release lease request are returned - * from ls_get_request(). - * Requests from all but the first client are rejected - * (sockets are closed). + * Expected results: One lease request per client is returned from + * ls_get_request(). + * (Test will process each connection in series and either + * keep the current connection or switch a new one.) + * Only one client remains connected at the end of the test, + * and it returns a validlease release request. */ START_TEST(issue_multiple_lease_requests) { + /* List of which client connections to accept. + * If the nth element is `false` that client request will be + * rejected, and should be reflected in the final configuration + * state */ + bool keep_current_client[] = {false, true, true, false, true}; + struct lease_handle *leases[] = { &test_lease, }; struct ls *ls = ls_create(leases, 1); - struct test_config accepted_config; - struct client_state *accepted_cstate; + const int clients = ARRAY_LENGTH(keep_current_client); + struct test_config configs[clients]; + struct client_state *cstates[clients]; - accepted_config = default_test_config; - accepted_cstate = test_client_start(&accepted_config); - get_and_check_request(ls, &test_lease, LS_REQ_GET_LEASE); - - /*Try to make additional connections while the first is still - *connected. */ - const int nextra_clients = 2; - struct test_config extra_configs[nextra_clients]; - struct client_state *extra_cstates[nextra_clients]; - - for (int i = 0; i < nextra_clients; i++) { - extra_configs[i] = default_test_config; - extra_cstates[i] = test_client_start(&extra_configs[i]); + struct ls_req req; + struct ls_client *current_client = NULL; + + /* Start all clients and accept / reject connections */ + for (int i = 0; i < clients; i++) { + configs[i] = default_test_config; + cstates[i] = test_client_start(&configs[i]); + ck_assert_int_eq(ls_get_request(ls, &req), true); + check_request(&req, &test_lease, LS_REQ_GET_LEASE); + if (current_client && keep_current_client[i]) { + ls_disconnect_client(ls, req.client); + } else { + if (current_client) + ls_disconnect_client(ls, current_client); + current_client = req.client; + } } - // Start asyncronously checking for the accepted client to release. - struct async_req *async_release_req = - get_and_check_request_async(ls, &test_lease, LS_REQ_RELEASE_LEASE); - - for (int i = 0; i < nextra_clients; i++) { - test_client_stop(extra_cstates[i]); + /* Shut down all clients */ + for (int i = 0; i < clients; i++) + test_client_stop(cstates[i]); + + /* Check that a valid release is received from the last accepted client + * connection */ + ck_assert_int_eq(ls_get_request(ls, &req), true); + check_request(&req, &test_lease, LS_REQ_RELEASE_LEASE); + ck_assert_ptr_eq(current_client, req.client); + + /* Check that no other client connections have completed */ + int connections_completed = 0; + for (int i = 0; i < clients; i++) { + if (configs[i].connection_completed) { + connections_completed++; + ck_assert_int_eq(connections_completed, 1); + } } - - /* Release the first connection and check results */ - test_client_stop(accepted_cstate); - check_async_req_result(async_release_req); - - /* Only one connection should be granted access by the lease manager */ - ck_assert_int_eq(accepted_config.connection_completed, true); - for (int i = 0; i < nextra_clients; i++) - ck_assert_int_eq(extra_configs[i].connection_completed, false); ls_destroy(ls); } END_TEST @@ -311,7 +274,7 @@ START_TEST(send_fd_to_client) /* send an fd to the client*/ int test_fd = get_dummy_fd(); - ck_assert_int_eq(ls_send_fd(ls, req.server, test_fd), true); + ck_assert_int_eq(ls_send_fd(ls, req.client, test_fd), true); test_client_stop(cstate); get_and_check_request(ls, &test_lease, LS_REQ_RELEASE_LEASE); @@ -343,7 +306,7 @@ START_TEST(ls_send_fd_is_noop_when_fd_is_invalid) int invalid_fd = get_dummy_fd(); close(invalid_fd); - ck_assert_int_eq(ls_send_fd(ls, req.server, invalid_fd), false); + ck_assert_int_eq(ls_send_fd(ls, req.client, invalid_fd), false); test_client_stop(cstate); get_and_check_request(ls, &test_lease, LS_REQ_RELEASE_LEASE); |