There was 3 patches posted on the mailing list, but only one here ?

For the upload tool and instructions:
http://blog.fishsoup.net/2008/11/16/git-bz-bugzilla-subcommand-for-git/
https://wiki.gnome.org/Git/Developers

(In reply to comment #1)
> There was 3 patches posted on the mailing list, but only one here ?
> 
> For the upload tool and instructions:
> http://blog.fishsoup.net/2008/11/16/git-bz-bugzilla-subcommand-for-git/
> https://wiki.gnome.org/Git/Developers

Yes but I used "Add an attachment" option in web interface instead of git-bz. Via web interface I could attach only one patch, so I merged all three patch into one file. But thanks for advise - next time I'll use git-bz.

You can add as many as you want after having the bug created,
https://bugzilla.gnome.org/attachment.cgi?bugid=721861&action=enter

It's better not to squash patches like this.

Created attachment 265895 [details] [review]
Import kdbus interface header

Created attachment 265896 [details] [review]
Add preliminary implementation of kdbus support

Created attachment 265897 [details] [review]
Integrate kdbus into GDBus core

I've splitted previous patch in 3 patches (as it was posted before on the mailing list). I hope hope it's better readable now.

Here [1] you can find some simple tests for method calls (test_server and test_client apps) and test for org.freedesktop.DBus methods (which are handled directly in glib code now). More info how to test it you can find in README file.

In case of test_server and test_client apps sometime you can get TIMEOUT error, but this is bug in kdbus code (I'm working on it now).

[1] https://github.com/lukasz-skalski/glib-kdbus-tests 

---
Please notice that glib-kdbus repo, which you can find on my private GitHub is not up to date, you have to use below patchset.

Hey Lukasz, thanks for putting this work here ! I have some initial impressions / questions - not a full review yet:

1. I wonder if it makes sense to expose GKdbus and GKdbusConnection as public API. As a first approximation, I would expect that apps just get a different bus address, and that makes things magically open a kdbus device instead of a regular dbus connection, and they never have to do anything else - in particular not use specific kdbus api.

2. It would be really good to have a TODO list for whats still missing from this patchset for a complete port. From quickly glancing through the code, I see at least:
- StartServiceByName
- Match rule / bloom filter implementation
- Credentials / authentication
Anything else ?

3. I notice that there's a bunch of empty boilerplate in gkdbusconnection.c, like empty get/set_property and finalize methods - best to just leave those out entirely to keep things clean and small.

4. The gkdbus.c code has get/set_property code for a number of properties, but does not actually define those properties in class_init. That won't work - gobject dispatches get/set_property calls to the class which defines the properties, so your setter code will never get run. Unless you install paramspecs for your properties.

I had a nice conversation with Lennart and Simon today.  We came to the following conclusions that are important to note here:

 - there now exists something called the bus driver which implements the
   org.freedesktop.DBus interface on kdbus.  We will use this for all of our
   bus calls in preference to emulating them in GLib.  This includes AddMatch
   (for which the driver has the ability to request that the kernel modify
   our bloom filter on our behalf)

 - there will be a new version negotiation protocol added whereby on connect
   we can find out that we don't support the version of kdbus that the kernel
   (and systemd) is speaking and fall back to using dbus-1 via socket instead.

   This is sort of the 'nuclear option' for dealing with making necessary
   incompatible changes to the kernel interface and it gives me more faith
   that we will be able to correct design mistakes if they arise.  In other
   words, connecting via kdbus is (strictly speaking) 'best effort', but it
   is an effort that will always succeed as long as a distribution pays
   attention to keeping their software versions properly in sync.

 - we're going to try to get memfd turned into a proper kernel interface so
   that we are not afraid of exposing it via a special type of GBytes.  We
   will discuss this more during FOSDEM.

 - we want to use the new blocking ioctl interface for dispatching sync calls
   directly from their thread of origin (instead of going via the worker) but
   we will first need support added for cancellation (from another thread,
   while the call is in progress)

 - it may be possible to shortcut the worker thread in some other situations
   as well -- we will have to look into this, while being careful not to break
   message ordering or filters.  Not a high priority.

 - we may get some kind of copy-paste code or library that we can use for
   filling in the bloom filter when broadcasting signals.  This together with
   using the AddMatch interface of the bus driver will help ensure that we can
   easily adjust to protocol improvements

 - there is some desire to add support for maybe types, isolated dictionary
   entries and the unit type to dbus-1

 - the original plan for how activation would work (passing the fd) has been
   scrapped in favour of an approach much closer to that of dbus-1, so this
   is no longer something that we need to worry about

 - systemd is going to grow support for scanning desktop files and treating
   the DBusActivatable ones as services, via a DBusService= or ServiceExec=
   line in the file (which will be added to the spec)

Created attachment 267328 [details] [review]
[PATCH 1/4][RFC v3] gdbus: Import kdbus interface header

Created attachment 267329 [details] [review]
[PATCH 2/4][RFC v3] gdbus: Add preliminary implementation of kdbus support

Created attachment 267330 [details] [review]
[PATCH 3/4][RFC v3] gdbus: Integrate kdbus into GDBus core

Created attachment 267331 [details] [review]
[PATCH 4/4][RFC v3] glib: Add SipHash algorithm

I've just uploaded new version of patchset - v3.

Changes since v2:
=================

 - switched to major protocol version 2 (GVariant Serialization),

 - added support for systemd-bus-driverd (bus driver implements
   all org.freedesktop.DBus methods on kdbus, so now we don't
   have to process the bus driver messages locally),

 - implemented SipHash algorithm and bloom filters (it fix problems
   with broadcast and unicast signals),

 - GKdbus and GKdbusConnection aren't exposed as a public API,

 - caught up all the latest kdbus changes (ff53619bb7),

 - rebased on top of glib's master branch (8548d16add),

 - fix problem with g_dbus_connection_close_sync(),

 - some minor changes (support for KDBUS_ITEM_CONN_NAME,
   mem leaks, ...),


 Still missing pieces:
 ~~~~~~~~~~~~~~~~~~~~~

 - implement rest of org.freedesktop.DBus methods which allows process
   the bus driver messages locally if system-bus-driverd isn't available
   in system,

 - add support for memfd and fds,

 - more tests...


Short description:
==================

Due to adding support for systemd-bus-driverd and GVariant Serialization
glib with kdbus support can be easily tested in systemd-nspawn containter.

1) setup a container with systemd-bus-proxyd:

http://people.freedesktop.org/~kay/kdbus-container-install.txt

2) compile latest glib witk kdbus support:

