Migrate from dbus-glib to GDBus #426

smallorange · 2023-11-23T07:41:23Z

Hi,

This work migrated the DBus backend to GDBus. Since dbus-glib will be deprecated, the applications depending on it should be migrated to use GDBus as the DBus backend. Notably, the thermald package, shipped by RHEL for its customers, is currently the only package dependent on glib-dbus. This patch migrates the DBus backend to GDBus and it also resolves the dependency issue of the packages. Furthermore, it brings benefits to various Linux distributions affected by the deprecation of dbus-glib.

Fixes: #348, #416

Load dbus introspection to gresource Signed-off-by: Kate Hsuan <hpa@redhat.com>

Fixed it to fit the current GObject definition scheme. Signed-off-by: Kate Hsuan <hpa@redhat.com>

This is the GDBus framework for the Dbus API implementation. The corresponding functions are put to the DBus API method. Signed-off-by: Kate Hsuan <hpa@redhat.com>

Convert the gresource definition to C code before compiling. Signed-off-by: Kate Hsuan <hpa@redhat.com>

The GDBus will be the default backend since glib-dbus will be deprecated soon. --disable-gdbus can be set to switch to glib-dbus. Signed-off-by: Kate Hsuan <hpa@redhat.com>

smallorange · 2023-12-04T03:22:59Z

Hi,

Could folks review this patch?

Thank you :)

spandruvada · 2023-12-06T16:32:34Z

Thanks. I am in process of testing.

zhang-rui · 2023-12-17T02:46:21Z

Hi, @smallorange
A dumb question, if I want to convert intel_lpmd at https://github.com/intel/intel-lpmd to GDBus, what should I do? it seems that only patch 3/5 is the critical part for this transition?

smallorange · 2023-12-18T05:33:58Z

Hi,

I'll deep dive into lpmd code and reply to this later :)

zhang-rui · 2023-12-18T11:33:03Z

Great, really appreciated! @smallorange

smallorange · 2023-12-19T09:49:01Z

Hi,

The structure of lpmd is pretty similar to thermal_daemon. For the original implementation, dbus-binding-tool generated the structure of the DBus and you just need to put all the helper functions (dbus_interface_l_pm__fo_rc_e__on()...etc.) to the right place and then the Dbus API works. That is good since the helpers are already done.

For GDBus, the first thing is to fix the GObject declaration to fit the current version of Glib. It looks like the GObject fixing commit. and then we can focus on the main commit.. Make sure g_bus_own_name() is properly set, for example: org.freedesktop.intel_lpmd, and the callbacks are properly set, (especially on on_bus_aquired).

Look into thd_dbus_on_bus_acquired(). on_bus_aquired callback will be invoked when g_bus_own_name() is successfully done. First, we start to introspect the Dbus XML introspection file (intel_lpmd_dbus_interface.xml). Then, we can start to register all the necessary callbacks, for example, method_call, get_preoperty, and set_property. If no error returns, it should start working.

Moreover, the Dbus introspection file should be set as a gresource. Look in to the commit. glib-compile-resources will compile the dbus introspection file into a C source code. thd_dbus_load_introspection() contains a example for looking up the data from gresource. Moreover, replacing autoconf with meson is a better solution to compile Glib programs since only a few meson commands can generate the code you want, such as greource.

That is the minimum requirement for getting GDbus running.

If you have question or need help, please let me know :)

zhang-rui · 2023-12-20T06:30:41Z

Hi, @smallorange , really appreciate your help on this.
I will ramp up on this and propose some patches later, will reach you for more details if I run into some problems. :)
Really appreciated!

spandruvada · 2024-01-19T14:06:34Z

I get this build error
make[2]: *** No rule to make target 'thermald-resource.c', needed by 'thermald-thermald-resource.o'. Stop.
make[2]: Leaving directory '/home/spandruv/development/tools/thermal_daemon'
make[1]: *** [Makefile:1530: all-recursive] Error 1
make[1]: Leaving directory '/home/spandruv/development/tools/thermal_daemon'

spandruvada · 2024-01-19T14:32:42Z

This diff was required to build:
diff --git a/Makefile.am b/Makefile.am
index 2830bd0..a0c051a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -41,7 +41,8 @@ thermald_LDADD =
$(EVDEV_LIBS)

BUILT_SOURCES = \

```
  thd_dbus_interface.h
```

```
  thd_dbus_interface.h \
```
```
  thermald-resource.c
```

