On 10/01/16 16:27, Martin Kletzander wrote: > On Mon, Nov 30, 2015 at 04:03:01PM +0100, Erik Skultety wrote: >> Since the daemon can manage and add (at fresh start) multiple servers, >> we also should be able to add them from a JSON state file in case of a >> daemon restart. This patch introduces virNetDaemonAddServersPostExec >> method which harvests the data about servers from a JSON file supporting >> both old format with a single server and a new one storing an array of >> servers. The method makes use of the original >> virNetDaemonAddServerPostExec, >> declaring the latter as static. >> Patch also updates virnetdaemontest accordingly. > > There's only one thing wrong with this patch: In virnetdaemontest you > remove the ability for testing something that should've failed. You > also effectively remove one such test. Was it because you forgot about > it or did you remove it intentionally? I would prefer it to stay there. > The removal is the only thing keeping me from acking this patch. > Hmm, I can't think of any reasons why I did that, but you're definitely right about including such a test in there. However, I did a small investigation and found out, that the current solution doesn't work as expected, see the following scenarios: SCENARIO 1} Include a test, that should pass, but fails unexpectedly OUT: TEST: virnetdaemontest 1) ExecRestart initial ... OK 2) ExecRestart initial-nomdns ... OK 3) ExecRestart anon-clients ... OK 4) ExecRestart anon-clients ... libvirt: XML-RPC error : internal error: Cannot add more servers post-exec than there were pre-exec libvirt: XML-RPC error : internal error: Cannot add more servers post-exec than there were pre-exec FAILED 5) ExecRestart admin-nomdns ... OK I separated test case 4 so it is clear that there are 2 identical error messages caused by multiple calls to virDispatchError, first being in virnetdaemontest and the other one in virtTestRun routine. SCENARIO 2} Include a test which should fail, but would actually pass TEST: virnetdaemontest 1) ExecRestart initial ... OK 2) ExecRestart initial-nomdns ... OK 3) ExecRestart anon-clients ... OK 4) ExecRestart anon-clients ... libvirt: XML-RPC error : internal error: Cannot add more servers post-exec than there were pre-exec OK 5) ExecRestart admin-nomdns ... libvirt: XML-RPC error : internal error: Test should've failed OK Testcase 5 failed with error stating that the test should have failed the execution but it didn't. Yet, the testcase returns success. I'll include all necessary changes in this patch once I adjust all the remaining ones in this v3 series and post them all. Erik -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list