From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56664) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XR09G-0003rI-8E for qemu-devel@nongnu.org; Mon, 08 Sep 2014 10:42:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XR098-0000Qh-V4 for qemu-devel@nongnu.org; Mon, 08 Sep 2014 10:42:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:19174) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XR098-0000Q2-Ni for qemu-devel@nongnu.org; Mon, 08 Sep 2014 10:42:22 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s88EgLWg030953 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Mon, 8 Sep 2014 10:42:21 -0400 Date: Mon, 8 Sep 2014 10:42:17 -0400 From: Luiz Capitulino Message-ID: <20140908104217.48f2354a@redhat.com> In-Reply-To: <20140829160727.69f66ecd@redhat.com> References: <20140829160727.69f66ecd@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] block: extend BLOCK_IO_ERROR event with nospace indicator List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: kwolf@redhat.com Cc: fromani@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com On Fri, 29 Aug 2014 16:07:27 -0400 Luiz Capitulino wrote: > Management software, such as RHEV's vdsm, want to be able to allocate > disk space on demand. The basic use case is to start a VM with a small > disk and then the disk is enlarged when QEMU hits a ENOSPC condition. > > To this end, the management software has to be notified when QEMU > encounters ENOSPC. The solution implemented by this commit is simple: > it extends the BLOCK_IO_ERROR with a 'nospace' key, which is true > when QEMU is stopped due to ENOSPC. > > Note that support for querying this event is already present in > query-block by means of the 'io-status' key. Also, the new 'nospace' > BLOCK_IO_ERROR field shares the same semantics with 'io-status', > which basically means that werror= has to be set to either > 'stop' or 'enospc' to enable 'nospace'. > > Finally, this commit also updates the 'io-status' key doc in the > schema with a list of supported device models. > > Signed-off-by: Luiz Capitulino Kevin, are you going to take this via block layer tree? > --- > > Three important observations: > > 1. We've talked with oVirt and OpenStack folks. oVirt folks say that > this implementation is enough for their use-case. OpenStack don't > need this feature > > 2. While testing this with a raw image on a (smaller) ext2 file mounted > via the loopback device, I get half "Invalid argument" I/O errors and > half "No space" errors". This means that half of the BLOCK_IO_ERROR > events that are emitted for this test-case will have nospace=false > and the other half nospace=true. I don't know why I'm getting those > "Invalid argument" errors, can anyone of the block layer comment > on this? I don't get that with a qcow2 image (I get nospace=true for > all events) > > 3. I think this should go via block tree > > block.c | 22 ++++++++++++++-------- > qapi/block-core.json | 8 +++++++- > 2 files changed, 21 insertions(+), 9 deletions(-) > > diff --git a/block.c b/block.c > index 1df13ac..b334e35 100644 > --- a/block.c > +++ b/block.c > @@ -3632,6 +3632,18 @@ BlockErrorAction bdrv_get_error_action(BlockDriverState *bs, bool is_read, int e > } > } > > +static void send_qmp_error_event(BlockDriverState *bs, > + BlockErrorAction action, > + bool is_read, int error) > +{ > + BlockErrorAction ac; > + > + ac = is_read ? IO_OPERATION_TYPE_READ : IO_OPERATION_TYPE_WRITE; > + qapi_event_send_block_io_error(bdrv_get_device_name(bs), ac, action, > + bdrv_iostatus_is_enabled(bs), > + error == ENOSPC, &error_abort); > +} > + > /* This is done by device models because, while the block layer knows > * about the error, it does not know whether an operation comes from > * the device or the block layer (from a job, for example). > @@ -3657,16 +3669,10 @@ void bdrv_error_action(BlockDriverState *bs, BlockErrorAction action, > * also ensures that the STOP/RESUME pair of events is emitted. > */ > qemu_system_vmstop_request_prepare(); > - qapi_event_send_block_io_error(bdrv_get_device_name(bs), > - is_read ? IO_OPERATION_TYPE_READ : > - IO_OPERATION_TYPE_WRITE, > - action, &error_abort); > + send_qmp_error_event(bs, action, is_read, error); > qemu_system_vmstop_request(RUN_STATE_IO_ERROR); > } else { > - qapi_event_send_block_io_error(bdrv_get_device_name(bs), > - is_read ? IO_OPERATION_TYPE_READ : > - IO_OPERATION_TYPE_WRITE, > - action, &error_abort); > + send_qmp_error_event(bs, action, is_read, error); > } > } > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index fb74c56..567e0a6 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -336,6 +336,7 @@ > # > # @io-status: #optional @BlockDeviceIoStatus. Only present if the device > # supports it and the VM is configured to stop on errors > +# (supported device models: virtio-blk, ide, scsi-disk) > # > # @inserted: #optional @BlockDeviceInfo describing the device if media is > # present > @@ -1569,6 +1570,11 @@ > # > # @action: action that has been taken > # > +# @nospace: #optional true if I/O error was caused due to a no-space > +# condition. This key is only present if query-block's > +# io-status is present, please see query-block documentation > +# for more information (since: 2.2) > +# > # Note: If action is "stop", a STOP event will eventually follow the > # BLOCK_IO_ERROR event > # > @@ -1576,7 +1582,7 @@ > ## > { 'event': 'BLOCK_IO_ERROR', > 'data': { 'device': 'str', 'operation': 'IoOperationType', > - 'action': 'BlockErrorAction' } } > + 'action': 'BlockErrorAction', '*nospace': 'bool' } } > > ## > # @BLOCK_JOB_COMPLETED