[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [v2 2/5] block/ssh: Add InetSocketAddress and accept it
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [v2 2/5] block/ssh: Add InetSocketAddress and accept it |
Date: |
Mon, 17 Oct 2016 17:53:07 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 17.10.2016 um 17:44 hat Ashijeet Acharya geschrieben:
> On Mon, Oct 17, 2016 at 6:27 PM, Kevin Wolf <address@hidden> wrote:
> > Am 17.10.2016 um 14:33 hat Ashijeet Acharya geschrieben:
> >> On Sun, Oct 16, 2016 at 4:00 AM, Max Reitz <address@hidden> wrote:
> >> > On 15.10.2016 11:04, Ashijeet Acharya wrote:
> >> >> Add InetSocketAddress compatibility to SSH driver.
> >> >>
> >> >> Add a new option "server" to the SSH block driver which then accepts
> >> >> a InetSocketAddress.
> >> >>
> >> >> "host" and "port" are supported as legacy options and are mapped to
> >> >> their InetSocketAddress representation.
> >> >>
> >> >> Signed-off-by: Ashijeet Acharya <address@hidden>
> >> >> ---
> >> >> block/ssh.c | 83
> >> >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
> >> >> 1 file changed, 74 insertions(+), 9 deletions(-)
> >> >>
> >> >>
> >> >> /* Open the socket and connect. */
> >> >> s->sock = inet_connect(s->hostport, errp);
> >> >> @@ -634,7 +698,8 @@ static int connect_to_ssh(BDRVSSHState *s, QDict
> >> >> *options,
> >> >> }
> >> >>
> >> >> /* Check the remote host's key against known_hosts. */
> >> >> - ret = check_host_key(s, host, port, host_key_check, errp);
> >> >> + ret = check_host_key(s, s->inet->host, port, host_key_check,
> >> >
> >> > But then you're still using the port here... And I can't come up with a
> >> > way (not even a bad one) to get the numeric port. Maybe interpret the
> >> > addrinfo in inet_connect_saddr()? But getting that information out would
> >> > be ugly, if even possible...
> >> >
> >> > So maybe the best is to keep it this way and put a FIXME above the
> >> > atoi() call. :-/
> >>
> >> Kevin, I believe (after talking with Max) that regarding the atoi()
> >> issue, I can't use any string to integer function since it won't
> >> succeed for cases like port = 'ssh' and putting a FIXME over it seems
> >> to be the only option. But Max did warn me, though, to get everybody's
> >> opinion before I do so. So I am awaiting your response on this one.
> >> Much better will be if you have a workaround solution in mind!! :-)
> >
> > The integer port is only needed for libssh2_knownhost_checkp(). One
> > option could be to consider passing -1 instead:
> >
> > port is the port number used by the host (or a negative number to
> > check the generic host). If the port number is given, libssh2 will
> > check the key for the specific host + port number combination in
> > addition to the plain host name only check.
> >
> > In 99% of the cases, this shouldn't make any difference.
>
> I think, its better to keep using atoi() and check if it returns a '0'
> value and display the error to the user to give the input as numeric.
> This is possible since this will not clash with the possibility that
> user gives the port input as port = '0' for no such port number exists
> as far as I know. Will this work?
It's fair enough. We will have a little inconsistency between ssh and
other users of SocketAddress, but the driver never supported service
names, so it isn't a regression either.
Kevin
> Ashijeet
> > Alternatively it could be possible to use getservbyname() to get the
> > port number from the name, but maybe that's a bit too much for a feature
> > that most people don't even know of.
> >
> > I'm also not completely opposed to simply requiring a numeric argument
> > for SSH. There is no real use to support service names here other than
> > being consistent with other places in qemu.
> >
> > Kevin
- [Qemu-block] [v2 1/5] block/ssh: Add ssh_has_filename_options_conflict(), (continued)
- [Qemu-block] [v2 1/5] block/ssh: Add ssh_has_filename_options_conflict(), Ashijeet Acharya, 2016/10/15
- [Qemu-block] [v2 2/5] block/ssh: Add InetSocketAddress and accept it, Ashijeet Acharya, 2016/10/15
- Re: [Qemu-block] [v2 2/5] block/ssh: Add InetSocketAddress and accept it, Kevin Wolf, 2016/10/17
- Re: [Qemu-block] [v2 2/5] block/ssh: Add InetSocketAddress and accept it, Ashijeet Acharya, 2016/10/17
- Re: [Qemu-block] [v2 2/5] block/ssh: Add InetSocketAddress and accept it, Kevin Wolf, 2016/10/17
- Re: [Qemu-block] [v2 2/5] block/ssh: Add InetSocketAddress and accept it, Ashijeet Acharya, 2016/10/17
- Re: [Qemu-block] [v2 2/5] block/ssh: Add InetSocketAddress and accept it,
Kevin Wolf <=
- Re: [Qemu-block] [v2 2/5] block/ssh: Add InetSocketAddress and accept it, Ashijeet Acharya, 2016/10/17
- Re: [Qemu-block] [v2 2/5] block/ssh: Add InetSocketAddress and accept it, Eric Blake, 2016/10/17
- Re: [Qemu-block] [v2 2/5] block/ssh: Add InetSocketAddress and accept it, Ashijeet Acharya, 2016/10/17
[Qemu-block] [v2 3/5] block/ssh: Use inet_connect_saddr() to establish socket connection, Ashijeet Acharya, 2016/10/15
[Qemu-block] [v2 4/5] block/ssh: Use InetSocketAddress options, Ashijeet Acharya, 2016/10/15
[Qemu-block] [v2 5/5] qapi: allow blockdev-add for ssh, Ashijeet Acharya, 2016/10/15
Re: [Qemu-block] [Qemu-devel] [v2 0/5] Allow blockdev-add for SSH, no-reply, 2016/10/15