[Qemu-devel] [PATCH v4 0/3] iotests: Fix test 162

[Qemu-devel] [PATCH v4 0/3] iotests: Fix test 162

[Max Reitz]

162 is potentially racy and makes some invalid assumptions about what
should happen when connecting to a non-existing domain name. This series
fixes both issues.


v4:
- Added documentation for the new --fork option [Kevin]


git-backport-diff against v3:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream
patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences,
respectively

001/3:[0004] [FC] 'qemu-nbd: Add --fork option'
002/3:[----] [--] 'iotests: Remove raciness from 162'
003/3:[----] [--] 'iotests: Do not rely on unavailable domains in 162'


Max Reitz (3):
  qemu-nbd: Add --fork option
  iotests: Remove raciness from 162
  iotests: Do not rely on unavailable domains in 162

 qemu-nbd.c                 | 17 ++++++++++++++++-
 qemu-nbd.texi              |  2 ++
 tests/qemu-iotests/162     | 22 ++++++++++++++++------
 tests/qemu-iotests/162.out |  2 +-
 4 files changed, 35 insertions(+), 8 deletions(-)

-- 
2.10.0

[Qemu-devel] [PATCH v4 1/3] qemu-nbd: Add --fork option

[Max Reitz]

Using the --fork option, one can make qemu-nbd fork the worker process.
The original process will exit on error of the worker or once the worker
enters the main loop.

Suggested-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-nbd.c    | 17 ++++++++++++++++-
 qemu-nbd.texi |  2 ++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 99297a5..0bd5ba5 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -48,6 +48,7 @@
 #define QEMU_NBD_OPT_OBJECT        260
 #define QEMU_NBD_OPT_TLSCREDS      261
 #define QEMU_NBD_OPT_IMAGE_OPTS    262
+#define QEMU_NBD_OPT_FORK          263
 
 #define MBR_SIZE 512
 
@@ -92,6 +93,8 @@ static void usage(const char *name)
 "                            passwords and/or encryption keys\n"
 "  -T, --trace [[enable=]<pattern>][,events=<file>][,file=<file>]\n"
 "                            specify tracing options\n"
+"  --fork                    fork off the server process and exit the parent\n"
+"                            once the server is running\n"
 #ifdef __linux__
 "Kernel NBD client support:\n"
 "  -c, --connect=DEV         connect FILE to the local NBD device DEV\n"
@@ -503,6 +506,7 @@ int main(int argc, char **argv)
         { "tls-creds", required_argument, NULL, QEMU_NBD_OPT_TLSCREDS },
         { "image-opts", no_argument, NULL, QEMU_NBD_OPT_IMAGE_OPTS },
         { "trace", required_argument, NULL, 'T' },
+        { "fork", no_argument, NULL, QEMU_NBD_OPT_FORK },
         { NULL, 0, NULL, 0 }
     };
     int ch;
@@ -524,6 +528,8 @@ int main(int argc, char **argv)
     bool imageOpts = false;
     bool writethrough = true;
     char *trace_file = NULL;
+    bool fork_process = false;
+    int old_stderr = -1;
 
     /* The client thread uses SIGTERM to interrupt the server.  A signal
      * handler ensures that "qemu-nbd -v -c" exits with a nice status code.
@@ -714,6 +720,9 @@ int main(int argc, char **argv)
             g_free(trace_file);
             trace_file = trace_opt_parse(optarg);
             break;
+        case QEMU_NBD_OPT_FORK:
+            fork_process = true;
+            break;
         }
     }
 
@@ -773,7 +782,7 @@ int main(int argc, char **argv)
         return 0;
     }
 
-    if (device && !verbose) {
+    if ((device && !verbose) || fork_process) {
         int stderr_fd[2];
         pid_t pid;
         int ret;
@@ -796,6 +805,7 @@ int main(int argc, char **argv)
             ret = qemu_daemon(1, 0);
 
             /* Temporarily redirect stderr to the parent's pipe...  */
+            old_stderr = dup(STDERR_FILENO);
             dup2(stderr_fd[1], STDERR_FILENO);
             if (ret < 0) {
                 error_report("Failed to daemonize: %s", strerror(errno));
@@ -951,6 +961,11 @@ int main(int argc, char **argv)
         exit(EXIT_FAILURE);
     }
 
