xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	"Stefano Stabellini" <sstabellini@kernel.org>,
	"Julien Grall" <julien@xen.org>,
	"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
	"Juergen Gross" <jgross@suse.com>,
	Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: Hypercall fault injection (Was [PATCH 0/3] xen/domain: More structured teardown)
Date: Tue, 22 Dec 2020 11:00:44 +0100	[thread overview]
Message-ID: <983a3fef-c80f-ec2a-bf3c-5e054fc6a7a9@suse.com> (raw)
In-Reply-To: <ac552c84-144c-c213-7985-84d92cbb5601@citrix.com>

On 21.12.2020 20:36, Andrew Cooper wrote:
> Hello,
> 
> We have some very complicated hypercalls, createdomain, and max_vcpus a
> close second, with immense complexity, and very hard-to-test error handling.
> 
> It is no surprise that the error handling is riddled with bugs.
> 
> Random failures from core functions is one way, but I'm not sure that
> will be especially helpful.  In particular, we'd need a way to exclude
> "dom0 critical" operations so we've got a usable system to run testing on.
> 
> As an alternative, how about adding a fault_ttl field into the hypercall?
> 
> The exact paths taken in {domain,vcpu}_create() are sensitive to the
> hardware, Xen Kconfig, and other parameters passed into the
> hypercall(s).  The testing logic doesn't really want to care about what
> failed; simply that the error was handled correctly.
> 
> So a test for this might look like:
> 
> cfg = { ... };
> while ( xc_create_domain(xch, cfg) < 0 )
>     cfg.fault_ttl++;
> 
> 
> The pro's of this approach is that for a specific build of Xen on a
> piece of hardware, it ought to check every failure path in
> domain_create(), until the ttl finally gets higher than the number of
> fail-able actions required to construct a domain.  Also, the test
> doesn't need changing as the complexity of domain_create() changes.
> 
> The main con will mostly likely be the invasiveness of code in Xen, but
> I suppose any fault injection is going to be invasive to a certain extent.

While I like the idea in principle, the innocent looking

cfg = { ... };

is quite a bit of a concern here as well: Depending on the precise
settings, paths taken in the hypervisor may heavily vary, and hence
such a test will only end up being useful if it covers a wide
variety of settings. Even if the number of tests to execute turned
out to still be manageable today, it may quickly turn out not
sufficiently scalable as we add new settings controllable right at
domain creation (which I understand is the plan).

Jan


  reply	other threads:[~2020-12-22 10:00 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-21 18:14 [PATCH 0/3] xen/domain: More structured teardown Andrew Cooper
2020-12-21 18:14 ` [PATCH 1/3] xen/domain: Reorder trivial initialisation in early domain_create() Andrew Cooper
2020-12-22 10:10   ` Jan Beulich
2020-12-22 10:24     ` Andrew Cooper
2020-12-22 10:50       ` Jan Beulich
2020-12-21 18:14 ` [PATCH 2/3] xen/domain: Introduce domain_teardown() Andrew Cooper
2020-12-21 18:36   ` Julien Grall
2020-12-21 18:45     ` Andrew Cooper
2020-12-22  7:50       ` Jan Beulich
2020-12-22 10:25         ` Julien Grall
2020-12-22 10:53           ` Jan Beulich
2020-12-22 11:05             ` Julien Grall
2020-12-22 11:11             ` Andrew Cooper
2020-12-22 10:35   ` Jan Beulich
2020-12-22 11:46     ` Andrew Cooper
2020-12-22 11:55       ` Jan Beulich
2020-12-21 18:14 ` [PATCH 3/3] xen/evtchn: Clean up teardown handling Andrew Cooper
2020-12-22 10:48   ` Jan Beulich
2020-12-22 11:28     ` Andrew Cooper
2020-12-22 11:52       ` Jan Beulich
2020-12-22 13:33         ` Andrew Cooper
2020-12-22 13:45           ` Jan Beulich
2020-12-21 19:36 ` Hypercall fault injection (Was [PATCH 0/3] xen/domain: More structured teardown) Andrew Cooper
2020-12-22 10:00   ` Jan Beulich [this message]
2020-12-22 11:14     ` Andrew Cooper
2020-12-22 15:47       ` Tamas K Lengyel
2020-12-22 17:17         ` Andrew Cooper
2020-12-22 18:24           ` Tamas K Lengyel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=983a3fef-c80f-ec2a-bf3c-5e054fc6a7a9@suse.com \
    --to=jbeulich@suse.com \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=jgross@suse.com \
    --cc=julien@xen.org \
    --cc=roger.pau@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).