| Message ID | 20220203163024.38913-5-hreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | block/nbd: Move s->ioc on AioContext change | expand |
03.02.2022 19:30, Hanna Reitz wrote: > This is a rather simple class that allows creating a QSD instance > running in the background and stopping it when no longer needed. > > The __del__ handler is a safety net for when something goes so wrong in > a test that e.g. the tearDown() method is not called (e.g. setUp() > launches the QSD, but then launching a VM fails). We do not want the > QSD to continue running after the test has failed, so __del__() will > take care to kill it. > > Signed-off-by: Hanna Reitz <hreitz@redhat.com> > --- > tests/qemu-iotests/iotests.py | 42 +++++++++++++++++++++++++++++++++++ > 1 file changed, 42 insertions(+) > > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py > index 8cdb381f2a..c75e402b87 100644 > --- a/tests/qemu-iotests/iotests.py > +++ b/tests/qemu-iotests/iotests.py > @@ -73,6 +73,8 @@ > qemu_prog = os.environ.get('QEMU_PROG', 'qemu') > qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ') > > +qsd_prog = os.environ.get('QSD_PROG', 'qemu-storage-daemon') > + > gdb_qemu_env = os.environ.get('GDB_OPTIONS') > qemu_gdb = [] > if gdb_qemu_env: > @@ -345,6 +347,46 @@ def cmd(self, cmd): > return self._read_output() > > > +class QemuStorageDaemon: > + def __init__(self, *args: str, instance_id: Optional[str] = None): > + if not instance_id: > + instance_id = 'a' this is equivalent to simply instance_id: str = 'a' > + I'd add assert '--pidfile' not in args to prove following logic > + self.pidfile = os.path.join(test_dir, f'qsd-{instance_id}-pid') > + all_args = [qsd_prog] + list(args) + ['--pidfile', self.pidfile] > + > + # Cannot use with here, we want the subprocess to stay around > + # pylint: disable=consider-using-with > + self._p = subprocess.Popen(all_args) > + while not os.path.exists(self.pidfile): > + if self._p.poll() is not None: > + cmd = ' '.join(all_args) > + raise RuntimeError( > + 'qemu-storage-daemon terminated with exit code ' + > + f'{self._p.returncode}: {cmd}') > + > + time.sleep(0.01) > + > + with open(self.pidfile, encoding='utf-8') as f: > + self._pid = int(f.read().strip()) > + > + assert self._pid == self._p.pid > + > + def stop(self, kill_signal=15): > + self._p.send_signal(kill_signal) > + self._p.wait() > + self._p = None > + > + try: > + os.remove(self.pidfile) > + except OSError: > + pass > + > + def __del__(self): > + if self._p is not None: > + self.stop(kill_signal=9) > + > + > 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)) > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
On 04.02.22 10:04, Vladimir Sementsov-Ogievskiy wrote: > 03.02.2022 19:30, Hanna Reitz wrote: >> This is a rather simple class that allows creating a QSD instance >> running in the background and stopping it when no longer needed. >> >> The __del__ handler is a safety net for when something goes so wrong in >> a test that e.g. the tearDown() method is not called (e.g. setUp() >> launches the QSD, but then launching a VM fails). We do not want the >> QSD to continue running after the test has failed, so __del__() will >> take care to kill it. >> >> Signed-off-by: Hanna Reitz <hreitz@redhat.com> >> --- >> tests/qemu-iotests/iotests.py | 42 +++++++++++++++++++++++++++++++++++ >> 1 file changed, 42 insertions(+) >> >> diff --git a/tests/qemu-iotests/iotests.py >> b/tests/qemu-iotests/iotests.py >> index 8cdb381f2a..c75e402b87 100644 >> --- a/tests/qemu-iotests/iotests.py >> +++ b/tests/qemu-iotests/iotests.py >> @@ -73,6 +73,8 @@ >> qemu_prog = os.environ.get('QEMU_PROG', 'qemu') >> qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ') >> +qsd_prog = os.environ.get('QSD_PROG', 'qemu-storage-daemon') >> + >> gdb_qemu_env = os.environ.get('GDB_OPTIONS') >> qemu_gdb = [] >> if gdb_qemu_env: >> @@ -345,6 +347,46 @@ def cmd(self, cmd): >> return self._read_output() >> +class QemuStorageDaemon: >> + def __init__(self, *args: str, instance_id: Optional[str] = None): >> + if not instance_id: >> + instance_id = 'a' > > this is equivalent to simply > > instance_id: str = 'a' Oh. Right. :) >> + > > I'd add > > assert '--pidfile' not in args > > to prove following logic Sounds good! >> + self.pidfile = os.path.join(test_dir, f'qsd-{instance_id}-pid') >> + all_args = [qsd_prog] + list(args) + ['--pidfile', >> self.pidfile] >> + >> + # Cannot use with here, we want the subprocess to stay around >> + # pylint: disable=consider-using-with >> + self._p = subprocess.Popen(all_args) >> + while not os.path.exists(self.pidfile): >> + if self._p.poll() is not None: >> + cmd = ' '.join(all_args) >> + raise RuntimeError( >> + 'qemu-storage-daemon terminated with exit code ' + >> + f'{self._p.returncode}: {cmd}') >> + >> + time.sleep(0.01) >> + >> + with open(self.pidfile, encoding='utf-8') as f: >> + self._pid = int(f.read().strip()) >> + >> + assert self._pid == self._p.pid >> + >> + def stop(self, kill_signal=15): >> + self._p.send_signal(kill_signal) >> + self._p.wait() >> + self._p = None >> + >> + try: >> + os.remove(self.pidfile) >> + except OSError: >> + pass >> + >> + def __del__(self): >> + if self._p is not None: >> + self.stop(kill_signal=9) >> + >> + >> 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)) >> > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Thanks a lot for reviewing!
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 8cdb381f2a..c75e402b87 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -73,6 +73,8 @@ qemu_prog = os.environ.get('QEMU_PROG', 'qemu') qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ') +qsd_prog = os.environ.get('QSD_PROG', 'qemu-storage-daemon') + gdb_qemu_env = os.environ.get('GDB_OPTIONS') qemu_gdb = [] if gdb_qemu_env: @@ -345,6 +347,46 @@ def cmd(self, cmd): return self._read_output() +class QemuStorageDaemon: + def __init__(self, *args: str, instance_id: Optional[str] = None): + if not instance_id: + instance_id = 'a' + + self.pidfile = os.path.join(test_dir, f'qsd-{instance_id}-pid') + all_args = [qsd_prog] + list(args) + ['--pidfile', self.pidfile] + + # Cannot use with here, we want the subprocess to stay around + # pylint: disable=consider-using-with + self._p = subprocess.Popen(all_args) + while not os.path.exists(self.pidfile): + if self._p.poll() is not None: + cmd = ' '.join(all_args) + raise RuntimeError( + 'qemu-storage-daemon terminated with exit code ' + + f'{self._p.returncode}: {cmd}') + + time.sleep(0.01) + + with open(self.pidfile, encoding='utf-8') as f: + self._pid = int(f.read().strip()) + + assert self._pid == self._p.pid + + def stop(self, kill_signal=15): + self._p.send_signal(kill_signal) + self._p.wait() + self._p = None + + try: + os.remove(self.pidfile) + except OSError: + pass + + def __del__(self): + if self._p is not None: + self.stop(kill_signal=9) + + 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))
This is a rather simple class that allows creating a QSD instance running in the background and stopping it when no longer needed. The __del__ handler is a safety net for when something goes so wrong in a test that e.g. the tearDown() method is not called (e.g. setUp() launches the QSD, but then launching a VM fails). We do not want the QSD to continue running after the test has failed, so __del__() will take care to kill it. Signed-off-by: Hanna Reitz <hreitz@redhat.com> --- tests/qemu-iotests/iotests.py | 42 +++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)