git clone git://review.tizen.org/platform/upstream/glib.git -b sandbox/lskalski/kdbus-integration
./configure --enable-kdbus-transport
make && make install

3) export LD_LIBRARY_PATH and DBUS_SYSTEM_BUS_ADDRESS:

export LD_LIBRARY_PATH=absolute_path_to_glib_libs_with_kdbus_patch
export DBUS_SYSTEM_BUS_ADDRESS=kernel:path=/dev/kdbus/0-system/bus

4) Test your apps...

We will be happy to hear any and all of your comments.

Thanks!
Lukasz Skalski
l.skalski@samsung.com

(In reply to comment #9)
> Hey Lukasz, thanks for putting this work here ! I have some initial impressions
> / questions - not a full review yet:
> 

Hi Matthias,

> 1. I wonder if it makes sense to expose GKdbus and GKdbusConnection as public
> API. As a first approximation, I would expect that apps just get a different
> bus address, and that makes things magically open a kdbus device instead of a
> regular dbus connection, and they never have to do anything else - in
> particular not use specific kdbus api.
> 

Yes, you've right. It's already done in patchset v3.

> 2. It would be really good to have a TODO list for whats still missing from
> this patchset for a complete port. From quickly glancing through the code, I
> see at least:
> - StartServiceByName
> - Match rule / bloom filter implementation
> - Credentials / authentication
> Anything else ?
> 

I'll prepare detailed TODO list after FOSDEM (I hope there will be some important discussions about further changes in kdbus).

> 3. I notice that there's a bunch of empty boilerplate in gkdbusconnection.c,
> like empty get/set_property and finalize methods - best to just leave those out
> entirely to keep things clean and small.
> 
> 4. The gkdbus.c code has get/set_property code for a number of properties, but
> does not actually define those properties in class_init. That won't work -
> gobject dispatches get/set_property calls to the class which defines the
> properties, so your setter code will never get run. Unless you install
> paramspecs for your properties.

I'll fix this soon.

Thanks,
Lukasz
l.skalski@samsung.com

(In reply to comment #10)

>  - there will be a new version negotiation protocol added whereby on connect
>    we can find out that we don't support the version of kdbus that the kernel
>    (and systemd) is speaking and fall back to using dbus-1 via socket instead.
> 
>    This is sort of the 'nuclear option' for dealing with making necessary
>    incompatible changes to the kernel interface and it gives me more faith
>    that we will be able to correct design mistakes if they arise.  In other
>    words, connecting via kdbus is (strictly speaking) 'best effort', but it
>    is an effort that will always succeed as long as a distribution pays
>    attention to keeping their software versions properly in sync.

This already exists in the bus_flags upper 32bit. Clients libs just need to implement this properly.
 
>  - we're going to try to get memfd turned into a proper kernel interface so
>    that we are not afraid of exposing it via a special type of GBytes.  We
>    will discuss this more during FOSDEM.

Ryan, you will be at FOSDEM? That'd be great!

Anyway, I wouldn't hold my breath to turn this into a syscall...

>  - we want to use the new blocking ioctl interface for dispatching sync calls
>    directly from their thread of origin (instead of going via the worker) but
>    we will first need support added for cancellation (from another thread,
>    while the call is in progress)

Cancellation is supported now by kdbus.

>  - we may get some kind of copy-paste code or library that we can use for
>    filling in the bloom filter when broadcasting signals.  This together with
>    using the AddMatch interface of the bus driver will help ensure that we can
>    easily adjust to protocol improvements

No, this idea is dead. Instead we devised some changes in the api to allow a) making the bloom filter size and number of hash functions variable, so that we can change these parameters later on, and b) allow extensions of the vocabulary. Together with the "nuclear" option this should be enough to make sure we can update things.

