[master/rhel7-branch] DASD storage re-write, take III

Vratislav Podzimek vpodzime at redhat.com
Fri Feb 28 07:48:37 UTC 2014


On Thu, 2014-02-27 at 09:17 -0500, Samantha N. Bueno wrote:
> On Wed, Feb 26, 2014 at 10:31:24AM +0100, Vratislav Podzimek wrote:
> > On Wed, 2014-02-26 at 00:07 -0500, Samantha N. Bueno wrote:
> > > This now includes fixes from the second review. I had some residual
> > > issues with thread handling and updating of the GUI spoke status as
> > > well (that I didn't fully realize until after I posted my last set), and
> > > those issues are fixed.
> > > 
> > > Just so things are clear, I've created four delightful and exciting
> > > screencasts for anyone interested, so get your popcorn ready:
> > > 
> > > [graphical install]
> > > http://sbueno.fedorapeople.org/1001070-graphical-20140225.webm
> > > 
> > > [graphical install w/zerombr in ks]
> > > http://sbueno.fedorapeople.org/1001070-graphical-zerombr-20140225.webm
> > > 
> > > [text install]
> > > http://sbueno.fedorapeople.org/1001070-text-20140225.webm
> > > 
> > > [text install w/zerombr in ks]
> > > http://sbueno.fedorapeople.org/1001070-text-zerombr-20140225.webm
> > From my perspective, these patches are good to go after doing few local
> > changes fixing the neatpicks. Thanks for working on this so hard!
> 
> Great! Didn't want to respond to each message individually since they
> were so small, but I've applied all your suggestions. Thanks very much
> for the reviews. Re: this one piece though--
> 
> > > @@ -357,6 +365,10 @@ class StorageSpoke(NormalSpoke, StorageChecker):             
> > >      def _doExecute(self):                                                        
> > >          self._ready = False                                                      
> > >          hubQ.send_not_ready(self.__class__.__name__)                             
> > > +        # ugly and stupid, but on the off-chance dasdfmt is running,
> > > we can't    
> > > +        # proceed further with this                                              
> > > +        while threadMgr.get(constants.THREAD_DASDFMT):                           
> > > +            continue                                                             
> > Is there any reason for doing this busy wait instead of calling                     
> > threadMgr.wait(constants.THREAD_DASDFMT)? I believe the result would be             
> > the same.
> 
> The only reason I do that is to avoid directly calling THREAD_DASDFMT to
> try and reduce the UI freezing. I guess it is kind of the same though
> since it is constantly checking the thread. I can't seem to come up
> with a better way though. That is my only residual issue with this patch
> set though.
The _doExecute method is called in a non-main thread, so it shouldn't
freeze the UI.

If you send me the final version of your patches, I'll apply them and
try to find out what may be the cause of the hangs you are seeing.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list