From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58429) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S2m3w-0003rq-07 for qemu-devel@nongnu.org; Wed, 29 Feb 2012 11:07:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S2m3p-0006Bs-Mr for qemu-devel@nongnu.org; Wed, 29 Feb 2012 11:07:31 -0500 Received: from plane.gmane.org ([80.91.229.3]:40770) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S2m3p-0006Bf-G1 for qemu-devel@nongnu.org; Wed, 29 Feb 2012 11:07:25 -0500 Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1S2m3n-00047e-QR for qemu-devel@nongnu.org; Wed, 29 Feb 2012 17:07:23 +0100 Received: from 93-34-182-16.ip50.fastwebnet.it ([93.34.182.16]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Wed, 29 Feb 2012 17:07:23 +0100 Received: from pbonzini by 93-34-182-16.ip50.fastwebnet.it with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Wed, 29 Feb 2012 17:07:23 +0100 From: Paolo Bonzini Date: Wed, 29 Feb 2012 17:07:13 +0100 Message-ID: References: <1330473276-8975-1-git-send-email-mjt@tls.msk.ru> <1330473276-8975-2-git-send-email-mjt@msgid.tls.msk.ru> <4F4E49FF.3080602@redhat.com> <4F4E4B9F.8060609@msgid.tls.msk.ru> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit In-Reply-To: <4F4E4B9F.8060609@msgid.tls.msk.ru> Subject: Re: [Qemu-devel] [PATCH 1/3] Combine bdrv_read and bdrv_write to bdrv_rw List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Il 29/02/2012 17:00, Michael Tokarev ha scritto: > And how it will be a cleanup? > > The whole cow code (and a few others) is not reenterant. Merely > moving this lock/unlock stuff inth actual methods eliminates two > current wrappers in cow_co_write() and cow_co_read(), which are > exactly the same now, and moves this exactly the same code into > actual methods, which has nothing to do with locking - they're > not reenterant, and they deal with internal to the format stuff. > Having this common locking layer on top and _outside_ of the > actual work helps removing irrelevant code from important paths. > Also, it will be too easy to forgot to unlock it there by doing > just "return" somewhere. It's not very different from leaking memory. It's just the way C works. In the future, you may add unlock around image access like in qcow2, and then an unlock/lock pair would be confusing without the lock/unlock outside. If you are worried about forgetting to unlock, add owner tracking to qemu-coroutine-lock.c, similar to PTHREAD_MUTEX_ERRORCHECK. That would be quite useful. Paolo