Re: [Qemu-devel] [PATCH RFC] blkdebug: make the fault injection functionality callable from QMP

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3825 bytes --]

On 08/12/2014 10:48 PM, Hitoshi Mitake wrote:
> This patch makes the fault injection functionality of blkdebug
> callable from QMP. Motivation of this change is for testing and
> debugging distributed systems. Ordinal distributed systems must handle
> hardware faults because of its reason for existence, but testing
> whether the systems can hanle such faults and recover in a correct
> manner is really hard.
> 

> 
> {"execute": "set-block-fault-state", "arguments": {"id": "ide0-hd0",
> "state": "error"}} # <- inject error to /dev/sda
> {"return": {}}
> 

Sounds interesting.

> Now the guest OS on the VM finds the disk is broken.
> 
> Of course, using QMP directly is painful for users (developers and
> admins of distributed systems). I'm implementing user friendly
> interface in vagrant-kvm [4] for blackbox testing. In addition, a
> testing framework for injecting faults at critical timing (which
> requires solid understanding of target systems) is in progress.
> 
> [1] http://ucare.cs.uchicago.edu/pdf/socc13-limplock.pdf
> [2] http://docs.openstack.org/developer/swift/howto_installmultinode.html
> [3] http://www.amazon.com/dp/B00C93QFHI
> [4] https://github.com/adrahon/vagrant-kvm
> 
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>
> ---

> +++ b/qapi-schema.json
> @@ -3481,3 +3481,42 @@
>  # Since: 2.1
>  ##
>  { 'command': 'rtc-reset-reinjection' }
> +
> +##
> +# @BlockFaultState:
> +#
> +# Injected fault state for a block device.
> +#
> +# @none: no fault
> +#
> +# @error: visible error
> +#

Needs a Since: 2.2 line, as well as the ## trailing marker.

> +{ 'enum': 'BlockFaultState', 'data': [ 'none', 'error' ] }
> +
> +##
> +# @query-block-fault-state:
> +#
> +# Return block fault state information for the given block device.
> +#
> +# @name: name of block device
> +#
> +# Returns: @BlockFaultState of a block device.
> +#
> +# Since: 1.6

s/1.6/2.2/

Personally, I think this command is overkill.  Just enhance the existing
query-block to output the fault state alongside everything else it
already does.  There's no need to add a new command that outputs a
subset of the existing command.

> +##
> +{ 'command': 'query-block-fault-state', 'data': { 'id': 'str' },
> +  'returns': 'BlockFaultState' }
> +##
> +# @set-block-fault-state:
> +#
> +# Set fault state for the given block device.
> +#
> +# @name: the chardev's ID, must exist and not be in use
> +# @state: fault state
> +#
> +# Returns: Nothing on success
> +#
> +# Since: 1.6

s/1.6/2.2/

> +##
> +{ 'command': 'set-block-fault-state', 'data': {'id': 'str',
> +  'state': 'BlockFaultState' }}
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 4be4765..96ae7e2 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3755,3 +3755,27 @@ Example:
>  <- { "return": {} }
>  
>  EQMP
> +    {
> +        .name       = "query-block-fault-state",
> +        .args_type  = "id:s",
> +        .mhandler.cmd_new = qmp_marshal_input_query_block_fault_state,
> +    },
> +
> +SQMP
> +query-block-fault-state
> +-----------------------
> +
> +Show block fault information.
> +
> +EQMP
> +    {
> +        .name       = "set-block-fault-state",
> +        .args_type  = "id:s,state:q",
> +        .mhandler.cmd_new = qmp_marshal_input_set_block_fault_state,
> +    },
> +
> +SQMP
> +set-block-fault-state
> +-----------------------
> +
> +Set block fault information.
> 

It would be nice to include an example section for the new command(s)
(well, I argue that only one new command is needed).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]