From 7b523884249f904f45b1e9a7eae8152c2ac1248d Mon Sep 17 00:00:00 2001 From: nyorain Date: Mon, 24 Sep 2018 23:17:08 +0200 Subject: [PATCH 1/2] Rework session handling Sessions can now be retrieved from a backend in a more general manner. Multi-backend gets back its `session` field that contains the session if one was created, removing the interfacing from multi backend with the drm backend directly. This adds the possibility to use sessions even without the drm backend. It additionally fixes the bug that 2 session objects got created when WLR_BACKENDS were set to "libinput,drm". To allow vt switching without drm backend (and drm fd) on logind, start listening to PropertiesChanged signals from dbus and parse the session "Active" property when no master fd was created (this does not change current drm backend behaviour in any way). --- backend/backend.c | 42 ++++++---- backend/drm/backend.c | 5 -- backend/multi/backend.c | 19 ++--- backend/session/logind.c | 143 ++++++++++++++++++++++++++++++++ include/backend/multi.h | 1 + include/wlr/backend.h | 5 ++ include/wlr/backend/drm.h | 2 - include/wlr/backend/interface.h | 1 + rootston/keyboard.c | 14 ++-- 9 files changed, 189 insertions(+), 43 deletions(-) diff --git a/backend/backend.c b/backend/backend.c index 78b90007..2d6464b7 100644 --- a/backend/backend.c +++ b/backend/backend.c @@ -15,6 +15,7 @@ #include #include #include +#include "backend/multi.h" /* WLR_HAS_X11_BACKEND needs to be after wlr/config.h */ #ifdef WLR_HAS_X11_BACKEND @@ -56,6 +57,13 @@ struct wlr_renderer *wlr_backend_get_renderer(struct wlr_backend *backend) { return NULL; } +struct wlr_session *wlr_backend_get_session(struct wlr_backend *backend) { + if (backend->impl->get_session) { + return backend->impl->get_session(backend); + } + return NULL; +} + static size_t parse_outputs_env(const char *name) { const char *outputs_str = getenv(name); if (outputs_str == NULL) { @@ -158,10 +166,12 @@ static struct wlr_backend *attempt_backend_by_name(struct wl_display *display, return attempt_headless_backend(display, create_renderer_func); } else if (strcmp(name, "drm") == 0 || strcmp(name, "libinput") == 0) { // DRM and libinput need a session - *session = wlr_session_create(display); if (!*session) { - wlr_log(WLR_ERROR, "failed to start a session"); - return NULL; + *session = wlr_session_create(display); + if (!*session) { + wlr_log(WLR_ERROR, "failed to start a session"); + return NULL; + } } if (strcmp(name, "libinput") == 0) { @@ -178,13 +188,12 @@ static struct wlr_backend *attempt_backend_by_name(struct wl_display *display, struct wlr_backend *wlr_backend_autocreate(struct wl_display *display, wlr_renderer_create_func_t create_renderer_func) { struct wlr_backend *backend = wlr_multi_backend_create(display); + struct wlr_multi_backend *multi = (struct wlr_multi_backend *)backend; if (!backend) { wlr_log(WLR_ERROR, "could not allocate multibackend"); return NULL; } - struct wlr_session *session = NULL; - char *names = getenv("WLR_BACKENDS"); if (names) { names = strdup(names); @@ -197,12 +206,12 @@ struct wlr_backend *wlr_backend_autocreate(struct wl_display *display, char *saveptr; char *name = strtok_r(names, ",", &saveptr); while (name != NULL) { - struct wlr_backend *subbackend = - attempt_backend_by_name(display, backend, &session, name, create_renderer_func); + struct wlr_backend *subbackend = attempt_backend_by_name(display, + backend, &multi->session, name, create_renderer_func); if (subbackend == NULL) { wlr_log(WLR_ERROR, "failed to start backend '%s'", name); wlr_backend_destroy(backend); - wlr_session_destroy(session); + wlr_session_destroy(multi->session); free(names); return NULL; } @@ -210,7 +219,7 @@ struct wlr_backend *wlr_backend_autocreate(struct wl_display *display, if (!wlr_multi_backend_add(backend, subbackend)) { wlr_log(WLR_ERROR, "failed to add backend '%s'", name); wlr_backend_destroy(backend); - wlr_session_destroy(session); + wlr_session_destroy(multi->session); free(names); return NULL; } @@ -245,29 +254,30 @@ struct wlr_backend *wlr_backend_autocreate(struct wl_display *display, #endif // Attempt DRM+libinput - session = wlr_session_create(display); - if (!session) { + multi->session = wlr_session_create(display); + if (!multi->session) { wlr_log(WLR_ERROR, "Failed to start a DRM session"); wlr_backend_destroy(backend); return NULL; } - struct wlr_backend *libinput = wlr_libinput_backend_create(display, session); + struct wlr_backend *libinput = wlr_libinput_backend_create(display, + multi->session); if (!libinput) { wlr_log(WLR_ERROR, "Failed to start libinput backend"); wlr_backend_destroy(backend); - wlr_session_destroy(session); + wlr_session_destroy(multi->session); return NULL; } wlr_multi_backend_add(backend, libinput); - struct wlr_backend *primary_drm = - attempt_drm_backend(display, backend, session, create_renderer_func); + struct wlr_backend *primary_drm = attempt_drm_backend(display, backend, + multi->session, create_renderer_func); if (!primary_drm) { wlr_log(WLR_ERROR, "Failed to open any DRM device"); wlr_backend_destroy(libinput); wlr_backend_destroy(backend); - wlr_session_destroy(session); + wlr_session_destroy(multi->session); return NULL; } diff --git a/backend/drm/backend.c b/backend/drm/backend.c index bba79bda..40f17bb7 100644 --- a/backend/drm/backend.c +++ b/backend/drm/backend.c @@ -193,8 +193,3 @@ error_fd: free(drm); return NULL; } - -struct wlr_session *wlr_drm_backend_get_session(struct wlr_backend *backend) { - struct wlr_drm_backend *drm = get_drm_backend_from_backend(backend); - return drm->session; -} diff --git a/backend/multi/backend.c b/backend/multi/backend.c index 3707fe34..2f4b929f 100644 --- a/backend/multi/backend.c +++ b/backend/multi/backend.c @@ -1,7 +1,6 @@ #include #include #include -#include #include #include #include @@ -72,10 +71,17 @@ static struct wlr_renderer *multi_backend_get_renderer( return NULL; } +static struct wlr_session *multi_backend_get_session( + struct wlr_backend *_backend) { + struct wlr_multi_backend *backend = multi_backend_from_backend(_backend); + return backend->session; +} + struct wlr_backend_impl backend_impl = { .start = multi_backend_start, .destroy = multi_backend_destroy, .get_renderer = multi_backend_get_renderer, + .get_session = multi_backend_get_session, }; static void handle_display_destroy(struct wl_listener *listener, void *data) { @@ -191,17 +197,6 @@ void wlr_multi_backend_remove(struct wlr_backend *_multi, } } -struct wlr_session *wlr_multi_get_session(struct wlr_backend *_backend) { - struct wlr_multi_backend *backend = multi_backend_from_backend(_backend); - struct subbackend_state *sub; - wl_list_for_each(sub, &backend->backends, link) { - if (wlr_backend_is_drm(sub->backend)) { - return wlr_drm_backend_get_session(sub->backend); - } - } - return NULL; -} - bool wlr_multi_is_empty(struct wlr_backend *_backend) { assert(wlr_backend_is_multi(_backend)); struct wlr_multi_backend *backend = (struct wlr_multi_backend *)_backend; diff --git a/backend/session/logind.c b/backend/session/logind.c index 39143a2f..b86cf3dc 100644 --- a/backend/session/logind.c +++ b/backend/session/logind.c @@ -35,6 +35,11 @@ struct logind_session { char *id; char *path; + + // specifies whether a drm device was taken + // if so, the session will be (de)activated with the drm fd, + // otherwise with the dbus PropertiesChanged on "active" signal + bool has_drm; }; static struct logind_session *logind_session_from_session( @@ -43,6 +48,30 @@ static struct logind_session *logind_session_from_session( return (struct logind_session *)base; } +static void parse_active(struct logind_session *session, + struct sd_bus_message *msg) { + int ret; + ret = sd_bus_message_enter_container(msg, 'v', "b"); + if (ret < 0) { + wlr_log(WLR_ERROR, "Failed to parse D-Bus active property (1): %s", + strerror(-ret)); + return; + } + + bool active; + ret = sd_bus_message_read_basic(msg, 'b', &active); + if (ret < 0) { + wlr_log(WLR_ERROR, "Failed to parse D-Bus active property (2): %s", + strerror(-ret)); + return; + } + + if (!session->has_drm) { + session->base.active = active; + wlr_signal_emit_safe(&session->base.session_signal, session); + } +} + static int logind_take_device(struct wlr_session *base, const char *path) { struct logind_session *session = logind_session_from_session(base); @@ -57,6 +86,10 @@ static int logind_take_device(struct wlr_session *base, const char *path) { return -1; } + if (major(st.st_rdev) == DRM_MAJOR) { + session->has_drm = true; + } + ret = sd_bus_call_method(session->bus, "org.freedesktop.login1", session->path, "org.freedesktop.login1.Session", "TakeDevice", &error, &msg, "uu", major(st.st_rdev), minor(st.st_rdev)); @@ -262,6 +295,7 @@ static int pause_device(sd_bus_message *msg, void *userdata, sd_bus_error *ret_e } if (major == DRM_MAJOR) { + assert(session->has_drm); session->base.active = false; wlr_signal_emit_safe(&session->base.session_signal, session); } @@ -307,6 +341,107 @@ error: return 0; } +static int properties_changed(sd_bus_message *msg, void *userdata, + sd_bus_error *ret_error) { + struct logind_session *session = userdata; + int ret = 0; + + // if we have drm fd we don't depend on this + if (session->has_drm) { + return 0; + } + + // PropertiesChanged 1: interface + const char *interface; + ret = sd_bus_message_read_basic(msg, 's', &interface); // skip path + if (ret < 0) { + wlr_log(WLR_ERROR, "Failed to parse D-Bus PropertiesChanged (1) %s", + strerror(-ret)); + goto error; + } + + if (strcmp(interface, "org.freedesktop.login1.Session") != 0) { + // not interesting for us; ignore + wlr_log(WLR_DEBUG, "ignoring PropertiesChanged from %s", interface); + return 0; + } + + // PropertiesChanged 2: changed properties with values + ret = sd_bus_message_enter_container(msg, 'a', "{sv}"); + if (ret < 0) { + wlr_log(WLR_ERROR, "Failed to parse D-Bus PropertiesChanged (2) %s", + strerror(-ret)); + goto error; + } + + const char *s; + while ((ret = sd_bus_message_enter_container(msg, 'e', "sv")) > 0) { + ret = sd_bus_message_read_basic(msg, 's', &s); + if (ret < 0) { + wlr_log(WLR_ERROR, "Failed to parse D-Bus PropertiesChanged (3) %s", + strerror(-ret)); + goto error; + } + + if (strcmp(s, "Active") == 0) { + parse_active(session, msg); + return 0; + } else { + sd_bus_message_skip(msg, "{sv}"); + } + + ret = sd_bus_message_exit_container(msg); + if (ret < 0) { + wlr_log(WLR_ERROR, "Failed to parse D-Bus PropertiesChanged (4) %s", + strerror(-ret)); + goto error; + } + } + + if (ret < 0) { + wlr_log(WLR_ERROR, "Failed to parse D-Bus PropertiesChanged (5) %s", + strerror(-ret)); + goto error; + } + + ret = sd_bus_message_exit_container(msg); + if (ret < 0) { + wlr_log(WLR_ERROR, "Failed to parse D-Bus PropertiesChanged (6) %s", + strerror(-ret)); + goto error; + } + + // PropertiesChanged 3: changed properties without values + sd_bus_message_enter_container(msg, 'a', "s"); + while ((ret = sd_bus_message_read_basic(msg, 's', &s)) > 0) { + if (strcmp(s, "Active") == 0) { + sd_bus_error error = SD_BUS_ERROR_NULL; + sd_bus_message *answer = NULL; + ret = sd_bus_call_method(session->bus, "org.freedesktop.login1", + session->path, "org.freedesktop.DBus.Properties", "Get", + &error, &answer, "ss", "org.freedesktop.login1.Session", + "Active"); + if (ret < 0) { + wlr_log(WLR_ERROR, "Failed to get active property: '%s' (%s)", + error.message, strerror(ret)); + goto error; + } + + parse_active(session, answer); + return 0; + } + } + + if (ret < 0) { + wlr_log(WLR_ERROR, "Failed to parse D-Bus PropertiesChanged (7) %s", + strerror(-ret)); + goto error; + } + +error: + return 0; +} + static bool add_signal_matches(struct logind_session *session) { int ret; @@ -338,6 +473,14 @@ static bool add_signal_matches(struct logind_session *session) { return false; } + ret = sd_bus_match_signal(session->bus, NULL, "org.freedesktop.login1", + session->path, "org.freedesktop.DBus.Properties", "PropertiesChanged", + properties_changed, session); + if (ret < 0) { + wlr_log(WLR_ERROR, "Failed to add D-Bus match: %s", strerror(-ret)); + return false; + } + return true; } diff --git a/include/backend/multi.h b/include/backend/multi.h index c57c48f3..2f5f1bd4 100644 --- a/include/backend/multi.h +++ b/include/backend/multi.h @@ -8,6 +8,7 @@ struct wlr_multi_backend { struct wlr_backend backend; + struct wlr_session *session; struct wl_list backends; diff --git a/include/wlr/backend.h b/include/wlr/backend.h index 39d072e2..dfb8d2a8 100644 --- a/include/wlr/backend.h +++ b/include/wlr/backend.h @@ -57,5 +57,10 @@ void wlr_backend_destroy(struct wlr_backend *backend); * Obtains the wlr_renderer reference this backend is using. */ struct wlr_renderer *wlr_backend_get_renderer(struct wlr_backend *backend); +/** + * Obtains the wlr_session reference from this backend if there is any. + * Might return NULL for backends that don't use a session. + */ +struct wlr_session *wlr_backend_get_session(struct wlr_backend *backend); #endif diff --git a/include/wlr/backend/drm.h b/include/wlr/backend/drm.h index 5d47647d..3724adfb 100644 --- a/include/wlr/backend/drm.h +++ b/include/wlr/backend/drm.h @@ -34,6 +34,4 @@ bool wlr_output_is_drm(struct wlr_output *output); typedef struct _drmModeModeInfo drmModeModeInfo; bool wlr_drm_connector_add_mode(struct wlr_output *output, const drmModeModeInfo *mode); -struct wlr_session *wlr_drm_backend_get_session(struct wlr_backend *backend); - #endif diff --git a/include/wlr/backend/interface.h b/include/wlr/backend/interface.h index f3dee69b..2c300709 100644 --- a/include/wlr/backend/interface.h +++ b/include/wlr/backend/interface.h @@ -17,6 +17,7 @@ struct wlr_backend_impl { bool (*start)(struct wlr_backend *backend); void (*destroy)(struct wlr_backend *backend); struct wlr_renderer *(*get_renderer)(struct wlr_backend *backend); + struct wlr_session *(*get_session)(struct wlr_backend *backend); }; /** diff --git a/rootston/keyboard.c b/rootston/keyboard.c index b5a8093b..b28bd773 100644 --- a/rootston/keyboard.c +++ b/rootston/keyboard.c @@ -5,7 +5,6 @@ #include #include #include -#include #include #include #include @@ -193,14 +192,13 @@ static bool keyboard_execute_compositor_binding(struct roots_keyboard *keyboard, if (keysym >= XKB_KEY_XF86Switch_VT_1 && keysym <= XKB_KEY_XF86Switch_VT_12) { struct roots_server *server = keyboard->input->server; - if (wlr_backend_is_multi(server->backend)) { - struct wlr_session *session = - wlr_multi_get_session(server->backend); - if (session) { - unsigned vt = keysym - XKB_KEY_XF86Switch_VT_1 + 1; - wlr_session_change_vt(session, vt); - } + + struct wlr_session *session = wlr_backend_get_session(server->backend); + if (session) { + unsigned vt = keysym - XKB_KEY_XF86Switch_VT_1 + 1; + wlr_session_change_vt(session, vt); } + return true; } From b4d46aa9c255ccb6f121b3657f6910c29e02d704 Mon Sep 17 00:00:00 2001 From: nyorain Date: Thu, 4 Oct 2018 13:46:01 +0200 Subject: [PATCH 2/2] Use sd_bus_get_property_trivial & remove numbering --- backend/session/logind.c | 86 +++++++++++++++---------------------- include/wlr/backend/multi.h | 1 - 2 files changed, 35 insertions(+), 52 deletions(-) diff --git a/backend/session/logind.c b/backend/session/logind.c index b86cf3dc..b44112e6 100644 --- a/backend/session/logind.c +++ b/backend/session/logind.c @@ -48,30 +48,6 @@ static struct logind_session *logind_session_from_session( return (struct logind_session *)base; } -static void parse_active(struct logind_session *session, - struct sd_bus_message *msg) { - int ret; - ret = sd_bus_message_enter_container(msg, 'v', "b"); - if (ret < 0) { - wlr_log(WLR_ERROR, "Failed to parse D-Bus active property (1): %s", - strerror(-ret)); - return; - } - - bool active; - ret = sd_bus_message_read_basic(msg, 'b', &active); - if (ret < 0) { - wlr_log(WLR_ERROR, "Failed to parse D-Bus active property (2): %s", - strerror(-ret)); - return; - } - - if (!session->has_drm) { - session->base.active = active; - wlr_signal_emit_safe(&session->base.session_signal, session); - } -} - static int logind_take_device(struct wlr_session *base, const char *path) { struct logind_session *session = logind_session_from_session(base); @@ -346,17 +322,15 @@ static int properties_changed(sd_bus_message *msg, void *userdata, struct logind_session *session = userdata; int ret = 0; - // if we have drm fd we don't depend on this + // if we have a drm fd we don't depend on this if (session->has_drm) { return 0; } - // PropertiesChanged 1: interface + // PropertiesChanged arg 1: interface const char *interface; ret = sd_bus_message_read_basic(msg, 's', &interface); // skip path if (ret < 0) { - wlr_log(WLR_ERROR, "Failed to parse D-Bus PropertiesChanged (1) %s", - strerror(-ret)); goto error; } @@ -366,11 +340,9 @@ static int properties_changed(sd_bus_message *msg, void *userdata, return 0; } - // PropertiesChanged 2: changed properties with values + // PropertiesChanged arg 2: changed properties with values ret = sd_bus_message_enter_container(msg, 'a', "{sv}"); if (ret < 0) { - wlr_log(WLR_ERROR, "Failed to parse D-Bus PropertiesChanged (2) %s", - strerror(-ret)); goto error; } @@ -378,13 +350,26 @@ static int properties_changed(sd_bus_message *msg, void *userdata, while ((ret = sd_bus_message_enter_container(msg, 'e', "sv")) > 0) { ret = sd_bus_message_read_basic(msg, 's', &s); if (ret < 0) { - wlr_log(WLR_ERROR, "Failed to parse D-Bus PropertiesChanged (3) %s", - strerror(-ret)); goto error; } if (strcmp(s, "Active") == 0) { - parse_active(session, msg); + int ret; + ret = sd_bus_message_enter_container(msg, 'v', "b"); + if (ret < 0) { + goto error; + } + + bool active; + ret = sd_bus_message_read_basic(msg, 'b', &active); + if (ret < 0) { + goto error; + } + + if (session->base.active != active) { + session->base.active = active; + wlr_signal_emit_safe(&session->base.session_signal, session); + } return 0; } else { sd_bus_message_skip(msg, "{sv}"); @@ -392,53 +377,52 @@ static int properties_changed(sd_bus_message *msg, void *userdata, ret = sd_bus_message_exit_container(msg); if (ret < 0) { - wlr_log(WLR_ERROR, "Failed to parse D-Bus PropertiesChanged (4) %s", - strerror(-ret)); goto error; } } if (ret < 0) { - wlr_log(WLR_ERROR, "Failed to parse D-Bus PropertiesChanged (5) %s", - strerror(-ret)); goto error; } ret = sd_bus_message_exit_container(msg); if (ret < 0) { - wlr_log(WLR_ERROR, "Failed to parse D-Bus PropertiesChanged (6) %s", - strerror(-ret)); goto error; } - // PropertiesChanged 3: changed properties without values + // PropertiesChanged arg 3: changed properties without values sd_bus_message_enter_container(msg, 'a', "s"); while ((ret = sd_bus_message_read_basic(msg, 's', &s)) > 0) { if (strcmp(s, "Active") == 0) { sd_bus_error error = SD_BUS_ERROR_NULL; - sd_bus_message *answer = NULL; - ret = sd_bus_call_method(session->bus, "org.freedesktop.login1", - session->path, "org.freedesktop.DBus.Properties", "Get", - &error, &answer, "ss", "org.freedesktop.login1.Session", - "Active"); + bool active; + ret = sd_bus_get_property_trivial(session->bus, + "org.freedesktop.login1", session->path, + "org.freedesktop.login1.Session", "Active", &error, + 'b', &active); if (ret < 0) { - wlr_log(WLR_ERROR, "Failed to get active property: '%s' (%s)", + wlr_log(WLR_ERROR, "Failed to get 'Active' property: '%s' (%s)", error.message, strerror(ret)); - goto error; + return 0; } - parse_active(session, answer); + if (session->base.active != active) { + session->base.active = active; + wlr_signal_emit_safe(&session->base.session_signal, session); + } return 0; } } if (ret < 0) { - wlr_log(WLR_ERROR, "Failed to parse D-Bus PropertiesChanged (7) %s", - strerror(-ret)); goto error; } + return 0; + error: + wlr_log(WLR_ERROR, "Failed to parse D-Bus PropertiesChanged %s", + strerror(-ret)); return 0; } diff --git a/include/wlr/backend/multi.h b/include/wlr/backend/multi.h index ef908d26..0687f4b6 100644 --- a/include/wlr/backend/multi.h +++ b/include/wlr/backend/multi.h @@ -28,7 +28,6 @@ void wlr_multi_backend_remove(struct wlr_backend *multi, struct wlr_backend *backend); bool wlr_backend_is_multi(struct wlr_backend *backend); -struct wlr_session *wlr_multi_get_session(struct wlr_backend *base); bool wlr_multi_is_empty(struct wlr_backend *backend); void wlr_multi_for_each_backend(struct wlr_backend *backend,