Re: [Qemu-devel] [PATCH] qapi: allow blockdev-add for ssh

[Kevin Wolf <kwolf@redhat.com>]

Am 10.10.2016 um 12:48 hat Ashijeet Acharya geschrieben:
> On Mon, Oct 10, 2016 at 2:45 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> > Am 08.10.2016 um 12:44 hat Ashijeet Acharya geschrieben:
> >> +{ 'struct': 'BlockdevoptionsSsh',
> >> +  'data': { 'server': 'InetSocketAddress',
> >> +            'path': 'str',
> >> +            'user': 'str',
> >> +            'host_key_check': 'str' } }

Another thing I just noticed now: I think host_key_check should be
marked optional (i.e. '*host_key_check')

> >
> > Did you test this? The ssh driver currently takes a separate 'host'
> > string and 'port' integer, not a 'server' InetSocketAddress, so I think
> > the C code needs an update, too.
> 
> Ohh, maybe I misunderstood. I will update the c code too.
> One question though, here when we talk about server, we refer to something like
>          <host>:<port>
> right?

You (correctly) defined 'server' as an InetSocketAddress. This in turn
is defined in qapi-schema.json:

{ 'struct': 'InetSocketAddress',
  'data': {
    'host': 'str',
    'port': 'str',
    '*to': 'uint16',
    '*ipv4': 'bool',
    '*ipv6': 'bool' } }

Your .bdrv_open() callback in ssh gets these options as keys in
QDict *options, using the dot syntax. options might look like this
(using "key" = "value" for the example):

    "server.host" = "localhost"
    "server.port" = "1234"
    "server.ipv4" = "on"
    "server.ipv6" = "on"
    "path" = "/tmp/test.img"
    "user" = "test"

You can manually parse the "server.*" fields with
qdict_extract_subqdict() and QemuOpts and then construct an
InetSocketAddress object in C similar to what NBD does currently.

The other option, and maybe a bit nicer, would be to use a visitor to
directly go from the (sub-)QDict to InetSocketAddress.

> And similarly, maybe not related to this but when we parse uri
> using 'uri_parse(filename)', does the 'uri->server' stores a similar
> kind of format I mentioned above?

It appears to return a struct URI, which contains 'char *server' and
'int port', but also many other fields.

> > As for how to use a SocketAddress in order to establish a connection,
> > you can look at block/nbd.c for reference.
> 
> Great! I will use that as a reference.

Another thing to have a look at might be the NBD series that Max posted
to convert it to blockdev-add. I still haven't done that myself, but I
suppose many of the things he does there apply to ssh as well.

Kevin