Re: [Qemu-devel] [PATCH v2 0/5]: QMP: Proper thin provisioning support

[Luiz Capitulino]

On Wed, 03 Aug 2011 17:39:04 +0200
Kevin Wolf <address@hidden> wrote:

> Am 03.08.2011 17:19, schrieb Luiz Capitulino:
> > Roughly speaking, thin provisioning is a feature where the VM is started 
> > with
> > a small disk and space is allocated on demand.
> > 
> > It's already possible for QMP clients to implement this feature by using
> > the BLOCK_IO_ERROR event. However, the event can be missed. When this
> > happens QMP clients need a way to query if any block device has hit a
> > no space condition.
> > 
> > This is what this series is about: it extends the query-block command to
> > contain the disk's I/O status.
> > 
> > Please, note that this series depends on the following two other series:
> > 
> >  1. [PATCH 00/55] Block layer cleanup & fixes (by Markus)
> >  2. [PATCH 0/7]: Introduce the QemuState type (by me)
> 
> It feels strange that device models are involved with this. The block
> layer already knows the error number, it just tells the device model so
> that it can ask it back. Wouldn't it be easier to store it in the
> BlockDriverState?

My first version did it, but looking at it now maybe I did it wrong,
because I was setting the iostatus in the devices (instead of setting
it in the block layer itself). Here's an example:

 http://lists.nongnu.org/archive/html/qemu-devel/2011-07/msg00421.html

But I'm not sure what the best place is. I'm hoping that you and/or
Markus can help me here. Also note that Markus already told us what
he thinks:

"""
Actually, this is a symptom of the midlayer disease.  I suspect things
would be simpler if we hold the status in its rightful owner, the device
model.  Need a getter for it.  I'm working on a patch series that moves
misplaced state out of the block layer into device models and block
drivers, and a I/O status getter will fit in easily there.
"""

 http://lists.nongnu.org/archive/html/qemu-devel/2011-07/msg00937.html

Can you guys get in agreement?
 
> Also, as I already commented on v1, I think it's a bad idea to let
> requests that complete later overwrite the error code with "success".
> Quite possible in an ENOSPC case, I think.

Really? I'm under the assumption that all pending I/O will be completed
as soon as the vm is stopped. Am I wrong?

What we need here is a way to "reset" the io-status field. That's, the vm will
stop due to ENOSPC, the mngt application will handle it and put the vm to run
again. At this point the io-status has to be reset otherwise it will contain
stale data.

By doing the overwrite this "reset" is done automatically as soon as the
vm is put to run again and doesn't hit the same (or other) errors.

The other option would be to allow the mngt application to manually reset
the field.

More ideas?