+    if (fork_process) {
+        dup2(old_stderr, STDERR_FILENO);
+        close(old_stderr);
+    }
+
     state = RUNNING;
     do {
         main_loop_wait(false);
diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index 91ebf04..b7a9c6d 100644
--- a/qemu-nbd.texi
+++ b/qemu-nbd.texi
@@ -86,6 +86,8 @@ the new style NBD protocol negotiation
 Enable mandatory TLS encryption for the server by setting the ID
 of the TLS credentials object previously created with the --object
 option.
+@item --fork
+Fork off the server process and exit the parent once the server is running.
 @item -v, --verbose
 Display extra debugging information
 @item -h, --help
-- 
2.10.0

[Qemu-devel] [PATCH v4 2/3] iotests: Remove raciness from 162

[Max Reitz]

With qemu-nbd's new --fork option, we no longer need to launch it the
hacky way.

Suggested-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/162 | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/162 b/tests/qemu-iotests/162
index 0b43ea3..c7e6593 100755
--- a/tests/qemu-iotests/162
+++ b/tests/qemu-iotests/162
@@ -51,8 +51,7 @@ $QEMU_IMG info 'json:{"driver": "nbd", "host": "does.not.exist.example.com", "po
 # This is a test for NBD's bdrv_refresh_filename() implementation: It expects
 # either host or path to be set, but it must not assume that they are set to
 # strings in the options QDict
-$QEMU_NBD -k "$PWD/42" -f raw null-co:// &
-sleep 0.5
+$QEMU_NBD -k "$PWD/42" -f raw --fork null-co://
 $QEMU_IMG info 'json:{"driver": "nbd", "path": 42}' | grep '^image'
 rm -f 42
 
-- 
2.10.0

[Qemu-devel] [PATCH v4 3/3] iotests: Do not rely on unavailable domains in 162

[Max Reitz]

There are some (mostly ISP-specific) name servers who will redirect
non-existing domains to special hosts. In this case, we will get a
different error message when trying to connect to such a host, which
breaks test 162.

162 needed this specific error message so it can confirm that qemu was
indeed trying to connect to the user-specified port. However, we can
also confirm this by setting up a local NBD server on exactly that port;
so we can fix the issue by doing just that.

Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/162     | 19 +++++++++++++++----
 tests/qemu-iotests/162.out |  2 +-
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/162 b/tests/qemu-iotests/162
index c7e6593..f8eecb3 100755
--- a/tests/qemu-iotests/162
+++ b/tests/qemu-iotests/162
@@ -43,10 +43,21 @@ echo '=== NBD ==='
 $QEMU_IMG info 'json:{"driver": "nbd", "host": 42}'
 
 # And this should not treat @port as if it had not been specified
-# (We cannot use localhost with an invalid port here, but we need to use a
-#  non-existing domain, because otherwise the error message will not contain
-#  the port)
-$QEMU_IMG info 'json:{"driver": "nbd", "host": "does.not.exist.example.com", "port": 42}'
+# (We need to set up a server here, because the error message for "Connection
+#  refused" does not contain the destination port)
+
+# Launching qemu-nbd is done in a loop: We try to set up an NBD server on some
+# random port and continue until success, i.e. until we have found a port that
+# is not in use yet.
+while true; do
+    port=$((RANDOM + 32768))
+    if $QEMU_NBD -p $port -f raw --fork null-co:// 2> /dev/null; then
+        break
+    fi
+done
+
+$QEMU_IMG info "json:{'driver': 'nbd', 'host': 'localhost', 'port': $port}" \
+    | grep '^image' | sed -e "s/$port/PORT/"
 
 # This is a test for NBD's bdrv_refresh_filename() implementation: It expects
 # either host or path to be set, but it must not assume that they are set to
diff --git a/tests/qemu-iotests/162.out b/tests/qemu-iotests/162.out
index 9bba723..3c5be2c 100644
--- a/tests/qemu-iotests/162.out
+++ b/tests/qemu-iotests/162.out
@@ -2,7 +2,7 @@ QA output created by 162
 
 === NBD ===
 qemu-img: Could not open 'json:{"driver": "nbd", "host": 42}': Failed to connect socket: Invalid argument
-qemu-img: Could not open 'json:{"driver": "nbd", "host": "does.not.exist.example.com", "port": 42}': address resolution failed for does.not.exist.example.com:42: Name or service not known
+image: nbd://localhost:PORT
 image: nbd+unix://?socket=42
 
 === SSH ===
-- 
2.10.0

Re: [Qemu-devel] [Qemu-block] [PATCH v4 0/3] iotests: Fix test 162

[Hao QingFeng]

Max,

Just a common question for this case, if sshx block driver wasn't built 
into qemu-img, this case would fail as below:

exec /home/haoqf/KVMonz/qemu/tests/qemu-iotests/../../qemu-img info 
--image-opts driver=ssh,host=localhost,port=0.42,path=/foo
qemu-img: Could not open 
'driver=ssh,host=localhost,port=0.42,path=/foo': Unknown driver 'ssh'

Adding 162.notrun can bypass this case but it would skip it even if 
qemu-img has sshx block driver, in which case I think it should be run.

So How about adding a script to dynamically check at runtime if the 
current env qemu-img can meet the requirement to run the test or not?

I made a sample here whose result is:

[Without sshx built in]

./check -qcow2 162
... ...
162 0s ... [not run] case 162 not applicable!
Not run: 162
Passed all 0 tests

[Without sshx built in]
./check -qcow2 162
... ...
162 0s ...
Passed all 1 tests

Rough code patch is(new file 162.check is introduced to check if sshx is 
built in):

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 4cba215..e7ef395 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -18,7 +18,6 @@
  #
  # Control script for QA
  #
-
  status=0
  needwrap=true
  try=0
@@ -291,6 +290,24 @@ do
          start=`_wallclock`
          $timestamp && echo -n "        ["`date "+%T"`"]"

+       check_ret=0
+       if [ -f "$source_iotests/$seq.check" -a -x 
"$source_iotests/$seq.check" ]; then
+           if [ "$(head -n 1 "$source_iotests/$seq.check")" == 
"#!/usr/bin/env python" ]; then
+                $PYTHON $seq.check
+            else
+                ./$seq.check
+           fi
+           check_ret=$?
+       fi
+       if [ $check_ret -ne 0 ]; then
+            $timestamp || echo -n " [not run] "
+            $timestamp && echo " [not run]" && echo -n "        $seq -- "
+           echo "case $seq not applicable!"
+            notrun="$notrun $seq"
+           seq="after_$seq"
+           continue
+       fi
+
          if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env 
python" ]; then
              run_command="$PYTHON $seq"
          else

diff --git a/tests/qemu-iotests/162.check b/tests/qemu-iotests/162.check
new file mode 100755
index 0000000..a80df7a
--- /dev/null
+++ b/tests/qemu-iotests/162.check
@@ -0,0 +1,35 @@
+#!/bin/bash
+#Return 0 if the case can run, others can not
+#Typically the block drivers can be queried by "qemu-img --help" and
+#the output is as:
+#Supported formats: dmg luks ssh sheepdog nbd null-aio null-co 
host_cdrom host_device file blkreplay blkverify blkdebug parallels 
quorum qed qcow2 vvfat vpc bochs cloop vmdk vdi qcow raw
+#set -x
+
+#. ./common.config
+found=0
+. ./common.rc
+. ./common.filter
+#_supported_fmt generic
+#_supported_os Linux
+blk_drivers=`$QEMU_IMG --help|grep "Supported formats:"|sed 
's/Supported formats://'`
+#echo "drivers:"$blk_drivers
+#echo $blk_drivers|awk '{print $0}'
+found=$(
+echo $blk_drivers|awk '{n=split($0, arr_drivers, " ");
+               for(i=1; i<=n; i++) print arr_drivers[i]}' | { while 
read driver
+               do
+                   if [ "$driver"x = "sshx" ]; then
+                       echo 1
+                       exit
+                   fi
+               done
+               echo 0
+       }
+)
+
+#echo "ret:$found"
+if [ "$found" = "1" ]; then
+   exit 0
+fi
+
+exit 1

