| Message ID | 20231116115354.228678-1-npiggin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] tests/avocado: reverse_debugging drain console to prevent hang | expand |
> On 16-Nov-2023, at 5:23 PM, Nicholas Piggin <npiggin@gmail.com> wrote: > > Like replay_linux.py, reverse_debugging.py starts the vm with console > set but does not interact with it (e.g., with wait_for_console_pattern). > In this situation, the console should have a drainer attached so the > socket does not fill. replay_linux.py has a drainer, but it is missing > from reverse_debugging.py. > > Per analysis in Link: this can cause the console socket/pipe to fill and > QEMU get stuck in qemu_chr_write_buffer, leading to strange test case > failures (ppc64 fails because it prints a lot to console in early bios). > Attaching a drainer prevents this. > > Note, this commit does not fix bugs introduced by the commits referenced > in the first two Fixes: tags, but together those commits conspire to > irritate the problem and cause test case failure, which this commit > fixes. > > Link: https://lore.kernel.org/qemu-devel/ZVT-bY9YOr69QTPX@redhat.com/ > Fixes: 1d4796cd0083 ("python/machine: use socketpair() for console connections") > Fixes: 761a13b23946 ("tests/avocado: ppc64 reverse debugging tests for pseries and powernv") > Fixes: be52eca30978 ("tests/acceptance: add reverse debugging test") > Tested-by: Thomas Huth <thuth@redhat.com> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Reviewed-by: Ani Sinha <anisinha@redhat.com <mailto:anisinha@redhat.com> <mailto:anisinha@redhat.com>> > --- > tests/avocado/reverse_debugging.py | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/tests/avocado/reverse_debugging.py b/tests/avocado/reverse_debugging.py > index fc47874eda..128d85bc0e 100644 > --- a/tests/avocado/reverse_debugging.py > +++ b/tests/avocado/reverse_debugging.py > @@ -12,6 +12,7 @@ > > from avocado import skipIf > from avocado_qemu import BUILD_DIR > +from avocado.utils import datadrainer > from avocado.utils import gdb > from avocado.utils import process > from avocado.utils.network.ports import find_free_port > @@ -52,6 +53,10 @@ def run_vm(self, record, shift, args, replay_path, image_path, port): > if args: > vm.add_args(*args) > vm.launch() > + console_drainer = datadrainer.LineLogger(vm.console_socket.fileno(), > + logger=self.log.getChild('console'), > + stop_check=(lambda : not vm.is_running())) > + console_drainer.start() > return vm > > @staticmethod > -- > 2.42.0
> On 16-Nov-2023, at 6:56 PM, Ani Sinha <anisinha@redhat.com> wrote: > > > >> On 16-Nov-2023, at 5:23 PM, Nicholas Piggin <npiggin@gmail.com> wrote: >> >> Like replay_linux.py, reverse_debugging.py starts the vm with console >> set but does not interact with it (e.g., with wait_for_console_pattern). >> In this situation, the console should have a drainer attached so the >> socket does not fill. replay_linux.py has a drainer, but it is missing >> from reverse_debugging.py. >> >> Per analysis in Link: this can cause the console socket/pipe to fill and >> QEMU get stuck in qemu_chr_write_buffer, leading to strange test case >> failures (ppc64 fails because it prints a lot to console in early bios). >> Attaching a drainer prevents this. >> >> Note, this commit does not fix bugs introduced by the commits referenced >> in the first two Fixes: tags, but together those commits conspire to >> irritate the problem and cause test case failure, which this commit >> fixes. >> >> Link: https://lore.kernel.org/qemu-devel/ZVT-bY9YOr69QTPX@redhat.com/ >> Fixes: 1d4796cd0083 ("python/machine: use socketpair() for console connections") >> Fixes: 761a13b23946 ("tests/avocado: ppc64 reverse debugging tests for pseries and powernv") >> Fixes: be52eca30978 ("tests/acceptance: add reverse debugging test") >> Tested-by: Thomas Huth <thuth@redhat.com> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > Reviewed-by: Ani Sinha <anisinha@redhat.com <mailto:anisinha@redhat.com>> This bs started happening recently once I upgraded my OS. Sorry about that. Reviewed-by: Ani Sinha <anisinha@redhat.com <mailto:anisinha@redhat.com>> > >> --- >> tests/avocado/reverse_debugging.py | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/tests/avocado/reverse_debugging.py b/tests/avocado/reverse_debugging.py >> index fc47874eda..128d85bc0e 100644 >> --- a/tests/avocado/reverse_debugging.py >> +++ b/tests/avocado/reverse_debugging.py >> @@ -12,6 +12,7 @@ >> >> from avocado import skipIf >> from avocado_qemu import BUILD_DIR >> +from avocado.utils import datadrainer >> from avocado.utils import gdb >> from avocado.utils import process >> from avocado.utils.network.ports import find_free_port >> @@ -52,6 +53,10 @@ def run_vm(self, record, shift, args, replay_path, image_path, port): >> if args: >> vm.add_args(*args) >> vm.launch() >> + console_drainer = datadrainer.LineLogger(vm.console_socket.fileno(), >> + logger=self.log.getChild('console'), >> + stop_check=(lambda : not vm.is_running())) >> + console_drainer.start() >> return vm >> >> @staticmethod >> -- >> 2.42.0
> On 16-Nov-2023, at 7:01 PM, Ani Sinha <anisinha@redhat.com> wrote: > > > >> On 16-Nov-2023, at 6:56 PM, Ani Sinha <anisinha@redhat.com> wrote: >> >> >> >>> On 16-Nov-2023, at 5:23 PM, Nicholas Piggin <npiggin@gmail.com> wrote: >>> >>> Like replay_linux.py, reverse_debugging.py starts the vm with console >>> set but does not interact with it (e.g., with wait_for_console_pattern). >>> In this situation, the console should have a drainer attached so the >>> socket does not fill. replay_linux.py has a drainer, but it is missing >>> from reverse_debugging.py. >>> >>> Per analysis in Link: this can cause the console socket/pipe to fill and >>> QEMU get stuck in qemu_chr_write_buffer, leading to strange test case >>> failures (ppc64 fails because it prints a lot to console in early bios). >>> Attaching a drainer prevents this. >>> >>> Note, this commit does not fix bugs introduced by the commits referenced >>> in the first two Fixes: tags, but together those commits conspire to >>> irritate the problem and cause test case failure, which this commit >>> fixes. >>> >>> Link: https://lore.kernel.org/qemu-devel/ZVT-bY9YOr69QTPX@redhat.com/ >>> Fixes: 1d4796cd0083 ("python/machine: use socketpair() for console connections") >>> Fixes: 761a13b23946 ("tests/avocado: ppc64 reverse debugging tests for pseries and powernv") >>> Fixes: be52eca30978 ("tests/acceptance: add reverse debugging test") >>> Tested-by: Thomas Huth <thuth@redhat.com> >>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> >> Reviewed-by: Ani Sinha <anisinha@redhat.com <mailto:anisinha@redhat.com>> > > This bs started happening recently once I upgraded my OS. Sorry about that. > > Reviewed-by: Ani Sinha <anisinha@redhat.com <mailto:anisinha@redhat.com>> Trying again Reviewed-by: Ani Sinha <anisinha@redhat.com> > > >> >>> --- >>> tests/avocado/reverse_debugging.py | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/tests/avocado/reverse_debugging.py b/tests/avocado/reverse_debugging.py >>> index fc47874eda..128d85bc0e 100644 >>> --- a/tests/avocado/reverse_debugging.py >>> +++ b/tests/avocado/reverse_debugging.py >>> @@ -12,6 +12,7 @@ >>> >>> from avocado import skipIf >>> from avocado_qemu import BUILD_DIR >>> +from avocado.utils import datadrainer >>> from avocado.utils import gdb >>> from avocado.utils import process >>> from avocado.utils.network.ports import find_free_port >>> @@ -52,6 +53,10 @@ def run_vm(self, record, shift, args, replay_path, image_path, port): >>> if args: >>> vm.add_args(*args) >>> vm.launch() >>> + console_drainer = datadrainer.LineLogger(vm.console_socket.fileno(), >>> + logger=self.log.getChild('console'), >>> + stop_check=(lambda : not vm.is_running())) >>> + console_drainer.start() >>> return vm >>> >>> @staticmethod >>> -- >>> 2.42.0
diff --git a/tests/avocado/reverse_debugging.py b/tests/avocado/reverse_debugging.py index fc47874eda..128d85bc0e 100644 --- a/tests/avocado/reverse_debugging.py +++ b/tests/avocado/reverse_debugging.py @@ -12,6 +12,7 @@ from avocado import skipIf from avocado_qemu import BUILD_DIR +from avocado.utils import datadrainer from avocado.utils import gdb from avocado.utils import process from avocado.utils.network.ports import find_free_port @@ -52,6 +53,10 @@ def run_vm(self, record, shift, args, replay_path, image_path, port): if args: vm.add_args(*args) vm.launch() + console_drainer = datadrainer.LineLogger(vm.console_socket.fileno(), + logger=self.log.getChild('console'), + stop_check=(lambda : not vm.is_running())) + console_drainer.start() return vm @staticmethod
Like replay_linux.py, reverse_debugging.py starts the vm with console set but does not interact with it (e.g., with wait_for_console_pattern). In this situation, the console should have a drainer attached so the socket does not fill. replay_linux.py has a drainer, but it is missing from reverse_debugging.py. Per analysis in Link: this can cause the console socket/pipe to fill and QEMU get stuck in qemu_chr_write_buffer, leading to strange test case failures (ppc64 fails because it prints a lot to console in early bios). Attaching a drainer prevents this. Note, this commit does not fix bugs introduced by the commits referenced in the first two Fixes: tags, but together those commits conspire to irritate the problem and cause test case failure, which this commit fixes. Link: https://lore.kernel.org/qemu-devel/ZVT-bY9YOr69QTPX@redhat.com/ Fixes: 1d4796cd0083 ("python/machine: use socketpair() for console connections") Fixes: 761a13b23946 ("tests/avocado: ppc64 reverse debugging tests for pseries and powernv") Fixes: be52eca30978 ("tests/acceptance: add reverse debugging test") Tested-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- tests/avocado/reverse_debugging.py | 5 +++++ 1 file changed, 5 insertions(+)