[Qemu-devel] block/ssh:Allow blockdev-add for ssh

[Qemu-devel] block/ssh:Allow blockdev-add for ssh

[Ashijeet Acharya]

Hi all,

I was trying to convert SSH driver to support 'blockdev-add' and so
far I have tried to figure out what the struct 'BlockdevOptionsSsh' in
block-core.json should look like,

{ 'struct': 'BlockdevOptionsSsh',
  'data': { 'tcp': 'InetSocketAddress',
             'path': 'str' } }

Naive question but I have to ask, Am I missing something?

As far as I know, ssh only supports 'tcp' right? So using
'InetSocketAddress' should be good enough. (like the TODO says)

I had a discussion with Kevin about this and he thinks, maybe
'SocketAddress' can be used too because the restriction comes from the
qemu block driver rather than the backend. He advised me to get an
opinion on this one from the maintainers of SSH.

Thanks for reading
Ashijeet

Re: [Qemu-devel] block/ssh:Allow blockdev-add for ssh

[Richard W.M. Jones]

On Thu, Sep 29, 2016 at 01:05:48PM +0530, Ashijeet Acharya wrote:
> Hi all,
> 
> I was trying to convert SSH driver to support 'blockdev-add' and so
> far I have tried to figure out what the struct 'BlockdevOptionsSsh' in
> block-core.json should look like,
> 
> { 'struct': 'BlockdevOptionsSsh',
>   'data': { 'tcp': 'InetSocketAddress',
>              'path': 'str' } }
> 
> Naive question but I have to ask, Am I missing something?
> 
> As far as I know, ssh only supports 'tcp' right? So using
> 'InetSocketAddress' should be good enough. (like the TODO says)
> 
> I had a discussion with Kevin about this and he thinks, maybe
> 'SocketAddress' can be used too because the restriction comes from the
> qemu block driver rather than the backend. He advised me to get an
> opinion on this one from the maintainers of SSH.

I have no idea.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Re: [Qemu-devel] [Qemu-block] block/ssh:Allow blockdev-add for ssh

[Kevin Wolf]

Am 29.09.2016 um 10:07 hat Richard W.M. Jones geschrieben:
> On Thu, Sep 29, 2016 at 01:05:48PM +0530, Ashijeet Acharya wrote:
> > Hi all,
> > 
> > I was trying to convert SSH driver to support 'blockdev-add' and so
> > far I have tried to figure out what the struct 'BlockdevOptionsSsh' in
> > block-core.json should look like,
> > 
> > { 'struct': 'BlockdevOptionsSsh',
> >   'data': { 'tcp': 'InetSocketAddress',
> >              'path': 'str' } }
> > 
> > Naive question but I have to ask, Am I missing something?
> > 
> > As far as I know, ssh only supports 'tcp' right? So using
> > 'InetSocketAddress' should be good enough. (like the TODO says)
> > 
> > I had a discussion with Kevin about this and he thinks, maybe
> > 'SocketAddress' can be used too because the restriction comes from the
> > qemu block driver rather than the backend. He advised me to get an
> > opinion on this one from the maintainers of SSH.
> 
> I have no idea.

I searched the net a bit and it seems that SSH over Unix domain sockets
isn't a thing. So it might actually be okay to restrict the QEMU block
driver to TCP, too, and therefore use InetSocketAddress.

Any other opinions?

Kevin

Re: [Qemu-devel] [Qemu-block] block/ssh:Allow blockdev-add for ssh

[Daniel P. Berrange]

On Thu, Sep 29, 2016 at 12:42:34PM +0200, Kevin Wolf wrote:
> Am 29.09.2016 um 10:07 hat Richard W.M. Jones geschrieben:
> > On Thu, Sep 29, 2016 at 01:05:48PM +0530, Ashijeet Acharya wrote:
> > > Hi all,
> > > 
> > > I was trying to convert SSH driver to support 'blockdev-add' and so
> > > far I have tried to figure out what the struct 'BlockdevOptionsSsh' in
> > > block-core.json should look like,
> > > 
> > > { 'struct': 'BlockdevOptionsSsh',
> > >   'data': { 'tcp': 'InetSocketAddress',
> > >              'path': 'str' } }
> > > 
> > > Naive question but I have to ask, Am I missing something?
> > > 
> > > As far as I know, ssh only supports 'tcp' right? So using
> > > 'InetSocketAddress' should be good enough. (like the TODO says)
> > > 
> > > I had a discussion with Kevin about this and he thinks, maybe
> > > 'SocketAddress' can be used too because the restriction comes from the
> > > qemu block driver rather than the backend. He advised me to get an
> > > opinion on this one from the maintainers of SSH.
> > 
> > I have no idea.
> 
> I searched the net a bit and it seems that SSH over Unix domain sockets
> isn't a thing. So it might actually be okay to restrict the QEMU block
> driver to TCP, too, and therefore use InetSocketAddress.
> 
> Any other opinions?