thermald_SOURCES =
src/main.cpp
@@ -93,7 +94,7 @@ man8_MANS = man/thermald.8
thd_dbus_interface.h: $(top_srcdir)/src/thd_dbus_interface.xml
$(AM_V_GEN) dbus-binding-tool --prefix=thd_dbus_interface --mode=glib-server --output=$@ $<

-thd_resources.c: $(top_srcdir)/thermald.gresource.xml
+thermald-resource.c: $(top_srcdir)/thermald-resource.gresource.xml
$(AM_V_GEN) glib-compile-resources --generate-source thermald-resource.gresource.xml

CLEANFILES = $(BUILT_SOURCES)

smallorange · 2024-01-19T15:57:11Z

Hi,

Thank you for testing this patch.

Sorry for my mistake. I've updated the Makefile and it should work.
Btw, I'll take a leave on Monday and I'll come back to work on Tuesday. :)

spandruvada · 2024-01-19T16:35:23Z

Fix the build issue and applied.

q66 · 2024-01-22T22:42:24Z

this PR is incomplete, right? because the project still does not build without dbus-glib, it references the old API in various places and the build system still forces it in

zhang-rui · 2024-01-23T06:31:56Z

src/thd_dbus_interface.cpp

+			 gpointer         user_data) {
+	guint registration_id;
+	GDBusProxy *proxy_id = NULL;
+	GError *error;


should we set error to NULL?
according to https://docs.gtk.org/gio/func.resources_lookup_data.html,
"
The argument can be NULL.
If the return location is not NULL, then you must initialize it to a NULL GError*.
The argument will left initialized to NULL by the function if there are no errors.
In case of error, the argument will be set to a newly allocated GError; the caller will take ownership of the data, and be responsible for freeing it."

It should be NULL. I'll fix this.

zhang-rui · 2024-01-23T06:32:40Z

src/thd_dbus_interface.cpp

+	}
+
+	introspection_data = thd_dbus_load_introspection("src/thd_dbus_interface.xml",
+							 &error);


In my case (converting to use GDBus in intel_lpmd), if I don't set it to NULL, the code fails here.

zhang-rui · 2024-01-23T06:33:32Z

src/thd_dbus_interface.cpp