Created attachment 273596 [details] [review]
gdbus: Import kdbus interface header

Created attachment 273597 [details] [review]
gdbus: Add preliminary implementation of kdbus support

Created attachment 273598 [details] [review]
glib: Add SipHash algorithm

Created attachment 273599 [details] [review]
gdbus: Integrate kdbus into GDBus core

I've just uploaded new version of patchset - v4.

Changes since v3:
=================

 - added support for file descriptor passing (KDBUS_ITEM_FSD) -
   it solves problem with gdbus-example-unix-fd-client test:

      root:/tests> ./gdbus-example-server &
      [2] 15662
      root:/tests> ./gdbus-example-unix-fd-client
      On Fri Apr  4, gdbus-example-unix-fd-client with pid 15683 was here!
      Wrote the following on server's stdout:
      On Fri Apr  4, gdbus-example-unix-fd-client with pid 15683 was here!

 - fixed problem with broadcasting signals without body,

 - fixed serious problem with with receiving messages if they are queued in
   memory pool,

 - added initial support for memfd objects,

 - added proper parsing of bus addresses (now it's consistent with PORTING-DBUS1
   document from systemd tree):

SYSTEM BUS:
kernel:path=/dev/kdbus/0-system/bus;unix:path=/var/run/dbus/system_bus_socket

USER BUS:
kernel:path=/dev/kdbus/$UID-user/bus;unix:path=$XDG_RUNTIME_DIR/bus

      If glib is compiled with --enable-kdbus-transport flag, kdbus is preferred
      over the legacy AF_UNIX socket, but compatibility is kept, so user can
      switch between standard socket and kdbus transport.

 - preserved compatibility with dbus-1 marshalling,

 - added initial support for splitting messages,

 - reworked gkdbusconnection code (now it's more consistent with
   gsocketconnection code),

 - reworked bloom filters code (bloom filers' parameters are read from HELLO
   struct),

 - caught up all the latest kdbus changes (commit b9ed5f94de3e),

 - rebased on top of glib's master branch (commit 20feb23569e6),

 - cleaned up code,


 Last missing pieces:
 ~~~~~~~~~~~~~~~~~~~~~

Native support for kdbus in glib is almost ready (ready to use in real system
with kdbus and systemd). Last missing pieces are:

 - add support for activators,

 - "native" AddMatch, RemoveMatch and StartServiceByName in glib:

   Do we really need this part?

   As Ryan wrote:

   "[...] now exists something called the bus driver which implements the
   org.freedesktop.DBus interface on kdbus.  We will use this for all of our
   bus calls in preference to emulating them in GLib.  This includes AddMatch
   (for which the driver has the ability to request that the kernel modify
   our bloom filter on our behalf)"

   Maybe we should remove all ioctls which are responsible for emulating
   org.freedesktop.DBus in GLib? Any suggestion?

 - rework memfd code:

   memfd code in kdbus will be completely reworked soon ([1],[2]), so I'm still
   waiting for this:

   [1] https://code.google.com/p/d-bus/source/browse/TODO
   [2] http://lwn.net/Articles/591108/


Short description:
==================

GLib (with native kdbus support) now is ready to test in real system with latest
kdbus and systemd, so we don't need now "systemd-bus-proxyd" for all application
which use gdbus.

Here are some outputs from my system:

http://www.lukasz-skalski.com/wp-content/uploads/2014/04/glib-log.txt

Any help with further testing will be greatly appreciated.


Thanks, 
Lukasz Skalski
Samsung R&D Institute Poland
Samsung Electronics
l.skalski@samsung.com

Info from systemd mailing list:

"Lukas, Ryan,

just wanted to let you know that I am working to get rid of the bus
driverd in systemd after all, for kdbus. The reason is that leaving it
in, as it is now, is very racy, and not just theorectically, but
IRL. Ryan, you might remember the "mouse cursor" bug when running gnome
on kdbus, we showed you two weeks ago in Nuremberg, this was caused by a
race between AddMatch being processed by the driver and a subsequent
method call to another service. Quite possibly, if the driver is
otherwise busy the method call might get executed earlier, possibly
triggering a signal, which is then not subject to the match that was to
be created. This bug only showed on gdbus since only there AddMatch is
called asychronously.

