Comment 16 for bug 1969734

Revision history for this message
Chris Halse Rogers (raof) wrote :

I think what I'm missing in reviewing this is some high-level description of what the patch does, and how it relates to the interaction of openconnect & network-manager-openconnect.

In the SRU team we get asked to review packages from all over the archive; the uploader presumably has a significantly deeper understanding of the package and how it fits into its immediate dependencies than we do. We might be missing system-level context that seems obvious to the uploader, so it's often useful for them to provide such context.

From the diff, it seems that the changes are in how translates the netmask option in the "config xml" into VPN config. But I don't know where the "config XML" comes from (is it NetworkManager? Is it sent over the wire from the VPN server? Is it read from an openconnect-specific config file, not set by anything else?).

From an SRU perspective, a significant concern of ours is “will this disrupt anyone's working system”. NetworkManager is not the only way to configure openconnect, right? How likely is it that a non-NetworkManager configuration could be using this feature? Since the bug seems to entirely break common NetworkManager/openconnect configurations, users are likely to have tried work-arounds. *Are* there any work-arounds for this bug, and how would they be affected by this revert? These are the sorts of questions we want answered in the [Where problems could occur] section.