From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LPhPm-0000IS-Vf for qemu-devel@nongnu.org; Wed, 21 Jan 2009 13:02:59 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LPhPl-0000He-7Z for qemu-devel@nongnu.org; Wed, 21 Jan 2009 13:02:58 -0500 Received: from [199.232.76.173] (port=49371 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LPhPk-0000HV-Ua for qemu-devel@nongnu.org; Wed, 21 Jan 2009 13:02:57 -0500 Received: from mx2.redhat.com ([66.187.237.31]:50121) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1LPhPj-0001zB-Uc for qemu-devel@nongnu.org; Wed, 21 Jan 2009 13:02:56 -0500 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n0LI2sRP010929 for ; Wed, 21 Jan 2009 13:02:54 -0500 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n0LI2sK7017405 for ; Wed, 21 Jan 2009 13:02:55 -0500 Received: from dhcp-1-237.tlv.redhat.com (dhcp-1-237.tlv.redhat.com [10.35.1.237]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n0LI2sgp002505 for ; Wed, 21 Jan 2009 13:02:54 -0500 Date: Wed, 21 Jan 2009 20:01:06 +0200 From: Gleb Natapov Subject: Re: [Qemu-devel] [PATCH v4] Stop VM on ENOSPC error. Message-ID: <20090121180106.GC16094@redhat.com> References: <20090120105610.GB27675@redhat.com> <18805.55601.339210.692083@mariner.uk.xensource.com> <20090120141651.GF27675@redhat.com> <18805.58968.356765.658443@mariner.uk.xensource.com> <20090120153159.GA8319@redhat.com> <18806.198.448290.48055@mariner.uk.xensource.com> <20090120182344.GA12718@redhat.com> <18807.20276.763061.774652@mariner.uk.xensource.com> <20090121170030.GA16094@redhat.com> <18807.23199.366958.156370@mariner.uk.xensource.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <18807.23199.366958.156370@mariner.uk.xensource.com> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On Wed, Jan 21, 2009 at 05:25:51PM +0000, Ian Jackson wrote: > Gleb Natapov writes ("Re: [Qemu-devel] [PATCH v4] Stop VM on ENOSPC error."): > > On Wed, Jan 21, 2009 at 04:37:08PM +0000, Ian Jackson wrote: > > > But ide.c doesn't call bdrv_pwrite; it calls bdrv_aio_write. > > > bdrv_aio_write would do what your ide.c code currently does. > > > > ide.c calls bdrv_write. > > In the non-DMA case, yes. That would have to be changed to call > bdrv_aio_write. > > > And synchronous calls will behave differently. I am all for doing it in > > one place, but I am against doing it just for part of API calls. So do > > you have better solution for sync calls except don't use them? > > Well, stopping the VM can't be done for synchronous calls so we > clearly have three choices > * write out the complicated asynchronous vm_stop-on-enospc logic > at least twice, once for ide and scsi > * do it only for ide > * do it only for part of the api, and use only that api in ide > and scsi > > I prefer the last of these. Doing it for only part of the internal > API is less bad than doing it only for some of the actual emulated > hardware. I also think doing it for only part of the internal API is > less bad than writing the same functionality twice. Tomorrow I'll send patch for scsi and PV block device. The patches are absolutely trivial. The only reason I've sent ide implementation first was that I wanted early feedback. As discussion went to this direction it just easier for me to send all patches to demonstrate that the changes a trivial. > > Writing the same functionality twice seems to me to be one of the > cardinal sins of programming. Doing something in the wrong level to save three line of code and make only part of API usable doesn't sound plausible too. > > But it would be good to hear what other people think. If they prefer > the other options then your patch should go in as-is. > > > And BTW block.c is compiled also for qemu-{img|mbd} no vmstop there. > > We can finesse this with a suitable hook or an #ifdef. > As far as I can see we compile block*.c only once and link them with qemu, qemu-img and qemu-nbd. AFAIR that was not the case so someone make an effort to fix it. Why break it one more time without a really good reason? -- Gleb.