Anyway, I fixed this now by making the requirement that driver calls are
executed in order on the sender side (ie. the client translates driver
bus calls into ioctls). This means the bus proxy will execute them when
a legacy dbus1 client is connected, but it is expected that a native
kdbus client will process them locally instead of ever letting them onto
the bus. Lukas, this effectively means that your initial gdbus patch was
the right way to go after all, and my request to let the driverd be in
charge was quite mislead. Sorry!

I will now remove the entirety of the driver daemon, not just
AddMatch/RemoveMatch as all but two of those calls are subject to
similar (theoretical...) races.

Ryan, I know that you really don't like having to translate the bus
driver calls on the client side, but I don't see any other way that
fixes the race and was somewhat clean to me. [...]"

Ok, so now I'll have to change my plans (till today I've worked on full test coverage for my patchset). 

Now my priority is prepare and upload new version of my patchset for gdbus
(with translating all bus calls into ioctls directly in library), as
soon as it's possible.

Latest TODO list:
- support for translating all bus calls into ioctls directly in lib,
- support for activators,
- rework memfd code,

Thanks, 
Lukasz Skalski
Samsung R&D Institute Poland
Samsung Electronics
l.skalski@samsung.com

After some discussion between Ryan and Lennart during the GNOME DX hackfest last week, we have some major changes in approach to kdbus userspace implementation in GLib. Below short summary that I got from Ryan:

" The official story is that we will do something as follows:

 - we create two new bus types called "user" and "machine"

 - these ones try to connect to kdbus, falling back to the old "session"
   and "system" buses (respectively) in case of failure

 - the "session" and "system" bus types always connect to the dbus
   socket, as today

 - it is not permitted to make calls to the org.freedesktop.DBus
   destination on the new bus types

 - we introduce new library API for common tasks such as fetching the
   list of name owners on the bus.  It's not yet clear if this will be blocking
   or async since in the kdbus case this call is very fast, but in the old d-bus
   case it could be slightly slow

 - we may also introduce some convenience API on GDBusMessage to ask
   questions like "what is the pid of the sender?" in case it is already known
   (since we can get this information from kdbus)

[...]"

Because new approach needs a lot of new changes (new API, new bus types, ...), until now all 'kdbus support' development in GLib will be continued on external GitHub repo and will be discussed on a regular basis with Ryan:

https://github.com/lukasz-skalski/glib.git (branch: wip/kdbus-support)

Are there actually applications we expect to connect to both session and user simultaneously?  Or both system and machine?

And what about GApplication? Will it switch to the user bus always? Or only when specified as the starter bus?

Indeed, what about the starter bus, if the passed in address is both a socket and a kdbus device node?

Cases like GApplication, where we expose the bus that we create as API are tricky.  Typically, the only thing that happens with that is that people register extra objects, but in theory, anything is possible.  My plan is to switch that over to the user bus and hopefully we don't get too many issues from that.

Note as well: user and session are not two separate buses -- only two names for the same bus, with different restrictions on what you can do.

The main reason behind this is that after the latest set of changes, kdbus is no longer providing the org.freedesktop.DBus bus destination.  As such, we cannot make calls to it.  Having "user" and "machine" bus is a way of having people opt-in to a situation where they can say "I will not make these calls" while still allowing old users to connect via the compatibility bridge.

I'm not crazy about this situation and it's been the topic of heated debate, but the "bus driver" (that used to provide org.freedesktop.DBus) has been killed and is apparently not coming back...

