Convert qubes.WindowIconUpdater to a socket service #37

pwmarcz · 2020-03-14T18:01:15Z

As a result, we have a single service running for all the VMs.

Start it using /etc/xdg/autostart to ensure it's running in the
proper graphical session.

Depends on Image.get_from_stream_async, implemented in
QubesOS/qubes-linux-utils#52.

pwmarcz · 2020-03-14T22:55:18Z

Ouch, this is not right, I think the cache needs to depend on domain ID...

marmarek · 2020-03-15T00:46:24Z

Not only cache, but also window ID map. Or at least the map should now have two fields (vm, remote window id). IMO it would be cleaner to have separate object instances for each incoming connection and keep domain-specific data there.
A little issue could be with dispatching local X events to the right instance, but we do already retrieve local window -> VM name mapping (search for atom_vmname), so it shouldn't be much of an issue.

pwmarcz · 2020-03-15T21:12:00Z

What do you think about the current code? I converted the "remote ID" to (domain_name, integer) tuple. This takes care of both cache and mapping.

I think creating separate object instances would make it less clean, because the X server connection is global and not related to either a specific domain or connection. You would need to filter X messages (which we receive globally) down to the right instance, and handle creating/destroying these instances.

With the current code, we store very little state per connection (just the local variables: domain and color), and we keep open the possibility of multiple connections from icon-sender, for instance per-request instead of per-session (that's not to say we should do that, but I think it shows things are more decoupled).

marmarek · 2020-03-15T21:50:37Z

Ok, makes sense.

window-icon-updater/icon-receiver

pwmarcz · 2020-03-18T21:27:55Z

By the way, master is still broken for me:

Mar 18 20:20:39 dom0 qubes.WindowIconUpdater+-work[7561]: Traceback (most recent call last):
Mar 18 20:20:39 dom0 qubes.WindowIconUpdater+-work[7561]:   File "/usr/lib/qubes/icon-receiver", line 361, in <module>
Mar 18 20:20:39 dom0 qubes.WindowIconUpdater+-work[7561]:     rcvd = IconReceiver()
Mar 18 20:20:39 dom0 qubes.WindowIconUpdater+-work[7561]:   File "/usr/lib/qubes/icon-receiver", line 72, in __init__
Mar 18 20:20:39 dom0 qubes.WindowIconUpdater+-work[7561]:     self.conn = xcb.connect()
Mar 18 20:20:39 dom0 qubes.WindowIconUpdater+-work[7561]:   File "/usr/lib/python3.7/site-packages/xcffib/__init__.py", line 526, in __init__
Mar 18 20:20:39 dom0 qubes.WindowIconUpdater+-work[7561]:     self.invalid()
Mar 18 20:20:39 dom0 qubes.WindowIconUpdater+-work[7561]:   File "/usr/lib/python3.7/site-packages/xcffib/__init__.py", line 558, in invalid
Mar 18 20:20:39 dom0 qubes.WindowIconUpdater+-work[7561]:     raise ConnectionException(err)
Mar 18 20:20:39 dom0 qubes.WindowIconUpdater+-work[7561]: xcffib.ConnectionException: xcb connection errors because of socket, pipe and other stream errors.

marmarek · 2020-03-18T21:47:42Z

Does it happen after logoff+logon?

pwmarcz · 2020-03-19T00:24:48Z

No, right after reboot.

marmarek · 2020-03-19T02:50:15Z

It might be because of the qrexec-policy-daemon. Since it's now this service calling qrexec-client, not qrexec-daemon, the actual service is called a) as root, b) with limited environment variables.

marmarek · 2020-03-28T03:01:57Z

Since QubesOS/qubes-core-qrexec#41 is merged, I assume this should make use of it, right?

marmarek · 2020-03-28T03:07:13Z

This also needs adjusted dependencies, for all the new things it requires:

qubes-core-qrexec >= 4.1.5 (socket services)
qubes-utils >= 4.1.4 (async image)

Both Fedora and Debian.

This makes icon-sender handle GUI restart, same as qubes-gui and audio. However, in this case we are not running raw vchan, but Qubes RPC, so the procedure is more complicated: we start a qrexec-client-vm subprocess, and restart it if it breaks. Needs icon-receiver to be a service with wait-for-session (QubesOS/qubes-gui-daemon#37), and fix for socket services in dom0 (QubesOS/qubes-core-qrexec#42).

marmarek · 2020-03-31T02:11:56Z

window-icon-updater/icon-receiver

+        while True:
+            await event.wait()
+            event.clear()
+            self.handle_pending_events()


Why not simply connect handle_pending_events() as a reader function directly?

Right. Updated.

As a result, we have a single service running for all the VMs. Start it using /etc/xdg/autostart to ensure it's running in the proper graphical session. Use (domain, ID) as remote IDs to prevent conflicts between windows. Depends on Image.get_from_stream_async, implemented in QubesOS/qubes-linux-utils#52.

Added to dom0 in QubesOS/qubes-core-qrexec#41.

- qubes-core-qrexec >= 4.1.5 (for socket services) - qubes-utils >= 4.1.4 (for Image.get_from_stream_async)

pwmarcz · 2020-03-31T17:06:38Z

Ouch, there is one more edge case: creating a new machine while the process is running :)

Newly created domains, and color changes.

pwmarcz force-pushed the icon-receiver-socket branch from def67a4 to c48f450 Compare March 14, 2020 18:17

pwmarcz changed the title ~~Convert qubes.WindowIconUpdater to a socket service~~ WIP: Convert qubes.WindowIconUpdater to a socket service Mar 14, 2020

pwmarcz force-pushed the icon-receiver-socket branch from c48f450 to 5b15353 Compare March 15, 2020 00:18

pwmarcz changed the title ~~WIP: Convert qubes.WindowIconUpdater to a socket service~~ Convert qubes.WindowIconUpdater to a socket service Mar 15, 2020

marmarek requested changes Mar 15, 2020

View reviewed changes

window-icon-updater/icon-receiver Outdated Show resolved Hide resolved

window-icon-updater/icon-receiver Outdated Show resolved Hide resolved

pwmarcz force-pushed the icon-receiver-socket branch from 5b15353 to 7b66d55 Compare March 16, 2020 13:01

pwmarcz mentioned this pull request Mar 28, 2020

icon-sender: reconnect on failure QubesOS/qubes-gui-agent-linux#92

Merged

marmarek requested changes Mar 31, 2020

View reviewed changes

pwmarcz added 3 commits March 31, 2020 13:20

qubes.WindowIconUpdater: use wait-for-session

bc7a21c

Added to dom0 in QubesOS/qubes-core-qrexec#41.

Add dependencies for qubes.WindowIconUpdater

4f67989

- qubes-core-qrexec >= 4.1.5 (for socket services) - qubes-utils >= 4.1.4 (for Image.get_from_stream_async)

pwmarcz force-pushed the icon-receiver-socket branch from 7b66d55 to 4f67989 Compare March 31, 2020 11:21

icon-receiver: use logging

ddc3377

icon-receiver: handle app.domains updates

05498da

Newly created domains, and color changes.

marmarek approved these changes Apr 4, 2020

View reviewed changes

marmarek merged commit e242064 into QubesOS:master Apr 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert qubes.WindowIconUpdater to a socket service #37

Convert qubes.WindowIconUpdater to a socket service #37

pwmarcz commented Mar 14, 2020

pwmarcz commented Mar 14, 2020

marmarek commented Mar 15, 2020

pwmarcz commented Mar 15, 2020

marmarek commented Mar 15, 2020

pwmarcz commented Mar 18, 2020

marmarek commented Mar 18, 2020

pwmarcz commented Mar 19, 2020

marmarek commented Mar 19, 2020

marmarek commented Mar 28, 2020

marmarek commented Mar 28, 2020

marmarek Mar 31, 2020

pwmarcz Mar 31, 2020

pwmarcz commented Mar 31, 2020

Convert qubes.WindowIconUpdater to a socket service #37

Convert qubes.WindowIconUpdater to a socket service #37

Conversation

pwmarcz commented Mar 14, 2020

pwmarcz commented Mar 14, 2020

marmarek commented Mar 15, 2020

pwmarcz commented Mar 15, 2020

marmarek commented Mar 15, 2020

pwmarcz commented Mar 18, 2020

marmarek commented Mar 18, 2020

pwmarcz commented Mar 19, 2020

marmarek commented Mar 19, 2020

marmarek commented Mar 28, 2020

marmarek commented Mar 28, 2020

marmarek Mar 31, 2020

Choose a reason for hiding this comment

pwmarcz Mar 31, 2020

Choose a reason for hiding this comment

pwmarcz commented Mar 31, 2020