Re: [Qemu-devel] [PATCH 2/4] qemu-iotests: make cancel_and_wait() common

[Stefan Hajnoczi]

On Wed, May 29, 2013 at 11:54:56AM +0200, Kevin Wolf wrote:
> Am 28.05.2013 um 17:11 hat Stefan Hajnoczi geschrieben:
> > -    def cancel_and_wait(self, drive='drive0', wait_ready=True):
> > -        '''Cancel a block job and wait for it to finish'''
> > -        if wait_ready:
> > -            ready = False
> > -            while not ready:
> > -                for event in self.vm.get_qmp_events(wait=True):
> > -                    if event['event'] == 'BLOCK_JOB_READY':
> > -                        self.assert_qmp(event, 'data/type', 'mirror')
> > -                        self.assert_qmp(event, 'data/device', drive)
> > -                        ready = True
> > -
> > -        result = self.vm.qmp('block-job-cancel', device=drive,
> > -                             force=not wait_ready)
> > -        self.assert_qmp(result, 'return', {})
> > -
> > -        cancelled = False
> > -        while not cancelled:
> > +    def wait_ready(self, drive='drive0'):
> > +        '''Wait until a block job BLOCK_JOB_READY event'''
> > +        ready = False
> > +        while not ready:
> >              for event in self.vm.get_qmp_events(wait=True):
> > -                if event['event'] == 'BLOCK_JOB_COMPLETED' or \
> > -                   event['event'] == 'BLOCK_JOB_CANCELLED':
> > +                if event['event'] == 'BLOCK_JOB_READY':
> >                      self.assert_qmp(event, 'data/type', 'mirror')
> >                      self.assert_qmp(event, 'data/device', drive)
> > -                    if wait_ready:
> > -                        self.assertEquals(event['event'], 
> > 'BLOCK_JOB_COMPLETED')
> > -                        self.assert_qmp(event, 'data/offset', 
> > self.image_len)
> > -                        self.assert_qmp(event, 'data/len', self.image_len)
> > -                    cancelled = True
> > +                    ready = True
> 
> Why don't you just move the whole function including the wait_ready
> parameter? It doesn't do any harm to other callers that don't need the
> feature, and will likely be useful for other test cases later.

The BLOCK_JOB_READY and block-job-complete concepts are used by
mirroring only.  Plus the mirroring test cases perform additional checks
on the event object which aren't common.

I figured the cleanest solution is the patch I posted.

Stefan