All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Ashijeet Acharya <ashijeetacharya@gmail.com>
Cc: "Richard W.M. Jones" <rjones@redhat.com>,
	jcody@redhat.com, Eric Blake <eblake@redhat.com>,
	mreitz@redhat.com, armbru@redhat.com,
	QEMU Developers <qemu-devel@nongnu.org>,
	qemu-block@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 0/4] Allow blockdev-add for SSH
Date: Wed, 12 Oct 2016 18:40:06 +0200	[thread overview]
Message-ID: <20161012164006.GR5544@noname.redhat.com> (raw)
In-Reply-To: <CAC2QTZYYiNiYJfiugKkO6hKwQw85cjkDuTWZfg8S7aQEMRevJw@mail.gmail.com>

Am 12.10.2016 um 18:20 hat Ashijeet Acharya geschrieben:
> On Wed, Oct 12, 2016 at 9:31 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> > Am 11.10.2016 um 09:37 hat Ashijeet Acharya geschrieben:
> >> This series adds blockdev-add support for SSH block driver.
> >>
> >> Patch 1 prepares the code for the addition of a new option prefix,
> >> which is "server.". This is accomplished by adding a
> >> ssh_has_filename_options_conflict() function which helps to iterate
> >> over the various options and check for conflict.
> >>
> >> Patch 2 first adds InetSocketAddress compatibility to SSH block driver
> >> and then makes it accept a InetSocketAddress under the "server" option.
> >> The old options "host" and "port" are supported as legacy options and
> >> then translated to the respective InetSocketAddress representation.
> >>
> >> Patch 3 drops the usage of "host" and "port" outside of
> >> ssh_has_filename_options_conflict() and
> >> ssh_process_legacy_socket_options() functions in order to make them
> >> legacy options completely.
> >>
> >> Patch 4 helps to allow blockdev-add support for the SSH block driver
> >> by making the SSH option available.
> >
> > Commented on patch 2, the rest looks good to me at first sight.
> 
> Yes, I am going through that currently and will ask you if I have any queries.
> 
> >
> > Just curious, what kind of testing did you give the series?
> 
> First thing I made sure if I wasn't breaking anything. Basically I
> checked if blockdev-add worked with it and then if I am able to remove
> it with x-blockdev-del.

Did you try out all of the options that we support, including those in
InetSocketAddress?

> Also, there were no major changes which could
> make the patches to break. I don't see tests available for SSH either
> so there was nothing much I can do.
> Do you have anything in mind?

Actually, qemu-iotests has ssh support. It's probably not run very
often, so I'm not sure whether it completely passes on master, but worth
a try anyway. Check out ./check --help in tests/qemu-iotests, you'll
probably want something like './check -T -ssh'.

The commit message that added ssh support to the tests says:

    Note in order to run these tests on ssh, you must be running a local
    ssh daemon, and that daemon must accept loopback connections, and
    ssh-agent has to be set up to allow logins on the local daemon.  In
    other words, the following command should just work without demanding
    any passphrase:

     ssh localhost

Hope this is helpful.

Kevin

  reply	other threads:[~2016-10-12 16:40 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-11  7:37 [Qemu-devel] [PATCH 0/4] Allow blockdev-add for SSH Ashijeet Acharya
2016-10-11  7:37 ` [Qemu-devel] [PATCH 1/4] block/ssh: Add ssh_has_filename_options_conflict() Ashijeet Acharya
2016-10-11  7:37 ` [Qemu-devel] [PATCH 2/4] block/ssh: Add InetSocketAddress and accept it Ashijeet Acharya
2016-10-12 15:51   ` Kevin Wolf
2016-10-12 16:49     ` Ashijeet Acharya
2016-10-13 18:34   ` Ashijeet Acharya
2016-10-11  7:37 ` [Qemu-devel] [PATCH 3/4] block/ssh: Use InetSocketAddress options Ashijeet Acharya
2016-10-11  7:37 ` [Qemu-devel] [PATCH 4/4] qapi: allow blockdev-add for ssh Ashijeet Acharya
2016-10-11 20:28 ` [Qemu-devel] [PATCH 0/4] Allow blockdev-add for SSH no-reply
2016-10-12  8:09 ` Ashijeet Acharya
2016-10-12  8:22   ` Kevin Wolf
2016-10-12  8:37     ` Ashijeet Acharya
2016-10-12  8:43       ` Kevin Wolf
2016-10-12  8:43   ` Ashijeet Acharya
2016-10-12 16:01 ` Kevin Wolf
2016-10-12 16:20   ` Ashijeet Acharya
2016-10-12 16:40     ` Kevin Wolf [this message]
2016-10-12 16:57       ` Ashijeet Acharya

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161012164006.GR5544@noname.redhat.com \
    --to=kwolf@redhat.com \
    --cc=armbru@redhat.com \
    --cc=ashijeetacharya@gmail.com \
    --cc=eblake@redhat.com \
    --cc=jcody@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rjones@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.