On 08/26/2014 07:59 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. >
> Example of QMP sequence (via telnet localhost 4444):
>
> { "execute": "qmp_capabilities" }
> {"return": {}}
>
> {"execute": "blkdebug-set-rules", "arguments": {"device": "ide0-hd0",
> "rules":[{"event": "write_aio", "type": "inject-error", "immediately":
Why 'write_aoi'? New QMP commands should prefer dashes (write-aio) if
there is no compelling reason for underscore.
> 1, "once": 0, "state": 1}]}} # <- inject error to /dev/sda
It looks like 'immediately', 'once', and 'state' are all bools - if so,
they should be typed as bool and the example should be written with
"immediately":true and so forth.
>
> {"return": {}}
>
> 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: Eric Blake <ebl...@redhat.com>
> Cc: Kevin Wolf <kw...@redhat.com>
> Cc: Stefan Hajnoczi <stefa...@redhat.com>
> Signed-off-by: Hitoshi Mitake <mitake.hito...@lab.ntt.co.jp>
> ---
> block/blkdebug.c | 199
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> include/block/block.h | 2 +
> qapi-schema.json | 14 ++++
> qmp-commands.hx | 18 +++++
> 4 files changed, 233 insertions(+)
>
> +
> +static void rules_list_iter(QObject *obj, void *opaque)
> +{
> + struct qmp_rules_list_iter *iter = (struct qmp_rules_list_iter *)opaque;
This is C, not C++. The cast is spurious.
> + } else if (!strcmp(type, "inject-error")) {
> + int _errno, sector;
The name _errno threw me; is there something better without a leading
underscore that would work better?
> + _errno = qdict_get_try_int(dict, "errno", EIO);
> + if (qemu_opt_set_number(new_opts, "errno", _errno) < 0) {
> + error_setg(&iter->err, "faild to set errno");
s/faild/failed/ (multiple times throughout your patch)
> +
> + once = qdict_get_try_bool(dict, "once", 0);
s/0/bool/ - we use <stdbool.h>, so you should use the named constants
when dealing with bool parameters.
> +++ b/qapi-schema.json
> @@ -3481,3 +3481,17 @@
> # Since: 2.1
> ##
> { 'command': 'rtc-reset-reinjection' }
> +
> +##
> +# @blockdebug-set-rules
> +#
> +# Set rules of blkdebug for the given block device.
> +#
> +# @device: device ID of target block device
> +# @rules: rules for setting, list of dictionary
> +#
> +# Since: 2.2
> +##
> +{ 'command': 'blkdebug-set-rules',
> + 'data': { 'device': 'str', 'rules': [ 'dict' ] },
> + 'gen': 'no'}
Are we really forced to use this non-type-safe variant? I'd REALLY love
to fully specify this interface in QAPI rather than just hand-waving
that an undocumented dictionary is in place.
Given your example, it looks like you'd want 'rules':['BlkdebugRule'],
where you then have something like:
{ 'enum':'BlkdebugRuleEvent', 'data':['write-aio', ...] }
{ 'enum':'BlkdebugRuleType', 'data':['inject-error', ...] }
{ 'type':'BlkdebugRule', 'arguments':{
'event':'BlkdebugRuleEvent', 'type':'BlkdebugRuleType',
'*immediately':'bool', '*once':'bool', '*state':'bool' } }
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
