Re: [PATCH v6 08/11] iotests: add testenv.py

[Vladimir Sementsov-Ogievskiy]

18.01.2021 12:59, Kevin Wolf wrote:

Am 16.01.2021 um 12:03 hat Vladimir Sementsov-Ogievskiy geschrieben:

15.01.2021 16:30, Vladimir Sementsov-Ogievskiy wrote:

15.01.2021 16:20, Kevin Wolf wrote:

Am 15.01.2021 um 14:10 hat Vladimir Sementsov-Ogievskiy geschrieben:

15.01.2021 15:45, Kevin Wolf wrote:

Am 15.01.2021 um 13:19 hat Vladimir Sementsov-Ogievskiy geschrieben:

15.01.2021 14:18, Kevin Wolf wrote:

Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:

Add TestEnv class, which will handle test environment in a new python
iotests running framework.

Difference with current ./check interface:
- -v (verbose) option dropped, as it is unused

- -xdiff option is dropped, until somebody complains that it is needed
- same for -n option

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
    tests/qemu-iotests/testenv.py | 328 ++
    1 file changed, 328 insertions(+)
    create mode 100755 tests/qemu-iotests/testenv.py



[..]


+    def init_binaries(self):
+        """Init binary path variables:
+             PYTHON (for bash tests)
+             QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, 
QEMU_NBD_PROG, QSD_PROG
+             SOCKET_SCM_HELPER
+        """
+        self.python = '/usr/bin/python3 -B'


This doesn't look right, we need to respect the Python binary set in
configure (which I think we get from common.env)


Oh, I missed the change. Then I should just drop this self.python.


Do we still get the value from elsewhere or do we need to manually parse
common.env?


Hmm.. Good question. We have either parse common.env, and still create 
self.python variable.

Or drop it, and include common.env directly to bash tests. For this we'll need 
to export

BUILD_IOTESTS, and do
  . $BUILD_IOTESTS/common.env

in common.rc..


check uses it, too, for running Python test cases.



But new check (written in python) doesn't.. Should I keep bash check, which 
will have only one line to call check.py with help of PYTHON?




Or finally, may be just drop it? Can we just use system python for
tests, now when we are already in a python3 world?


You can only assume the Python 3 (or more specifically 3.6+) world if
you are using the Python interpreter that was passed to configure. This
will commonly be the same thing as the first python3 in PATH, but it
doesn't have to be.

So your solution of just using the same interpreter as for check might
be okay as long as we make sure that 'make check' calls check with the
configured Python interpreter instead of relying on the shebang line.



I've sent v7 with exactly this thing: use same interpreter as for check and 
adjust check-block.sh which is called from make check


--
Best regards,
Vladimir

Re: [PATCH v6 08/11] iotests: add testenv.py

[Kevin Wolf]

Am 16.01.2021 um 12:03 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 15.01.2021 16:30, Vladimir Sementsov-Ogievskiy wrote:
> > 15.01.2021 16:20, Kevin Wolf wrote:
> > > Am 15.01.2021 um 14:10 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > > 15.01.2021 15:45, Kevin Wolf wrote:
> > > > > Am 15.01.2021 um 13:19 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > > > > 15.01.2021 14:18, Kevin Wolf wrote:
> > > > > > > Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy 
> > > > > > > geschrieben:
> > > > > > > > Add TestEnv class, which will handle test environment in a new 
> > > > > > > > python
> > > > > > > > iotests running framework.
> > > > > > > > 
> > > > > > > > Difference with current ./check interface:
> > > > > > > > - -v (verbose) option dropped, as it is unused
> > > > > > > > 
> > > > > > > > - -xdiff option is dropped, until somebody complains that it is 
> > > > > > > > needed
> > > > > > > > - same for -n option
> > > > > > > > 
> > > > > > > > Signed-off-by: Vladimir Sementsov-Ogievskiy 
> > > > > > > > 
> > > > > > > > ---
> > > > > > > >     tests/qemu-iotests/testenv.py | 328 
> > > > > > > > ++
> > > > > > > >     1 file changed, 328 insertions(+)
> > > > > > > >     create mode 100755 tests/qemu-iotests/testenv.py
> > > > > > > > 
> > > > 
> > > > [..]
> > > > 
> > > > > > > > +    def init_binaries(self):
> > > > > > > > +        """Init binary path variables:
> > > > > > > > +             PYTHON (for bash tests)
> > > > > > > > +             QEMU_PROG, QEMU_IMG_PROG, 
> > > > > > > > QEMU_IO_PROG, QEMU_NBD_PROG, QSD_PROG
> > > > > > > > +             SOCKET_SCM_HELPER
> > > > > > > > +        """
> > > > > > > > +        self.python = '/usr/bin/python3 -B'
> > > > > > > 
> > > > > > > This doesn't look right, we need to respect the Python binary set 
> > > > > > > in
> > > > > > > configure (which I think we get from common.env)
> > > > > > 
> > > > > > Oh, I missed the change. Then I should just drop this self.python.
> > > > > 
> > > > > Do we still get the value from elsewhere or do we need to manually 
> > > > > parse
> > > > > common.env?
> > > > 
> > > > Hmm.. Good question. We have either parse common.env, and still create 
> > > > self.python variable.
> > > > 
> > > > Or drop it, and include common.env directly to bash tests. For this 
> > > > we'll need to export
> > > > 
> > > > BUILD_IOTESTS, and do
> > > >   . $BUILD_IOTESTS/common.env
> > > > 
> > > > in common.rc..
> > > 
> > > check uses it, too, for running Python test cases.
> > > 
> > 
> > But new check (written in python) doesn't.. Should I keep bash check, which 
> > will have only one line to call check.py with help of PYTHON?
> > 
> > 
> 
> Or finally, may be just drop it? Can we just use system python for
> tests, now when we are already in a python3 world?

