[libvirt] [PATCH] Fix parallel runs of TLS test suites
From: "Daniel P. Berrange" <berrange@redhat.com> Use a seperate keyfile name for the two TLS test suites so that they don't clash when running tests in parallel Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- tests/virnettlscontexttest.c | 10 ++++++---- tests/virnettlshelpers.c | 6 ++---- tests/virnettlshelpers.h | 6 ++---- tests/virnettlssessiontest.c | 10 ++++++---- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c index 9ade785..53792ee 100644 --- a/tests/virnettlscontexttest.c +++ b/tests/virnettlscontexttest.c @@ -40,6 +40,8 @@ # define VIR_FROM_THIS VIR_FROM_RPC +# define KEYFILE "key-ctx.pem" + struct testTLSContextData { bool isServer; const char *cacrt; @@ -66,7 +68,7 @@ static int testTLSContextInit(const void *opaque) ctxt = virNetTLSContextNewServer(data->cacrt, NULL, data->crt, - keyfile, + KEYFILE, NULL, true, true); @@ -74,7 +76,7 @@ static int testTLSContextInit(const void *opaque) ctxt = virNetTLSContextNewClient(data->cacrt, NULL, data->crt, - keyfile, + KEYFILE, true, true); } @@ -109,7 +111,7 @@ mymain(void) { int ret = 0; - testTLSInit(); + testTLSInit(KEYFILE); # define DO_CTX_TEST(_isServer, _caCrt, _crt, _expectFail) \ do { \ @@ -617,7 +619,7 @@ mymain(void) testTLSDiscardCert(&clientcertlevel2breq); unlink("cacertchain.pem"); - testTLSCleanup(); + testTLSCleanup(KEYFILE); return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/virnettlshelpers.c b/tests/virnettlshelpers.c index 39a2df6..8a10340 100644 --- a/tests/virnettlshelpers.c +++ b/tests/virnettlshelpers.c @@ -34,8 +34,6 @@ # define VIR_FROM_THIS VIR_FROM_RPC -const char *keyfile = abs_builddir "/virnettlscontexttest-key.pem"; - /* * These store some static data that is needed when * encoding extensions in the x509 certs @@ -99,7 +97,7 @@ static gnutls_x509_privkey_t testTLSLoadKey(void) } -void testTLSInit(void) +void testTLSInit(const char *keyfile) { gnutls_global_init(); @@ -112,7 +110,7 @@ void testTLSInit(void) } -void testTLSCleanup(void) +void testTLSCleanup(const char *keyfile) { asn1_delete_structure(&pkix_asn1); unlink(keyfile); diff --git a/tests/virnettlshelpers.h b/tests/virnettlshelpers.h index 7c3f8da..3f6afb9 100644 --- a/tests/virnettlshelpers.h +++ b/tests/virnettlshelpers.h @@ -28,8 +28,6 @@ # include "rpc/virnettlscontext.h" -extern const char *keyfile; - /* * This contains parameter about how to generate * certificates. @@ -76,7 +74,7 @@ void testTLSWriteCertChain(const char *filename, size_t ncerts); void testTLSDiscardCert(struct testTLSCertReq *req); -void testTLSInit(void); -void testTLSCleanup(void); +void testTLSInit(const char *keyfile); +void testTLSCleanup(const char *keyfile); #endif diff --git a/tests/virnettlssessiontest.c b/tests/virnettlssessiontest.c index bc176aa..9b171ed 100644 --- a/tests/virnettlssessiontest.c +++ b/tests/virnettlssessiontest.c @@ -38,6 +38,8 @@ # define VIR_FROM_THIS VIR_FROM_RPC +# define KEYFILE "key-sess.pem" + struct testTLSSessionData { const char *servercacrt; const char *clientcacrt; @@ -107,7 +109,7 @@ static int testTLSSessionInit(const void *opaque) serverCtxt = virNetTLSContextNewServer(data->servercacrt, NULL, data->servercrt, - keyfile, + KEYFILE, data->wildcards, false, true); @@ -115,7 +117,7 @@ static int testTLSSessionInit(const void *opaque) clientCtxt = virNetTLSContextNewClient(data->clientcacrt, NULL, data->clientcrt, - keyfile, + KEYFILE, false, true); @@ -236,7 +238,7 @@ mymain(void) { int ret = 0; - testTLSInit(); + testTLSInit(KEYFILE); # define DO_SESS_TEST(_caCrt, _serverCrt, _clientCrt, _expectServerFail, \ _expectClientFail, _hostname, _wildcards) \ @@ -474,7 +476,7 @@ mymain(void) testTLSDiscardCert(&clientcertlevel2breq); unlink("cacertchain.pem"); - testTLSCleanup(); + testTLSCleanup(KEYFILE); return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 1.8.3.1
On 08/08/2013 04:09 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Use a seperate keyfile name for the two TLS test suites so that
s/seperate/separate/
they don't clash when running tests in parallel
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- tests/virnettlscontexttest.c | 10 ++++++---- tests/virnettlshelpers.c | 6 ++---- tests/virnettlshelpers.h | 6 ++---- tests/virnettlssessiontest.c | 10 ++++++---- 4 files changed, 16 insertions(+), 16 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/09/2013 12:29 AM, Eric Blake wrote:
On 08/08/2013 04:09 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Use a seperate keyfile name for the two TLS test suites so that
s/seperate/separate/
they don't clash when running tests in parallel
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- tests/virnettlscontexttest.c | 10 ++++++---- tests/virnettlshelpers.c | 6 ++---- tests/virnettlshelpers.h | 6 ++---- tests/virnettlssessiontest.c | 10 ++++++---- 4 files changed, 16 insertions(+), 16 deletions(-)
ACK.
I noticed this yesterday and fixed it in a different way, but ended up with one more problem. It was probably the way I fixed it combined with one more filename changed. Anyway, why I'm saying this is that one more filename should be renamed in order to avoid a race (which I was unable to reproduce, though). Consider squashing the following in before pushing. I checked this is the last file those two tests have in common by going through the code and the re-checked by this "script": strace -o session.trace -e open ./virnettlssessiontest strace -o context.trace -e open ./virnettlscontexttest sort \ <(sed -n '/^open/s/open("\([^"]*\)",.*$/\1/p' context.trace | sort -u)\ <(sed -n '/^open/s/open("\([^"]*\)",.*$/\1/p' session.trace | sort -u)\ | uniq -d| grep '.pem$' So it should be enough to make these tests independent of each other. diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c index 53792ee..2c7d400 100644 --- a/tests/virnettlscontexttest.c +++ b/tests/virnettlscontexttest.c @@ -556,12 +556,12 @@ mymain(void) cacertlevel2areq.crt, }; - testTLSWriteCertChain("cacertchain.pem", + testTLSWriteCertChain("cacertchain-ctx.pem", certchain, ARRAY_CARDINALITY(certchain)); - DO_CTX_TEST(true, "cacertchain.pem", servercertlevel3areq.filename, false); - DO_CTX_TEST(false, "cacertchain.pem", clientcertlevel2breq.filename, false); + DO_CTX_TEST(true, "cacertchain-ctx.pem", servercertlevel3areq.filename, false); + DO_CTX_TEST(false, "cacertchain-ctx.pem", clientcertlevel2breq.filename, false); testTLSDiscardCert(&cacertreq); testTLSDiscardCert(&cacert1req); @@ -617,7 +617,7 @@ mymain(void) testTLSDiscardCert(&cacertlevel2areq); testTLSDiscardCert(&servercertlevel3areq); testTLSDiscardCert(&clientcertlevel2breq); - unlink("cacertchain.pem"); + unlink("cacertchain-ctx.pem"); testTLSCleanup(KEYFILE); diff --git a/tests/virnettlssessiontest.c b/tests/virnettlssessiontest.c index 9b171ed..f5f7212 100644 --- a/tests/virnettlssessiontest.c +++ b/tests/virnettlssessiontest.c @@ -451,11 +451,11 @@ mymain(void) cacertlevel2areq.crt, }; - testTLSWriteCertChain("cacertchain.pem", + testTLSWriteCertChain("cacertchain-sess.pem", certchain, ARRAY_CARDINALITY(certchain)); - DO_SESS_TEST("cacertchain.pem", servercertlevel3areq.filename, clientcertlevel2breq.filename, + DO_SESS_TEST("cacertchain-sess.pem", servercertlevel3areq.filename, clientcertlevel2breq.filename, false, false, "libvirt.org", NULL); testTLSDiscardCert(&clientcertreq); @@ -474,7 +474,7 @@ mymain(void) testTLSDiscardCert(&cacertlevel2areq); testTLSDiscardCert(&servercertlevel3areq); testTLSDiscardCert(&clientcertlevel2breq); - unlink("cacertchain.pem"); + unlink("cacertchain-sess.pem"); testTLSCleanup(KEYFILE); --
On Fri, Aug 09, 2013 at 09:53:30AM +0200, Martin Kletzander wrote:
On 08/09/2013 12:29 AM, Eric Blake wrote:
On 08/08/2013 04:09 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Use a seperate keyfile name for the two TLS test suites so that
s/seperate/separate/
they don't clash when running tests in parallel
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- tests/virnettlscontexttest.c | 10 ++++++---- tests/virnettlshelpers.c | 6 ++---- tests/virnettlshelpers.h | 6 ++---- tests/virnettlssessiontest.c | 10 ++++++---- 4 files changed, 16 insertions(+), 16 deletions(-)
ACK.
I noticed this yesterday and fixed it in a different way, but ended up with one more problem. It was probably the way I fixed it combined with one more filename changed.
Anyway, why I'm saying this is that one more filename should be renamed in order to avoid a race (which I was unable to reproduce, though). Consider squashing the following in before pushing.
I checked this is the last file those two tests have in common by going through the code and the re-checked by this "script":
strace -o session.trace -e open ./virnettlssessiontest strace -o context.trace -e open ./virnettlscontexttest sort \ <(sed -n '/^open/s/open("\([^"]*\)",.*$/\1/p' context.trace | sort -u)\ <(sed -n '/^open/s/open("\([^"]*\)",.*$/\1/p' session.trace | sort -u)\ | uniq -d| grep '.pem$'
So it should be enough to make these tests independent of each other.
diff --git a/tests/virnettlscontexttest.c b/tests/virnettlscontexttest.c index 53792ee..2c7d400 100644 --- a/tests/virnettlscontexttest.c +++ b/tests/virnettlscontexttest.c @@ -556,12 +556,12 @@ mymain(void) cacertlevel2areq.crt, };
- testTLSWriteCertChain("cacertchain.pem", + testTLSWriteCertChain("cacertchain-ctx.pem", certchain, ARRAY_CARDINALITY(certchain));
- DO_CTX_TEST(true, "cacertchain.pem", servercertlevel3areq.filename, false); - DO_CTX_TEST(false, "cacertchain.pem", clientcertlevel2breq.filename, false); + DO_CTX_TEST(true, "cacertchain-ctx.pem", servercertlevel3areq.filename, false); + DO_CTX_TEST(false, "cacertchain-ctx.pem", clientcertlevel2breq.filename, false);
testTLSDiscardCert(&cacertreq); testTLSDiscardCert(&cacert1req); @@ -617,7 +617,7 @@ mymain(void) testTLSDiscardCert(&cacertlevel2areq); testTLSDiscardCert(&servercertlevel3areq); testTLSDiscardCert(&clientcertlevel2breq); - unlink("cacertchain.pem"); + unlink("cacertchain-ctx.pem");
testTLSCleanup(KEYFILE);
diff --git a/tests/virnettlssessiontest.c b/tests/virnettlssessiontest.c index 9b171ed..f5f7212 100644 --- a/tests/virnettlssessiontest.c +++ b/tests/virnettlssessiontest.c @@ -451,11 +451,11 @@ mymain(void) cacertlevel2areq.crt, };
- testTLSWriteCertChain("cacertchain.pem", + testTLSWriteCertChain("cacertchain-sess.pem", certchain, ARRAY_CARDINALITY(certchain));
- DO_SESS_TEST("cacertchain.pem", servercertlevel3areq.filename, clientcertlevel2breq.filename, + DO_SESS_TEST("cacertchain-sess.pem", servercertlevel3areq.filename, clientcertlevel2breq.filename, false, false, "libvirt.org", NULL);
testTLSDiscardCert(&clientcertreq); @@ -474,7 +474,7 @@ mymain(void) testTLSDiscardCert(&cacertlevel2areq); testTLSDiscardCert(&servercertlevel3areq); testTLSDiscardCert(&clientcertlevel2breq); - unlink("cacertchain.pem"); + unlink("cacertchain-sess.pem");
testTLSCleanup(KEYFILE);
ACK, yes this is also needed ontop of my patch. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 08/09/2013 03:23 AM, Daniel P. Berrange wrote:
Anyway, why I'm saying this is that one more filename should be renamed in order to avoid a race (which I was unable to reproduce, though).
I have reproduced the race.
ACK, yes this is also needed ontop of my patch.
I went ahead and pushed this in your name, since it didn't make it into Dan's original patch, and since I was hitting it. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/13/2013 04:20 AM, Eric Blake wrote:
On 08/09/2013 03:23 AM, Daniel P. Berrange wrote:
Anyway, why I'm saying this is that one more filename should be renamed in order to avoid a race (which I was unable to reproduce, though).
I have reproduced the race.
ACK, yes this is also needed ontop of my patch.
I went ahead and pushed this in your name, since it didn't make it into Dan's original patch, and since I was hitting it.
Thank you, I wasn't checking whether this got squashed in or not. I'm glad to hear the patch fixed your issue. Martin
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Martin Kletzander