(In reply to comment #27)
> Cases like GApplication, where we expose the bus that we create as API are
> tricky.  Typically, the only thing that happens with that is that people
> register extra objects, but in theory, anything is possible.  My plan is to
> switch that over to the user bus and hopefully we don't get too many issues
> from that.

Well, they'll probably handle some calls on those objects too, and I would not find unlikely that some apps are calling GetConnectionUnixProcessId or similar to find the identity of the sender of a message.

How about a simpler g_dbus_set_kdbus_enabled(bool) global call?
It is opt-in in the same way and we can flip the default in a couple of releases if stuff works by then.

(In reply to comment #29)

> How about a simpler g_dbus_set_kdbus_enabled(bool) global call?
> It is opt-in in the same way and we can flip the default in a couple of
> releases if stuff works by then.

Because kdbus *requires* applications to perform access control (usually indirectly via PolicyKit), it would be creating security problems for us to silently switch _SYSTEM to _MACHINE.

(In reply to comment #30)
> (In reply to comment #29)
> 
> > How about a simpler g_dbus_set_kdbus_enabled(bool) global call?
> > It is opt-in in the same way and we can flip the default in a couple of
> > releases if stuff works by then.
> 
> Because kdbus *requires* applications to perform access control (usually
> indirectly via PolicyKit), it would be creating security problems for us to
> silently switch _SYSTEM to _MACHINE.

Then we shall never change the default. But having two buses doesn't seem a big improvement over a single switch, flipped when the app is ready.

Additionally, _set_kdbus_enabled() would be a disaster on a shared bus.

(In reply to comment #32)
> Additionally, _set_kdbus_enabled() would be a disaster on a shared bus.

Not if it is only called by the application - which supposedly knows when all libraries are ready to be ported, and can enable kdbus at once for everything, or keep it off until ready.

(In reply to comment #33)
> (In reply to comment #32)
> > Additionally, _set_kdbus_enabled() would be a disaster on a shared bus.
> 
> Not if it is only called by the application - which supposedly knows when all
> libraries are ready to be ported

It can't know that. The libraries might pull in plugins.

https://github.com/lukasz-skalski/glib/commits/wip/kdbus-support was last updated 4 weeks ago, so I wonder what is left to be done to get this merged? I have some time to work on this now, so it would be good to get it in this cycle (as kdbus looks like it iwll be proposed for kernel 3.18.

(In reply to comment #35)
> https://github.com/lukasz-skalski/glib/commits/wip/kdbus-support was last
> updated 4 weeks ago, so I wonder what is left to be done to get this merged? I
> have some time to work on this now, so it would be good to get it in this cycle
> (as kdbus looks like it iwll be proposed for kernel 3.18.

David, 

Right, kdbus after a long period of inactivity, is again under development - memfd mechanism was merged in 3.17 and whole kdbus seems to be getting closer to being in shape for merging, so now is the best time to return to 'kdbus support' development in GLib.

As I wrote in one of my previous messages, list of TODO's is as follow:

(A) Introduce new library API (only sync; new calls can be used on all bus types) for common tasks such as fetching the list of name owners on the bus, ...

Because it will not be permitted to make calls to the org.freedesktop.DBus destination on these new bus types I have to prepare API for such tasks as:

  * synchronously getting the unique ID of the bus (replacement for "GetId" on org.freedesktop.DBus),

  * synchronously getting a list of all currently-owned names on the bus (replacement for "ListNames" on org.freedesktop.DBus),

  * synchronously getting a list of all names that can be activated on the bus (replacement for "ListActivatableNames" on org.freedesktop.DBus),

  * synchronously getting the unique bus names of connections currently queued for the choosen name (replacement for "ListQueuedOwners" on org.freedesktop.DBus),

  * synchronously getting the unique connection name of the primary owner of given name (replacement for "GetNameOwner" on org.freedesktop.DBus),

  * synchronously checking if the specified name exists - currently has an owner (replacement for "NameHasOwner" on org.freedesktop.DBus),

  * synchronously getting the Unix user ID of the process connected to the bus (replacement for "GetConnectionUnixUser" on org.freedesktop.DBus),

  * synchronously getting the Unix process ID of the process connected to the bus (replacement for "GetConnectionUnixProcessID" on org.freedesktop.DBus),

and also:

  * synchronously launching the executable associated with a name (replacement for "StartServiceByName" on org.freedesktop.DBus),

  * synchronously getting the security context used by SELinux (replacement for "GetConnectionSELinux" on org.freedesktop.DBus),

(B) Add initial support for two new bus types called "user" and "machine".

As Ryan proposed "[...] these ones try to connect to kdbus, falling back to the old "session" and "system" buses (respectively) in case of failure. The "session" and "system" bus types always connect to the dbus socket, as today."

(C) Rework existing kdbus core

It will be the last and the most time-consuming task. Old ioctl mechanism for memfd's was replaced by syscalls so I'll have to rework/update once again my previous patchset for GLib.

---

Some weeks ago, I sent to Ryan mail with patch[1] which introduce new library API for kdbus purpose - point 'A' in the above list (of course at this moment this new API supports only standard DBUS, but I'll extend these calls after adding final kdbus support). Today I got the answer that he will look at it soon. So 'A' it is already almost done. The next step is to add the support for new buses - "user" and "machine" and now I'm working on this.

Any help will be greatly appreciated.

[1] https://github.com/lukasz-skalski/glib/commit/9a1a360095a91a74c1c487e56434adf2c431f9a5

Thanks, 
Lukasz Skalski
Samsung R&D Institute Poland
Samsung Electronics
l.skalski@samsung.com

(In reply to comment #36)
> Some weeks ago, I sent to Ryan mail with patch[1] which introduce new library
> API for kdbus purpose - point 'A' in the above list (of course at this moment
> this new API supports only standard DBUS, but I'll extend these calls after
> adding final kdbus support). Today I got the answer that he will look at it
> soon. So 'A' it is already almost done. The next step is to add the support for
> new buses - "user" and "machine" and now I'm working on this.
> 
> Any help will be greatly appreciated.
> 
> [1]
> https://github.com/lukasz-skalski/glib/commit/9a1a360095a91a74c1c487e56434adf2c431f9a5

Would it make more sense to just let gdbus-codegen generate proxy methods from dbus-daemon.xml, and expose that API? And then you've got async versions of everything too.

hi,

I've been reviewing the code in github over the past couple of days.  Here are my initial set of comments:


0) Change-Id tags in commit messages are not helpful

1) g_sighash24() should not be public API

  - ideally we should have this as a simple private utility function to
    kdbus

  - could also add to GChecksum, but i don't think it's generally useful
    enough to warrant this

2) namespace g_dbus_ for utility calls on GDBusConnection is not a good
   choice

  - logically these belong in g_dbus_connection_

  - might be even better to group this into g_dbus_bus_ as a special set
    of connection-related API but this could complicate things for
    language bindings

3) NameHasOwner is redundant given that we have g_dbus_name_get_owner()
   which itself is questionably useful given our higher-level APIs for
   this -- I'd still keep get_owner(), however

4) eventually, we should not host our own copy of the kdbus header but
   include it from the system kernel headers -- i assume that its
   addition in the branch is only meant to be temporary

5) i don't like the hardcoding of the /dev/kdbus/ paths -- I talked to
   Lennart about this at plumbers and he promised that we'd get symlinks
   in "more official" locations: /run and XDG_RUNTIME_DIR

6) doc strings "new bus type for kdbus transport" are not helpful: we
   need more text here about what these bus types are and how they
   differ from their older counterparts.  avoid "new" because the docs
   will stay the same long after kdbus has stopped being "new".

7) allowing DBUS_SESSION_BUS_ADDRESS to short-circuit connecting to
   kdbus is not going to work very well -- we need to have a different
   variable for this.  ditto the system bus.

8) reorder commits to avoid addition of new bus types with default
   addresses containing "kernel:" before this address type is supported
   in the lower layers

9) the G_IS_KDBUS_CONNECTION (connection->stream) checks everywhere are
   distasteful.  we should either have vfuncs or some more explicit
   "this is a kdbus connection" check there

10) GKdbusConnection is not a GIOStream in any meaningful sense so it
    should not subclass it

