[libvirt] [PATCH 0/4] test: have default URI use file parsing helpers
This series reworks test:///default open handling to use the same XML parsing helpers that a non-default test://$PATH open uses. Saves some code, and makes it easier to extend test:///default, for example in patch #4 which we want for testing nodedev events Cole Robinson (4): test: Introduce testOpenParse test: Move testOpenDefault definition later test: Have test:///default open use file parsing helpers test: Add scsi vport nodedev to test:///default src/test/test_driver.c | 326 +++++++++++++++++++++---------------------------- 1 file changed, 141 insertions(+), 185 deletions(-) -- 2.7.4
--- src/test/test_driver.c | 51 +++++++++++++++++++++++++++++++------------------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index baff085..53901d6 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1296,6 +1296,37 @@ testParseAuthUsers(testDriverPtr privconn, return ret; } +static int +testOpenParse(testDriverPtr privconn, + const char *file, + xmlXPathContextPtr ctxt) +{ + if (!xmlStrEqual(ctxt->node->name, BAD_CAST "node")) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Root element is not 'node'")); + goto error; + } + + if (testParseNodeInfo(&privconn->nodeInfo, ctxt) < 0) + goto error; + if (testParseDomains(privconn, file, ctxt) < 0) + goto error; + if (testParseNetworks(privconn, file, ctxt) < 0) + goto error; + if (testParseInterfaces(privconn, file, ctxt) < 0) + goto error; + if (testParseStorage(privconn, file, ctxt) < 0) + goto error; + if (testParseNodedevs(privconn, file, ctxt) < 0) + goto error; + if (testParseAuthUsers(privconn, ctxt) < 0) + goto error; + + return 0; + error: + return -1; +} + /* No shared state between simultaneous test connections initialized * from a file. */ static int @@ -1317,28 +1348,10 @@ testOpenFromFile(virConnectPtr conn, const char *file) if (!(doc = virXMLParseFileCtxt(file, &ctxt))) goto error; - if (!xmlStrEqual(ctxt->node->name, BAD_CAST "node")) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Root element is not 'node'")); - goto error; - } - privconn->numCells = 0; memmove(&privconn->nodeInfo, &defaultNodeInfo, sizeof(defaultNodeInfo)); - if (testParseNodeInfo(&privconn->nodeInfo, ctxt) < 0) - goto error; - if (testParseDomains(privconn, file, ctxt) < 0) - goto error; - if (testParseNetworks(privconn, file, ctxt) < 0) - goto error; - if (testParseInterfaces(privconn, file, ctxt) < 0) - goto error; - if (testParseStorage(privconn, file, ctxt) < 0) - goto error; - if (testParseNodedevs(privconn, file, ctxt) < 0) - goto error; - if (testParseAuthUsers(privconn, ctxt) < 0) + if (testOpenParse(privconn, file, ctxt) < 0) goto error; xmlXPathFreeContext(ctxt); -- 2.7.4
Upcoming patches need this defined later --- src/test/test_driver.c | 273 ++++++++++++++++++++++++------------------------- 1 file changed, 136 insertions(+), 137 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 53901d6..e0d3e60 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -615,143 +615,6 @@ testDomainStartState(testDriverPtr privconn, } -/* Simultaneous test:///default connections should share the same - * common state (among other things, this allows testing event - * detection in one connection for an action caused in another). */ -static int -testOpenDefault(virConnectPtr conn) -{ - int u; - testDriverPtr privconn = NULL; - virDomainDefPtr domdef = NULL; - virDomainObjPtr domobj = NULL; - virNetworkDefPtr netdef = NULL; - virNetworkObjPtr netobj = NULL; - virInterfaceDefPtr interfacedef = NULL; - virInterfaceObjPtr interfaceobj = NULL; - virStoragePoolDefPtr pooldef = NULL; - virStoragePoolObjPtr poolobj = NULL; - virNodeDeviceDefPtr nodedef = NULL; - virNodeDeviceObjPtr nodeobj = NULL; - - virMutexLock(&defaultLock); - if (defaultConnections++) { - conn->privateData = defaultConn; - virMutexUnlock(&defaultLock); - return VIR_DRV_OPEN_SUCCESS; - } - - if (!(privconn = testDriverNew())) - goto error; - - conn->privateData = privconn; - - memmove(&privconn->nodeInfo, &defaultNodeInfo, sizeof(defaultNodeInfo)); - - /* Numa setup */ - privconn->numCells = 2; - for (u = 0; u < 2; ++u) { - privconn->cells[u].numCpus = 8; - privconn->cells[u].mem = (u + 1) * 2048 * 1024; - } - for (u = 0; u < 16; u++) { - virBitmapPtr siblings = virBitmapNew(16); - if (!siblings) - goto error; - ignore_value(virBitmapSetBit(siblings, u)); - privconn->cells[u / 8].cpus[(u % 8)].id = u; - privconn->cells[u / 8].cpus[(u % 8)].socket_id = u / 8; - privconn->cells[u / 8].cpus[(u % 8)].core_id = u % 8; - privconn->cells[u / 8].cpus[(u % 8)].siblings = siblings; - } - - if (!(privconn->caps = testBuildCapabilities(conn))) - goto error; - - if (!(domdef = virDomainDefParseString(defaultDomainXML, - privconn->caps, - privconn->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) - goto error; - - if (testDomainGenerateIfnames(domdef) < 0) - goto error; - if (!(domobj = virDomainObjListAdd(privconn->domains, - domdef, - privconn->xmlopt, - 0, NULL))) - goto error; - domdef = NULL; - - domobj->persistent = 1; - if (testDomainStartState(privconn, domobj, - VIR_DOMAIN_RUNNING_BOOTED) < 0) { - virObjectUnlock(domobj); - goto error; - } - - virObjectUnlock(domobj); - - if (!(netdef = virNetworkDefParseString(defaultNetworkXML))) - goto error; - if (!(netobj = virNetworkAssignDef(privconn->networks, netdef, 0))) { - virNetworkDefFree(netdef); - goto error; - } - netobj->active = 1; - virNetworkObjEndAPI(&netobj); - - if (!(interfacedef = virInterfaceDefParseString(defaultInterfaceXML))) - goto error; - if (!(interfaceobj = virInterfaceAssignDef(&privconn->ifaces, interfacedef))) { - virInterfaceDefFree(interfacedef); - goto error; - } - interfaceobj->active = 1; - virInterfaceObjUnlock(interfaceobj); - - if (!(pooldef = virStoragePoolDefParseString(defaultPoolXML))) - goto error; - - if (!(poolobj = virStoragePoolObjAssignDef(&privconn->pools, - pooldef))) { - virStoragePoolDefFree(pooldef); - goto error; - } - - if (testStoragePoolObjSetDefaults(poolobj) == -1) { - virStoragePoolObjUnlock(poolobj); - goto error; - } - poolobj->active = 1; - virStoragePoolObjUnlock(poolobj); - - /* Init default node device */ - if (!(nodedef = virNodeDeviceDefParseString(defaultNodeXML, 0, NULL))) - goto error; - if (!(nodeobj = virNodeDeviceAssignDef(&privconn->devs, - nodedef))) { - virNodeDeviceDefFree(nodedef); - goto error; - } - virNodeDeviceObjUnlock(nodeobj); - - defaultConn = privconn; - - virMutexUnlock(&defaultLock); - - return VIR_DRV_OPEN_SUCCESS; - - error: - testDriverFree(privconn); - conn->privateData = NULL; - virDomainDefFree(domdef); - defaultConnections--; - virMutexUnlock(&defaultLock); - return VIR_DRV_OPEN_ERROR; -} - - static char *testBuildFilename(const char *relativeTo, const char *filename) { @@ -1368,6 +1231,142 @@ testOpenFromFile(virConnectPtr conn, const char *file) return VIR_DRV_OPEN_ERROR; } +/* Simultaneous test:///default connections should share the same + * common state (among other things, this allows testing event + * detection in one connection for an action caused in another). */ +static int +testOpenDefault(virConnectPtr conn) +{ + int u; + testDriverPtr privconn = NULL; + virDomainDefPtr domdef = NULL; + virDomainObjPtr domobj = NULL; + virNetworkDefPtr netdef = NULL; + virNetworkObjPtr netobj = NULL; + virInterfaceDefPtr interfacedef = NULL; + virInterfaceObjPtr interfaceobj = NULL; + virStoragePoolDefPtr pooldef = NULL; + virStoragePoolObjPtr poolobj = NULL; + virNodeDeviceDefPtr nodedef = NULL; + virNodeDeviceObjPtr nodeobj = NULL; + + virMutexLock(&defaultLock); + if (defaultConnections++) { + conn->privateData = defaultConn; + virMutexUnlock(&defaultLock); + return VIR_DRV_OPEN_SUCCESS; + } + + if (!(privconn = testDriverNew())) + goto error; + + conn->privateData = privconn; + + memmove(&privconn->nodeInfo, &defaultNodeInfo, sizeof(defaultNodeInfo)); + + /* Numa setup */ + privconn->numCells = 2; + for (u = 0; u < 2; ++u) { + privconn->cells[u].numCpus = 8; + privconn->cells[u].mem = (u + 1) * 2048 * 1024; + } + for (u = 0; u < 16; u++) { + virBitmapPtr siblings = virBitmapNew(16); + if (!siblings) + goto error; + ignore_value(virBitmapSetBit(siblings, u)); + privconn->cells[u / 8].cpus[(u % 8)].id = u; + privconn->cells[u / 8].cpus[(u % 8)].socket_id = u / 8; + privconn->cells[u / 8].cpus[(u % 8)].core_id = u % 8; + privconn->cells[u / 8].cpus[(u % 8)].siblings = siblings; + } + + if (!(privconn->caps = testBuildCapabilities(conn))) + goto error; + + if (!(domdef = virDomainDefParseString(defaultDomainXML, + privconn->caps, + privconn->xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE))) + goto error; + + if (testDomainGenerateIfnames(domdef) < 0) + goto error; + if (!(domobj = virDomainObjListAdd(privconn->domains, + domdef, + privconn->xmlopt, + 0, NULL))) + goto error; + domdef = NULL; + + domobj->persistent = 1; + if (testDomainStartState(privconn, domobj, + VIR_DOMAIN_RUNNING_BOOTED) < 0) { + virObjectUnlock(domobj); + goto error; + } + + virObjectUnlock(domobj); + + if (!(netdef = virNetworkDefParseString(defaultNetworkXML))) + goto error; + if (!(netobj = virNetworkAssignDef(privconn->networks, netdef, 0))) { + virNetworkDefFree(netdef); + goto error; + } + netobj->active = 1; + virNetworkObjEndAPI(&netobj); + + if (!(interfacedef = virInterfaceDefParseString(defaultInterfaceXML))) + goto error; + if (!(interfaceobj = virInterfaceAssignDef(&privconn->ifaces, interfacedef))) { + virInterfaceDefFree(interfacedef); + goto error; + } + interfaceobj->active = 1; + virInterfaceObjUnlock(interfaceobj); + + if (!(pooldef = virStoragePoolDefParseString(defaultPoolXML))) + goto error; + + if (!(poolobj = virStoragePoolObjAssignDef(&privconn->pools, + pooldef))) { + virStoragePoolDefFree(pooldef); + goto error; + } + + if (testStoragePoolObjSetDefaults(poolobj) == -1) { + virStoragePoolObjUnlock(poolobj); + goto error; + } + poolobj->active = 1; + virStoragePoolObjUnlock(poolobj); + + /* Init default node device */ + if (!(nodedef = virNodeDeviceDefParseString(defaultNodeXML, 0, NULL))) + goto error; + if (!(nodeobj = virNodeDeviceAssignDef(&privconn->devs, + nodedef))) { + virNodeDeviceDefFree(nodedef); + goto error; + } + virNodeDeviceObjUnlock(nodeobj); + + defaultConn = privconn; + + virMutexUnlock(&defaultLock); + + return VIR_DRV_OPEN_SUCCESS; + + error: + testDriverFree(privconn); + conn->privateData = NULL; + virDomainDefFree(domdef); + defaultConnections--; + virMutexUnlock(&defaultLock); + return VIR_DRV_OPEN_ERROR; +} + static int testConnectAuthenticate(virConnectPtr conn, virConnectAuthPtr auth) -- 2.7.4
Convert the individual XML documents into one big XML document in the format expected by the non-default test://$PATH URI, and use the same internal helpers for assembling the driver contents. --- src/test/test_driver.c | 142 +++++++++++++------------------------------------ 1 file changed, 37 insertions(+), 105 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index e0d3e60..3f3e405 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -407,7 +407,8 @@ testDriverNew(void) } -static const char *defaultDomainXML = +static const char *defaultConnXML = +"<node>" "<domain type='test'>" " <name>test</name>" " <uuid>6695eb01-f6a4-8304-79aa-97f2502e193f</uuid>" @@ -417,10 +418,8 @@ static const char *defaultDomainXML = " <os>" " <type>hvm</type>" " </os>" -"</domain>"; - - -static const char *defaultNetworkXML = +"</domain>" +"" "<network>" " <name>default</name>" " <uuid>dd8fe884-6c02-601e-7551-cca97df1c5df</uuid>" @@ -431,9 +430,8 @@ static const char *defaultNetworkXML = " <range start='192.168.122.2' end='192.168.122.254'/>" " </dhcp>" " </ip>" -"</network>"; - -static const char *defaultInterfaceXML = +"</network>" +"" "<interface type=\"ethernet\" name=\"eth1\">" " <start mode=\"onboot\"/>" " <mac address=\"aa:bb:cc:dd:ee:ff\"/>" @@ -442,16 +440,34 @@ static const char *defaultInterfaceXML = " <ip address=\"192.168.0.5\" prefix=\"24\"/>" " <route gateway=\"192.168.0.1\"/>" " </protocol>" -"</interface>"; - -static const char *defaultPoolXML = +"</interface>" +"" "<pool type='dir'>" " <name>default-pool</name>" " <uuid>dfe224cb-28fb-8dd0-c4b2-64eb3f0f4566</uuid>" " <target>" " <path>/default-pool</path>" " </target>" -"</pool>"; +"</pool>" +"" +"<device>" +" <name>computer</name>" +" <capability type='system'>" +" <hardware>" +" <vendor>Libvirt</vendor>" +" <version>Test driver</version>" +" <serial>123456</serial>" +" <uuid>11111111-2222-3333-4444-555555555555</uuid>" +" </hardware>" +" <firmware>" +" <vendor>Libvirt</vendor>" +" <version>Test Driver</version>" +" <release_date>01/22/2007</release_date>" +" </firmware>" +" </capability>" +"</device>" +"</node>"; + static const char *defaultPoolSourcesLogicalXML = "<sources>\n" @@ -476,24 +492,6 @@ static const char *defaultPoolSourcesNetFSXML = " </source>\n" "</sources>\n"; -static const char *defaultNodeXML = -"<device>" -" <name>computer</name>" -" <capability type='system'>" -" <hardware>" -" <vendor>Libvirt</vendor>" -" <version>Test driver</version>" -" <serial>123456</serial>" -" <uuid>11111111-2222-3333-4444-555555555555</uuid>" -" </hardware>" -" <firmware>" -" <vendor>Libvirt</vendor>" -" <version>Test Driver</version>" -" <release_date>01/22/2007</release_date>" -" </firmware>" -" </capability>" -"</device>"; - static const unsigned long long defaultPoolCap = (100 * 1024 * 1024 * 1024ull); static const unsigned long long defaultPoolAlloc; @@ -1239,16 +1237,8 @@ testOpenDefault(virConnectPtr conn) { int u; testDriverPtr privconn = NULL; - virDomainDefPtr domdef = NULL; - virDomainObjPtr domobj = NULL; - virNetworkDefPtr netdef = NULL; - virNetworkObjPtr netobj = NULL; - virInterfaceDefPtr interfacedef = NULL; - virInterfaceObjPtr interfaceobj = NULL; - virStoragePoolDefPtr pooldef = NULL; - virStoragePoolObjPtr poolobj = NULL; - virNodeDeviceDefPtr nodedef = NULL; - virNodeDeviceObjPtr nodeobj = NULL; + xmlDocPtr doc = NULL; + xmlXPathContextPtr ctxt = NULL; virMutexLock(&defaultLock); if (defaultConnections++) { @@ -1284,84 +1274,26 @@ testOpenDefault(virConnectPtr conn) if (!(privconn->caps = testBuildCapabilities(conn))) goto error; - if (!(domdef = virDomainDefParseString(defaultDomainXML, - privconn->caps, - privconn->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + if (!(doc = virXMLParseStringCtxt(defaultConnXML, + _("(test driver)"), &ctxt))) goto error; - if (testDomainGenerateIfnames(domdef) < 0) - goto error; - if (!(domobj = virDomainObjListAdd(privconn->domains, - domdef, - privconn->xmlopt, - 0, NULL))) + if (testOpenParse(privconn, NULL, ctxt) < 0) goto error; - domdef = NULL; - - domobj->persistent = 1; - if (testDomainStartState(privconn, domobj, - VIR_DOMAIN_RUNNING_BOOTED) < 0) { - virObjectUnlock(domobj); - goto error; - } - - virObjectUnlock(domobj); - - if (!(netdef = virNetworkDefParseString(defaultNetworkXML))) - goto error; - if (!(netobj = virNetworkAssignDef(privconn->networks, netdef, 0))) { - virNetworkDefFree(netdef); - goto error; - } - netobj->active = 1; - virNetworkObjEndAPI(&netobj); - - if (!(interfacedef = virInterfaceDefParseString(defaultInterfaceXML))) - goto error; - if (!(interfaceobj = virInterfaceAssignDef(&privconn->ifaces, interfacedef))) { - virInterfaceDefFree(interfacedef); - goto error; - } - interfaceobj->active = 1; - virInterfaceObjUnlock(interfaceobj); - - if (!(pooldef = virStoragePoolDefParseString(defaultPoolXML))) - goto error; - - if (!(poolobj = virStoragePoolObjAssignDef(&privconn->pools, - pooldef))) { - virStoragePoolDefFree(pooldef); - goto error; - } - - if (testStoragePoolObjSetDefaults(poolobj) == -1) { - virStoragePoolObjUnlock(poolobj); - goto error; - } - poolobj->active = 1; - virStoragePoolObjUnlock(poolobj); - - /* Init default node device */ - if (!(nodedef = virNodeDeviceDefParseString(defaultNodeXML, 0, NULL))) - goto error; - if (!(nodeobj = virNodeDeviceAssignDef(&privconn->devs, - nodedef))) { - virNodeDeviceDefFree(nodedef); - goto error; - } - virNodeDeviceObjUnlock(nodeobj); defaultConn = privconn; + xmlXPathFreeContext(ctxt); + xmlFreeDoc(doc); virMutexUnlock(&defaultLock); return VIR_DRV_OPEN_SUCCESS; error: testDriverFree(privconn); + xmlXPathFreeContext(ctxt); + xmlFreeDoc(doc); conn->privateData = NULL; - virDomainDefFree(domdef); defaultConnections--; virMutexUnlock(&defaultLock); return VIR_DRV_OPEN_ERROR; -- 2.7.4
A nodedev device definition like this is required for testing NodeDeviceCreateXML and NodeDeviceDestroy. So unless it's part of the stock test:///default set there's no way to actually invoke those functions for the default URI --- src/test/test_driver.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 3f3e405..4f94317 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -466,6 +466,18 @@ static const char *defaultConnXML = " </firmware>" " </capability>" "</device>" +"<device>" +" <name>test-scsi-host-vport</name>" +" <parent>computer</parent>" +" <capability type='scsi_host'>" +" <host>1</host>" +" <capability type='fc_host'>" +" <wwnn>2000000012341234</wwnn>" +" <wwpn>1000000012341234</wwpn>" +" </capability>" +" <capability type='vport_ops'/>" +" </capability>" +"</device>" "</node>"; -- 2.7.4
On 07/01/2016 07:37 AM, Cole Robinson wrote:
This series reworks test:///default open handling to use the same XML parsing helpers that a non-default test://$PATH open uses. Saves some code, and makes it easier to extend test:///default, for example in patch #4 which we want for testing nodedev events
Cole Robinson (4): test: Introduce testOpenParse test: Move testOpenDefault definition later test: Have test:///default open use file parsing helpers test: Add scsi vport nodedev to test:///default
src/test/test_driver.c | 326 +++++++++++++++++++++---------------------------- 1 file changed, 141 insertions(+), 185 deletions(-)
Nice... Something for a "todo" list - adding snapshots for domains (to test testParseDomainSnapshots) and volumes for pools (to test testOpenVolumesForPool) for defaultConnXML. w/r/t: 4/4... If 'wwnn' and 'wwpn' weren't supplied, then virRandomGenerateWWN should have done the trick; however, testParseNodedevs passes a NULL virt_type so the generation fails. Part of me wonders if should we create a TEST_DRIVER_OUI using "200000" as the prefix just to "prove" that the code path works. Your call though - easy enough to mock up a virRandomGenerateWWN or modify the non mocked version to support the test driver. ACK series regardless of how you handle 4/4. John
On 07/12/2016 10:45 AM, John Ferlan wrote:
On 07/01/2016 07:37 AM, Cole Robinson wrote:
This series reworks test:///default open handling to use the same XML parsing helpers that a non-default test://$PATH open uses. Saves some code, and makes it easier to extend test:///default, for example in patch #4 which we want for testing nodedev events
Cole Robinson (4): test: Introduce testOpenParse test: Move testOpenDefault definition later test: Have test:///default open use file parsing helpers test: Add scsi vport nodedev to test:///default
src/test/test_driver.c | 326 +++++++++++++++++++++---------------------------- 1 file changed, 141 insertions(+), 185 deletions(-)
Nice...
Something for a "todo" list - adding snapshots for domains (to test testParseDomainSnapshots) and volumes for pools (to test testOpenVolumesForPool) for defaultConnXML.
w/r/t: 4/4... If 'wwnn' and 'wwpn' weren't supplied, then virRandomGenerateWWN should have done the trick; however, testParseNodedevs passes a NULL virt_type so the generation fails. Part of me wonders if should we create a TEST_DRIVER_OUI using "200000" as the prefix just to "prove" that the code path works. Your call though - easy enough to mock up a virRandomGenerateWWN or modify the non mocked version to support the test driver.
ACK series regardless of how you handle 4/4.
Thanks for the reviews. I'd pushed the patches as posted. I agree getting some extra coverage for the WWN generation would be a nice addition, but this is blocking some of the incoming GSOC work and I'm short on time at the moment... Thanks, Cole
participants (2)
-
Cole Robinson -
John Ferlan