We have multiple block device drivers that all need network addresses.
I think it'd be nice if we can use the same schema for all of them,
even if some of them don't (currently) require certain options. 

eg rename 'GlusterServer' to 'BlockServer' and use it for all nework
transports. Those that don't want unix support can just reject it
at runtime, likewise those that don't need multiple-server support.

This will let mgmt apps have the same code for generating the
blockdev options for all network based transports, instead of having
to write different code for each.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

Re: [Qemu-devel] [Qemu-block] block/ssh:Allow blockdev-add for ssh

[Kevin Wolf]

Am 29.09.2016 um 13:07 hat Daniel P. Berrange geschrieben:
> On Thu, Sep 29, 2016 at 12:42:34PM +0200, Kevin Wolf wrote:
> > Am 29.09.2016 um 10:07 hat Richard W.M. Jones geschrieben:
> > > On Thu, Sep 29, 2016 at 01:05:48PM +0530, Ashijeet Acharya wrote:
> > > > Hi all,
> > > > 
> > > > I was trying to convert SSH driver to support 'blockdev-add' and so
> > > > far I have tried to figure out what the struct 'BlockdevOptionsSsh' in
> > > > block-core.json should look like,
> > > > 
> > > > { 'struct': 'BlockdevOptionsSsh',
> > > >   'data': { 'tcp': 'InetSocketAddress',
> > > >              'path': 'str' } }
> > > > 
> > > > Naive question but I have to ask, Am I missing something?
> > > > 
> > > > As far as I know, ssh only supports 'tcp' right? So using
> > > > 'InetSocketAddress' should be good enough. (like the TODO says)
> > > > 
> > > > I had a discussion with Kevin about this and he thinks, maybe
> > > > 'SocketAddress' can be used too because the restriction comes from the
> > > > qemu block driver rather than the backend. He advised me to get an
> > > > opinion on this one from the maintainers of SSH.
> > > 
> > > I have no idea.
> > 
> > I searched the net a bit and it seems that SSH over Unix domain sockets
> > isn't a thing. So it might actually be okay to restrict the QEMU block
> > driver to TCP, too, and therefore use InetSocketAddress.
> > 
> > Any other opinions?
> 
> We have multiple block device drivers that all need network addresses.
> I think it'd be nice if we can use the same schema for all of them,
> even if some of them don't (currently) require certain options. 
> 
> eg rename 'GlusterServer' to 'BlockServer' and use it for all nework
> transports. Those that don't want unix support can just reject it
> at runtime, likewise those that don't need multiple-server support.
> 
> This will let mgmt apps have the same code for generating the
> blockdev options for all network based transports, instead of having
> to write different code for each.

Allowing everything and then rejecting things at runtime isn't really
the idea behind having a schema... It would also make it impossible for
libvirt to detect whether a new backend has been implemented. If we do
schemas properly and only advertise what's really there, schema
introspection can answer that question.

Kevin

Re: [Qemu-devel] [Qemu-block] block/ssh:Allow blockdev-add for ssh

[Daniel P. Berrange]

On Thu, Sep 29, 2016 at 12:42:34PM +0200, Kevin Wolf wrote:
> Am 29.09.2016 um 10:07 hat Richard W.M. Jones geschrieben:
> > On Thu, Sep 29, 2016 at 01:05:48PM +0530, Ashijeet Acharya wrote:
> > > Hi all,
> > > 
> > > I was trying to convert SSH driver to support 'blockdev-add' and so
> > > far I have tried to figure out what the struct 'BlockdevOptionsSsh' in
> > > block-core.json should look like,
> > > 
> > > { 'struct': 'BlockdevOptionsSsh',
> > >   'data': { 'tcp': 'InetSocketAddress',
> > >              'path': 'str' } }
> > > 
> > > Naive question but I have to ask, Am I missing something?
> > > 
> > > As far as I know, ssh only supports 'tcp' right? So using
> > > 'InetSocketAddress' should be good enough. (like the TODO says)
> > > 
> > > I had a discussion with Kevin about this and he thinks, maybe
> > > 'SocketAddress' can be used too because the restriction comes from the
> > > qemu block driver rather than the backend. He advised me to get an
> > > opinion on this one from the maintainers of SSH.
> > 
> > I have no idea.
> 
> I searched the net a bit and it seems that SSH over Unix domain sockets
> isn't a thing. So it might actually be okay to restrict the QEMU block
> driver to TCP, too, and therefore use InetSocketAddress.

