From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Goldish Subject: Re: [KVM-AUTOTEST PATCH v2 3/6] [RFC] Introduce exception context strings Date: Thu, 06 Jan 2011 13:56:04 +0200 Message-ID: <4D25ADD4.5080502@redhat.com> References: <1294242329-11034-1-git-send-email-mgoldish@redhat.com> <1294242329-11034-3-git-send-email-mgoldish@redhat.com> <4D249855.30607@redhat.com> <4D249A8F.8090009@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: autotest@test.kernel.org, kvm@vger.kernel.org, Eduardo Habkost To: Avi Kivity Return-path: Received: from mx1.redhat.com ([209.132.183.28]:32374 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752817Ab1AFLzk (ORCPT ); Thu, 6 Jan 2011 06:55:40 -0500 In-Reply-To: <4D249A8F.8090009@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 01/05/2011 06:21 PM, Avi Kivity wrote: > On 01/05/2011 06:12 PM, Avi Kivity wrote: >> On 01/05/2011 05:45 PM, Michael Goldish wrote: >>> In complex tests (KVM) an exception string is often not informative >>> enough and >>> the traceback and source code have to be examined in order to figure >>> out what >>> caused the exception. Context strings are a way for tests to provide >>> information about what they're doing, so that when an exception is >>> raised, this >>> information will be embedded in the exception string. The result is >>> a concise >>> yet highly informative exception string, which should make it very >>> easy to >>> figure out where/when the exception was raised. >>> >>> A typical example for a test where this may be useful is KVM's reboot >>> test. >>> Some exceptions can be raised either before or after the VM is >>> rebooted (e.g. >>> logging into the guest can fail) and whether they are raised before >>> or after >>> is critical to the understanding of the failure. Normally the >>> traceback would >>> have to be examined, but the proposed method makes it easy to know >>> where the >>> exception is raised without doing so. To achieve this, the reboot >>> test should >>> place calls to error.context() as follows: >>> >>> error.context("before reboot") >>> >>> error.context("sending reboot command") >>> >>> error.context("after reboot") >>> >>> >>> If login fails in the pre-reboot section, the resulting exception >>> string can >>> can have something like "context: before reboot" embedded in it. >>> (The actual >>> embedding is done in the next patch in the series.) >> >> It would be nice to make the error context a stack, and to use the >> with statement to manage the stack: >> >> >> with error.context("main test"): >> foo() >> with error.context("before reboot"): >> bar() >> >> If foo() throws an exception, the context would be "main test", while >> if bar() throws an exception, the context would be "before reboot" in >> "main test". This seems like the best solution and it's unfortunate that we can't use it. > btw, you can have a decorator for enclosing an entire function in an > error context: > > @function_error_context('migration test') > def migration_test(...): > ... > > anything in migration_test() is enclosed in that context. But we're > just repeating the ordinary stack trace with something more readable. The problem is that the string passed to function_error_context can't be based on function parameters, so declaring a context like 'migrating vm1' is impossible. I do think we can benefit from 2 context levels per function though: @context_aware def migrate(...): base_context("migrating %s" % vm.name) context("collecting parameters") ... context("sending monitor command") ... context("cleanup") ... base_context() and context() will just be joined together using ' --> ' like regular contexts. base_context() can be useful for long utility functions. Does this sound like a reasonable solution, or do you think it's cleaner to always define a new nested function for each context level?