Re: [Qemu-devel] [PATCH 0/4] Allow blockdev-add for SSH

2016-10-12 Thread Ashijeet Acharya
On Wed, Oct 12, 2016 at 10:10 PM, Kevin Wolf  wrote:
> Am 12.10.2016 um 18:20 hat Ashijeet Acharya geschrieben:
>> On Wed, Oct 12, 2016 at 9:31 PM, Kevin Wolf  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.

>> 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?

No, only 'host' and 'port' from InetSocketAddress which is why I think
the tests were successful in the first place. Using other options seem
to break it. Like you suggested, I think using
qobject_input_visitor_new_autocast() should fix this.

>
>> 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'.
>

Oh, I wasn't aware of that. I will look around now. Thanks!

> 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.

Yes very helpful!!!

Thanks again!

Ashijeet
>
> Kevin



Re: [Qemu-devel] [PATCH 0/4] Allow blockdev-add for SSH

2016-10-12 Thread Kevin Wolf
Am 12.10.2016 um 18:20 hat Ashijeet Acharya geschrieben:
> On Wed, Oct 12, 2016 at 9:31 PM, Kevin Wolf  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



Re: [Qemu-devel] [PATCH 0/4] Allow blockdev-add for SSH

2016-10-12 Thread Ashijeet Acharya
On Wed, Oct 12, 2016 at 9:31 PM, Kevin Wolf  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. 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?

Ashijeet

>
> Kevin



Re: [Qemu-devel] [PATCH 0/4] Allow blockdev-add for SSH

2016-10-12 Thread Kevin Wolf
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.

Just curious, what kind of testing did you give the series?

Kevin



Re: [Qemu-devel] [PATCH 0/4] Allow blockdev-add for SSH

