summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDamian Hobson-Garcia <dhobsong@igel.co.jp>2021-03-09 12:05:29 +0900
committerDamian Hobson-Garcia <dhobsong@igel.co.jp>2021-04-06 10:33:11 +0900
commitecaaf9e2ad40181d916049510823ce8557ecd91e (patch)
treeb78438e8c5c610cef5a79dfaeba060c2a521125d
parent220eb2fad6c21338c32989e69f45646b4e8d5f0f (diff)
lease-server: Allow multiple client connections
Allow multiple clients to issue lease requests on a server at the same time. This is necessary to be able to grant or deny leases, not just on a first-come-first-served basis. Future patches will add extra contitions, such as command-line options and lease configuration settings to determine when and how lease requests should be granted. This update changes the behaviour of the lease-server interface so that it reports every client connection request, instead of when a server has accepted a request, so update the test suite to reflect this. Bug-AGL: SPEC-3816 Change-Id: I48cc392dd62a8c06ea74178bc52c627032817203 Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
-rw-r--r--drm-lease-manager/lease-manager.c6
-rw-r--r--drm-lease-manager/lease-server.c117
-rw-r--r--drm-lease-manager/lease-server.h8
-rw-r--r--drm-lease-manager/main.c8
-rw-r--r--drm-lease-manager/test/lease-server-test.c141
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);