Thanks!

Hao QingFeng


在 2016-09-29 4:46, Max Reitz 写道:
> 162 is potentially racy and makes some invalid assumptions about what
> should happen when connecting to a non-existing domain name. This series
> fixes both issues.
>
>
> v4:
> - Added documentation for the new --fork option [Kevin]
>
>
> git-backport-diff against v3:
>
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream
> patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences,
> respectively
>
> 001/3:[0004] [FC] 'qemu-nbd: Add --fork option'
> 002/3:[----] [--] 'iotests: Remove raciness from 162'
> 003/3:[----] [--] 'iotests: Do not rely on unavailable domains in 162'
>
>
> Max Reitz (3):
>    qemu-nbd: Add --fork option
>    iotests: Remove raciness from 162
>    iotests: Do not rely on unavailable domains in 162
>
>   qemu-nbd.c                 | 17 ++++++++++++++++-
>   qemu-nbd.texi              |  2 ++
>   tests/qemu-iotests/162     | 22 ++++++++++++++++------
>   tests/qemu-iotests/162.out |  2 +-
>   4 files changed, 35 insertions(+), 8 deletions(-)
>

-- 
QingFeng Hao

Re: [Qemu-devel] [Qemu-block] [PATCH v4 0/3] iotests: Fix test 162

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]

