On 01/15/2014 03:22 AM, Kevin Wolf wrote: > From: Max Reitz <mre...@redhat.com> > > Add structures to support blkdebug and blkverify in blockdev-add. > > Signed-off-by: Max Reitz <mre...@redhat.com> > Signed-off-by: Kevin Wolf <kw...@redhat.com> > --- > qapi-schema.json | 113 > +++++++++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 109 insertions(+), 4 deletions(-)
Sorry for not noticing this sooner, but we still have some interface
problems that need to be ironed out.
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index f27c48a..35f7b34 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4201,6 +4201,113 @@
> '*pass-discard-other': 'bool' } }
>
> ##
> +# @BlkdebugEvent
> +#
> +# Trigger events supported by blkdebug.
> +##
> +{ 'enum': 'BlkdebugEvent',
Missing a 'Since: 2.0' designation; would be worth adding that in a
followup patch.
> + 'data': [ 'l1_update', 'l1_grow.alloc_table', 'l1_grow.write_table',
> + 'l1_grow.activate_table', 'l2_load', 'l2_update',
> + 'l2_update_compressed', 'l2_alloc.cow_read', 'l2_alloc.write',
> + 'read_aio', 'read_backing_aio', 'read_compressed', 'write_aio',
> + 'write_compressed', 'vmstate_load', 'vmstate_save', 'cow_read',
> + 'cow_write', 'reftable_load', 'reftable_grow', 'reftable_update',
> + 'refblock_load', 'refblock_update', 'refblock_update_part',
> + 'refblock_alloc', 'refblock_alloc.hookup',
> 'refblock_alloc.write',
> + 'refblock_alloc.write_blocks', 'refblock_alloc.write_table',
> + 'refblock_alloc.switch_table', 'cluster_alloc',
> + 'cluster_alloc_bytes', 'cluster_free', 'flush_to_os',
> + 'flush_to_disk' ] }
Do we want to prefer '-' over '_' in all these names?
> +
> +##
> +# @BlkdebugInjectErrorOptions
> +#
> +# Describes a single error injection for blkdebug.
> +#
> +# @event: trigger event
> +#
> +# @state: #optional the state identifier blkdebug needs to be in to
> +# actually trigger the event; defaults to "any"
This sounds like it is a finite set of states, and therefore should be
an enum rather than an 'int'.
> +#
> +# @errno: #optional error identifier (errno) to be returned; defaults
> to
> +# EIO
errno is not a portable. The values of errno on one machine may not
match the values of errno on the other machine using the QMP connection
via a remote socket. It might be cleaner to have an enum of named error
values, rather than integer errno values.
> +#
> +# @sector: #optional specifies the sector index which has to be affected
> +# in order to actually trigger the event; defaults to "any
> +# sector"
> +#
> +# @once: #optional disables further events after this one has been
> +# triggered; defaults to false
> +#
> +# @immediately: #optional fail immediately; defaults to false
> +#
> +# Since: 2.0
> +##
> +{ 'type': 'BlkdebugInjectErrorOptions',
> + 'data': { 'event': 'BlkdebugEvent',
> + '*state': 'int',
> + '*errno': 'int',
> + '*sector': 'int',
> + '*once': 'bool',
> + '*immediately': 'bool' } }
> +
> +##
> +# @BlkdebugSetStateOptions
> +#
> +# Describes a single state-change event for blkdebug.
> +#
> +# @event: trigger event
> +#
> +# @state: #optional the current state identifier blkdebug needs to be
> in;
> +# defaults to "any"
> +#
Again, this should be an enum, not an int.
> +# @new_state: the state identifier blkdebug is supposed to assume if
> +# this event is triggered
s/new_state/new-state/
> @@ -4224,10 +4331,8 @@
> # TODO sheepdog: Wait for structured options
> # TODO ssh: Should take InetSocketAddress for 'host'?
> 'vvfat': 'BlockdevOptionsVVFAT',
> -
> -# TODO blkdebug: Wait for structured options
> -# TODO blkverify: Wait for structured options
> -
> + 'blkdebug': 'BlockdevOptionsBlkdebug',
> + 'blkverify': 'BlockdevOptionsBlkverify',
As we are adding new arms to the union, we should document that these
values are available since 2.0.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
