Re: [PATCH 13/14] iotests/151: ensure subprocesses are cleaned up

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Aug 19, 2025 at 05:37:04PM -0400, John Snow wrote:
> On Tue, Jul 15, 2025 at 10:32 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > The iotest 151 creates a bunch of subprocesses, with their stdout
> > connected to a pipe but never reads any data from them and does
> > not gurantee the processes are killed on cleanup.
> >
> > This triggers resource leak warnings from python when the
> > subprocess.Popen object is garbage collected.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  tests/qemu-iotests/151 | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/tests/qemu-iotests/151 b/tests/qemu-iotests/151
> > index f2ff9c5dac..06ee3585db 100755
> > --- a/tests/qemu-iotests/151
> > +++ b/tests/qemu-iotests/151
> > @@ -263,6 +263,11 @@ class TestThrottledWithNbdExportBase(iotests.QMPTestCase):
> >                          break
> >                      except subprocess.TimeoutExpired:
> >                          self.vm.qtest(f'clock_step {1 * 1000 * 1000 * 1000}')
> > +                try:
> > +                    p.kill()
> > +                    p.stdout.close()
> > +                except:
> > +                    pass
> 
> You can decide how much you care here, but using "except Exception:"
> will allow critical exceptions to still crash the program, like
> KeyboardInterrupt and SystemExit et al. It's good hygiene for things
> that may or may not hang so you can still use Ctrl+C.

Yes, that should be changed as you describe.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|