You can only assume the Python 3 (or more specifically 3.6+) world if
you are using the Python interpreter that was passed to configure. This
will commonly be the same thing as the first python3 in PATH, but it
doesn't have to be.

So your solution of just using the same interpreter as for check might
be okay as long as we make sure that 'make check' calls check with the
configured Python interpreter instead of relying on the shebang line.

Kevin

Re: [PATCH v6 08/11] iotests: add testenv.py

[Vladimir Sementsov-Ogievskiy]

16.01.2021 14:03, Vladimir Sementsov-Ogievskiy wrote:

15.01.2021 16:30, Vladimir Sementsov-Ogievskiy wrote:

15.01.2021 16:20, Kevin Wolf wrote:

Am 15.01.2021 um 14:10 hat Vladimir Sementsov-Ogievskiy geschrieben:

15.01.2021 15:45, Kevin Wolf wrote:

Am 15.01.2021 um 13:19 hat Vladimir Sementsov-Ogievskiy geschrieben:

15.01.2021 14:18, Kevin Wolf wrote:

Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:

Add TestEnv class, which will handle test environment in a new python
iotests running framework.

Difference with current ./check interface:
- -v (verbose) option dropped, as it is unused

- -xdiff option is dropped, until somebody complains that it is needed
- same for -n option

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
    tests/qemu-iotests/testenv.py | 328 ++
    1 file changed, 328 insertions(+)
    create mode 100755 tests/qemu-iotests/testenv.py



[..]


+    def init_binaries(self):
+    """Init binary path variables:
+ PYTHON (for bash tests)
+ QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, QSD_PROG
+ SOCKET_SCM_HELPER
+    """
+    self.python = '/usr/bin/python3 -B'


This doesn't look right, we need to respect the Python binary set in
configure (which I think we get from common.env)


Oh, I missed the change. Then I should just drop this self.python.


Do we still get the value from elsewhere or do we need to manually parse
common.env?


Hmm.. Good question. We have either parse common.env, and still create 
self.python variable.

Or drop it, and include common.env directly to bash tests. For this we'll need 
to export

BUILD_IOTESTS, and do
  . $BUILD_IOTESTS/common.env

in common.rc..


check uses it, too, for running Python test cases.



But new check (written in python) doesn't.. Should I keep bash check, which 
will have only one line to call check.py with help of PYTHON?




Or finally, may be just drop it? Can we just use system python for tests, now 
when we are already in a python3 world?



Oh, I know. We just need to use "self.python = sys.executable", so that tests 
uses the interpreter that runs check.

--
Best regards,
Vladimir

Re: [PATCH v6 08/11] iotests: add testenv.py

[Vladimir Sementsov-Ogievskiy]

15.01.2021 16:30, Vladimir Sementsov-Ogievskiy wrote:

15.01.2021 16:20, Kevin Wolf wrote:

Am 15.01.2021 um 14:10 hat Vladimir Sementsov-Ogievskiy geschrieben:

15.01.2021 15:45, Kevin Wolf wrote:

Am 15.01.2021 um 13:19 hat Vladimir Sementsov-Ogievskiy geschrieben:

15.01.2021 14:18, Kevin Wolf wrote:

Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:

Add TestEnv class, which will handle test environment in a new python
iotests running framework.

Difference with current ./check interface:
- -v (verbose) option dropped, as it is unused

- -xdiff option is dropped, until somebody complains that it is needed
- same for -n option

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
    tests/qemu-iotests/testenv.py | 328 ++
    1 file changed, 328 insertions(+)
    create mode 100755 tests/qemu-iotests/testenv.py



[..]


+    def init_binaries(self):
+    """Init binary path variables:
+ PYTHON (for bash tests)
+ QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, QSD_PROG
+ SOCKET_SCM_HELPER
+    """
+    self.python = '/usr/bin/python3 -B'


This doesn't look right, we need to respect the Python binary set in
configure (which I think we get from common.env)


Oh, I missed the change. Then I should just drop this self.python.


Do we still get the value from elsewhere or do we need to manually parse
common.env?


Hmm.. Good question. We have either parse common.env, and still create 
self.python variable.

Or drop it, and include common.env directly to bash tests. For this we'll need 
to export

BUILD_IOTESTS, and do
  . $BUILD_IOTESTS/common.env

in common.rc..


check uses it, too, for running Python test cases.



But new check (written in python) doesn't.. Should I keep bash check, which 
will have only one line to call check.py with help of PYTHON?




Or finally, may be just drop it? Can we just use system python for tests, now 
when we are already in a python3 world?

--
Best regards,
Vladimir

Re: [PATCH v6 08/11] iotests: add testenv.py

[Vladimir Sementsov-Ogievskiy]

15.01.2021 16:20, Kevin Wolf wrote:

Am 15.01.2021 um 14:10 hat Vladimir Sementsov-Ogievskiy geschrieben:

15.01.2021 15:45, Kevin Wolf wrote:

Am 15.01.2021 um 13:19 hat Vladimir Sementsov-Ogievskiy geschrieben:

15.01.2021 14:18, Kevin Wolf wrote:

Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:

Add TestEnv class, which will handle test environment in a new python
iotests running framework.

Difference with current ./check interface:
- -v (verbose) option dropped, as it is unused

- -xdiff option is dropped, until somebody complains that it is needed
- same for -n option

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
tests/qemu-iotests/testenv.py | 328 ++
1 file changed, 328 insertions(+)
create mode 100755 tests/qemu-iotests/testenv.py



[..]


