On Mon, Apr 8, 2013 at 11:46 AM, Bernardo Dal Seno <[email protected]> wrote: > On 7 April 2013 12:35, Michele Tartara <[email protected]> wrote: >> >> Il giorno 07/apr/2013 07:41, "Guido Trotter" <[email protected]> ha >> scritto: >> >> >>> >>> Note that this fixes the "current issue" but doesn't fix the underlying >>> problem. :/ >>> >>> Signed-off-by: Guido Trotter <[email protected]> >>> --- >>> qa/ganeti-qa.py | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/qa/ganeti-qa.py b/qa/ganeti-qa.py >>> index 5ffd44f..1ea0b25 100755 >>> --- a/qa/ganeti-qa.py >>> +++ b/qa/ganeti-qa.py >>> @@ -364,7 +364,11 @@ def RunExportImportTests(instance, inodes): >>> @param inodes: current nodes of the instance >>> >>> """ >>> - if qa_config.TestEnabled("instance-export"): >>> + # FIXME: export explicitly bails out on file based storage. other >>> non-lvm >>> + # based storage types are untested, though. Also note that import could >>> still >>> + # work, but is deeply embedded into the "export" case. >>> + if (qa_config.TestEnabled("instance-export") and >>> + instance.disk_template != constants.DT_FILE): > > With other tests we have put such checks inside the test function. I > think is better, as you have one place that handles any exception, > instead of keeping in sync the logic of the caller and the callee. >
I went for there because there were a lot of them, and because it's good that it's only one place especially as there's a bug open and we want to get this fixed. In other cases we don't plan to "solve" the issue, so inside the test function is more appropriate. Thanks, Guido
