Re: [Qemu-devel] [PATCH v2] block: Add support for Secure Shell (ssh) block device.

[Stefan Hajnoczi]

On Mon, Mar 25, 2013 at 11:49:41PM +0000, Richard W.M. Jones wrote:
> From: "Richard W.M. Jones" <address@hidden>
> 
>   qemu-system-x86_64 -drive file=ssh://hostname/some/image
> 
> QEMU will ssh into 'hostname' and open '/some/image' which is made
> available as a standard block device.
> 
> You can specify a username (ssh://address@hidden/...) and/or a port number
> (ssh://host:port/...).
> 
> Current limitations:
> 
> - Authentication must be done without passwords or passphrases, using
>   ssh-agent.  Other authentication methods are not supported. (*)
> 
> - Does not check host key. (*)
> 
> - New remote files cannot be created. (*)
> 
> - Uses a single connection, instead of concurrent AIO with multiple
>   SSH connections.
> 
> - Blocks during connection and authentication.

This isn't ideal but .bdrv_open() is a blocking function anyway.  If you
open a raw image on an NFS export the open(2) call can block too.

Blocking in .bdrv_open() is fine during startup.  It's bad when
hotplugging drives since the running VM will experience downtime.

> +    /* Start SFTP. */
> +    s->sftp = libssh2_sftp_init(s->session);
> +    if (!s->sftp) {
> +        session_error_report(s, "failed to initialize sftp handle");
> +        ret = -EINVAL;
> +        goto err;
> +    }
> +
> +    s->sftp_handle = libssh2_sftp_open(s->sftp, path,
> +                                       LIBSSH2_FXF_READ|LIBSSH2_FXF_WRITE,

Probably worth handling -drive ...,readonly=on.  !(flags & BDRV_O_RDWR)

> +static coroutine_fn void set_fd_handler(BDRVSSHState *s)
> +{
> +    int r;
> +    void (*rd_handler)(void*);
> +    void (*wr_handler)(void*);

There's a typedef for this: IOHandler.

> +    Coroutine *co = qemu_coroutine_self();
> +
> +    rd_handler = wr_handler = NULL;
> +
> +    r = libssh2_session_block_directions(s->session);
> +
> +    if (r & LIBSSH2_SESSION_BLOCK_INBOUND) {
> +        rd_handler = restart_coroutine;
> +    }
> +    if (r & LIBSSH2_SESSION_BLOCK_OUTBOUND) {
> +        wr_handler = restart_coroutine;
> +    }
> +
> +    DPRINTF("s->sock=%d rd_handler=%p wr_handler=%p", s->sock,
> +            rd_handler, wr_handler);
> +
> +    qemu_aio_set_fd_handler(s->sock, rd_handler, wr_handler, return_true, 
> co);
> +}
> +
> +/* A non-blocking call returned EAGAIN, so yield, ensuring the
> + * handlers are set up so that we'll be rescheduled when there is an
> + * interesting event on the socket.
> + */
> +static coroutine_fn void co_yield(BDRVSSHState *s)
> +{
> +    set_fd_handler(s);
> +    qemu_coroutine_yield();

Should we clear the fd handler here?  That way the caller doesn't have
to worry about fd handler state.