1:52 p.m.
After 0.8.0 release, libssh deprecated some functions like:
ssh_is_server_known() and ssh_write_knownhost(). They were replaced by
ssh_session_is_known_server() and ssh_session_update_known_hosts()
respectively. This serie creates the alias to keep the compatibility and
create an auxiliar enum to help it because
ssh_session_update_known_hosts() introduced new state returns.
v1-v2: Rebasing ssh_session_is_known_server() return states.
v2-v3: Only code syntax fixes.
Julio Faracco (4):
m4: checking if ssh_session_is_known_server() exists.
rpc: replacing ssh_is_server_known() by ssh_session_is_known_server().
m4: checking if ssh_session_update_known_hosts() exists.
rpc: replacing ssh_write_knownhost() by
ssh_session_update_known_hosts().
m4/virt-libssh.m4 | 12 ++++++++++++
src/rpc/virnetlibsshsession.c | 18 +++++++++++++++++-
2 files changed, 29 insertions(+), 1 deletion(-)
--
2.19.1
1:52 p.m.
New subject: [libvirt] [PATCH v3 1/4] m4: checking if ssh_session_is_known_server() exists.
This commit adds some checks inside libssh m4 checking to verify if
ssh_session_is_known_server function is available. This new function
scope replaces the old ssh_is_server_known() from libssh 0.8.0 and
below versions.
Another auxiliar enumerator was added to keep the compatibility with the
old standard used by ssh_is_server_known() function.
Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com>
---
m4/virt-libssh.m4 | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/m4/virt-libssh.m4 b/m4/virt-libssh.m4
index 01c3b75c72..52bd4d3639 100644
--- a/m4/virt-libssh.m4
+++ b/m4/virt-libssh.m4
@@ -33,6 +33,14 @@ AC_DEFUN([LIBVIRT_CHECK_LIBSSH],[
[],
[AC_DEFINE_UNQUOTED([ssh_get_server_publickey], [ssh_get_publickey],
[ssh_get_publickey is deprecated and replaced by
ssh_get_server_publickey.])])
+ AC_CHECK_FUNC([ssh_session_is_known_server],
+ [],
+ [AC_DEFINE_UNQUOTED([ssh_session_is_known_server], [ssh_is_server_known],
+ [ssh_is_server_known is deprecated and replaced by
ssh_session_is_known_server.])])
+ AC_CHECK_TYPES([enum ssh_known_hosts_e],
+ [AC_DEFINE([HAVE_SSH_KNOWN_HOSTS_E], [1],
+ [Defined if enum ssh_known_hosts_e exists in libssh.h])],
+ [], [[#include <libssh/libssh.h>]])
CFLAGS="$old_CFLAGS"
LIBS="$old_LIBS"
fi
--
2.19.1
8:06 a.m.
New subject: [libvirt] [PATCH v3 1/4] m4: checking if ssh_session_is_known_server() exists.
On Sat, 2018-11-24 at 03:52 +0800, Julio Faracco wrote:
This commit adds some checks inside libssh m4 checking to verify if
ssh_session_is_known_server function is available. This new function
scope replaces the old ssh_is_server_known() from libssh 0.8.0 and
below versions.
Another auxiliar enumerator was added to keep the compatibility with the
old standard used by ssh_is_server_known() function.
You only have the check for the enum, not the definition of the
replacement one here. I guess you want to squash patch 2 in here?
[...]
+ AC_CHECK_FUNC([ssh_session_is_known_server],
+ [],
+ [AC_DEFINE_UNQUOTED([ssh_session_is_known_server], [ssh_is_server_known],
+ [ssh_is_server_known is deprecated and replaced by
ssh_session_is_known_server.])])
+ AC_CHECK_TYPES([enum ssh_known_hosts_e],
+ [AC_DEFINE([HAVE_SSH_KNOWN_HOSTS_E], [1],
+ [Defined if enum ssh_known_hosts_e exists in libssh.h])],
+ [], [[#include <libssh/libssh.h>]])
We check for the availability of the new function and the enum
containing its possible return values separately: is there a chance
we might find one but not the other? I'm hoping not, but it would be
nice if you could confirm that's indeed never going to happen, as it
would be quite problematic if it did...
--
Andrea Bolognani / Red Hat / Virtualization
1:52 p.m.
New subject: [libvirt] [PATCH v3 2/4] rpc: replacing ssh_is_server_known() by ssh_session_is_known_server().
After version 0.8.0, libssh deprecated the function scope
ssh_is_server_known() and moved to ssh_session_is_known_server().
So, libvirt is failing to compile using this new function name.
Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com>
---
src/rpc/virnetlibsshsession.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c
index 7c5f158f4d..eb6d6843a2 100644
--- a/src/rpc/virnetlibsshsession.c
+++ b/src/rpc/virnetlibsshsession.c
@@ -80,6 +80,22 @@ struct _virNetLibsshAuthMethod {
int tries;
};
+#ifndef HAVE_SSH_KNOWN_HOSTS_E
+/* This is an auxiliar enum to help libssh migration to version 0.8.0
+ * or higher. This enum associates the enumerator ssh_server_known_e
+ * with new ssh_known_hosts_e enum. In other words, it can be removed
+ * in the future. ERROR was moved from -1 to -2 and NOT_FOUND from 4
+ * to -1. */
+enum _vir_ssh_known_hosts_e {
+ SSH_KNOWN_HOSTS_ERROR = SSH_SERVER_ERROR,
+ SSH_KNOWN_HOSTS_UNKNOWN = SSH_SERVER_NOT_KNOWN,
+ SSH_KNOWN_HOSTS_OK,
+ SSH_KNOWN_HOSTS_CHANGED,
+ SSH_KNOWN_HOSTS_OTHER,
+ SSH_KNOWN_HOSTS_NOT_FOUND,
+};
+#endif
+
struct _virNetLibsshSession {
virObjectLockable parent;
virNetLibsshSessionState state;
--
2.19.1
8:18 a.m.
New subject: [libvirt] [PATCH v3 2/4] rpc: replacing ssh_is_server_known() by ssh_session_is_known_server().
On Sat, 2018-11-24 at 03:52 +0800, Julio Faracco wrote:
After version 0.8.0, libssh deprecated the function scope
ssh_is_server_known() and moved to ssh_session_is_known_server().
So, libvirt is failing to compile using this new function name.
Based on the commit message, I imagine this patch was supposed to
look a lot like patch 4 but the corresponding hunk somehow got lost,
and without it the whole thing doesn't make a whole lot of sense :)
[...]
+#ifndef HAVE_SSH_KNOWN_HOSTS_E
+/* This is an auxiliar enum to help libssh migration to version 0.8.0
+ * or higher. This enum associates the enumerator ssh_server_known_e
+ * with new ssh_known_hosts_e enum. In other words, it can be removed
+ * in the future. ERROR was moved from -1 to -2 and NOT_FOUND from 4
+ * to -1. */
The specific values are irrelevant, all we care about is that the
new function returns values from a new enum and the old one from the
old enum, so we need to create a bit of compatibility gunk ourselves
in order for internal callers not to care which version of libssh
we're building against; in the same way, the fact that we're going
to be able to drop the compatibility is not very interesting if the
condition for dropping it is omitted.
Please reword the comment accordingly.
+enum _vir_ssh_known_hosts_e {
Since we only get here if libssh's own enum is not present, I don't
see why we wouldn't just use ssh_known_hosts_e here.
+ SSH_KNOWN_HOSTS_ERROR = SSH_SERVER_ERROR,
+ SSH_KNOWN_HOSTS_UNKNOWN = SSH_SERVER_NOT_KNOWN,
+ SSH_KNOWN_HOSTS_OK,
+ SSH_KNOWN_HOSTS_CHANGED,
+ SSH_KNOWN_HOSTS_OTHER,
+ SSH_KNOWN_HOSTS_NOT_FOUND,
+};
+#endif
Okay, this works, but it's not extremely clear... I would very much
prefer if you explicitly assigned values taken from the old enum to
the new enum every single time instead of just for the first two.
Looks good if you address the comments above and squash it into the
previous patch.
--
Andrea Bolognani / Red Hat / Virtualization
1:52 p.m.
New subject: [libvirt] [PATCH v3 3/4] m4: checking if ssh_session_update_known_hosts() exists.
This commit adds some checks inside libssh m4 checking to verify if
ssh_session_update_known_hosts function is available. This new function
scope replaces the old ssh_write_knownhost() from libssh 0.8.0 and
below versions.
Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com>
---
m4/virt-libssh.m4 | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/m4/virt-libssh.m4 b/m4/virt-libssh.m4
index 52bd4d3639..f6d307f8d1 100644
--- a/m4/virt-libssh.m4
+++ b/m4/virt-libssh.m4
@@ -41,6 +41,10 @@ AC_DEFUN([LIBVIRT_CHECK_LIBSSH],[
[AC_DEFINE([HAVE_SSH_KNOWN_HOSTS_E], [1],
[Defined if enum ssh_known_hosts_e exists in libssh.h])],
[], [[#include <libssh/libssh.h>]])
+ AC_CHECK_FUNC([ssh_session_update_known_hosts],
+ [],
+ [AC_DEFINE_UNQUOTED([ssh_session_update_known_hosts], [ssh_write_knownhost],
+ [ssh_write_knownhost is deprecated and replaced by
ssh_session_update_known_hosts.])])
CFLAGS="$old_CFLAGS"
LIBS="$old_LIBS"
fi
--
2.19.1
8:22 a.m.
New subject: [libvirt] [PATCH v3 3/4] m4: checking if ssh_session_update_known_hosts() exists.
On Sat, 2018-11-24 at 03:52 +0800, Julio Faracco wrote:
[...]
+ AC_CHECK_FUNC([ssh_session_update_known_hosts],
+ [],
+ [AC_DEFINE_UNQUOTED([ssh_session_update_known_hosts], [ssh_write_knownhost],
+ [ssh_write_knownhost is deprecated and replaced by
ssh_session_update_known_hosts.])])
Looks good.
--
Andrea Bolognani / Red Hat / Virtualization
1:52 p.m.
New subject: [libvirt] [PATCH v3 4/4] rpc: replacing ssh_write_knownhost() by ssh_session_update_known_hosts().
After version 0.8.0, libssh deprecated the function scope
ssh_write_knownhost() and moved to ssh_session_update_known_hosts().
So, libvirt is failing to compile using this new function name.
Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com>
---
src/rpc/virnetlibsshsession.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c
index eb6d6843a2..5a7ca04dec 100644
--- a/src/rpc/virnetlibsshsession.c
+++ b/src/rpc/virnetlibsshsession.c
@@ -397,7 +397,7 @@ virNetLibsshCheckHostKey(virNetLibsshSessionPtr sess)
/* write the host key file, if specified */
if (sess->knownHostsFile) {
- if (ssh_write_knownhost(sess->session) < 0) {
+ if (ssh_session_update_known_hosts(sess->session) < 0) {
errmsg = ssh_get_error(sess->session);
virReportError(VIR_ERR_LIBSSH,
_("failed to write known_host file '%s':
%s"),
--
2.19.1
8:22 a.m.
New subject: [libvirt] [PATCH v3 4/4] rpc: replacing ssh_write_knownhost() by ssh_session_update_known_hosts().
On Sat, 2018-11-24 at 03:52 +0800, Julio Faracco wrote:
[...]
/* write the host key file, if specified */
if (sess->knownHostsFile) {
- if (ssh_write_knownhost(sess->session) < 0) {
+ if (ssh_session_update_known_hosts(sess->session) < 0) {
errmsg = ssh_get_error(sess->session);
virReportError(VIR_ERR_LIBSSH,
_("failed to write known_host file '%s':
%s"),
Looks good.
--
Andrea Bolognani / Red Hat / Virtualization
8 a.m.
On Sat, 2018-11-24 at 03:52 +0800, Julio Faracco wrote:
After 0.8.0 release, libssh deprecated some functions like:
ssh_is_server_known() and ssh_write_knownhost().
This is not accurate: 0.8.5 was released less than two months ago
and does not consider any of the functions you listed as deprecated;
the deprecation is indeed in place on the master branch, which will
eventually be released as 0.9.0 or whatever.
Speaking of which, I'm not entirely convinced being *this* proactive
with respect to libssh deprecations is useful when we could at least
wait for the next stable release to be out. I'll review the patches
in more detail regardless :)
--
Andrea Bolognani / Red Hat / Virtualization
1:43 p.m.
I agree about versioning and stability.
The idea behind the patches is just predict the future problems related to
libssh.
Specially to discuss changes associated with deprecated functions.
And also specially because we have several Linux OSes keeping its own
package versioning.
We would not like to break anything.
I will split this serie and wait for 0.9.0 stable release.
Thanks to review, Andrea. ;-)
Em ter, 18 de dez de 2018 às 12:01, Andrea Bolognani <abologna(a)redhat.com>
escreveu:
On Sat, 2018-11-24 at 03:52 +0800, Julio Faracco wrote:
> After 0.8.0 release, libssh deprecated some functions like:
> ssh_is_server_known() and ssh_write_knownhost().
This is not accurate: 0.8.5 was released less than two months ago
and does not consider any of the functions you listed as deprecated;
the deprecation is indeed in place on the master branch, which will
eventually be released as 0.9.0 or whatever.
Speaking of which, I'm not entirely convinced being *this* proactive
with respect to libssh deprecations is useful when we could at least
wait for the next stable release to be out. I'll review the patches
in more detail regardless :)
--
Andrea Bolognani / Red Hat / Virtualization
1979
days inactive
2004
days old
10 comments
2 participants
participants (2)
-
Andrea Bolognani -
Julio Faracco