On 12.10.2016 10:55, Hao QingFeng wrote:
> Max,
> 
> Just a common question for this case, if sshx block driver wasn't built
> into qemu-img, this case would fail as below:

Good point, and thanks for bringing it up, but it's not directly linked
to this series other than by its subject, of course, so I'd rather add a
fix on top.

> exec /home/haoqf/KVMonz/qemu/tests/qemu-iotests/../../qemu-img info
> --image-opts driver=ssh,host=localhost,port=0.42,path=/foo
> qemu-img: Could not open
> 'driver=ssh,host=localhost,port=0.42,path=/foo': Unknown driver 'ssh'
> 
> Adding 162.notrun can bypass this case but it would skip it even if
> qemu-img has sshx block driver, in which case I think it should be run.
> 
> So How about adding a script to dynamically check at runtime if the
> current env qemu-img can meet the requirement to run the test or not?

Unfortunately, the list of block drivers listed by will not contain ssh
if ssh is built as a module, which is possible.

This is a bug that should be fixed, but I'd rather do so in a separate
series from this one.

In any case, once it is fixed I'd rather just take the approach quorum
tests take already (e.g. test 081), which is something like:

test_ssh=$($QEMU_IMG --help | grep '^Supported formats:.* ssh\( \|$\)')
[ "$test_ssh" = "" ] && _notrun "ssh support required"

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 480 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH v4 0/3] iotests: Fix test 162

[Hao QingFeng]

在 2016-10-13 3:46, Max Reitz 写道:
> On 12.10.2016 10:55, Hao QingFeng wrote:
>> Max,
>>
>> Just a common question for this case, if sshx block driver wasn't built
>> into qemu-img, this case would fail as below:
> Good point, and thanks for bringing it up, but it's not directly linked
> to this series other than by its subject, of course, so I'd rather add a
> fix on top.
Thanks and sorry for sending to the improper mail series.
>> exec /home/haoqf/KVMonz/qemu/tests/qemu-iotests/../../qemu-img info
>> --image-opts driver=ssh,host=localhost,port=0.42,path=/foo
>> qemu-img: Could not open
>> 'driver=ssh,host=localhost,port=0.42,path=/foo': Unknown driver 'ssh'
>>
>> Adding 162.notrun can bypass this case but it would skip it even if
>> qemu-img has sshx block driver, in which case I think it should be run.
>>
>> So How about adding a script to dynamically check at runtime if the
>> current env qemu-img can meet the requirement to run the test or not?
> Unfortunately, the list of block drivers listed by will not contain ssh
> if ssh is built as a module, which is possible.
Actually I am not sure if I understood it. Do you mean 
"CONFIG_LIBSSH2=m" set
rather than "CONFIG_LIBSSH2=y" in config-host.mak? But in the configure it's
set to be "CONFIG_LIBSSH2=y":
if test "$libssh2" = "yes" ; then
   echo "CONFIG_LIBSSH2=y" >> $config_host_mak
   echo "LIBSSH2_CFLAGS=$libssh2_cflags" >> $config_host_mak
   echo "LIBSSH2_LIBS=$libssh2_libs" >> $config_host_mak
