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
next prev parent 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.