systemd: Don't open and close tuned DBus connections asynchronously

[mvollmer]

The TunedPerformanceProfile component was keeping a DBus connection open to tuned, and was closing and reopening it whenever something might have changed that requires it.

This happened asynchronously with the rest of the code that was using this DBus connection, and would frequently cause that code to fail with "disconnected" errors.

Also, the TunedDialog was itself causing changes that require a new DBus connection (namely starting the tuned.service), but was always working with the one that was originally passed into it via its properties.

Let's straighten this all out:

• Polling for the button state will open its own dedicated DBUs connection and close it when polling is done. There is no need to keep a DBus connection open for the whole Cockpit session.

• The tuned.service is started when the dialog is opened and a DBus connection is opened explicitly for the dialog after that and closed when the dialog is closed.

[mvollmer]

Starting the service might be slow and it would be best to do it inside the dialog, while the spinner is showing. That is probably straightforward.

Also, I think our DBusClient is supposed to survive the dbus service coming and going, maybe with some flags and owner watching. That would be another, even cleaner, way of handling this, by only having a single DBusClient that is never closed and always works.

[mvollmer]

by only having a single DBusClient that is never closed and always works.

(except when superuser.allowed changes)

[mvollmer]

Starting the service might be slow and it would be best to do it inside the dialog, while the spinner is showing. That is probably straightforward.

Also, I think our DBusClient is supposed to survive the dbus service coming and going, maybe with some flags and owner watching. That would be another, even cleaner, way of handling this, by only having a single DBusClient that is never closed and always works.

Done in #21115.

[mvollmer]

Tested locally, the dialog loads a lot faster!

This is due to this: https://github.com/cockpit-project/cockpit/pull/21114/files#diff-edcd27aeefd91df68948709cd630aaefbc569ac0674cc54693139880404e17bdR101-R108

[mvollmer]

Starting the service might be slow and it would be best to do it inside the dialog, while the spinner is showing. That is probably straightforward.

@jelly, what do you think about this? Should we start tuned.service while the dialog is already open? Then the TunedDialog component needs to maintain its own DBusClient ins a useState hook, probably.

[jelly]

Starting the service might be slow and it would be best to do it inside the dialog, while the spinner is showing. That is probably straightforward.

@jelly, what do you think about this? Should we start tuned.service while the dialog is already open? Then the TunedDialog component needs to maintain its own DBusClient ins a useState hook, probably.

Hmmm right, what if it fails the error message get's "lost", so maybe we should do the extra work (and right thing) And start the service in the dialog?

[mvollmer]

Starting the service might be slow and it would be best to do it inside the dialog, while the spinner is showing. That is probably straightforward.

@jelly, what do you think about this? Should we start tuned.service while the dialog is already open? Then the TunedDialog component needs to maintain its own DBusClient ins a useState hook, probably.

Hmmm right, what if it fails the error message get's "lost", so maybe we should do the extra work (and right thing) And start the service in the dialog?

Yes, catching the error is convincing. I have done this now. The explicit close() function for closing the DBus channel is not great, that should rather be done in a "destroy" function of useObject or useEffect... do we care enough to refactor the code more?

[mvollmer]

that should rather be done in a "destroy" function of useObject or useEffect... do we care enough to refactor the code more?

Yes, I did that as well.