[libvirt] [PATCH v3 2/4] rpc: replacing ssh_is_server_known() by ssh_session_is_known_server().

Julio Faracco posted 4 patches 7 years, 2 months ago
[libvirt] [PATCH v3 2/4] rpc: replacing ssh_is_server_known() by ssh_session_is_known_server().
Posted by Julio Faracco 7 years, 2 months ago
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@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/4] rpc: replacing ssh_is_server_known() by ssh_session_is_known_server().
Posted by Andrea Bolognani 7 years, 1 month ago
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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list