2010/11/29 Jiri Denemark <jdenemar@xxxxxxxxxx>: > --- > Âtests/nwfilterxml2xmltest.c |  15 +++++---------- > Âtests/qemuxml2argvtest.c  Â|  Â8 +++----- > Â2 files changed, 8 insertions(+), 15 deletions(-) > > diff --git a/tests/nwfilterxml2xmltest.c b/tests/nwfilterxml2xmltest.c > index 070aa98..9cad913 100644 > --- a/tests/nwfilterxml2xmltest.c > +++ b/tests/nwfilterxml2xmltest.c > @@ -25,7 +25,7 @@ static char *abs_srcdir; > > Âstatic int testCompareXMLToXMLFiles(const char *inxml, >                   const char *outxml, > -                  Âbool expect_warning) { > +                  Âbool expect_error) { >   char inXmlData[MAX_FILE]; >   char *inXmlPtr = &(inXmlData[0]); >   char outXmlData[MAX_FILE]; > @@ -33,26 +33,21 @@ static int testCompareXMLToXMLFiles(const char *inxml, >   char *actual = NULL; >   int ret = -1; >   virNWFilterDefPtr dev = NULL; > -  Âchar *log; > >   if (virtTestLoadFile(inxml, &inXmlPtr, MAX_FILE) < 0) >     goto fail; >   if (virtTestLoadFile(outxml, &outXmlPtr, MAX_FILE) < 0) >     goto fail; > > -  Âif (!(dev = virNWFilterDefParseString(NULL, inXmlData))) > -    Âgoto fail; > +  ÂvirResetLastError(); > > -  Âif ((log = virtTestLogContentAndReset()) == NULL) > +  Âif (!(dev = virNWFilterDefParseString(NULL, inXmlData))) >     goto fail; > > -  Âif ((*log != '\0') != expect_warning) { > -    Âfree(log); > +  Âif (!!virGetLastError() != expect_error) >     goto fail; > -  Â} > -  Âfree(log); > > -  Âif (expect_warning) { > +  Âif (expect_error) { >     /* need to suppress the errors */ >     virResetLastError(); >   } > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index b149ef4..c5e0448 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -84,6 +84,7 @@ static int testCompareXMLToArgvFiles(const char *xml, > > >   free(virtTestLogContentAndReset()); > +  ÂvirResetLastError(); > >   if (qemudBuildCommandLine(conn, &driver, >                vmdef, &monitor_chr, 0, flags, > @@ -91,11 +92,8 @@ static int testCompareXMLToArgvFiles(const char *xml, >                NULL, NULL, migrateFrom, NULL) < 0) >     goto fail; > > -  Âif ((log = virtTestLogContentAndReset()) == NULL) > -    Âgoto fail; > - > -  Âif (!!strstr(log, ": error :") != expectError) { > -    Âif (virTestGetDebug()) > +  Âif (!!virGetLastError() != expectError) { > +    Âif (virTestGetDebug() && (log = virtTestLogContentAndReset())) >       fprintf(stderr, "\n%s", log); >     goto fail; >   } > -- > 1.7.3.2 > ACK to the patch as is. It fixes the problem, but I wonder why qemudBuildCommandLine returns 0 and reports and error. Actually it doesn't report an error, because then it should return non-zero. What it does in the 3 cases where the test expects and "error" it reports actually a warning. For example this one: qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("the QEMU binary %s does not support kvm"), emulator); This is currently reported at error level (VIR_ERR_ERROR) but it should actually be reported at warning level (VIR_ERR_WARNING), because it's treated as non-fatal. The nwfilter test expects errors from this line in virNWFilterRuleDetailsParse: virNWFilterReportError(VIR_ERR_INTERNAL_ERROR, _("%s has illegal value %s"), att[idx].name, prop); The only caller of virNWFilterRuleDetailsParse is virNWFilterRuleParse, but it decided to ignore malformed rules: if (virNWFilterRuleDetailsParse(cur, ret, virAttr[i].att) < 0) { /* we ignore malformed rules goto err_exit; */ } I wonder if in this case virNWFilterRuleParse should call virRestLastError, because it decided to ignore malformed rules. In that case the nwfilter test would not expect an error, because there is actually no error to expect. Matthias. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list