Proposal to change backwards compatibility constraints on D-Bus interface revisions

[mulkieran]

Background: Currently we release new revisions of the D-Bus interfaces that stratisd supports with every stratisd minor release. The constraint that we previously imposed, in the 2.0 releases, on successive revisions is that each revision supports all the D-Bus methods and properties that the previous revision supported. We have always allowed D-Bus methods to extend their argument list with each new revision.

Proposal: In a new D-Bus interface revision for the 3.0 release, allow dropping properties and methods of the previous revision. Also allow dropping arguments from the argument list for existing D-Bus methods.

Motivation: Our code will be simpler and the simplicity will prevent some bugs that otherwise would occur.

Justification: In semantic versioning, the act of increasing the minor version is used to indicate that the associated release contains an enhancement. For stratisd, we consider the new revisions of each stratisd interface to constitute an optional enhancement that a client may choose to take advantage of, but is not obliged to. The client may continue to use the revisions of the Stratis D-Bus interfaces associated with the previous minor version, and in that case, it may expect that stratisd will continue to work as before. Thus, backwards compatibility in stratisd across minor releases is correctly preserved.

Remarks: stratisd has never been tested with and does not support a mixture of revision numbers for different Stratis D-Bus interfaces. A client must select just one revision number for all Stratis D-Bus interfaces.

[Conan-Kudo]

Remarks: stratisd has never been tested with and does not support a mixture of revision numbers for different Stratis D-Bus interfaces. A client must select just one revision number for all Stratis D-Bus interfaces.

I'm not sure that will happen in practice. People tend to add API usage ad-hoc based on what they see, so you'll wind up seeing mixtures in practice.

[mvollmer]

Yeah, this is what has happened in Cockpit. We use

• org.storage.stratis2.Manager.r1

• org.storage.stratis2.FetchProperties.r2

• org.storage.stratis2.FetchProperties.r4

• org.storage.stratis2.pool.r1

• org.storage.stratis2.blockdev.r2

• org.storage.stratis2.filesystem

• org.storage.stratis3.Manager.r0

• org.storage.stratis3.pool.r0

• org.storage.stratis3.blockdev.r0

• org.storage.stratis3.filesystem.r0

So it's messy for stratis2 but okay for stratis3. But going forward, I was planning to only increase revisions if there is a need... I'll adjust the code to make sure that interfacce revisions are always the same.

[mulkieran]

Just a note: In the original design of the backwards compatible D-Bus interfaces, the revision # for a particular interface was only bumped if there was a change to the interface that had to be made. So, it would be natural for the numbers of the revisions that a client was using to differ, because one revision # might cover multiple conceptual revisions corresponding to totally identical interfaces. This was more confusing than bumping every revision #, regardless of whether the interface had actually changed or not.

So, in the new system, bumping the revision #s in lockstep should be less mental work for the developer of a client as well.

[mulkieran]

Example: Interface "org.storage.stratis3.blockdev.r0"[1] has a D-Bus property UserInfo and a method SetUserInfo which allows a client to set the associated block device's user info data. For the proposed "org.storage.stratis3.blockdev.r1" interface we propose to drop the SetUserInfo method and make the UserInfo property writeable as well as readable. With the restrictions we had in place for stratisd 2.0, it is permissible to make the UserInfo property writeable, but dropping the SetUserInfo method is forbidden.

[1] https://stratis-storage.github.io/blockdev.xml

[mvollmer]

Personally, I am a big fan of backwards compatibility in APIs and this proposal actually keeps it. Compatibility would only be lost when old revisions of interfaces are removed completely, but you are not proposing this. Nobody will be broken just because stratisd is updated to a new minor version.

Updating a client to use a newer minor version of the interfaces might be more work with this proposal than before, but as long as it is possible without changes to the actual functionality, there should be not too many complaints. Good documentation always helps, of course.

What you propose in your example is okay, but removing org.storage.stratis3.blockdev.r0 or removing any means to manage the user info with org.storage.stratis3.blockdev.r1 would not be okay.

[mulkieran]

And we will happily take advice on how we can improve our D-Bus API documentation. We felt that posting the introspection data, w/ detailed comments, on our website[1], was a good step in that direction already, but further suggestions would also be appreciated, especially if easy to maintain.

[1] https://stratis-storage.github.io/

[mulkieran]

Example: Removing useless argument in CreatePool. Currently the CreatePool method in the org.storage.stratis3.Manager.r0 method takes an argument, redundancy which is meaningless[1] as the only acceptable values are (False, <any u16>) meaning "accept the default" which is 0, and (True, 0) which specifies the default, still 0. Under the proposed rule, it would be allowed to remove the redundancy argument entirely in org.storage.stratis3.Manager.r1.

[1] https://stratis-storage.github.io/manager.xml

[mulkieran]

We went ahead with this with what we believe will be a satisfactory solution. Discussions don't close, and this one doesn't turn well into an issue, so I'm just locking it.