11) the choice to call the kdbus.c functions or _call_sync() based on
    which bus type potentially changes the order of message delivery: we
    short-circuit normal delivery (via the thread) by doing the sync
    calls directly, thus violating ordering guarantees.  I think we want
    to send directly from the main thread in some cases but we need to
    be more careful about order guarantees.

12) the previous comment is false because sending and receiving of
    messages is not yet supported at all....


I think we need to have some sort of a face-to-face (or at least IRC) meeting about high level design here.  I am interested in spending some time working on this over the coming weeks and David King is also going to help.

(In reply to comment #38)
> hi,
> 

Hi,

> I've been reviewing the code in github over the past couple of days.  Here are
> my initial set of comments:
> 
> 
> 0) Change-Id tags in commit messages are not helpful
> 

Yes of course - now I treat my github repo as a 'development repo' and before sending patches to official GLib repository I'll appropriately split all patches and rewrite commit messages.

> 1) g_sighash24() should not be public API
> 
>   - ideally we should have this as a simple private utility function to
>     kdbus
> 
>   - could also add to GChecksum, but i don't think it's generally useful
>     enough to warrant this
> 

Ok, I'll do it.

> 2) namespace g_dbus_ for utility calls on GDBusConnection is not a good
>    choice
> 
>   - logically these belong in g_dbus_connection_
> 
>   - might be even better to group this into g_dbus_bus_ as a special set
>     of connection-related API but this could complicate things for
>     language bindings
> 

Before I'll start reworking this part of code I think we should 

> 3) NameHasOwner is redundant given that we have g_dbus_name_get_owner()
>    which itself is questionably useful given our higher-level APIs for
>    this -- I'd still keep get_owner(), however
> 

Right - I'll remove it.

