On Tue, 2020-07-21 at 18:49 -0500, Joshua Watt wrote:
> 
> 
> On Tue, Jul 21, 2020, 4:56 PM Richard Purdie 
> <[email protected]> wrote:
> > We continue to see qemu races where qemu fails to write out a pidfile. The
> > pidfile name being used was based on the parent process starting the qemus
> > so in theory, a qemu shutting down could have deleted the file into which
> > the new qemu's pid was about to be written. This shouldn't happen as qemu
> > processes should be exitting completely before the code returns.
> > 
> > In the absence of better theory for the failure, use a random pid filename
> > rather than the parent pid so there is no overlap in the pid filename,
> > thereby removing any theoretical race.
> > 
> > Signed-off-by: Richard Purdie <[email protected]>
> > ---
> >  meta/lib/oeqa/utils/qemurunner.py | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/meta/lib/oeqa/utils/qemurunner.py 
> > b/meta/lib/oeqa/utils/qemurunner.py
> > index 519aa9aa1e5..73a3efdc964 100644
> > --- a/meta/lib/oeqa/utils/qemurunner.py
> > +++ b/meta/lib/oeqa/utils/qemurunner.py
> > @@ -20,6 +20,7 @@ import string
> >  import threading
> >  import codecs
> >  import logging
> > +import tempfile
> >  from oeqa.utils.dump import HostDumper
> >  from collections import defaultdict
> > 
> > @@ -65,7 +66,11 @@ class QemuRunner:
> >          self.runqemutime = 120
> >          if not workdir:
> >              workdir = os.getcwd()
> > -        self.qemu_pidfile = workdir + '/pidfile_' + str(os.getpid())
> > +        # Use tempfile to obtain a unique name. We own the directory
> > +        # so closing the file isn't an issue as long as we don't delete it
> > +        self.qemu_pidfile = tempfile.mkstemp(dir=workdir, 
> > prefix="qemupid_")
> > +        os.close(self.qemu_pidfile[0])
> > +        self.qemu_pidfile = self.qemu_pidfile[1]
> 
> Minor, but more idiomatic python:
> 
>  (fd, self.qemu_pidfile) = tempfile.mkstemp(...)
>  os.close(fd)

Yes!

I'd changed the code a few too many times and it ended up a bit silly.

Sadly the autobuilder showed the same fault with this applied so this
patch doesn't work anyway and probably isn't worth applying :(

If anyone can spot the race...

Cheers,

Richard

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.

View/Reply Online (#140852): 
https://lists.openembedded.org/g/openembedded-core/message/140852
Mute This Topic: https://lists.openembedded.org/mt/75713689/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub  
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to