SSH over UNIX sockets isn't common, but it is possible. eg say you want
to connect to a remote machine that isn't directly acessible. You might
use SSH tunnelling to setup a local UNIX domain socket that is connected
to the remote machine eg

   ssh -L /tmp/catbus-sock:catbus.mydomain.com:22 domokun

Now, connecting to the UNIX domain socket /tmp/catbus-sock would in
fact forward traffic to the remote TCP server.

This isn't specific to SSH really - any network protocol could be tunnelled
in this way, so from that POV there is value in all the network block
drivers being able to accept UNIX domain socket addresses.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|

Re: [Qemu-devel] [Qemu-block] block/ssh:Allow blockdev-add for ssh

[Kevin Wolf]

Am 29.09.2016 um 13:22 hat Daniel P. Berrange geschrieben:
> On Thu, Sep 29, 2016 at 12:42:34PM +0200, Kevin Wolf wrote:
> > Am 29.09.2016 um 10:07 hat Richard W.M. Jones geschrieben:
> > > On Thu, Sep 29, 2016 at 01:05:48PM +0530, Ashijeet Acharya wrote:
> > > > Hi all,
> > > > 
> > > > I was trying to convert SSH driver to support 'blockdev-add' and so
> > > > far I have tried to figure out what the struct 'BlockdevOptionsSsh' in
> > > > block-core.json should look like,
> > > > 
> > > > { 'struct': 'BlockdevOptionsSsh',
> > > >   'data': { 'tcp': 'InetSocketAddress',
> > > >              'path': 'str' } }
> > > > 
> > > > Naive question but I have to ask, Am I missing something?
> > > > 
> > > > As far as I know, ssh only supports 'tcp' right? So using
> > > > 'InetSocketAddress' should be good enough. (like the TODO says)
> > > > 
> > > > I had a discussion with Kevin about this and he thinks, maybe
> > > > 'SocketAddress' can be used too because the restriction comes from the
> > > > qemu block driver rather than the backend. He advised me to get an
> > > > opinion on this one from the maintainers of SSH.
> > > 
> > > I have no idea.
> > 
> > I searched the net a bit and it seems that SSH over Unix domain sockets
> > isn't a thing. So it might actually be okay to restrict the QEMU block
> > driver to TCP, too, and therefore use InetSocketAddress.
> 
> SSH over UNIX sockets isn't common, but it is possible. eg say you want
> to connect to a remote machine that isn't directly acessible. You might
> use SSH tunnelling to setup a local UNIX domain socket that is connected
> to the remote machine eg
> 
>    ssh -L /tmp/catbus-sock:catbus.mydomain.com:22 domokun
> 
> Now, connecting to the UNIX domain socket /tmp/catbus-sock would in
> fact forward traffic to the remote TCP server.
> 
> This isn't specific to SSH really - any network protocol could be tunnelled
> in this way, so from that POV there is value in all the network block
> drivers being able to accept UNIX domain socket addresses.