> 4) eventually, we should not host our own copy of the kdbus header but
>    include it from the system kernel headers -- i assume that its
>    addition in the branch is only meant to be temporary
> 
> 5) i don't like the hardcoding of the /dev/kdbus/ paths -- I talked to
>    Lennart about this at plumbers and he promised that we'd get symlinks
>    in "more official" locations: /run and XDG_RUNTIME_DIR
> 
> 6) doc strings "new bus type for kdbus transport" are not helpful: we
>    need more text here about what these bus types are and how they
>    differ from their older counterparts.  avoid "new" because the docs
>    will stay the same long after kdbus has stopped being "new".
> 
> 7) allowing DBUS_SESSION_BUS_ADDRESS to short-circuit connecting to
>    kdbus is not going to work very well -- we need to have a different
>    variable for this.  ditto the system bus.
> 
> 8) reorder commits to avoid addition of new bus types with default
>    addresses containing "kernel:" before this address type is supported
>    in the lower layers
> 
> 9) the G_IS_KDBUS_CONNECTION (connection->stream) checks everywhere are
>    distasteful.  we should either have vfuncs or some more explicit
>    "this is a kdbus connection" check there
> 
> 10) GKdbusConnection is not a GIOStream in any meaningful sense so it
>     should not subclass it
> 
> 11) the choice to call the kdbus.c functions or _call_sync() based on
>     which bus type potentially changes the order of message delivery: we
>     short-circuit normal delivery (via the thread) by doing the sync
>     calls directly, thus violating ordering guarantees.  I think we want
>     to send directly from the main thread in some cases but we need to
>     be more careful about order guarantees.
> 
> 12) the previous comment is false because sending and receiving of
>     messages is not yet supported at all....
> 
> 
> I think we need to have some sort of a face-to-face (or at least IRC) meeting
> about high level design here.  I am interested in spending some time working on
> this over the coming weeks and David King is also going to help.

Discussion about high level design (and points 4-12 from above list) will be very useful, especially that kdbus was already submitted for review to the kernel [1] and it's quite stable now. So, at the beginning, I propose IRC chat in next week. Ryan and David, what date and time would be the most convenient for you?

Thank you for your review and comments.

[1] https://lkml.org/lkml/2014/10/29/854

Created attachment 313275 [details] [review]
[PATCH 1/7][RFC v5] gbytes: substantial rework for kdbus purposes

Created attachment 313276 [details] [review]
[PATCH 2/7][RFC v5] glib-unix: add function to ensure an fd is sealed

Created attachment 313277 [details] [review]
[PATCH 3/7][RFC v5] glib-unix: add new internal header glib-linux.h

Created attachment 313278 [details] [review]
[PATCH 4/7][RFC v5] gvariant: substantial rework for kdbus purposes

Created attachment 313279 [details] [review]
[PATCH 5/7][RFC v5] gdbus: import kdbus interface header

Created attachment 313280 [details] [review]
[PATCH 6/7][RFC v5] gdbus: add kdbus core files

Created attachment 313281 [details] [review]
[PATCH 7/7][RFC v5] gdbus: integrate kdbus into GDBus core

Almost after one year since last status update, I would like to back to "kdbus
support in GLib" discussion, so that I've just uploaded new version of patchset.

Since v4, kdbus support for GLib has been completely rewritten (it could be even
quite difficult to find some common pieces with previous patchset). What is the
most important, now we are able to run whole GNOME session without dbus-daemon
and with native kdbus transport for all apps which use gdbus. Almost everything
works without significant regressions:

[root@archlinux lskalski]# systemctl --state=failed
0 loaded units listed. Pass --all to see loaded but inactive units, too.
To show all installed unit files use 'systemctl list-unit-files'.

[lskalski@archlinux~]$ systemctl --user --state=failed
0 loaded units listed. Pass --all to see loaded but inactive units, too.
To show all installed unit files use 'systemctl list-unit-files'.

And some screenshots:

https://plus.google.com/+LukaszSkalski/posts/X6VVT7BbKbE?pid=6181810101797755458&oid=102520525834824455120

This patchset is squashed version of my work [1] and Ryan Lortie's patches
(mainly for gvariant and gbytes) from wip/kdbus-junk branch [2]

GLib 2.44 with kdbus transport repository (for tests):

https://github.com/lukasz-skalski/glib

Changes since v4:
=================

 - gbytes substantial rework for kdbus purposes

   Main changes:

     * introduce a new type of GBytes: 'inline' -  this allows us to make a
       single allocation instead of two in the g_bytes_new() case,

     * new g_bytes_take_zero_copy_fd() function - function takes a memfd, seals
       it, and creates a GBytes based on it,

     * add g_bytes_get_zero_copy_fd() function - add a way to get the zero-copy
       fd back out of a GBytes that was created from one.

 - gvariant: substantial rework for kdbus purposes
  
   Main changes:

     * add GVariantVectors utility struct,

     * support serialising/deserialising to/from GVariantVectors,

     * support for single precision floats,

     * GVariantVectors serialisation tests,

 - add GKDBusWorker struct, bloom filters improvements, ...

 - caught up all the latest kdbus changes,

 - rebased on GLib 2.44.1,

 Still missing pieces:
 ~~~~~~~~~~~~~~~~~~~~~

 - security policies (to run GNOME session on my ArchLinux I had to comment out 
   internal kdbus policy in kdbus module - can talk,see and own checking -
   I hope that soon on systemd.conf, there will be occasion to talk about it
   with kdbus developers),

 - rebase onto 2.46,

 - more tests...