+def init_binaries(self):
+"""Init binary path variables:
+ PYTHON (for bash tests)
+ QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, QSD_PROG
+ SOCKET_SCM_HELPER
+"""
+self.python = '/usr/bin/python3 -B'


This doesn't look right, we need to respect the Python binary set in
configure (which I think we get from common.env)


Oh, I missed the change. Then I should just drop this self.python.


Do we still get the value from elsewhere or do we need to manually parse
common.env?


Hmm.. Good question. We have either parse common.env, and still create 
self.python variable.

Or drop it, and include common.env directly to bash tests. For this we'll need 
to export

BUILD_IOTESTS, and do
  . $BUILD_IOTESTS/common.env

in common.rc..


check uses it, too, for running Python test cases.



But new check (written in python) doesn't.. Should I keep bash check, which 
will have only one line to call check.py with help of PYTHON?


--
Best regards,
Vladimir

Re: [PATCH v6 08/11] iotests: add testenv.py

[Kevin Wolf]

Am 15.01.2021 um 14:10 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 15.01.2021 15:45, Kevin Wolf wrote:
> > Am 15.01.2021 um 13:19 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > 15.01.2021 14:18, Kevin Wolf wrote:
> > > > Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > > > Add TestEnv class, which will handle test environment in a new python
> > > > > iotests running framework.
> > > > > 
> > > > > Difference with current ./check interface:
> > > > > - -v (verbose) option dropped, as it is unused
> > > > > 
> > > > > - -xdiff option is dropped, until somebody complains that it is needed
> > > > > - same for -n option
> > > > > 
> > > > > Signed-off-by: Vladimir Sementsov-Ogievskiy 
> > > > > ---
> > > > >tests/qemu-iotests/testenv.py | 328 
> > > > > ++
> > > > >1 file changed, 328 insertions(+)
> > > > >create mode 100755 tests/qemu-iotests/testenv.py
> > > > > 
> 
> [..]
> 
> > > > > +def init_binaries(self):
> > > > > +"""Init binary path variables:
> > > > > + PYTHON (for bash tests)
> > > > > + QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, 
> > > > > QSD_PROG
> > > > > + SOCKET_SCM_HELPER
> > > > > +"""
> > > > > +self.python = '/usr/bin/python3 -B'
> > > > 
> > > > This doesn't look right, we need to respect the Python binary set in
> > > > configure (which I think we get from common.env)
> > > 
> > > Oh, I missed the change. Then I should just drop this self.python.
> > 
> > Do we still get the value from elsewhere or do we need to manually parse
> > common.env?
> 
> Hmm.. Good question. We have either parse common.env, and still create 
> self.python variable.
> 
> Or drop it, and include common.env directly to bash tests. For this we'll 
> need to export
> 
> BUILD_IOTESTS, and do
>  . $BUILD_IOTESTS/common.env
> 
> in common.rc..

check uses it, too, for running Python test cases.

Kevin

Re: [PATCH v6 08/11] iotests: add testenv.py

[Vladimir Sementsov-Ogievskiy]

15.01.2021 15:45, Kevin Wolf wrote:

Am 15.01.2021 um 13:19 hat Vladimir Sementsov-Ogievskiy geschrieben:

15.01.2021 14:18, Kevin Wolf wrote:

Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:

Add TestEnv class, which will handle test environment in a new python
iotests running framework.

Difference with current ./check interface:
- -v (verbose) option dropped, as it is unused

- -xdiff option is dropped, until somebody complains that it is needed
- same for -n option

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
   tests/qemu-iotests/testenv.py | 328 ++
   1 file changed, 328 insertions(+)
   create mode 100755 tests/qemu-iotests/testenv.py



[..]


+def init_binaries(self):
+"""Init binary path variables:
+ PYTHON (for bash tests)
+ QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, QSD_PROG
+ SOCKET_SCM_HELPER
+"""
+self.python = '/usr/bin/python3 -B'


This doesn't look right, we need to respect the Python binary set in
configure (which I think we get from common.env)


Oh, I missed the change. Then I should just drop this self.python.


Do we still get the value from elsewhere or do we need to manually parse
common.env?


Hmm.. Good question. We have either parse common.env, and still create 
self.python variable.

Or drop it, and include common.env directly to bash tests. For this we'll need 
to export

BUILD_IOTESTS, and do
 . $BUILD_IOTESTS/common.env

in common.rc..







+def root(*names):
+return os.path.join(self.build_root, *names)
+
+arch = os.uname().machine
+if 'ppc64' in arch:
+arch = 'ppc64'
+
+self.qemu_prog = os.getenv('QEMU_PROG', root(f'qemu-system-{arch}'))
+self.qemu_img_prog = os.getenv('QEMU_IMG_PROG', root('qemu-img'))
+self.qemu_io_prog = os.getenv('QEMU_IO_PROG', root('qemu-io'))
+self.qemu_nbd_prog = os.getenv('QEMU_NBD_PROG', root('qemu-nbd'))
+self.qsd_prog = os.getenv('QSD_PROG', root('storage-daemon',
+   'qemu-storage-daemon'))
+
+for b in [self.qemu_img_prog, self.qemu_io_prog, self.qemu_nbd_prog,
+  self.qemu_prog, self.qsd_prog]:
+if not os.path.exists(b):
+exit('Not such file: ' + b)
+if not os.access(b, os.X_OK):
+exit('Not executable: ' + b)
+
+helper_path = os.path.join(self.build_iotests, 'socket_scm_helper')
+if os.access(helper_path, os.X_OK):
+self.socket_scm_helper = helper_path  # SOCKET_SCM_HELPER
+
+def __init__(self, argv: List[str]) -> None:
+"""Parse args and environment"""
+
+# Initialize generic paths: build_root, build_iotests, source_iotests,
+# which are needed to initialize some environment variables. They are
+# used by init_*() functions as well.
+
+
+if os.path.islink(sys.argv[0]):
+# called from the build tree
+self.source_iotests = os.path.dirname(os.readlink(sys.argv[0]))
+self.build_iotests = os.path.dirname(os.path.abspath(sys.argv[0]))
+else:
+# called from the source tree
+self.source_iotests = os.getcwd()
+self.build_iotests = self.source_iotests
+
+self.build_root = os.path.join(self.build_iotests, '..', '..')
+
+self.init_handle_argv(argv)
+self.init_directories()
+self.init_binaries()
+
+# QEMU_OPTIONS
+self.qemu_options = '-nodefaults -display none -accel qtest'
+machine_map = (
+(('arm', 'aarch64'), 'virt'),


How does this work? Won't we check for "qemu-system-('arm', 'aarch64')"
below, which we'll never find?


Hmm. I just took existing logic from check:

[..]
   case "$QEMU_PROG" in
   *qemu-system-arm|*qemu-system-aarch64)
   export QEMU_OPTIONS="$QEMU_OPTIONS -machine virt"
   ;;
