Re: [Qemu-devel] [PATCH 2/4] block/ssh: Add InetSocketAddress and accept it

[Ashijeet Acharya <ashijeetacharya@gmail.com>]

On Tue, Oct 11, 2016 at 1:07 PM, Ashijeet Acharya
<ashijeetacharya@gmail.com> 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 <ashijeetacharya@gmail.com>
> ---
>  block/ssh.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 78 insertions(+), 9 deletions(-)
>
> diff --git a/block/ssh.c b/block/ssh.c
> index 75cb7bc..702871a 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -32,8 +32,11 @@
>  #include "qemu/error-report.h"
>  #include "qemu/sockets.h"
>  #include "qemu/uri.h"
> +#include "qapi-visit.h"
>  #include "qapi/qmp/qint.h"
>  #include "qapi/qmp/qstring.h"
> +#include "qapi/qmp-input-visitor.h"
> +#include "qapi/qmp-output-visitor.h"
>
>  /* DEBUG_SSH=1 enables the DPRINTF (debugging printf) statements in
>   * this block driver code.
> @@ -74,6 +77,8 @@ typedef struct BDRVSSHState {
>       */
>      LIBSSH2_SFTP_ATTRIBUTES attrs;
>
> +    InetSocketAddress *inet;
> +
>      /* Used to warn if 'flush' is not supported. */
>      char *hostport;
>      bool unsafe_flush_warning;
> @@ -263,7 +268,9 @@ static bool ssh_has_filename_options_conflict(QDict *options, Error **errp)
>              !strcmp(qe->key, "port") ||
>              !strcmp(qe->key, "path") ||
>              !strcmp(qe->key, "user") ||
> -            !strcmp(qe->key, "host_key_check"))
> +            !strcmp(qe->key, "host_key_check") ||
> +            !strcmp(qe->key, "server") ||
> +            !strncmp(qe->key, "server.", 7))
>          {
>              error_setg(errp, "Option '%s' cannot be used with a file name",
>                         qe->key);
> @@ -555,13 +562,71 @@ static QemuOptsList ssh_runtime_opts = {
>      },
>  };
>
> +static bool ssh_process_legacy_socket_options(QDict *output_opts,
> +                                              QemuOpts *legacy_opts,
> +                                              Error **errp)
> +{
> +    const char *host = qemu_opt_get(legacy_opts, "host");
> +    const char *port = qemu_opt_get(legacy_opts, "port");
> +
> +    if (!host && port) {
> +        error_setg(errp, "port may not be used without host");
> +        return false;
> +    }
> +
> +    if (!host) {
> +        error_setg(errp, "No hostname was specified");
> +        return false;

I was debugging this part with gdb while making the changes for v2,
and I hit something very strange.
The code always gives the error of "No hostname was specified". To be
more clear, I reverted back to original driver state and the problem
did not seem to appear for the same qemu command line and I can't find
the bug.

Command I used:

$ ./bin/qemu-system-x86_64 -m 512 -drive
file=ssh://ashijeet@127.0.0.1/home/ashijeet/qemu_build/test.qcow2,
if=virtio

Is there something wrong with the command line?

Ashijeet