fi
Meanwhile I changed it to be "CONFIG_LIBSSH2=m" and reconfig, make the qemu,
qemu-img --help can still prompt ssh.
> This is a bug that should be fixed, but I'd rather do so in a separate
> series from this one.
>
> In any case, once it is fixed I'd rather just take the approach quorum
> tests take already (e.g. test 081), which is something like:
>
> test_ssh=$($QEMU_IMG --help | grep '^Supported formats:.* ssh\( \|$\)')
> [ "$test_ssh" = "" ] && _notrun "ssh support required"
Cool. Agree with this like what was done in 081.  thanks
> Max
>

-- 
QingFeng Hao

Re: [Qemu-devel] [Qemu-block] [PATCH v4 0/3] iotests: Fix test 162

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 2501 bytes --]

On 13.10.2016 07:20, Hao QingFeng wrote:
> 
> 
> 在 2016-10-13 3:46, Max Reitz 写道:
>> On 12.10.2016 10:55, Hao QingFeng wrote:
>>> Max,
>>>
>>> Just a common question for this case, if sshx block driver wasn't built
>>> into qemu-img, this case would fail as below:
>> Good point, and thanks for bringing it up, but it's not directly linked
>> to this series other than by its subject, of course, so I'd rather add a
>> fix on top.
> Thanks and sorry for sending to the improper mail series.
>>> exec /home/haoqf/KVMonz/qemu/tests/qemu-iotests/../../qemu-img info
>>> --image-opts driver=ssh,host=localhost,port=0.42,path=/foo
>>> qemu-img: Could not open
>>> 'driver=ssh,host=localhost,port=0.42,path=/foo': Unknown driver 'ssh'
>>>
>>> Adding 162.notrun can bypass this case but it would skip it even if
>>> qemu-img has sshx block driver, in which case I think it should be run.
>>>
>>> So How about adding a script to dynamically check at runtime if the
>>> current env qemu-img can meet the requirement to run the test or not?
>> Unfortunately, the list of block drivers listed by will not contain ssh
>> if ssh is built as a module, which is possible.
> Actually I am not sure if I understood it. Do you mean
> "CONFIG_LIBSSH2=m" set
> rather than "CONFIG_LIBSSH2=y" in config-host.mak? But in the configure
> it's
> set to be "CONFIG_LIBSSH2=y":
> if test "$libssh2" = "yes" ; then
>   echo "CONFIG_LIBSSH2=y" >> $config_host_mak
>   echo "LIBSSH2_CFLAGS=$libssh2_cflags" >> $config_host_mak
>   echo "LIBSSH2_LIBS=$libssh2_libs" >> $config_host_mak
> fi

I don't know which version of qemu you are looking at, but on master it
says "m" instead of "y" there:

http://git.qemu.org/?p=qemu.git;a=blob;f=configure;h=dd9e6792bbe04411d81eb5438d58eb1999d4dcd2;hb=HEAD#l5477

> Meanwhile I changed it to be "CONFIG_LIBSSH2=m" and reconfig, make the
> qemu,
> qemu-img --help can still prompt ssh.

Have you tried building master with --enable-modules specified for
configure?

Max

>> This is a bug that should be fixed, but I'd rather do so in a separate
>> series from this one.
>>
>> In any case, once it is fixed I'd rather just take the approach quorum
>> tests take already (e.g. test 081), which is something like:
>>
>> test_ssh=$($QEMU_IMG --help | grep '^Supported formats:.* ssh\( \|$\)')
>> [ "$test_ssh" = "" ] && _notrun "ssh support required"
> Cool. Agree with this like what was done in 081.  thanks
>> Max
>>
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 480 bytes --]

Re: [Qemu-devel] [PATCH v4 0/3] iotests: Fix test 162

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1175 bytes --]

