From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35502) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bClGC-0001vg-S3 for qemu-devel@nongnu.org; Tue, 14 Jun 2016 06:07:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bClG8-0000aF-Kw for qemu-devel@nongnu.org; Tue, 14 Jun 2016 06:07:51 -0400 Received: from mail-am1on0117.outbound.protection.outlook.com ([157.56.112.117]:19007 helo=emea01-am1-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bClG7-0000ZW-Ug for qemu-devel@nongnu.org; Tue, 14 Jun 2016 06:07:48 -0400 References: <1465378757-15271-1-git-send-email-den@openvz.org> <20160608112357.GD5324@noname.str.redhat.com> From: "Denis V. Lunev" Message-ID: <575FC184.4090903@openvz.org> Date: Tue, 14 Jun 2016 11:34:12 +0300 MIME-Version: 1.0 In-Reply-To: <20160608112357.GD5324@noname.str.redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/1] hmp: acquire aio_context in hmp_qemu_io List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, Vladimir Sementsov-Ogievskiy , Paolo Bonzini On 06/08/2016 02:23 PM, Kevin Wolf wrote: > Am 08.06.2016 um 11:39 hat Denis V. Lunev geschrieben: >> From: Vladimir Sementsov-Ogievskiy >> >> Acquire aio context before run command, this is mandatory for unit tests. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy >> Signed-off-by: Denis V. Lunev >> CC: Kevin Wolf >> CC: Paolo Bonzini > Looks right to me, but why "mandatory for unit tests"? Does this fix an > observable bug in unit tests? If so, we should be more specific in the > commit message. > > But in fact, I would only expect it to make a difference for dataplane > and I don't think we have test cases that use both the 'qemu-io' HMP > command and dataplane. > > Kevin the problem is that it is usually difficult to understand that the problem is in locking and find where the locking is missed. Here we do have a place, which is used by unit testing, with a locking missed. May be now tests are not failing, but IMHO these tests MUST be fixed to accommodate future changes and thus this patch is mandatory for them. Den