+	GError *error = NULL;
+	PrefObject *value_obj;
+
+	if (error != NULL) {


this check is redundant, right?

Thank you for pointing it out.
It can be removed. I'll submit a PR to fix :).

smallorange · 2024-01-23T08:08:12Z

this PR is incomplete, right? because the project still does not build without dbus-glib, it references the old API in various places and the build system still forces it in
Hi,

I finished all necessary GDbus implementation of it but I have not yet tweaked the dependency (dbus-glib-1) of it. I need to find a way to switch between dbus-binding-tool and glib-compile-resources for dbus-glib and gdbus in Makefile.am.
I wonder if the dbus-glib can be completely removed from thermald or it has to be kept?

q66 · 2024-01-23T12:12:47Z

it has to be removed completely for this to make sense (because distros will be dropping dbus-glib)

you should not need dbus-binding-tool in the end, just remove the header inclusion and tweak things around it; the other problem is that not all assumptions around dbus-glib are gone yet (e.g. there are still calls to API to set up integration of glib mainloop with dbus in other places here)

zhang-rui · 2024-01-23T13:26:23Z

src/thd_dbus_interface.cpp

+int thd_dbus_server_init(gboolean (*exit_handler)(void)) {
+	GError *error = NULL;
+	PrefObject *value_obj;
+


Maybe need to add this line?
thd_dbus_exit_callback = exit_handler;

@spandruvada , can you check if terminate dbus message still work?

In my case, intel_lpmd fails to terminate via dbus message without setting the proper exit handler.

smallorange · 2024-01-24T06:11:54Z

it has to be removed completely for this to make sense (because distros will be dropping dbus-glib)

For this, I hope @spandruvada can answer this or @zhang-rui can help with this. Should the dbus-glib be removed completely or keep it to support the user who use dbus-glib?

you should not need dbus-binding-tool in the end, just remove the header inclusion and tweak things around it; the other problem is that not all assumptions around dbus-glib are gone yet (e.g. there are still calls to API to set up integration of glib mainloop with dbus in other places here)

Right. Some of the dbus-glib placed in thd_cdev_modem.cpp should be removed but it is a bit complicated for me. I need to find a way to test it since I don't have the appropriate hardware to test it. According to the commit message "Added support for modem throttling on SoFIA-3GR", it seems to support throttling SoFIA-3GR platform.

Kate Hsuan added 2 commits November 23, 2023 15:31

The definition of gresource for dbus introspection

94a09a8

Load dbus introspection to gresource Signed-off-by: Kate Hsuan <hpa@redhat.com>

src: thd_dbus_interface: Fix GObject definition

8df1de9

Fixed it to fit the current GObject definition scheme. Signed-off-by: Kate Hsuan <hpa@redhat.com>

smallorange force-pushed the kate/gdbus branch from b02bcbc to de1ae2e Compare November 24, 2023 02:42

Kate Hsuan added 3 commits November 24, 2023 11:15

src: The GDBus framework and the API functions

1910055

This is the GDBus framework for the Dbus API implementation. The corresponding functions are put to the DBus API method. Signed-off-by: Kate Hsuan <hpa@redhat.com>

Makefile.am: Generate gresource code before compiling

3c08971

Convert the gresource definition to C code before compiling. Signed-off-by: Kate Hsuan <hpa@redhat.com>

configure.ac: A parameter to disable GDBus

5730c30

The GDBus will be the default backend since glib-dbus will be deprecated soon. --disable-gdbus can be set to switch to glib-dbus. Signed-off-by: Kate Hsuan <hpa@redhat.com>

smallorange force-pushed the kate/gdbus branch from de1ae2e to 5730c30 Compare November 24, 2023 03:15

smallorange changed the title ~~Draft: Change Dbus library from glib-dbus to GDBus~~ Change Dbus library from glib-dbus to GDBus Nov 24, 2023

smallorange changed the title ~~Change Dbus library from glib-dbus to GDBus~~ Migrate from glib-dbus to GDBus Nov 24, 2023

smallorange changed the title ~~Migrate from glib-dbus to GDBus~~ Migrate from dbus-glib to GDBus Nov 28, 2023

Fix Makefile

ab3a332

spandruvada mentioned this pull request Jan 19, 2024

V2.5.5 merge branch #427

Merged

spandruvada closed this Jan 19, 2024

zhang-rui reviewed Jan 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate from dbus-glib to GDBus #426

Migrate from dbus-glib to GDBus #426

smallorange commented Nov 23, 2023 •

edited

smallorange commented Dec 4, 2023

spandruvada commented Dec 6, 2023

zhang-rui commented Dec 17, 2023

smallorange commented Dec 18, 2023

zhang-rui commented Dec 18, 2023

smallorange commented Dec 19, 2023

zhang-rui commented Dec 20, 2023

spandruvada commented Jan 19, 2024

spandruvada commented Jan 19, 2024

smallorange commented Jan 19, 2024 •

edited

spandruvada commented Jan 19, 2024

q66 commented Jan 22, 2024

zhang-rui Jan 23, 2024

smallorange Jan 23, 2024

zhang-rui Jan 23, 2024

zhang-rui Jan 23, 2024

smallorange Jan 23, 2024 •

edited

smallorange commented Jan 23, 2024

q66 commented Jan 23, 2024

zhang-rui Jan 23, 2024

smallorange commented Jan 24, 2024

Migrate from dbus-glib to GDBus #426

Migrate from dbus-glib to GDBus #426

Conversation

smallorange commented Nov 23, 2023 • edited

smallorange commented Dec 4, 2023

spandruvada commented Dec 6, 2023

zhang-rui commented Dec 17, 2023

smallorange commented Dec 18, 2023

zhang-rui commented Dec 18, 2023

smallorange commented Dec 19, 2023

zhang-rui commented Dec 20, 2023

spandruvada commented Jan 19, 2024

spandruvada commented Jan 19, 2024

smallorange commented Jan 19, 2024 • edited

spandruvada commented Jan 19, 2024

q66 commented Jan 22, 2024

zhang-rui Jan 23, 2024

Choose a reason for hiding this comment

smallorange Jan 23, 2024

Choose a reason for hiding this comment

zhang-rui Jan 23, 2024

Choose a reason for hiding this comment

zhang-rui Jan 23, 2024

Choose a reason for hiding this comment

smallorange Jan 23, 2024 • edited

Choose a reason for hiding this comment

smallorange commented Jan 23, 2024

q66 commented Jan 23, 2024

zhang-rui Jan 23, 2024

Choose a reason for hiding this comment

smallorange commented Jan 24, 2024

smallorange commented Nov 23, 2023 •

edited

smallorange commented Jan 19, 2024 •

edited

smallorange Jan 23, 2024 •

edited