Re: [PATCH] detect and report qemu-dm failure

[Ian Jackson <Ian.Jackson@eu.citrix.com>]

Daniel P. Berrange writes ("Re: [Xen-devel] [PATCH] detect and report qemu-dm failure"):
> It seems rather complicated, but I can't think of a much better way to deal
> with it, so just a handful of minor nit-picks inline...

Yes, it is rather complicated.  There are slightly better ways of
doing thing kind of an event-driven way but even so the fact that xend
isn't necessarily the parent causes trouble.

Thanks for the comments; I'll resubmit shortly (and give it a more
thorough reading and search for tabs first).

> On Thu, Jun 12, 2008 at 12:23:06PM +0100, Ian Jackson wrote:
> > +	    image.cleanup_stale_sentinel_fifos()
> > +
> 
> This indentation looks wrong to me.

tab/space problem.  I should untabify it.

> > +import fcntl
> 
> This this change intended ?  You're not adding any calls to fcntl in
> this patch.

Oh, that'll be from an earlier attempt.

> > +        logfd = os.open(self.logfile, os.O_WRONLY|os.O_CREAT|os.O_TRUNC|os.O_APPEND)
> 
> Don't need to add O_APPEND, since we're truncating the file.

Not true.  We share the fd with our child, and want output to be
properly interleaved.  Without this output can sometimes be lost due
to overwriting.

> > +	sentinel_write.close()
> 
> Broken indentation here. 

tab/space damage again.

> >      def destroyDeviceModel(self):
> >          if self.device_model is None:
> >              return
> > +        self
> 
> Typo, I guess ?

That's weird.  I must have introduced that by mistake.  It has no
effect so my testing won't have spotted it.

Ian.