2016-10-12 Thread Ashijeet Acharya
> I received a mail saying my series failed the automatic build test but
> it builds completely fine (after applying Dan's patch obviously) in my
> local environment.
>
> Going through the config output of the test script, I see that the
> supporting library for SSH which is "libssh2" seems to be disabled and
> maybe causes the build to fail. I am able to reproduce it locally with
> "libssh2" disabled.

Sorry!
s/ "libssh2" disabled/ "libssh2" enabled.

Ashijeet



Re: [Qemu-devel] [PATCH 0/4] Allow blockdev-add for SSH

2016-10-12 Thread Kevin Wolf
Am 12.10.2016 um 10:37 hat Ashijeet Acharya geschrieben:
> On Wed, Oct 12, 2016 at 1:52 PM, Kevin Wolf  wrote:
> > Am 12.10.2016 um 10:09 hat Ashijeet Acharya geschrieben:
> 
> > Of course, we must be able to build qemu correctly both with ssh enabled
> > and disabled, so if you can indeed see a (different) build error with
> > disabled libssh2, that needs to be fixed.
> 
> I am able to reproduce it when I don't apply Dan's patch with libssh2 enabled.
> After disabling libssh2, it compiles perfectly because then the SSH
> driver will get ignored I guess.
> libssh2 doesn't seem to be the problem.
> 
> >
> > But I can't see how that would happen, you don't touch configure and it
> > already compiled the ssh driver out if libssh2 is disabled.
> 
> Yes, with libssh2 disabled it builds with no error just like the first
> output of auto-build shows.
> So, I think with libssh2 enabled it throws an error (just like second
> output of auto-build) since Dan's patches are not merged yet. Right?

Yes. You can ignore that, we'll make sure to merge the patches in the
right order.

Kevin



Re: [Qemu-devel] [PATCH 0/4] Allow blockdev-add for SSH

2016-10-12 Thread Ashijeet Acharya
On Wed, Oct 12, 2016 at 1:52 PM, Kevin Wolf  wrote:
> Am 12.10.2016 um 10:09 hat Ashijeet Acharya geschrieben:

> Of course, we must be able to build qemu correctly both with ssh enabled
> and disabled, so if you can indeed see a (different) build error with
> disabled libssh2, that needs to be fixed.

I am able to reproduce it when I don't apply Dan's patch with libssh2 enabled.
After disabling libssh2, it compiles perfectly because then the SSH
driver will get ignored I guess.
libssh2 doesn't seem to be the problem.

>
> But I can't see how that would happen, you don't touch configure and it
> already compiled the ssh driver out if libssh2 is disabled.

Yes, with libssh2 disabled it builds with no error just like the first
output of auto-build shows.
So, I think with libssh2 enabled it throws an error (just like second
output of auto-build) since Dan's patches are not merged yet. Right?

Ashijeet
> Kevin



Re: [Qemu-devel] [PATCH 0/4] Allow blockdev-add for SSH

2016-10-12 Thread Kevin Wolf
Am 12.10.2016 um 10:09 hat Ashijeet Acharya geschrieben:
> I received a mail saying my series failed the automatic build test but
> it builds completely fine (after applying Dan's patch obviously) in my
> local environment.

The reason why patchew fails to build your series is because it doesn't
understand the dependency on Dan's patches. Just ignore it.

> Going through the config output of the test script, I see that the
> supporting library for SSH which is "libssh2" seems to be disabled and
> maybe causes the build to fail. I am able to reproduce it locally with
> "libssh2" disabled.

Of course, we must be able to build qemu correctly both with ssh enabled
and disabled, so if you can indeed see a (different) build error with
disabled libssh2, that needs to be fixed.

But I can't see how that would happen, you don't touch configure and it
already compiled the ssh driver out if libssh2 is disabled.

Kevin



Re: [Qemu-devel] [PATCH 0/4] Allow blockdev-add for SSH

2016-10-12 Thread Ashijeet Acharya
On Tue, Oct 11, 2016 at 1:07 PM, Ashijeet Acharya
 wrote:
> 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.
>
>
> *** This series depends on the following patch: ***
> "qdict: implement a qdict_crumple method for un-flattening a dict"
> from Daniel's "QAPI/QOM work for non-scalar object properties"
> series.
>
> Ashijeet Acharya (4):
>   block/ssh: Add ssh_has_filename_options_conflict()
>   block/ssh: Add InetSocketAddress and accept it
>   block/ssh: Use InetSocketAddress options
>   qapi: allow blockdev-add for ssh
>
>  block/ssh.c  | 121 
> +++
>  qapi/block-core.json |  24 +-
>  2 files changed, 125 insertions(+), 20 deletions(-)
>
> --
> 2.6.2

I received a mail saying my series failed the automatic build test but
it builds completely fine (after applying Dan's patch obviously) in my
local environment.

Going through the config output of the test script, I see that the
supporting library for SSH which is "libssh2" seems to be disabled and
maybe causes the build to fail. I am able to reproduce it locally with
"libssh2" disabled.

Any other thoughts?

Ashijeet



Re: [Qemu-devel] [PATCH 0/4] Allow blockdev-add for SSH

2016-10-11 Thread no-reply
Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Message-id: 1476171437-11830-1-git-send-email-ashijeetacha...@gmail.com
Subject: [Qemu-devel] [PATCH 0/4] Allow blockdev-add for SSH
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
make J=8 docker-test-quick@centos6
make J=8 docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
ada148c qapi: allow blockdev-add for ssh
c15d50b block/ssh: Use InetSocketAddress options
370773b block/ssh: Add InetSocketAddress and accept it
7b6cfe0 block/ssh: Add ssh_has_filename_options_conflict()

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD   centos6
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPYRUNNER
  RUN test-quick in centos6
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
ccache-3.1.6-2.el6.x86_64
epel-release-6-8.noarch
gcc-4.4.7-17.el6.x86_64
git-1.7.1-4.el6_7.1.x86_64
glib2-devel-2.28.8-5.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
make-3.81-23.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
tar-1.23-15.el6_8.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=libfdt-devel ccache tar git make gcc g++ zlib-devel 
glib2-devel SDL-devel pixman-devel epel-release
HOSTNAME=15ba99d379b9
TERM=xterm
MAKEFLAGS= -j8
HISTSIZE=1000
J=8
USER=root
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
MAIL=/var/spool/mail/root
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
LANG=en_US.UTF-8
TARGET_LIST=
HISTCONTROL=ignoredups
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
LOGNAME=root
LESSOPEN=||/usr/bin/lesspipe.sh %s
FEATURES= dtc
DEBUG=
G_BROKEN_FILENAMES=1
CCACHE_HASHDIR=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu 
--prefix=/var/tmp/qemu-build/install
No C++ compiler available; disabling C++ specific optional code
Install prefix/var/tmp/qemu-build/install
BIOS directory/var/tmp/qemu-build/install/share/qemu
binary directory  /var/tmp/qemu-build/install/bin
library directory /var/tmp/qemu-build/install/lib
module directory  /var/tmp/qemu-build/install/lib/qemu
libexec directory /var/tmp/qemu-build/install/libexec
include directory /var/tmp/qemu-build/install/include
config directory  /var/tmp/qemu-build/install/etc
local state directory   /var/tmp/qemu-build/install/var
Manual directory  /var/tmp/qemu-build/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /tmp/qemu-test/src
C compilercc
Host C compiler   cc
C++ compiler  
Objective-C compiler cc
ARFLAGS   rv
CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g 
QEMU_CFLAGS   -I/usr/include/pixman-1-pthread -I/usr/include/glib-2.0 
-I/usr/lib64/glib-2.0/include   -fPIE -DPIE -m64 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes 
-fno-strict-aliasing -fno-common -fwrapv  -Wendif-labels -Wmissing-include-dirs 
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wold-style-declaration -Wold-style-definition 
-Wtype-limits -fstack-protector-all
LDFLAGS   -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
make  make
install   install
pythonpython -B
smbd  /usr/sbin/smbd
module supportno
host CPU  x86_64
host big endian   no
target list   x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
pixmansystem
SDL support   yes (1.2.14)
GTK support   no 
GTK GL supportno
VTE support   no 
TLS priority  NORMAL
GNUTLS supportno
GNUTLS rndno
libgcrypt no
libgcrypt kdf no
nettleno 
nettle kdfno
libtasn1  no
curses supportno
virgl support no
curl support  no
mingw32 support   no
Audio drivers oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS supportno
VNC support   yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   no
xen support   no
brlapi supportno
bluez  supportno
Documentation no
PIE   yes
vde support   no
netmap supportno
Linux AIO support no
ATTR/XATTR support yes
Install blobs yes
KVM support   yes
RDMA support  no
TCG interpreter   no
fdt support   yes
preadv supportyes
fdatasync yes
madvise   yes
posix_madvise yes