On Wed, Apr 04, 2018 at 09:42:41AM +0800, Peter Xu wrote: > On Tue, Apr 03, 2018 at 01:59:18PM +0100, Stefan Hajnoczi wrote: > > On Tue, Mar 27, 2018 at 10:21:55AM +0800, Peter Xu wrote: > > > On Mon, Mar 26, 2018 at 12:47:39PM +0200, Kevin Wolf wrote: > > > > Am 26.03.2018 um 08:11 hat Peter Xu geschrieben: > > > The patch fixes the other case when there > > > are two events: one JOB_COMPLETED plus another (e.g., RESUME) event. > > > When that happens, logically we should return one JOB_COMPLETED event, > > > but the old code will return the other event (e.g., RESUME). > > > > > > > > > > > Wouldn't it be much easier to just add a 'break'? > > > > > > Yes, it's the same. But IMHO those logics (e.g., the completed > > > variable) are not really needed at all. This one is simpler. > > > > No, the outer loop is needed so that the function waits until > > BLOCK_JOB_COMPLETED is received. It's not possible to do it with a > > single for loop. > > Indeed. But then I would still slightly prefer removing the > "completed" var: > > def wait_until_completed(self, drive='drive0', check_offset=True): > '''Wait for a block job to finish, returning the event''' > while True: > for event in self.vm.get_qmp_events(wait=True): > if event['event'] == 'BLOCK_JOB_COMPLETED': > self.assert_qmp(event, 'data/device', drive) > self.assert_qmp_absent(event, 'data/error') > if check_offset: > self.assert_qmp(event, 'data/offset', > event['data']['len']) > self.assert_no_active_block_jobs() > return event > > Or a single break would work too. Do either of you have any > preference? I can repost in either way. Thanks,
Looks good to me! Stefan
signature.asc
Description: PGP signature
