[libvirt PATCH 07/11] virnetclient: Use 'if' consistently

[Andrea Bolognani]

On Mon, Feb 14, 2022 at 04:56:37PM +0000, Daniel P. Berrangé wrote:
> On Mon, Feb 14, 2022 at 08:09:57AM -0800, Andrea Bolognani wrote:
> > > > Can you please provide pointers to the OpenStack implementation of
> > > > this and the issue that resulted from introducing virt-ssh-helper?
> > >
> > > I don't know where the code is. I just know that they were broken
> > > by our changes in this area.
> >
> > I have found some of the issues filed at the time
> >
> >   https://bugs.launchpad.net/tripleo/+bug/1918250
> >   https://bugzilla.redhat.com/show_bug.cgi?id=1936804
> >
> > and the corresponding fix
> >
> >   https://github.com/rdo-packages/nova-distgit/commit/d5aba75f3b5589e156afeec915dcfcd4eca4eafe
> >
> > The detection logic, as currently implemented, is a bit fragile, but
> > updating it so that it keeps working even as we make minor
> > adjustments to our ssh tunnel script wasn't particularly difficult
> >
> >   https://github.com/andreabolognani/nova-distgit/commit/27cee8da127c1d447cfb694d54c209a94dd804de
>
> It isn't about whether it is difficult or not though. It is showing
> that the changes in libvirt are creating a compatibility problem for
> existing application code, and that any changes we make here will
> require further changes in such applications.  I've not been explicit
> about back compatibility in this area, as we didn't realize apps would
> be sensitive to changes.  Now we know that, I don't think we should be
> knowingly making further changes that are likely to break apps.

I think it's unlikely that other management applications are
performing this kind of naive string matching on our ssh tunnel
script, as evidenced by the fact that there haven't been widespread
breakages reported at the time virt-ssh-helper was introduced.

While obviously not definitive proof of this, a search for
"virt-ssh-helper" on GitHub

  https://github.com/search?q=virt-ssh-helper&type=code

only finds the OpenStack code that was affected by this; searching
for "nc ARG -U"

  https://github.com/search?q=%22nc+ARG+-U%22&type=code

brings up OpenStack again and a project called "virt-access" which
hasn't seen a single update in 7 years. So it seems reasonable to
conclude that, as long as we update OpenStack before merging these
changes, they're not going to break any deployment.

I've also just realized that the OpenStack fix mentioned above is
less than two weeks old, so there's probably a fair chance that it
hasn't made it into a release yet and we can replace the current
approach with the more resilient one I implemented in a completely
seamless manner.

-- 
Andrea Bolognani / Red Hat / Virtualization