John Levon <levon(a)movementarian.org&gt; wrote: The test failure affects all systems, since two newly required test files were not committed: tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.args tests/qemuxml2argvdata/qemuxml2argv-disk-drive-shared.xml Obviously, the testing framework should report the failure to open the .args file: Here's the patch to do that: From: Jim Meyering <meyering(a)redhat.com&gt; Date: Fri, 30 Jan 2009 20:06:48 +0100 Subject: [PATCH] tests: diagnose open failure * tests/qemuxml2argvtest.c (testCompareXMLToArgvFiles): Diagnose failure to open an input file. --- tests/qemuxml2argvtest.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 90b4740..1d7aeb9 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -36,8 +36,10 @@ static int testCompareXMLToArgvFiles(const char *xml, virDomainDefPtr vmdef = NULL; virDomainObj vm; - if (virtTestLoadFile(cmd, &expectargv, MAX_FILE) < 0) + if (virtTestLoadFile(cmd, &expectargv, MAX_FILE) < 0) { + fprintf(stderr, "failed to open %s: %s\n", cmd, strerror (errno)); goto fail; + } if (!(vmdef = virDomainDefParseFile(NULL, driver.caps, xml, VIR_DOMAIN_XML_INACTIVE))) -- 1.6.1.2.418.gd79e6

John Levon <levon(a)movementarian.org&gt; wrote: As mentioned, I'm fixing it like this: From: Jim Meyering <meyering(a)redhat.com&gt; Date: Mon, 2 Feb 2009 18:46:32 +0100 Subject: [PATCH 1/2] tests: diagnose more open failures * tests/qemuxml2argvtest.c: Revert the change, "tests: diagnose open failure" of 2009-01-30. --- tests/qemuxml2argvtest.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1d7aeb9..90b4740 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -36,10 +36,8 @@ static int testCompareXMLToArgvFiles(const char *xml, virDomainDefPtr vmdef = NULL; virDomainObj vm; - if (virtTestLoadFile(cmd, &expectargv, MAX_FILE) < 0) { - fprintf(stderr, "failed to open %s: %s\n", cmd, strerror (errno)); + if (virtTestLoadFile(cmd, &expectargv, MAX_FILE) < 0) goto fail; - } if (!(vmdef = virDomainDefParseFile(NULL, driver.caps, xml, VIR_DOMAIN_XML_INACTIVE))) -- 1.6.1.2.443.g0d7c2 From: Jim Meyering <meyering(a)redhat.com&gt; Date: Mon, 2 Feb 2009 19:13:24 +0100 Subject: [PATCH 2/2] * tests/testutils.c (virtTestLoadFile): Diagnose failure here. --- tests/testutils.c | 17 +++++++++++++---- 1 files changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/testutils.c b/tests/testutils.c index e8b07fc..8d8f82d 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -1,7 +1,7 @@ /* * testutils.c: basic test utils * - * Copyright (C) 2005-2008 Red Hat, Inc. + * Copyright (C) 2005-2009 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -106,27 +106,36 @@ virtTestRun(const char *title, int nloops, int (*body)(const void *data), const return ret; } -int virtTestLoadFile(const char *name, +/* Read FILE into buffer BUF of length BUFLEN. + Upon any failure, or if FILE appears to contain more than BUFLEN bytes, + diagnose it and return -1, but don't bother trying to preserve errno. + Otherwise, return the number of bytes read (and copied into BUF). */ +int virtTestLoadFile(const char *file, char **buf, int buflen) { - FILE *fp = fopen(name, "r"); + FILE *fp = fopen(file, "r"); struct stat st; - if (!fp) + if (!fp) { + fprintf (stderr, "%s: failed to open: %s\n", file, strerror(errno)); return -1; + } if (fstat(fileno(fp), &st) < 0) { + fprintf (stderr, "%s: failed to fstat: %s\n", file, strerror(errno)); fclose(fp); return -1; } if (st.st_size > (buflen-1)) { + fprintf (stderr, "%s: larger than buffer (> %d)\n", file, buflen-1); fclose(fp); return -1; } if (st.st_size) { if (fread(*buf, st.st_size, 1, fp) != 1) { + fprintf (stderr, "%s: read failed: %s\n", file, strerror(errno)); fclose(fp); return -1; } -- 1.6.1.2.443.g0d7c2

"Daniel P. Berrange" <berrange(a)redhat.com&gt; wrote: Applied. But as John Levon pointed out, there are many more tests/*test.c programs that use virtTestLoadFile in exactly the same way: $ git grep -h 'if (virtTestLoadFile' if (virtTestLoadFile(outputfile, &expect, MAX_FILE) < 0) if (virtTestLoadFile(cmd, &expectargv, MAX_FILE) < 0) { if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0) if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0) if (virtTestLoadFile(sexpr, &sexprPtr, MAX_FILE) < 0) if (virtTestLoadFile(expect, &expectPtr, MAX_FILE) < 0) if (virtTestLoadFile(xml, &expectxml, MAX_FILE) < 0) if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0) if (virtTestLoadFile(xmcfg, &xmcfgPtr, MAX_FILE) < 0) if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0) if (virtTestLoadFile(xmcfg, &xmcfgPtr, MAX_FILE) < 0) if (virtTestLoadFile(xml, &xmlPtr, MAX_FILE) < 0) if (virtTestLoadFile(sexpr, &sexprPtr, MAX_FILE) < 0) So rather than applying the same multi-line fix to all of those uses, I expect to revert that patch and move the error reporting into virtTestLoadFile itself.