Re: [Qemu-devel] [PATCH v4] Stop VM on ENOSPC error.

[Gleb Natapov <gleb@redhat.com>]

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.