Re: [PATCH 03/10] python/machine: use subprocess.run instead of subprocess.Popen

[John Snow]

On 5/14/21 10:08 AM, Wainer dos Santos Moschetta wrote:
> Hi,
>
> On 5/12/21 6:46 PM, John Snow wrote:
> > use run() instead of Popen() -- to assert to pylint that we are not
> > forgetting to close a long-running program.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >   python/qemu/machine.py | 15 +++++++++------
> >   1 file changed, 9 insertions(+), 6 deletions(-)
> >
> > diff --git a/python/qemu/machine.py b/python/qemu/machine.py
> > index 41f51bd27d0..c13ff9b32bf 100644
> > --- a/python/qemu/machine.py
> > +++ b/python/qemu/machine.py
> > @@ -223,13 +223,16 @@ def send_fd_scm(self, fd: Optional[int] = None,
> >               assert fd is not None
> >               fd_param.append(str(fd))
> > -        proc = subprocess.Popen(
> > -            fd_param, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE,
> > -            stderr=subprocess.STDOUT, close_fds=False
> > +        proc = subprocess.run(
> > +            fd_param,
> > +            stdin=subprocess.DEVNULL,
> > +            stdout=subprocess.PIPE,
> > +            stderr=subprocess.STDOUT,
> > +            check=True,
> > +            close_fds=False,
> >           )
>
> Now it might throw a CalledProcessError given that `check=True`. 
> Shouldn't it capture the exception and (possible) re-throw as an 
> QEMUMachineError?
>
> - Wainer

I suppose I ought to so that it matches the other errors of this method, 
yes.

Setting it to false and checking manually may be less code, but yeah. 
I'll change this.

Thanks!