But the ssh client doesn't seem to support connection to that Unix domain
socket, even if it would be possible to support in theory. And probably
none of the SSH libraries that we consider to use support it either (but
I haven't checked that).

Kevin

Re: [Qemu-devel] [Qemu-block] block/ssh:Allow blockdev-add for ssh

[Richard W.M. Jones]

On Thu, Sep 29, 2016 at 01:36:35PM +0200, Kevin Wolf wrote:
> But the ssh client doesn't seem to support connection to that Unix domain
> socket, even if it would be possible to support in theory. And probably
> none of the SSH libraries that we consider to use support it either (but
> I haven't checked that).

So I checked this.  libssh2 (which is the one we're using) does
not appear to support AF_UNIX for the ssh connection.

We should most likely change over the libssh which is a better
library.  That library does in fact support AF_UNIX connections
through the ssh_socket_unix API, although that's of dubious utility
considering that almost no one runs an sshd listening on a Unix domain
socket.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

Re: [Qemu-devel] [Qemu-block] block/ssh:Allow blockdev-add for ssh

[Ashijeet Acharya]

On Thu, Sep 29, 2016 at 5:29 PM, Richard W.M. Jones <rjones@redhat.com> wrote:
> On Thu, Sep 29, 2016 at 01:36:35PM +0200, Kevin Wolf wrote:
>> But the ssh client doesn't seem to support connection to that Unix domain
>> socket, even if it would be possible to support in theory. And probably
>> none of the SSH libraries that we consider to use support it either (but
>> I haven't checked that).
>
> So I checked this.  libssh2 (which is the one we're using) does
> not appear to support AF_UNIX for the ssh connection.
>
> We should most likely change over the libssh which is a better
> library.  That library does in fact support AF_UNIX connections
> through the ssh_socket_unix API, although that's of dubious utility
> considering that almost no one runs an sshd listening on a Unix domain
> socket.
>

I think that compared to SSH, 'GlusterServer' would be a better
candidate to use 'SocketAddress' rather than using 'InetSocketAddress'
and 'UnixSocketAddress' separately for tcp and Unix sockets
respectively AFAIK.

Also as Richard said, I don't think anyone would use SSH over a Unix
and go though the whole tunneling mechanism which Daniel described and
maybe just simply use tcp directly (right?). So using
'InetSocketAddress' should actually do the job here.

Other than that I also asked if I have accidentally missed any other
important field regarding the structure 'BlockdevOptionsSsh' I
described in the previous mail (I will post it here again for
convenience)

{ 'struct': 'BlockdevOptionsSsh',
  'data': { 'tcp': 'InetSocketAddress',
             'path': 'str' } }

It will be better if I know it beforehand and build up the patch based
on the suggestions.

Ashijeet

> Rich.
>
> --
> Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
> Read my programming and virtualization blog: http://rwmj.wordpress.com
> virt-p2v converts physical machines to virtual machines.  Boot with a
> live CD or over the network (PXE) and turn machines into KVM guests.
> http://libguestfs.org/virt-v2v

Re: [Qemu-devel] [Qemu-block] block/ssh:Allow blockdev-add for ssh

[Kevin Wolf]

Am 29.09.2016 um 16:07 hat Ashijeet Acharya geschrieben:
> Other than that I also asked if I have accidentally missed any other
> important field regarding the structure 'BlockdevOptionsSsh' I
> described in the previous mail (I will post it here again for
> convenience)
> 
> { 'struct': 'BlockdevOptionsSsh',
>   'data': { 'tcp': 'InetSocketAddress',
>              'path': 'str' } }
> 
> It will be better if I know it beforehand and build up the patch based
> on the suggestions.

Sorry, I missed this part. Yes, there are a few more options. Just check
ssh_runtime_opts. You wil replace the existing 'host'/'port' by 'tcp' (I
think I would call the option something like 'server' instead of 'tcp',
though). 'path' is already here. This leaves 'user' and 'host_key_check'
that are missing.

Kevin

Re: [Qemu-devel] [Qemu-block] block/ssh:Allow blockdev-add for ssh

[Ashijeet Acharya]

On Thu, Sep 29, 2016 at 8:32 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 29.09.2016 um 16:07 hat Ashijeet Acharya geschrieben:
>> Other than that I also asked if I have accidentally missed any other
>> important field regarding the structure 'BlockdevOptionsSsh' I
>> described in the previous mail (I will post it here again for
>> convenience)
>>
>> { 'struct': 'BlockdevOptionsSsh',
>>   'data': { 'tcp': 'InetSocketAddress',
>>              'path': 'str' } }
>>
>> It will be better if I know it beforehand and build up the patch based
>> on the suggestions.
>
> Sorry, I missed this part. Yes, there are a few more options. Just check
> ssh_runtime_opts. You wil replace the existing 'host'/'port' by 'tcp' (I
> think I would call the option something like 'server' instead of 'tcp',
> though). 'path' is already here. This leaves 'user' and 'host_key_check'
> that are missing.

Sure, I will rename it to 'server'.
Thanks for the help. I will post the patch for review soon.

Ashijeet
> Kevin