[..]


What I mean is that the code below doesn't look like it's prepared to
interpret a tuple like ('arm', 'aarch64'), it expects a single string:




+('avr', 'mega2560'),
+('rx', 'gdbsim-r5f562n8'),
+('tricore', 'tricore_testboard')
+)
+for suffix, machine in machine_map:
+if self.qemu_prog.endswith(f'qemu-system-{suffix}'):


Here we get effectively:

 suffix: Tuple[str, str] = ('arm', 'aarch64')

The formatted string uses str(suffix), which makes the result
"qemu-system-('arm', 'aarch64')".

Or am I misunderstanding something here?


Ah, you are right:) Will fix. I interpreted your

  Won't we check for "qemu-system-('arm', 'aarch64')"

as
  
  Won't we check for "qemu-system-arm" or "qemu-system-aarch64"




--
Best regards,
Vladimir

Re: [PATCH v6 08/11] iotests: add testenv.py

[Kevin Wolf]

Am 15.01.2021 um 13:19 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 15.01.2021 14:18, Kevin Wolf wrote:
> > Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > Add TestEnv class, which will handle test environment in a new python
> > > iotests running framework.
> > > 
> > > Difference with current ./check interface:
> > > - -v (verbose) option dropped, as it is unused
> > > 
> > > - -xdiff option is dropped, until somebody complains that it is needed
> > > - same for -n option
> > > 
> > > Signed-off-by: Vladimir Sementsov-Ogievskiy 
> > > ---
> > >   tests/qemu-iotests/testenv.py | 328 ++
> > >   1 file changed, 328 insertions(+)
> > >   create mode 100755 tests/qemu-iotests/testenv.py
> > > 
> > > diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
> > > new file mode 100755
> > > index 00..ecaf76fb85
> > > --- /dev/null
> > > +++ b/tests/qemu-iotests/testenv.py
> > > @@ -0,0 +1,328 @@
> > > +#!/usr/bin/env python3
> > > +#
> > > +# Parse command line options to manage test environment variables.
> > > +#
> > > +# Copyright (c) 2020 Virtuozzo International GmbH
> > > +#
> > > +# This program is free software; you can redistribute it and/or modify
> > > +# it under the terms of the GNU General Public License as published by
> > > +# the Free Software Foundation; either version 2 of the License, or
> > > +# (at your option) any later version.
> > > +#
> > > +# This program is distributed in the hope that it will be useful,
> > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > +# GNU General Public License for more details.
> > > +#
> > > +# You should have received a copy of the GNU General Public License
> > > +# along with this program.  If not, see .
> > > +#
> > > +
> > > +import os
> > > +import sys
> > > +import tempfile
> > > +from pathlib import Path
> > > +import shutil
> > > +import collections
> > > +import subprocess
> > > +import argparse
> > > +from typing import List, Dict
> > > +
> > > +
> > > +def get_default_machine(qemu_prog: str) -> str:
> > > +outp = subprocess.run([qemu_prog, '-machine', 'help'], check=True,
> > > +  text=True, stdout=subprocess.PIPE).stdout
> > > +
> > > +machines = outp.split('\n')
> > > +default_machine = next(m for m in machines if m.endswith(' 
> > > (default)'))
> > > +default_machine = default_machine.split(' ', 1)[0]
> > > +
> > > +alias_suf = ' (alias of {})'.format(default_machine)
> > > +alias = next((m for m in machines if m.endswith(alias_suf)), None)
> > > +if alias is not None:
> > > +default_machine = alias.split(' ', 1)[0]
> > > +
> > > +return default_machine
> > > +
> > > +
> > > +class TestEnv:
> > > +"""
> > > +Manage system environment for running tests
> > > +
> > > +The following variables are supported/provided. They are represented 
> > > by
> > > +lower-cased TestEnv attributes.
> > > +"""
> > > +env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 
> > > 'SAMPLE_IMG_DIR',
> > > + 'OUTPUT_DIR', 'PYTHON', 'QEMU_PROG', 
> > > 'QEMU_IMG_PROG',
> > > + 'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
> > > + 'SOCKET_SCM_HELPER', 'QEMU_OPTIONS', 
> > > 'QEMU_IMG_OPTIONS',
> > > + 'QEMU_IO_OPTIONS', 'QEMU_NBD_OPTIONS', 'IMGOPTS',
> > > + 'IMGFMT', 'IMGPROTO', 'AIOMODE', 'CACHEMODE',
> > > + 'VALGRIND_QEMU', 'CACHEMODE_IS_DEFAULT', 
> > > 'IMGFMT_GENERIC',
> > > + 'IMGOPTSSYNTAX', 'IMGKEYSECRET', 
> > > 'QEMU_DEFAULT_MACHINE']
> > > +
> > > +def get_env(self) -> Dict[str, str]:
> > > +env = {}
> > > +for v in self.env_variables:
> > > +val = getattr(self, v.lower(), None)
> > > +if val is not None:
> > > +env[v] = val
> > > +
> > > +return env
> > > +
> > > +_argparser = None
> > > +@classmethod
> > > +def get_argparser(cls) -> argparse.ArgumentParser:
> > > +if cls._argparser is not None:
> > > +return cls._argparser
> > > +
> > > +p = argparse.ArgumentParser(description="= test environment 
> > > options =",
> > > +add_help=False, 
> > > usage=argparse.SUPPRESS)
> > > +
> > > +p.add_argument('-d', dest='debug', action='store_true', 
> > > help='debug')
> > > +p.add_argument('-misalign', action='store_true',
> > > +   help='misalign memory allocations')
> > > +
> > > +p.set_defaults(imgfmt='raw', imgproto='file')
> > > +
> > > +format_list = ['raw', 'bochs', 'cloop', 'parallels', 'qcow', 
> > > 'qcow2',
> > > +   'qed', 'vdi', 'vpc', 'vhdx', 'vmdk', 'luks', 
> > > 'dmg']
> > > +g = p.add_argumen

Re: [PATCH v6 08/11] iotests: add testenv.py

[Vladimir Sementsov-Ogievskiy]

15.01.2021 14:18, Kevin Wolf wrote:

Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:

Add TestEnv class, which will handle test environment in a new python
iotests running framework.

Difference with current ./check interface:
- -v (verbose) option dropped, as it is unused

- -xdiff option is dropped, until somebody complains that it is needed
- same for -n option

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  tests/qemu-iotests/testenv.py | 328 ++
  1 file changed, 328 insertions(+)
  create mode 100755 tests/qemu-iotests/testenv.py

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
new file mode 100755
index 00..ecaf76fb85
--- /dev/null
+++ b/tests/qemu-iotests/testenv.py
@@ -0,0 +1,328 @@
+#!/usr/bin/env python3
+#
+# Parse command line options to manage test environment variables.
+#
+# Copyright (c) 2020 Virtuozzo International GmbH
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import os
+import sys
+import tempfile
+from pathlib import Path
+import shutil
+import collections
+import subprocess
+import argparse
+from typing import List, Dict
+
+
+def get_default_machine(qemu_prog: str) -> str:
+outp = subprocess.run([qemu_prog, '-machine', 'help'], check=True,
+  text=True, stdout=subprocess.PIPE).stdout
+
+machines = outp.split('\n')
+default_machine = next(m for m in machines if m.endswith(' (default)'))
+default_machine = default_machine.split(' ', 1)[0]
+
+alias_suf = ' (alias of {})'.format(default_machine)
+alias = next((m for m in machines if m.endswith(alias_suf)), None)
+if alias is not None:
+default_machine = alias.split(' ', 1)[0]
+
+return default_machine
+
+
+class TestEnv:
+"""
+Manage system environment for running tests
+
+The following variables are supported/provided. They are represented by
+lower-cased TestEnv attributes.
+"""
+env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
+ 'OUTPUT_DIR', 'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
+ 'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
+ 'SOCKET_SCM_HELPER', 'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
+ 'QEMU_IO_OPTIONS', 'QEMU_NBD_OPTIONS', 'IMGOPTS',
+ 'IMGFMT', 'IMGPROTO', 'AIOMODE', 'CACHEMODE',
+ 'VALGRIND_QEMU', 'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC',
+ 'IMGOPTSSYNTAX', 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE']
+
+def get_env(self) -> Dict[str, str]:
+env = {}
+for v in self.env_variables:
+val = getattr(self, v.lower(), None)
+if val is not None:
+env[v] = val
+
+return env
+
+_argparser = None
+@classmethod
+def get_argparser(cls) -> argparse.ArgumentParser:
+if cls._argparser is not None:
+return cls._argparser
+
+p = argparse.ArgumentParser(description="= test environment options =",
+add_help=False, usage=argparse.SUPPRESS)
+
+p.add_argument('-d', dest='debug', action='store_true', help='debug')
+p.add_argument('-misalign', action='store_true',
+   help='misalign memory allocations')
+
+p.set_defaults(imgfmt='raw', imgproto='file')
+
+format_list = ['raw', 'bochs', 'cloop', 'parallels', 'qcow', 'qcow2',
+   'qed', 'vdi', 'vpc', 'vhdx', 'vmdk', 'luks', 'dmg']
+g = p.add_argument_group(
+'image format options',
+'The following options sets IMGFMT environment variable. '


s/sets/set the/


+'At most one chose is allowed, default is "raw"')


s/chose/choice/


+g = g.add_mutually_exclusive_group()
+for fmt in format_list:
+g.add_argument('-' + fmt, dest='imgfmt', action='store_const',
+   const=fmt)
+
+protocol_list = ['file', 'rbd', 'sheepdoc', 'nbd', 'ssh', 'nfs',
+ 'fuse']
+g = p.add_argument_group(
+'image protocol options',
+'The following options sets IMGPROTO environment variably. '
+'At most one chose is allowed, default is "file"')


Same as above, but also s/variably/variable/.

Do we consider these environment var

Re: [PATCH v6 08/11] iotests: add testenv.py

[Kevin Wolf]

Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Add TestEnv class, which will handle test environment in a new python
> iotests running framework.
> 
> Difference with current ./check interface:
> - -v (verbose) option dropped, as it is unused
> 
> - -xdiff option is dropped, until somebody complains that it is needed
> - same for -n option
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  tests/qemu-iotests/testenv.py | 328 ++
>  1 file changed, 328 insertions(+)
>  create mode 100755 tests/qemu-iotests/testenv.py
> 
> diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
> new file mode 100755
> index 00..ecaf76fb85
> --- /dev/null
> +++ b/tests/qemu-iotests/testenv.py
> @@ -0,0 +1,328 @@
> +#!/usr/bin/env python3
> +#
> +# Parse command line options to manage test environment variables.
> +#
> +# Copyright (c) 2020 Virtuozzo International GmbH
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see .
> +#
> +
> +import os
> +import sys
> +import tempfile
> +from pathlib import Path
> +import shutil
> +import collections
> +import subprocess
> +import argparse
> +from typing import List, Dict
> +
> +
> +def get_default_machine(qemu_prog: str) -> str:
> +outp = subprocess.run([qemu_prog, '-machine', 'help'], check=True,
> +  text=True, stdout=subprocess.PIPE).stdout
> +
> +machines = outp.split('\n')
> +default_machine = next(m for m in machines if m.endswith(' (default)'))
> +default_machine = default_machine.split(' ', 1)[0]
> +
> +alias_suf = ' (alias of {})'.format(default_machine)
> +alias = next((m for m in machines if m.endswith(alias_suf)), None)
> +if alias is not None:
> +default_machine = alias.split(' ', 1)[0]
> +
> +return default_machine
> +
> +
> +class TestEnv:
> +"""
> +Manage system environment for running tests
> +
> +The following variables are supported/provided. They are represented by
> +lower-cased TestEnv attributes.
> +"""
> +env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
> + 'OUTPUT_DIR', 'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
> + 'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
> + 'SOCKET_SCM_HELPER', 'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
> + 'QEMU_IO_OPTIONS', 'QEMU_NBD_OPTIONS', 'IMGOPTS',
> + 'IMGFMT', 'IMGPROTO', 'AIOMODE', 'CACHEMODE',
> + 'VALGRIND_QEMU', 'CACHEMODE_IS_DEFAULT', 
> 'IMGFMT_GENERIC',
> + 'IMGOPTSSYNTAX', 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE']
> +
> +def get_env(self) -> Dict[str, str]:
> +env = {}
> +for v in self.env_variables:
> +val = getattr(self, v.lower(), None)
> +if val is not None:
> +env[v] = val
> +
> +return env
> +
> +_argparser = None
> +@classmethod
> +def get_argparser(cls) -> argparse.ArgumentParser:
> +if cls._argparser is not None:
> +return cls._argparser
> +
> +p = argparse.ArgumentParser(description="= test environment options 
> =",
> +add_help=False, usage=argparse.SUPPRESS)
> +
> +p.add_argument('-d', dest='debug', action='store_true', help='debug')
> +p.add_argument('-misalign', action='store_true',
> +   help='misalign memory allocations')
> +
> +p.set_defaults(imgfmt='raw', imgproto='file')
> +
> +format_list = ['raw', 'bochs', 'cloop', 'parallels', 'qcow', 'qcow2',
> +   'qed', 'vdi', 'vpc', 'vhdx', 'vmdk', 'luks', 'dmg']
> +g = p.add_argument_group(
> +'image format options',
> +'The following options sets IMGFMT environment variable. '

s/sets/set the/

> +'At most one chose is allowed, default is "raw"')

s/chose/choice/

> +g = g.add_mutually_exclusive_group()
> +for fmt in format_list:
> +g.add_argument('-' + fmt, dest='imgfmt', action='store_const',
> +   const=fmt)
> +
> +protocol_list = ['file', 'rbd', 'sheepdoc', 'nbd', 'ssh', 'nfs',
> + 'fuse']
> +g = p.add_argument_group(
> +'image protocol options',
> +'T

Re: [PATCH v6 08/11] iotests: add testenv.py

[Vladimir Sementsov-Ogievskiy]

14.01.2021 14:14, Kevin Wolf wrote:

Am 14.01.2021 um 05:28 hat Vladimir Sementsov-Ogievskiy geschrieben:

09.01.2021 15:26, Vladimir Sementsov-Ogievskiy wrote:

Add TestEnv class, which will handle test environment in a new python
iotests running framework.

Difference with current ./check interface:
- -v (verbose) option dropped, as it is unused

- -xdiff option is dropped, until somebody complains that it is needed
- same for -n option


One more thing dropped is looking for binaries in $build_iotests
directory.  Do someone use this feature? Or we can drop it, and
reimplement when someone report the degradation?


I seem to have socket_scm_helper only there, but you use
self.build_iotests for that, so I suppose this is different from what
you mean?


Yes socket_scm_helper is searched in build_iotests both pre- and after- my 
patches.

But for other tools (qemu-img, qemu-io, qemu-nbd, qemu, qemu-storage-daemon) 
old check script first search[1] in build_iotests, and then in standard 
location.. In this commit I don't do the step [1].



My tools and system emulator binaries are in the standard location in
the build directory, not in the iotests build directory, so they don't
need it.



--
Best regards,
Vladimir

Re: [PATCH v6 08/11] iotests: add testenv.py

[Kevin Wolf]

Am 14.01.2021 um 05:28 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 09.01.2021 15:26, Vladimir Sementsov-Ogievskiy wrote:
> > Add TestEnv class, which will handle test environment in a new python
> > iotests running framework.
> > 
> > Difference with current ./check interface:
> > - -v (verbose) option dropped, as it is unused
> > 
> > - -xdiff option is dropped, until somebody complains that it is needed
> > - same for -n option
> 
> One more thing dropped is looking for binaries in $build_iotests
> directory.  Do someone use this feature? Or we can drop it, and
> reimplement when someone report the degradation?

I seem to have socket_scm_helper only there, but you use
self.build_iotests for that, so I suppose this is different from what
you mean?

My tools and system emulator binaries are in the standard location in
the build directory, not in the iotests build directory, so they don't
need it.

Kevin

Re: [PATCH v6 08/11] iotests: add testenv.py

[Vladimir Sementsov-Ogievskiy]

14.01.2021 09:14, Vladimir Sementsov-Ogievskiy wrote:

12.01.2021 20:36, Kevin Wolf wrote:

Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:

Add TestEnv class, which will handle test environment in a new python
iotests running framework.

Difference with current ./check interface:
- -v (verbose) option dropped, as it is unused

- -xdiff option is dropped, until somebody complains that it is needed
- same for -n option

Signed-off-by: Vladimir Sementsov-Ogievskiy 


+* Module testenv
+testenv.py:48:0: R0902: Too many instance attributes (34/7) 
(too-many-instance-attributes)
+testenv.py:212:16: R1722: Consider using sys.exit() (consider-using-sys-exit)
+testenv.py:214:16: R1722: Consider using sys.exit() (consider-using-sys-exit)
+testenv.py:324:8: R1722: Consider using sys.exit() (consider-using-sys-exit)
+testenv.py:1:0: R0801: Similar lines in 2 files
+==findtests:45
+==testenv:72
+    _argparser = None
+    @classmethod
+    def get_argparser(cls) -> argparse.ArgumentParser:
+    if cls._argparser is not None:
+    return cls._argparser
+ (duplicate-code)
+testenv.py:294: error: Function is missing a type annotation for one or more 
arguments


Interesting, I don't see "similar lines" error in the output..



Ah, that's because I added a newline in findtests.py. Add it in testenv.py and 
the error comes back.





I wonder whether we should just disable the "similar lines" check? The
instance attributes one looks kind of arbitrary, too. The rest looks
valid enough.

Kevin







--
Best regards,
Vladimir

Re: [PATCH v6 08/11] iotests: add testenv.py

[Vladimir Sementsov-Ogievskiy]

12.01.2021 20:36, Kevin Wolf wrote:

Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:

Add TestEnv class, which will handle test environment in a new python
iotests running framework.

Difference with current ./check interface:
- -v (verbose) option dropped, as it is unused

- -xdiff option is dropped, until somebody complains that it is needed
- same for -n option

Signed-off-by: Vladimir Sementsov-Ogievskiy 


+* Module testenv
+testenv.py:48:0: R0902: Too many instance attributes (34/7) 
(too-many-instance-attributes)
+testenv.py:212:16: R1722: Consider using sys.exit() (consider-using-sys-exit)
+testenv.py:214:16: R1722: Consider using sys.exit() (consider-using-sys-exit)
+testenv.py:324:8: R1722: Consider using sys.exit() (consider-using-sys-exit)
+testenv.py:1:0: R0801: Similar lines in 2 files
+==findtests:45
+==testenv:72
+_argparser = None
+@classmethod
+def get_argparser(cls) -> argparse.ArgumentParser:
+if cls._argparser is not None:
+return cls._argparser
+ (duplicate-code)
+testenv.py:294: error: Function is missing a type annotation for one or more 
arguments


Interesting, I don't see "similar lines" error in the output..

[root@kvm qemu-iotests]# pylint-3 --score=n findtests.py testenv.py
* Module testenv
testenv.py:48:0: R0902: Too many instance attributes (34/7) 
(too-many-instance-attributes)
testenv.py:212:16: R1722: Consider using sys.exit() (consider-using-sys-exit)
testenv.py:214:16: R1722: Consider using sys.exit() (consider-using-sys-exit)
testenv.py:324:8: R1722: Consider using sys.exit() (consider-using-sys-exit)
[root@kvm qemu-iotests]# pylint --score=n findtests.py testenv.py
* Module testenv
testenv.py:48:0: R0902: Too many instance attributes (34/7) 
(too-many-instance-attributes)
testenv.py:212:16: R1722: Consider using sys.exit() (consider-using-sys-exit)
testenv.py:214:16: R1722: Consider using sys.exit() (consider-using-sys-exit)
testenv.py:324:8: R1722: Consider using sys.exit() (consider-using-sys-exit)
[root@kvm qemu-iotests]# pylint-3 --version
pylint 2.6.0
astroid 2.4.2
Python 3.9.1 (default, Dec  8 2020, 00:00:00)
[GCC 10.2.1 20201125 (Red Hat 10.2.1-9)]
[root@kvm qemu-iotests]# pylint --version
pylint 2.6.0
astroid 2.4.2
Python 3.9.1 (default, Dec  8 2020, 00:00:00)
[GCC 10.2.1 20201125 (Red Hat 10.2.1-9)]
[root@kvm qemu-iotests]# which pylint-3
/usr/bin/pylint-3
[root@kvm qemu-iotests]# rpm -qf /usr/bin/pylint-3
python3-pylint-2.6.0-2.fc33.noarch



I wonder whether we should just disable the "similar lines" check? The
instance attributes one looks kind of arbitrary, too. The rest looks
valid enough.

Kevin




--
Best regards,
Vladimir

Re: [PATCH v6 08/11] iotests: add testenv.py

[Vladimir Sementsov-Ogievskiy]

09.01.2021 15:26, Vladimir Sementsov-Ogievskiy wrote:

Add TestEnv class, which will handle test environment in a new python
iotests running framework.

Difference with current ./check interface:
- -v (verbose) option dropped, as it is unused

- -xdiff option is dropped, until somebody complains that it is needed
- same for -n option


One more thing dropped is looking for binaries in $build_iotests directory.
Do someone use this feature? Or we can drop it, and reimplement when someone 
report the degradation?

--
Best regards,
Vladimir

Re: [PATCH v6 08/11] iotests: add testenv.py

[Kevin Wolf]

Am 09.01.2021 um 13:26 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Add TestEnv class, which will handle test environment in a new python
> iotests running framework.
> 
> Difference with current ./check interface:
> - -v (verbose) option dropped, as it is unused
> 
> - -xdiff option is dropped, until somebody complains that it is needed
> - same for -n option
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 

+* Module testenv
+testenv.py:48:0: R0902: Too many instance attributes (34/7) 
(too-many-instance-attributes)
+testenv.py:212:16: R1722: Consider using sys.exit() (consider-using-sys-exit)
+testenv.py:214:16: R1722: Consider using sys.exit() (consider-using-sys-exit)
+testenv.py:324:8: R1722: Consider using sys.exit() (consider-using-sys-exit)
+testenv.py:1:0: R0801: Similar lines in 2 files
+==findtests:45
+==testenv:72
+_argparser = None
+@classmethod
+def get_argparser(cls) -> argparse.ArgumentParser:
+if cls._argparser is not None:
+return cls._argparser
+ (duplicate-code)
+testenv.py:294: error: Function is missing a type annotation for one or more 
arguments

I wonder whether we should just disable the "similar lines" check? The
instance attributes one looks kind of arbitrary, too. The rest looks
valid enough.

Kevin

[PATCH v6 08/11] iotests: add testenv.py

[Vladimir Sementsov-Ogievskiy]

Add TestEnv class, which will handle test environment in a new python
iotests running framework.

Difference with current ./check interface:
- -v (verbose) option dropped, as it is unused

- -xdiff option is dropped, until somebody complains that it is needed
- same for -n option

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/testenv.py | 328 ++
 1 file changed, 328 insertions(+)
 create mode 100755 tests/qemu-iotests/testenv.py

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
new file mode 100755
index 00..ecaf76fb85
--- /dev/null
+++ b/tests/qemu-iotests/testenv.py
@@ -0,0 +1,328 @@
+#!/usr/bin/env python3
+#
+# Parse command line options to manage test environment variables.
+#
+# Copyright (c) 2020 Virtuozzo International GmbH
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import os
+import sys
+import tempfile
+from pathlib import Path
+import shutil
+import collections
+import subprocess
+import argparse
+from typing import List, Dict
+
+
+def get_default_machine(qemu_prog: str) -> str:
+outp = subprocess.run([qemu_prog, '-machine', 'help'], check=True,
+  text=True, stdout=subprocess.PIPE).stdout
+
+machines = outp.split('\n')
+default_machine = next(m for m in machines if m.endswith(' (default)'))
+default_machine = default_machine.split(' ', 1)[0]
+
+alias_suf = ' (alias of {})'.format(default_machine)
+alias = next((m for m in machines if m.endswith(alias_suf)), None)
+if alias is not None:
+default_machine = alias.split(' ', 1)[0]
+
+return default_machine
+
+
+class TestEnv:
+"""
+Manage system environment for running tests
+
+The following variables are supported/provided. They are represented by
+lower-cased TestEnv attributes.
+"""
+env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
+ 'OUTPUT_DIR', 'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
+ 'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
+ 'SOCKET_SCM_HELPER', 'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
+ 'QEMU_IO_OPTIONS', 'QEMU_NBD_OPTIONS', 'IMGOPTS',
+ 'IMGFMT', 'IMGPROTO', 'AIOMODE', 'CACHEMODE',
+ 'VALGRIND_QEMU', 'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC',
+ 'IMGOPTSSYNTAX', 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE']
+
+def get_env(self) -> Dict[str, str]:
+env = {}
+for v in self.env_variables:
+val = getattr(self, v.lower(), None)
+if val is not None:
+env[v] = val
+
+return env
+
+_argparser = None
+@classmethod
+def get_argparser(cls) -> argparse.ArgumentParser:
+if cls._argparser is not None:
+return cls._argparser
+
+p = argparse.ArgumentParser(description="= test environment options =",
+add_help=False, usage=argparse.SUPPRESS)
+
+p.add_argument('-d', dest='debug', action='store_true', help='debug')
+p.add_argument('-misalign', action='store_true',
+   help='misalign memory allocations')
+
+p.set_defaults(imgfmt='raw', imgproto='file')
+
+format_list = ['raw', 'bochs', 'cloop', 'parallels', 'qcow', 'qcow2',
+   'qed', 'vdi', 'vpc', 'vhdx', 'vmdk', 'luks', 'dmg']
+g = p.add_argument_group(
+'image format options',
+'The following options sets IMGFMT environment variable. '
+'At most one chose is allowed, default is "raw"')
+g = g.add_mutually_exclusive_group()
+for fmt in format_list:
+g.add_argument('-' + fmt, dest='imgfmt', action='store_const',
+   const=fmt)
+
+protocol_list = ['file', 'rbd', 'sheepdoc', 'nbd', 'ssh', 'nfs',
+ 'fuse']
+g = p.add_argument_group(
+'image protocol options',
+'The following options sets IMGPROTO environment variably. '
+'At most one chose is allowed, default is "file"')
+g = g.add_mutually_exclusive_group()
+for prt in protocol_list:
+g.add_argument('-' + prt, dest='imgproto', action='store_const',
+   const=prt)
+
+g = p.add_mutually_exclusiv