On 28.09.2016 22:46, Max Reitz wrote:
> 162 is potentially racy and makes some invalid assumptions about what
> should happen when connecting to a non-existing domain name. This series
> fixes both issues.
> 
> 
> v4:
> - Added documentation for the new --fork option [Kevin]
> 
> 
> git-backport-diff against v3:
> 
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream
> patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences,
> respectively
> 
> 001/3:[0004] [FC] 'qemu-nbd: Add --fork option'
> 002/3:[----] [--] 'iotests: Remove raciness from 162'
> 003/3:[----] [--] 'iotests: Do not rely on unavailable domains in 162'
> 
> 
> Max Reitz (3):
>   qemu-nbd: Add --fork option
>   iotests: Remove raciness from 162
>   iotests: Do not rely on unavailable domains in 162
> 
>  qemu-nbd.c                 | 17 ++++++++++++++++-
>  qemu-nbd.texi              |  2 ++
>  tests/qemu-iotests/162     | 22 ++++++++++++++++------
>  tests/qemu-iotests/162.out |  2 +-
>  4 files changed, 35 insertions(+), 8 deletions(-)

Ping


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 480 bytes --]

Re: [Qemu-devel] [PATCH v4 0/3] iotests: Fix test 162

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 1392 bytes --]

Am 17.10.2016 um 19:07 hat Max Reitz geschrieben:
> On 28.09.2016 22:46, Max Reitz wrote:
> > 162 is potentially racy and makes some invalid assumptions about what
> > should happen when connecting to a non-existing domain name. This series
> > fixes both issues.
> > 
> > 
> > v4:
> > - Added documentation for the new --fork option [Kevin]
> > 
> > 
> > git-backport-diff against v3:
> > 
> > Key:
> > [----] : patches are identical
> > [####] : number of functional differences between upstream/downstream
> > patch
> > [down] : patch is downstream-only
> > The flags [FC] indicate (F)unctional and (C)ontextual differences,
> > respectively
> > 
> > 001/3:[0004] [FC] 'qemu-nbd: Add --fork option'
> > 002/3:[----] [--] 'iotests: Remove raciness from 162'
> > 003/3:[----] [--] 'iotests: Do not rely on unavailable domains in 162'
> > 
> > 
> > Max Reitz (3):
> >   qemu-nbd: Add --fork option
> >   iotests: Remove raciness from 162
> >   iotests: Do not rely on unavailable domains in 162
> > 
> >  qemu-nbd.c                 | 17 ++++++++++++++++-
> >  qemu-nbd.texi              |  2 ++
> >  tests/qemu-iotests/162     | 22 ++++++++++++++++------
> >  tests/qemu-iotests/162.out |  2 +-
> >  4 files changed, 35 insertions(+), 8 deletions(-)
> 
> Ping

I expected that Sascha would review this as he had comments on earlier
versions?

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH v4 0/3] iotests: Fix test 162

[Hao QingFeng]

