[libvirt] [PATCH v3 2/6] virnetdaemon: Add post exec restart support for multiple servers

[Erik Skultety]

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