diff mbox

[v4,09/12] iotests.py: Add qemu_nbd function

Message ID 20160928205602.17275-10-mreitz@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Max Reitz Sept. 28, 2016, 8:55 p.m. UTC 
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Kevin Wolf Oct. 13, 2016, 1:11 p.m. UTC | #1 
Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/iotests.py | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 3329bc1..5a2678f 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -39,6 +39,10 @@ qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
>  if os.environ.get('QEMU_IO_OPTIONS'):
>      qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
>  
> +qemu_nbd_args = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]
> +if os.environ.get('QEMU_NBD_OPTIONS'):
> +    qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
> +
>  qemu_prog = os.environ.get('QEMU_PROG', 'qemu')
>  qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ')
>  
> @@ -87,6 +91,10 @@ def qemu_io(*args):
>          sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' '.join(args)))
>      return subp.communicate()[0]
>  
> +def qemu_nbd(*args):
> +    '''Run qemu-nbd in daemon mode and return the parent's exit code'''
> +    return subprocess.call(qemu_nbd_args + ['--fork'] + list(args))

Wouldn't it be better to always use -t, track the PID and shut it down
explicitly when the test exits?

The way you're using qemu-nbd here is fine if the test case passes, but
if it fails before we access the NBD server, the server keeps running in
the background.

Kevin
Max Reitz Oct. 15, 2016, 5:17 p.m. UTC | #2 
On 13.10.2016 15:11, Kevin Wolf wrote:
> Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  tests/qemu-iotests/iotests.py | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 3329bc1..5a2678f 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -39,6 +39,10 @@ qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
>>  if os.environ.get('QEMU_IO_OPTIONS'):
>>      qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
>>  
>> +qemu_nbd_args = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]
>> +if os.environ.get('QEMU_NBD_OPTIONS'):
>> +    qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
>> +
>>  qemu_prog = os.environ.get('QEMU_PROG', 'qemu')
>>  qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ')
>>  
>> @@ -87,6 +91,10 @@ def qemu_io(*args):
>>          sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' '.join(args)))
>>      return subp.communicate()[0]
>>  
>> +def qemu_nbd(*args):
>> +    '''Run qemu-nbd in daemon mode and return the parent's exit code'''
>> +    return subprocess.call(qemu_nbd_args + ['--fork'] + list(args))
> 
> Wouldn't it be better to always use -t, track the PID and shut it down
> explicitly when the test exits?

Probably. It's a lot more complicated, though. I'll see what I can do
but I'm not sure if I can do a lot before 2.8.

Max

> The way you're using qemu-nbd here is fine if the test case passes, but
> if it fails before we access the NBD server, the server keeps running in
> the background.
> 
> Kevin
Kevin Wolf Oct. 17, 2016, 8:33 a.m. UTC | #3 
Am 15.10.2016 um 19:17 hat Max Reitz geschrieben:
> On 13.10.2016 15:11, Kevin Wolf wrote:
> > Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
> >> Signed-off-by: Max Reitz <mreitz@redhat.com>
> >> ---
> >>  tests/qemu-iotests/iotests.py | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> >> index 3329bc1..5a2678f 100644
> >> --- a/tests/qemu-iotests/iotests.py
> >> +++ b/tests/qemu-iotests/iotests.py
> >> @@ -39,6 +39,10 @@ qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
> >>  if os.environ.get('QEMU_IO_OPTIONS'):
> >>      qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
> >>  
> >> +qemu_nbd_args = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]
> >> +if os.environ.get('QEMU_NBD_OPTIONS'):
> >> +    qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
> >> +
> >>  qemu_prog = os.environ.get('QEMU_PROG', 'qemu')
> >>  qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ')
> >>  
> >> @@ -87,6 +91,10 @@ def qemu_io(*args):
> >>          sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' '.join(args)))
> >>      return subp.communicate()[0]
> >>  
> >> +def qemu_nbd(*args):
> >> +    '''Run qemu-nbd in daemon mode and return the parent's exit code'''
> >> +    return subprocess.call(qemu_nbd_args + ['--fork'] + list(args))
> > 
> > Wouldn't it be better to always use -t, track the PID and shut it down
> > explicitly when the test exits?
> 
> Probably. It's a lot more complicated, though. I'll see what I can do
> but I'm not sure if I can do a lot before 2.8.

In that case, I'd prefer to have this series in 2.8 and improve the test
case later, so don't let this stop you from sending the next version.

Kevin
diff mbox

Patch

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 3329bc1..5a2678f 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -39,6 +39,10 @@  qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
 if os.environ.get('QEMU_IO_OPTIONS'):
     qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
 
+qemu_nbd_args = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]
+if os.environ.get('QEMU_NBD_OPTIONS'):
+    qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
+
 qemu_prog = os.environ.get('QEMU_PROG', 'qemu')
 qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ')
 
@@ -87,6 +91,10 @@  def qemu_io(*args):
         sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' '.join(args)))
     return subp.communicate()[0]
 
+def qemu_nbd(*args):
+    '''Run qemu-nbd in daemon mode and return the parent's exit code'''
+    return subprocess.call(qemu_nbd_args + ['--fork'] + list(args))
+
 def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt):
     '''Return True if two image files are identical'''
     return qemu_img('compare', '-f', fmt1,