[1] https://github.com/lukasz-skalski/glib/commits/glib-kdbus-devel
[2] https://git.gnome.org/browse/glib/log/?h=wip/kdbus-junk

Thanks!
Lukasz Skalski
l.skalski@samsung.com

Hi, Mr Skalski.

I tried compile glib from your repo (https://github.com/lukasz-skalski/glib/tree/glib-kdbus-2-46-1) on my ArchLinux using PKGBUILD from https://projects.archlinux.org/svntogit/packages.git/plain/trunk/PKGBUILD?h=packages/glib2.

The compilation is success, but when I reboot my laptop, gdm failed to start. I think it caused by polkit.service that also failed to start.

Here is my polkit log from journalctl:
-- Reboot --
Nov 02 19:26:54 hafshin polkitd[385]: Started polkitd version 0.113
Nov 02 19:26:54 hafshin polkitd[385]: Loading rules from directory /etc/polkit-1/rules.d
Nov 02 19:26:54 hafshin polkitd[385]: Loading rules from directory /usr/share/polkit-1/rules.d
Nov 02 19:26:54 hafshin polkitd[385]: Finished loading, compiling and executing 3 rules
Nov 02 19:26:54 hafshin polkitd[385]: Lost the name org.freedesktop.PolicyKit1 - exiting
Nov 02 19:26:54 hafshin polkitd[407]: Started polkitd version 0.113
Nov 02 19:26:54 hafshin polkitd[407]: Loading rules from directory /etc/polkit-1/rules.d
Nov 02 19:26:54 hafshin polkitd[407]: Loading rules from directory /usr/share/polkit-1/rules.d
Nov 02 19:26:54 hafshin polkitd[407]: Finished loading, compiling and executing 3 rules
Nov 02 19:26:54 hafshin polkitd[407]: Lost the name org.freedesktop.PolicyKit1 - exiting
Nov 02 19:26:54 hafshin polkitd[424]: Started polkitd version 0.113
Nov 02 19:26:54 hafshin polkitd[424]: Loading rules from directory /etc/polkit-1/rules.d
Nov 02 19:26:54 hafshin polkitd[424]: Loading rules from directory /usr/share/polkit-1/rules.d
Nov 02 19:26:54 hafshin polkitd[424]: Finished loading, compiling and executing 3 rules
Nov 02 19:26:54 hafshin polkitd[424]: Lost the name org.freedesktop.PolicyKit1 - exiting
Nov 02 19:26:54 hafshin polkitd[442]: Started polkitd version 0.113
Nov 02 19:26:54 hafshin polkitd[442]: Loading rules from directory /etc/polkit-1/rules.d
Nov 02 19:26:54 hafshin polkitd[442]: Loading rules from directory /usr/share/polkit-1/rules.d
Nov 02 19:26:54 hafshin polkitd[442]: Finished loading, compiling and executing 3 rules
Nov 02 19:26:54 hafshin polkitd[442]: Lost the name org.freedesktop.PolicyKit1 - exiting
Nov 02 19:26:54 hafshin polkitd[457]: Started polkitd version 0.113
Nov 02 19:26:54 hafshin polkitd[457]: Loading rules from directory /etc/polkit-1/rules.d
Nov 02 19:26:54 hafshin polkitd[457]: Loading rules from directory /usr/share/polkit-1/rules.d
Nov 02 19:26:54 hafshin polkitd[457]: Finished loading, compiling and executing 3 rules
Nov 02 19:26:54 hafshin polkitd[457]: Lost the name org.freedesktop.PolicyKit1 - exiting
Nov 02 19:34:00 hafshin polkitd[737]: Started polkitd version 0.113
Nov 02 19:34:00 hafshin polkitd[737]: Loading rules from directory /etc/polkit-1/rules.d
Nov 02 19:34:00 hafshin polkitd[737]: Loading rules from directory /usr/share/polkit-1/rules.d
Nov 02 19:34:00 hafshin polkitd[737]: Finished loading, compiling and executing 3 rules
Nov 02 19:34:00 hafshin polkitd[737]: Lost the name org.freedesktop.PolicyKit1 - exiting

ups, sorry. didn't read about the module. which part in the module I must change to make it works.

(In reply to Abi Hafshin from comment #49)
> ups, sorry. didn't read about the module. which part in the module I must
> change to make it works.

I think polkit not working with kdbus is a known issue https://lists.tizen.org/pipermail/dev/2014-March/002220.html
But you may want to report it to polkit developers.

kdbus never got merged into the kernel and has been dropped by its developers in favour of bus1.