在 2016-10-16 1:27, Max Reitz 写道:
> On 13.10.2016 07:20, Hao QingFeng wrote:
>>
>> 在 2016-10-13 3:46, Max Reitz 写道:
>>> On 12.10.2016 10:55, Hao QingFeng wrote:
>>>> Max,
>>>>
>>>> Just a common question for this case, if sshx block driver wasn't built
>>>> into qemu-img, this case would fail as below:
>>> Good point, and thanks for bringing it up, but it's not directly linked
>>> to this series other than by its subject, of course, so I'd rather add a
>>> fix on top.
>> Thanks and sorry for sending to the improper mail series.
>>>> exec /home/haoqf/KVMonz/qemu/tests/qemu-iotests/../../qemu-img info
>>>> --image-opts driver=ssh,host=localhost,port=0.42,path=/foo
>>>> qemu-img: Could not open
>>>> 'driver=ssh,host=localhost,port=0.42,path=/foo': Unknown driver 'ssh'
>>>>
>>>> Adding 162.notrun can bypass this case but it would skip it even if
>>>> qemu-img has sshx block driver, in which case I think it should be run.
>>>>
>>>> So How about adding a script to dynamically check at runtime if the
>>>> current env qemu-img can meet the requirement to run the test or not?
>>> Unfortunately, the list of block drivers listed by will not contain ssh
>>> if ssh is built as a module, which is possible.
>> Actually I am not sure if I understood it. Do you mean
>> "CONFIG_LIBSSH2=m" set
>> rather than "CONFIG_LIBSSH2=y" in config-host.mak? But in the configure
>> it's
>> set to be "CONFIG_LIBSSH2=y":
>> if test "$libssh2" = "yes" ; then
>>    echo "CONFIG_LIBSSH2=y" >> $config_host_mak
>>    echo "LIBSSH2_CFLAGS=$libssh2_cflags" >> $config_host_mak
>>    echo "LIBSSH2_LIBS=$libssh2_libs" >> $config_host_mak
>> fi
> I don't know which version of qemu you are looking at, but on master it
> says "m" instead of "y" there:
>
> http://git.qemu.org/?p=qemu.git;a=blob;f=configure;h=dd9e6792bbe04411d81eb5438d58eb1999d4dcd2;hb=HEAD#l5477
>
>> Meanwhile I changed it to be "CONFIG_LIBSSH2=m" and reconfig, make the
>> qemu,
>> qemu-img --help can still prompt ssh.
> Have you tried building master with --enable-modules specified for
> configure?
You are right, I tried the latest master code(commit 
b49e452fe994f8fbcd2) with
"./configure --target-list=s390x-softmmu --enable-debug --enable-modules 
--enable-libssh2 --prefix=/usr"
and qemu-img --help didn't include the ssh and CONFIG_LIBSSH2=m in 
config-host.mak.
Maybe it was changed in recent code. So how to deal with this case for 
162 or just ignore it now?
thanks!

> Max
>
>>> This is a bug that should be fixed, but I'd rather do so in a separate
>>> series from this one.
>>>
>>> In any case, once it is fixed I'd rather just take the approach quorum
>>> tests take already (e.g. test 081), which is something like:
>>>
>>> test_ssh=$($QEMU_IMG --help | grep '^Supported formats:.* ssh\( \|$\)')
>>> [ "$test_ssh" = "" ] && _notrun "ssh support required"
>> Cool. Agree with this like what was done in 081.  thanks
>>> Max
>>>
>

-- 
QingFeng Hao(Robin)

Re: [Qemu-devel] [Qemu-block] [PATCH v4 0/3] iotests: Fix test 162

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 1594 bytes --]

Am 18.10.2016 um 11:36 hat Kevin Wolf geschrieben:
> Am 17.10.2016 um 19:07 hat Max Reitz geschrieben:
> > On 28.09.2016 22:46, Max Reitz wrote:
> > > 162 is potentially racy and makes some invalid assumptions about what
> > > should happen when connecting to a non-existing domain name. This series
> > > fixes both issues.
> > > 
> > > 
> > > v4:
> > > - Added documentation for the new --fork option [Kevin]
> > > 
> > > 
> > > git-backport-diff against v3:
> > > 
> > > Key:
> > > [----] : patches are identical
> > > [####] : number of functional differences between upstream/downstream
> > > patch
> > > [down] : patch is downstream-only
> > > The flags [FC] indicate (F)unctional and (C)ontextual differences,
> > > respectively
> > > 
> > > 001/3:[0004] [FC] 'qemu-nbd: Add --fork option'
> > > 002/3:[----] [--] 'iotests: Remove raciness from 162'
> > > 003/3:[----] [--] 'iotests: Do not rely on unavailable domains in 162'
> > > 
> > > 
> > > Max Reitz (3):
> > >   qemu-nbd: Add --fork option
> > >   iotests: Remove raciness from 162
> > >   iotests: Do not rely on unavailable domains in 162
> > > 
> > >  qemu-nbd.c                 | 17 ++++++++++++++++-
> > >  qemu-nbd.texi              |  2 ++
> > >  tests/qemu-iotests/162     | 22 ++++++++++++++++------
> > >  tests/qemu-iotests/162.out |  2 +-
> > >  4 files changed, 35 insertions(+), 8 deletions(-)
> > 
> > Ping
> 
> I expected that Sascha would review this as he had comments on earlier
> versions?

Apparently that's not happening, so I've applied the series now.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]