| Message ID | 20210520075236.44723-6-eesposit@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qemu_iotests: improve debugging options | expand |
20.05.2021 10:52, Emanuele Giuseppe Esposito wrote: > Attaching gdbserver implies that the qmp socket > should wait indefinitely for an answer from QEMU. > > Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> > --- > tests/qemu-iotests/iotests.py | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py > index d667fde6f8..cf1ca60376 100644 > --- a/tests/qemu-iotests/iotests.py > +++ b/tests/qemu-iotests/iotests.py > @@ -478,11 +478,13 @@ def __init__(self, seconds, errmsg="Timeout"): > self.seconds = seconds > self.errmsg = errmsg > def __enter__(self): > - signal.signal(signal.SIGALRM, self.timeout) > - signal.setitimer(signal.ITIMER_REAL, self.seconds) > + if not qemu_gdb: > + signal.signal(signal.SIGALRM, self.timeout) > + signal.setitimer(signal.ITIMER_REAL, self.seconds) > return self > def __exit__(self, exc_type, value, traceback): > - signal.setitimer(signal.ITIMER_REAL, 0) > + if not qemu_gdb: > + signal.setitimer(signal.ITIMER_REAL, 0) > return False > def timeout(self, signum, frame): > raise Exception(self.errmsg) So, you just make the class do nothing.. I'd prefer something like this: @contextmanager def NoTimeout: yield if qemu_gdb: Timeout = NoTimeout anyway: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > @@ -576,7 +578,7 @@ class VM(qtest.QEMUQtestMachine): > > def __init__(self, path_suffix=''): > name = "qemu%s-%d" % (path_suffix, os.getpid()) > - timer = 15.0 > + timer = 15.0 if not qemu_gdb else None > super().__init__(qemu_prog, qemu_opts, name=name, > test_dir=test_dir, > socket_scm_helper=socket_scm_helper, > Still, it's not simple to avoid any kind of timeouts. The most annoying would be timeouts in event_wait() / events_wait()
> > So, you just make the class do nothing.. I'd prefer something like this: > > @contextmanager > def NoTimeout: > yield > > if qemu_gdb: > Timeout = NoTimeout I am not sure I understand what you want to do here. I have a basic understanding of @contextmanager, but can you explain more what you want to do? Alternatively, I send the patch as-is, and then you can send this change. Thank you, Emanuele > > > anyway: > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >
03.06.2021 14:03, Emanuele Giuseppe Esposito wrote: > >> >> So, you just make the class do nothing.. I'd prefer something like this: >> >> @contextmanager >> def NoTimeout: >> yield >> >> if qemu_gdb: >> Timeout = NoTimeout > > I am not sure I understand what you want to do here. > I have a basic understanding of @contextmanager, but can you explain more what you want to do? With qemu_gdb you make the class do absolutely nothing. So, it's just provides an interface of context manager, but does nothing. >> @contextmanager >> def NoTimeout: >> yield is the same thing: it's context manager, so you can do with NoTimeout(): .... but that context manager does absolutely nothing. @contextmanager gives you a simple way to create a context manager. You define a function which has yield point. So, everything before yield is __enter__, everything after yield is __exit__. And you can return a value by yield, it will be a return value of __enter__. So, what I meant: keep Timeout class as is and just add code that I suggested after it. > > Alternatively, I send the patch as-is, and then you can send this change. OK for me. > > Thank you, > Emanuele > >> >> >> anyway: >> >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> >
On 03/06/2021 14:25, Vladimir Sementsov-Ogievskiy wrote: > 03.06.2021 14:03, Emanuele Giuseppe Esposito wrote: >> >>> >>> So, you just make the class do nothing.. I'd prefer something like this: >>> >>> @contextmanager >>> def NoTimeout: >>> yield >>> >>> if qemu_gdb: >>> Timeout = NoTimeout >> >> I am not sure I understand what you want to do here. >> I have a basic understanding of @contextmanager, but can you explain >> more what you want to do? > > With qemu_gdb you make the class do absolutely nothing. So, it's just > provides an interface of context manager, but does nothing. > >>> @contextmanager >>> def NoTimeout: >>> yield > > is the same thing: it's context manager, so you can do > > with NoTimeout(): > .... > > but that context manager does absolutely nothing. > > > @contextmanager gives you a simple way to create a context manager. You > define a function which has yield point. So, everything before yield is > __enter__, everything after yield is __exit__. And you can return a > value by yield, it will be a return value of __enter__. > > > So, what I meant: keep Timeout class as is and just add code that I > suggested after it. Oh okay. I didn't know Timeout = NoTimeout was possible (and I didn't know where to put it anyways). Looks cleaner than the normal ifs, will apply this change and send v5. Thank you, Emanuele
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index d667fde6f8..cf1ca60376 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -478,11 +478,13 @@ def __init__(self, seconds, errmsg="Timeout"): self.seconds = seconds self.errmsg = errmsg def __enter__(self): - signal.signal(signal.SIGALRM, self.timeout) - signal.setitimer(signal.ITIMER_REAL, self.seconds) + if not qemu_gdb: + signal.signal(signal.SIGALRM, self.timeout) + signal.setitimer(signal.ITIMER_REAL, self.seconds) return self def __exit__(self, exc_type, value, traceback): - signal.setitimer(signal.ITIMER_REAL, 0) + if not qemu_gdb: + signal.setitimer(signal.ITIMER_REAL, 0) return False def timeout(self, signum, frame): raise Exception(self.errmsg) @@ -576,7 +578,7 @@ class VM(qtest.QEMUQtestMachine): def __init__(self, path_suffix=''): name = "qemu%s-%d" % (path_suffix, os.getpid()) - timer = 15.0 + timer = 15.0 if not qemu_gdb else None super().__init__(qemu_prog, qemu_opts, name=name, test_dir=test_dir, socket_scm_helper=socket_scm_helper,
Attaching gdbserver implies that the qmp socket should wait indefinitely for an answer from QEMU. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> --- tests/qemu-iotests/iotests.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)