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