[libvirt] PATCH: 0/2: port Test driver to new XML apis

The next two patches port the test driver over to use the new XML APIs for dealing with domain and network XML. More details with each one... Daniel. -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

This patch ports the Test driver to use the network XML apis. Basically the 'struct _testNet' is removed, and replaced by usage of the generic virNetworkObjPtr and virNetworkDefPtr objects. The XML parser and formatters are ripped out and replaced by calls to the generic APIs in network_conf.h. Finally there is just alot of cleanup of all the individual network driver implementations to adjust to the style of the new APIs used. test.c | 477 ++++++++++++++++------------------------------------------------- 1 file changed, 121 insertions(+), 356 deletions(-) Regards, Daniel diff -r 77e5fcbd24b7 src/test.c --- a/src/test.c Sun Jun 22 17:55:31 2008 -0400 +++ b/src/test.c Sun Jun 22 17:56:04 2008 -0400 @@ -46,6 +46,7 @@ #include "uuid.h" #include "capabilities.h" #include "memory.h" +#include "network_conf.h" /* Flags that determine the action to take on a shutdown or crash of a domain */ @@ -85,26 +86,6 @@ typedef struct _testDom testDom; typedef struct _testDom *testDomPtr; -struct _testNet { - int active; - int config; - int running; - char name[20]; - char bridge[20]; - unsigned char uuid[VIR_UUID_BUFLEN]; - int forward; - char forwardDev[IF_NAMESIZE]; - char ipAddress[INET_ADDRSTRLEN]; - char ipNetmask[INET_ADDRSTRLEN]; - - char dhcpStart[INET_ADDRSTRLEN]; - char dhcpEnd[INET_ADDRSTRLEN]; - - int autostart; -}; -typedef struct _testNet testNet; -typedef struct _testNet *testNetPtr; - #define MAX_DOMAINS 20 #define MAX_NETWORKS 20 #define MAX_CPUS 128 @@ -125,8 +106,7 @@ virNodeInfo nodeInfo; int numDomains; testDom domains[MAX_DOMAINS]; - int numNetworks; - testNet networks[MAX_NETWORKS]; + virNetworkObjPtr networks; int numCells; testCell cells[MAX_CELLS]; }; @@ -161,17 +141,18 @@ privdom = &privconn->domains[domidx]; #define GET_NETWORK(net, ret) \ - int netidx; \ testConnPtr privconn; \ - testNetPtr privnet; \ + virNetworkObjPtr privnet; \ \ privconn = (testConnPtr)net->conn->privateData; \ - if ((netidx = getNetworkIndex(net)) < 0) { \ - testError((net)->conn, NULL, (net), VIR_ERR_INVALID_ARG, \ - __FUNCTION__); \ - return (ret); \ - } \ - privnet = &privconn->networks[netidx]; + do { \ + if ((privnet = virNetworkFindByName(privconn->networks, \ + (net)->name)) == NULL) { \ + testError((net)->conn, NULL, (net), VIR_ERR_INVALID_ARG, \ + __FUNCTION__); \ + return (ret); \ + } \ + } while (0) #define GET_CONNECTION(conn, ret) \ testConnPtr privconn; \ @@ -423,191 +404,54 @@ } -static int testLoadNetwork(virConnectPtr conn, - xmlDocPtr xml) { - xmlNodePtr root = NULL; - xmlXPathContextPtr ctxt = NULL; - char *name = NULL, *bridge = NULL; - unsigned char uuid[VIR_UUID_BUFLEN]; - char *str; - char *ipaddress = NULL, *ipnetmask = NULL, *dhcpstart = NULL, *dhcpend = NULL; - int forward; - char *forwardDev = NULL; - int handle = -1, i; +#define MAX_NETWORK_XML_LEN 16000 + +static virNetworkObjPtr +testLoadNetworkFromFile(virConnectPtr conn, + const char *filename) { + char *data; + virNetworkDefPtr def; GET_CONNECTION(conn, -1); - root = xmlDocGetRootElement(xml); - if ((root == NULL) || (!xmlStrEqual(root->name, BAD_CAST "network"))) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("network")); - goto error; + if (virFileReadAll(filename, MAX_NETWORK_XML_LEN, &data) < 0) { + testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("load network definition file")); + return NULL; } - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("creating xpath context")); - goto error; + if ((def = virNetworkDefParse(conn, data, filename)) == NULL) { + VIR_FREE(data); + return NULL; } + VIR_FREE(data); - name = virXPathString("string(/network/name[1])", ctxt); - if (name == NULL) { - testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("network name")); - goto error; - } - - bridge = virXPathString("string(/network/bridge[1]/@name)", ctxt); - - str = virXPathString("string(/network/uuid[1])", ctxt); - if (str == NULL) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("network uuid")); - goto error; - } - if (virUUIDParse(str, uuid) < 0) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("network uuid")); - goto error; - } - VIR_FREE(str); - - - forward = virXPathBoolean("count(/network/forward) != 0", ctxt); - if (forward < 0) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("network forward")); - goto error; - } - - forwardDev = virXPathString("string(/network/forward/@dev)", ctxt); - - - ipaddress = virXPathString("string(/network/ip/@address)", ctxt); - if (ipaddress == NULL) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("ip address")); - goto error; - } - ipnetmask = virXPathString("string(/network/ip/@netmask)", ctxt); - if (ipnetmask == NULL) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("ip netmask")); - goto error; - } - dhcpstart = virXPathString("string(/network/ip/dhcp/range[1]/@start)", ctxt); - if (dhcpstart == NULL) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("ip address")); - goto error; - } - dhcpend = virXPathString("string(/network/ip/dhcp/range[1]/@end)", ctxt); - if (dhcpend == NULL) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("ip address")); - goto error; - } - - for (i = 0 ; i < MAX_NETWORKS ; i++) { - if (!privconn->networks[i].active) { - handle = i; - break; - } - } - if (handle < 0) - return (-1); - - privconn->networks[handle].active = 1; - privconn->networks[handle].running = 1; - strncpy(privconn->networks[handle].name, name, sizeof(privconn->networks[handle].name)-1); - privconn->networks[handle].name[sizeof(privconn->networks[handle].name)-1] = '\0'; - strncpy(privconn->networks[handle].bridge, bridge ? bridge : name, sizeof(privconn->networks[handle].bridge)-1); - privconn->networks[handle].bridge[sizeof(privconn->networks[handle].bridge)-1] = '\0'; - VIR_FREE(name); - name = NULL; - if (bridge) { - VIR_FREE(bridge); - bridge = NULL; - } - - memmove(privconn->networks[handle].uuid, uuid, VIR_UUID_BUFLEN); - privconn->networks[handle].forward = forward; - if (forwardDev) { - strncpy(privconn->networks[handle].forwardDev, forwardDev, sizeof(privconn->networks[handle].forwardDev)-1); - privconn->networks[handle].forwardDev[sizeof(privconn->networks[handle].forwardDev)-1] = '\0'; - VIR_FREE(forwardDev); - } - - strncpy(privconn->networks[handle].ipAddress, ipaddress, sizeof(privconn->networks[handle].ipAddress)-1); - privconn->networks[handle].ipAddress[sizeof(privconn->networks[handle].ipAddress)-1] = '\0'; - VIR_FREE(ipaddress); - strncpy(privconn->networks[handle].ipNetmask, ipnetmask, sizeof(privconn->networks[handle].ipNetmask)-1); - privconn->networks[handle].ipNetmask[sizeof(privconn->networks[handle].ipNetmask)-1] = '\0'; - VIR_FREE(ipnetmask); - strncpy(privconn->networks[handle].dhcpStart, dhcpstart, sizeof(privconn->networks[handle].dhcpStart)-1); - privconn->networks[handle].dhcpStart[sizeof(privconn->networks[handle].dhcpStart)-1] = '\0'; - VIR_FREE(dhcpstart); - strncpy(privconn->networks[handle].dhcpEnd, dhcpend, sizeof(privconn->networks[handle].dhcpEnd)-1); - privconn->networks[handle].dhcpEnd[sizeof(privconn->networks[handle].dhcpEnd)-1] = '\0'; - VIR_FREE(dhcpend); - xmlXPathFreeContext(ctxt); - return (handle); - - error: - xmlXPathFreeContext(ctxt); - VIR_FREE (forwardDev); - VIR_FREE(ipaddress); - VIR_FREE(ipnetmask); - VIR_FREE(dhcpstart); - VIR_FREE(dhcpend); - VIR_FREE(name); - return (-1); + return virNetworkAssignDef(conn, &privconn->networks, + def); } -static int testLoadNetworkFromDoc(virConnectPtr conn, - const char *doc) { - int ret; - xmlDocPtr xml; - if (!(xml = xmlReadDoc(BAD_CAST doc, "network.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("network")); - return (-1); - } - - ret = testLoadNetwork(conn, xml); - - xmlFreeDoc(xml); - - return (ret); -} - - -static int testLoadNetworkFromFile(virConnectPtr conn, - const char *filename) { - int ret, fd; - xmlDocPtr xml; - - if ((fd = open(filename, O_RDONLY)) < 0) { - testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("load network definition file")); - return (-1); - } - - if (!(xml = xmlReadFd(fd, filename, NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("network")); - return (-1); - } - - ret = testLoadNetwork(conn, xml); - - xmlFreeDoc(xml); - close(fd); - - return (ret); -} +static const char *defaultNetworkXML = +"<network>" +" <name>default</name>" +" <bridge name='virbr0' />" +" <forward/>" +" <ip address='192.168.122.1' netmask='255.255.255.0'>" +" <dhcp>" +" <range start='192.168.122.2' end='192.168.122.254' />" +" </dhcp>" +" </ip>" +"</network>"; static int testOpenDefault(virConnectPtr conn) { int u; struct timeval tv; testConnPtr privconn; + virNetworkDefPtr netdef; + virNetworkObjPtr netobj; + if (VIR_ALLOC(privconn) < 0) { testError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, "testConn"); return VIR_DRV_OPEN_ERROR; } - memset(privconn, 0, sizeof(testConn)); if (gettimeofday(&tv, NULL) < 0) { testError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("getting time of day")); @@ -636,21 +480,15 @@ privconn->domains[0].info.cpuTime = ((tv.tv_sec * 1000ll * 1000ll * 1000ll) + (tv.tv_usec * 1000ll)); - privconn->numNetworks = 1; - privconn->networks[0].active = 1; - privconn->networks[0].config = 1; - privconn->networks[0].running = 1; - strcpy(privconn->networks[0].name, "default"); - strcpy(privconn->networks[0].bridge, "default"); - for (u = 0 ; u < VIR_UUID_BUFLEN ; u++) { - privconn->networks[0].uuid[u] = (u * 75)%255; + if (!(netdef = virNetworkDefParse(conn, defaultNetworkXML, "default.xml"))) { + return VIR_DRV_OPEN_ERROR; } - privconn->networks[0].forward = 1; - strcpy(privconn->networks[0].forwardDev, "eth0"); - strcpy(privconn->networks[0].ipAddress, "192.168.122.1"); - strcpy(privconn->networks[0].ipNetmask, "255.255.255.0"); - strcpy(privconn->networks[0].dhcpStart, "192.168.122.128"); - strcpy(privconn->networks[0].dhcpEnd, "192.168.122.253"); + if (!(netobj = virNetworkAssignDef(conn, &privconn->networks, netdef))) { + virNetworkDefFree(netdef); + return VIR_DRV_OPEN_ERROR; + } + netobj->active = 1; + netobj->persistent = 1; // Numa setup privconn->numCells = 2; @@ -736,7 +574,6 @@ conn->privateData = privconn; privconn->nextDomID = 1; privconn->numDomains = 0; - privconn->numNetworks = 0; privconn->numCells = 0; strncpy(privconn->path, file, PATH_MAX-1); privconn->path[PATH_MAX-1] = '\0'; @@ -840,21 +677,20 @@ ret = virXPathNodeSet("/node/network", ctxt, &networks); if (ret > 0) { for (i = 0 ; i < ret ; i++) { + virNetworkObjPtr net; xmlChar *netFile = xmlGetProp(networks[i], BAD_CAST "file"); char *absFile = testBuildFilename(file, (const char *)netFile); - int handle; VIR_FREE(netFile); if (!absFile) { testError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("resolving network filename")); goto error; } - if ((handle = testLoadNetworkFromFile(conn, absFile)) < 0) { + if ((net = testLoadNetworkFromFile(conn, absFile)) == NULL) { VIR_FREE(absFile); goto error; } - privconn->networks[handle].config = 1; - VIR_FREE(absFile); - privconn->numNetworks++; + net->persistent = 1; + net->configFile = absFile; } if (networks != NULL) { VIR_FREE(networks); @@ -892,17 +728,6 @@ if (STREQ(domain->name, privconn->domains[i].name)) return (i); } - } - return (-1); -} - -static int getNetworkIndex(virNetworkPtr network) { - int i; - GET_CONNECTION(network->conn, -1); - - for (i = 0 ; i < MAX_NETWORKS ; i++) { - if (STREQ(network->name, privconn->networks[i].name)) - return (i); } return (-1); } @@ -1742,160 +1567,131 @@ static virNetworkPtr testLookupNetworkByUUID(virConnectPtr conn, const unsigned char *uuid) { - int i, idx = -1; + virNetworkObjPtr net = NULL; GET_CONNECTION(conn, NULL); - for (i = 0 ; i < MAX_NETWORKS ; i++) { - if (privconn->networks[i].active && - memcmp(uuid, privconn->networks[i].uuid, VIR_UUID_BUFLEN) == 0) { - idx = i; - break; - } - } - - if (idx < 0) { + if ((net = virNetworkFindByUUID(privconn->networks, uuid)) == NULL) { testError (conn, NULL, NULL, VIR_ERR_NO_NETWORK, NULL); return NULL; } - return virGetNetwork(conn, privconn->networks[idx].name, privconn->networks[idx].uuid); + return virGetNetwork(conn, net->def->name, net->def->uuid); } static virNetworkPtr testLookupNetworkByName(virConnectPtr conn, - const char *name) + const char *name) { - int i, idx = -1; + virNetworkObjPtr net = NULL; GET_CONNECTION(conn, NULL); - for (i = 0 ; i < MAX_NETWORKS ; i++) { - if (privconn->networks[i].active && - STREQ(name, privconn->networks[i].name)) { - idx = i; - break; - } - } - - if (idx < 0) { + if ((net = virNetworkFindByName(privconn->networks, name)) == NULL) { testError (conn, NULL, NULL, VIR_ERR_NO_NETWORK, NULL); return NULL; } - return virGetNetwork(conn, privconn->networks[idx].name, privconn->networks[idx].uuid); + return virGetNetwork(conn, net->def->name, net->def->uuid); } static int testNumNetworks(virConnectPtr conn) { - int numInactive = 0, i; + int numActive = 0; + virNetworkObjPtr net; GET_CONNECTION(conn, -1); - for (i = 0 ; i < MAX_NETWORKS ; i++) { - if (!privconn->networks[i].active || - !privconn->networks[i].running) - continue; - numInactive++; + net = privconn->networks; + while (net) { + if (virNetworkIsActive(net)) + numActive++; + net = net->next; } - return (numInactive); + return numActive; } static int testListNetworks(virConnectPtr conn, char **const names, int nnames) { - int n = 0, i; + int n = 0; + virNetworkObjPtr net; GET_CONNECTION(conn, -1); - for (i = 0, n = 0 ; i < MAX_NETWORKS && n < nnames ; i++) { - if (privconn->networks[i].active && - privconn->networks[i].running) { - names[n++] = strdup(privconn->networks[i].name); - } + net = privconn->networks; + while (net && n < nnames) { + if (virNetworkIsActive(net)) + names[n++] = strdup(net->def->name); + net = net->next; } - return (n); + return n; } static int testNumDefinedNetworks(virConnectPtr conn) { - int numInactive = 0, i; + int numInactive = 0; + virNetworkObjPtr net; GET_CONNECTION(conn, -1); - for (i = 0 ; i < MAX_NETWORKS ; i++) { - if (!privconn->networks[i].active || - privconn->networks[i].running) - continue; - numInactive++; + net = privconn->networks; + while (net) { + if (!virNetworkIsActive(net)) + numInactive++; + net = net->next; } - return (numInactive); + return numInactive; } static int testListDefinedNetworks(virConnectPtr conn, char **const names, int nnames) { - int n = 0, i; + int n = 0; + virNetworkObjPtr net; GET_CONNECTION(conn, -1); - for (i = 0, n = 0 ; i < MAX_NETWORKS && n < nnames ; i++) { - if (privconn->networks[i].active && - !privconn->networks[i].running) { - names[n++] = strdup(privconn->networks[i].name); - } + net = privconn->networks; + while (net && n < nnames) { + if (!virNetworkIsActive(net)) + names[n++] = strdup(net->def->name); + net = net->next; } - return (n); + return n; } static virNetworkPtr testNetworkCreate(virConnectPtr conn, const char *xml) { - int handle = -1; - virNetworkPtr net; + virNetworkDefPtr def; + virNetworkObjPtr net; GET_CONNECTION(conn, NULL); - if (xml == NULL) { - testError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); - return (NULL); - } + if ((def = virNetworkDefParse(conn, xml, NULL)) == NULL) + return NULL; - if (privconn->numNetworks == MAX_NETWORKS) { - testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("too many networks")); - return (NULL); - } + if ((net = virNetworkAssignDef(conn, &privconn->networks, + def)) == NULL) + return NULL; + net->active = 1; - if ((handle = testLoadNetworkFromDoc(conn, xml)) < 0) - return (NULL); - privconn->networks[handle].config = 0; - - net = virGetNetwork(conn, privconn->networks[handle].name, privconn->networks[handle].uuid); - if (net == NULL) return NULL; - privconn->numNetworks++; - return (net); + return virGetNetwork(conn, def->name, def->uuid); } static virNetworkPtr testNetworkDefine(virConnectPtr conn, const char *xml) { - int handle = -1; - virNetworkPtr net; + virNetworkDefPtr def; + virNetworkObjPtr net; GET_CONNECTION(conn, NULL); - if (xml == NULL) { - testError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); - return (NULL); - } + if ((def = virNetworkDefParse(conn, xml, NULL)) == NULL) + return NULL; - if (privconn->numNetworks == MAX_NETWORKS) { - testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("too many networks")); - return (NULL); - } + if ((net = virNetworkAssignDef(conn, &privconn->networks, + def)) == NULL) + return NULL; + net->persistent = 1; - if ((handle = testLoadNetworkFromDoc(conn, xml)) < 0) - return (NULL); - - net = virGetNetwork(conn, privconn->networks[handle].name, privconn->networks[handle].uuid); - privconn->networks[handle].config = 1; - if (net == NULL) return NULL; - privconn->numNetworks++; - return (net); + return virGetNetwork(conn, def->name, def->uuid); } static int testNetworkUndefine(virNetworkPtr network) { GET_NETWORK(network, -1); - if (privnet->running) { + if (virNetworkIsActive(privnet)) { testError(network->conn, NULL, network, VIR_ERR_INTERNAL_ERROR, _("Network is still running")); return (-1); } - privnet->active = 0; + virNetworkRemoveInactive(&privconn->networks, + privnet); return (0); } @@ -1903,13 +1699,13 @@ static int testNetworkStart(virNetworkPtr network) { GET_NETWORK(network, -1); - if (privnet->running) { + if (virNetworkIsActive(privnet)) { testError(network->conn, NULL, network, VIR_ERR_INTERNAL_ERROR, _("Network is already running")); return (-1); } - privnet->running = 1; + privnet->active = 1; return (0); } @@ -1917,55 +1713,24 @@ static int testNetworkDestroy(virNetworkPtr network) { GET_NETWORK(network, -1); - if (privnet->config) { - privnet->running = 0; - } else { - privnet->active = 0; + privnet->active = 0; + if (!privnet->persistent) { + virNetworkRemoveInactive(&privconn->networks, + privnet); } return (0); } static char *testNetworkDumpXML(virNetworkPtr network, int flags ATTRIBUTE_UNUSED) { - virBuffer buf = VIR_BUFFER_INITIALIZER; - unsigned char *uuid; - char uuidstr[VIR_UUID_STRING_BUFLEN]; GET_NETWORK(network, NULL); - virBufferAddLit(&buf, "<network>\n"); - virBufferVSprintf(&buf, " <name>%s</name>\n", network->name); - uuid = network->uuid; - virUUIDFormat(uuid, uuidstr); - virBufferVSprintf(&buf, " <uuid>%s</uuid>\n", uuidstr); - virBufferVSprintf(&buf, " <bridge name='%s'/>\n", privnet->bridge); - if (privnet->forward) { - if (privnet->forwardDev[0]) - virBufferVSprintf(&buf, " <forward dev='%s'/>\n", privnet->forwardDev); - else - virBufferAddLit(&buf, " <forward/>\n"); - } - - virBufferVSprintf(&buf, " <ip address='%s' netmask='%s'>\n", - privnet->ipAddress, privnet->ipNetmask); - virBufferAddLit(&buf, " <dhcp>\n"); - virBufferVSprintf(&buf, " <range start='%s' end='%s'/>\n", - privnet->dhcpStart, privnet->dhcpEnd); - virBufferAddLit(&buf, " </dhcp>\n"); - virBufferAddLit(&buf, " </ip>\n"); - - virBufferAddLit(&buf, "</network>\n"); - - if (virBufferError(&buf)) { - testError(network->conn, NULL, network, VIR_ERR_NO_MEMORY, __FUNCTION__); - return NULL; - } - - return virBufferContentAndReset(&buf); + return virNetworkDefFormat(network->conn, privnet->def); } static char *testNetworkGetBridgeName(virNetworkPtr network) { char *bridge; GET_NETWORK(network, NULL); - bridge = strdup(privnet->bridge); + bridge = privnet->def->bridge ? strdup(privnet->def->bridge) : NULL; if (!bridge) { testError(network->conn, NULL, network, VIR_ERR_NO_MEMORY, "network"); return NULL; -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Tue, Jun 24, 2008 at 04:59:18PM +0100, Daniel P. Berrange wrote:
This patch ports the Test driver to use the network XML apis.
Basically the 'struct _testNet' is removed, and replaced by usage of the generic virNetworkObjPtr and virNetworkDefPtr objects. The XML parser and formatters are ripped out and replaced by calls to the generic APIs in network_conf.h. Finally there is just alot of cleanup of all the individual network driver implementations to adjust to the style of the new APIs used.
Specific code shrinking, good :-) Looks fine to me, having the default network configuration isolated as an XML made it clearer to me, even better, +1 Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

Hi Dan, Nice work! "Daniel P. Berrange" <berrange@redhat.com> wrote:
diff -r 77e5fcbd24b7 src/test.c ... +static virNetworkObjPtr +testLoadNetworkFromFile(virConnectPtr conn, ... + if ((def = virNetworkDefParse(conn, data, filename)) == NULL) { + VIR_FREE(data); + return NULL; } + VIR_FREE(data);
Here's another case where you can avoid a duplicate free: def = virNetworkDefParse(conn, data, filename); VIR_FREE(data); if (def == NULL) return NULL; ...
static virNetworkPtr testLookupNetworkByUUID(virConnectPtr conn, const unsigned char *uuid) { - int i, idx = -1; + virNetworkObjPtr net = NULL;
s/ = NULL// ...
static virNetworkPtr testLookupNetworkByName(virConnectPtr conn, - const char *name) + const char *name) { - int i, idx = -1; + virNetworkObjPtr net = NULL;
Same here. ...
static int testNumNetworks(virConnectPtr conn) { - int numInactive = 0, i; + int numActive = 0; + virNetworkObjPtr net; GET_CONNECTION(conn, -1);
- for (i = 0 ; i < MAX_NETWORKS ; i++) { - if (!privconn->networks[i].active || - !privconn->networks[i].running) - continue; - numInactive++; + net = privconn->networks; + while (net) { + if (virNetworkIsActive(net)) + numActive++; + net = net->next; } - return (numInactive); + return numActive; }
static int testListNetworks(virConnectPtr conn, char **const names, int nnames) { - int n = 0, i; + int n = 0; + virNetworkObjPtr net; GET_CONNECTION(conn, -1);
- for (i = 0, n = 0 ; i < MAX_NETWORKS && n < nnames ; i++) { - if (privconn->networks[i].active && - privconn->networks[i].running) { - names[n++] = strdup(privconn->networks[i].name); - } + net = privconn->networks; + while (net && n < nnames) { + if (virNetworkIsActive(net)) + names[n++] = strdup(net->def->name);
I know this isn't new with your changes, but there seems to be a potential problem here, when strdup fails. The resulting NULL pointer looks like it will be dereferenced at least in virsh.c via cmdNetworkList's use of qsort+namesorter (it can call this function through virConnectListNetworks). As a work-around, this could increment "n" only for each non-NULL pointer: if (virNetworkIsActive(net)) { char *p = strdup(net->def->name); names[n] = p; if (p) ++n; } ...
static int testListDefinedNetworks(virConnectPtr conn, char **const names, int nnames) { - int n = 0, i; + int n = 0; + virNetworkObjPtr net; GET_CONNECTION(conn, -1);
- for (i = 0, n = 0 ; i < MAX_NETWORKS && n < nnames ; i++) { - if (privconn->networks[i].active && - !privconn->networks[i].running) { - names[n++] = strdup(privconn->networks[i].name);
Same here.
- } + net = privconn->networks; + while (net && n < nnames) { + if (!virNetworkIsActive(net)) + names[n++] = strdup(net->def->name);
and here.
+ net = net->next; } - return (n); + return n; }
static virNetworkPtr testNetworkCreate(virConnectPtr conn, const char *xml) { - int handle = -1; - virNetworkPtr net; + virNetworkDefPtr def; + virNetworkObjPtr net; GET_CONNECTION(conn, NULL);
- if (xml == NULL) { - testError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); - return (NULL); - } + if ((def = virNetworkDefParse(conn, xml, NULL)) == NULL) + return NULL;
- if (privconn->numNetworks == MAX_NETWORKS) { - testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("too many networks")); - return (NULL); - } + if ((net = virNetworkAssignDef(conn, &privconn->networks, + def)) == NULL)
Don't we need to call virNetworkDefFree(def) before returning?
+ return NULL; + net->active = 1;
- if ((handle = testLoadNetworkFromDoc(conn, xml)) < 0) - return (NULL); - privconn->networks[handle].config = 0; - - net = virGetNetwork(conn, privconn->networks[handle].name, privconn->networks[handle].uuid); - if (net == NULL) return NULL; - privconn->numNetworks++; - return (net); + return virGetNetwork(conn, def->name, def->uuid);
And before this one, too? i.e., virNetworkPtr result_net = virGetNetwork(conn, def->name, def->uuid); virNetworkDefFree(def); return result_net;
}
static virNetworkPtr testNetworkDefine(virConnectPtr conn, const char *xml) { - int handle = -1; - virNetworkPtr net; + virNetworkDefPtr def; + virNetworkObjPtr net; GET_CONNECTION(conn, NULL);
- if (xml == NULL) { - testError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); - return (NULL); - } + if ((def = virNetworkDefParse(conn, xml, NULL)) == NULL) + return NULL;
- if (privconn->numNetworks == MAX_NETWORKS) { - testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("too many networks")); - return (NULL); - } + if ((net = virNetworkAssignDef(conn, &privconn->networks, + def)) == NULL)
If needed above, then it's needed here, too.
+ return NULL; + net->persistent = 1;
- if ((handle = testLoadNetworkFromDoc(conn, xml)) < 0) - return (NULL); - - net = virGetNetwork(conn, privconn->networks[handle].name, privconn->networks[handle].uuid); - privconn->networks[handle].config = 1; - if (net == NULL) return NULL; - privconn->numNetworks++; - return (net); + return virGetNetwork(conn, def->name, def->uuid);
And here.
}
static int testNetworkUndefine(virNetworkPtr network) { ...
I'll finish tomorrow.

On Wed, Jul 02, 2008 at 10:13:00PM +0200, Jim Meyering wrote:
static virNetworkPtr testNetworkCreate(virConnectPtr conn, const char *xml) { - int handle = -1; - virNetworkPtr net; + virNetworkDefPtr def; + virNetworkObjPtr net; GET_CONNECTION(conn, NULL);
- if (xml == NULL) { - testError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); - return (NULL); - } + if ((def = virNetworkDefParse(conn, xml, NULL)) == NULL) + return NULL;
- if (privconn->numNetworks == MAX_NETWORKS) { - testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("too many networks")); - return (NULL); - } + if ((net = virNetworkAssignDef(conn, &privconn->networks, + def)) == NULL)
Don't we need to call virNetworkDefFree(def) before returning?
No need as the 'net' object holds a pointer to the 'def' struct _virDomainObj { [...snip...] virDomainDefPtr def; /* The current definition */ virDomainDefPtr newDef; /* New definition to activate at shutdown */ virDomainObjPtr next; }; Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

"Daniel P. Berrange" <berrange@redhat.com> wrote:
On Wed, Jul 02, 2008 at 10:13:00PM +0200, Jim Meyering wrote:
static virNetworkPtr testNetworkCreate(virConnectPtr conn, const char *xml) { - int handle = -1; - virNetworkPtr net; + virNetworkDefPtr def; + virNetworkObjPtr net; GET_CONNECTION(conn, NULL);
- if (xml == NULL) { - testError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); - return (NULL); - } + if ((def = virNetworkDefParse(conn, xml, NULL)) == NULL) + return NULL;
...
+ if ((net = virNetworkAssignDef(conn, &privconn->networks, + def)) == NULL)
Don't we need to call virNetworkDefFree(def) before returning?
No need as the 'net' object holds a pointer to the 'def'
But inside that if-block (where the function returns), "net" is NULL. Am I'm missing something?

On Thu, Jul 03, 2008 at 12:09:37PM +0200, Jim Meyering wrote:
"Daniel P. Berrange" <berrange@redhat.com> wrote:
On Wed, Jul 02, 2008 at 10:13:00PM +0200, Jim Meyering wrote:
static virNetworkPtr testNetworkCreate(virConnectPtr conn, const char *xml) { - int handle = -1; - virNetworkPtr net; + virNetworkDefPtr def; + virNetworkObjPtr net; GET_CONNECTION(conn, NULL);
- if (xml == NULL) { - testError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); - return (NULL); - } + if ((def = virNetworkDefParse(conn, xml, NULL)) == NULL) + return NULL;
...
+ if ((net = virNetworkAssignDef(conn, &privconn->networks, + def)) == NULL)
Don't we need to call virNetworkDefFree(def) before returning?
No need as the 'net' object holds a pointer to the 'def'
But inside that if-block (where the function returns), "net" is NULL.
Oh, I see what you mean - I didn't realize you were referring to the failure path there. Yes, I need to free it. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

"Daniel P. Berrange" <berrange@redhat.com> wrote: ... cont'd
static int testNetworkUndefine(virNetworkPtr network) { ... static char *testNetworkGetBridgeName(virNetworkPtr network) { char *bridge; GET_NETWORK(network, NULL); - bridge = strdup(privnet->bridge); + bridge = privnet->def->bridge ? strdup(privnet->def->bridge) : NULL; if (!bridge) { testError(network->conn, NULL, network, VIR_ERR_NO_MEMORY, "network"); return NULL;
This change makes it so a NULL privnet->def->bridge will mistakenly provoke an out of memory error.

On Thu, Jul 03, 2008 at 08:27:07AM +0200, Jim Meyering wrote:
"Daniel P. Berrange" <berrange@redhat.com> wrote: ... cont'd
static int testNetworkUndefine(virNetworkPtr network) { ... static char *testNetworkGetBridgeName(virNetworkPtr network) { char *bridge; GET_NETWORK(network, NULL); - bridge = strdup(privnet->bridge); + bridge = privnet->def->bridge ? strdup(privnet->def->bridge) : NULL; if (!bridge) { testError(network->conn, NULL, network, VIR_ERR_NO_MEMORY, "network"); return NULL;
This change makes it so a NULL privnet->def->bridge will mistakenly provoke an out of memory error.
Yep, that's bogus Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

This patch ports the Test driver over to the domain XML apis. Basically the 'struct _testDom' is removed, and replaced by usage of the generic virDomainObjPtr and virDomainDefPtr objects. The XML parser and formatters are ripped out and replaced by calls to the generic APIs in domain_conf.h. Finally there is just alot of cleanup of all the individual domain driver implementations to adjust to the style of the new APIs used. This actually dramatically increases the functionality of the test driver - previously its XML parser didn't support any of the <device> section in the XML. This should make it much more useful for advanced apps like virt-manager docs/testdomfc4.xml | 2 src/test.c | 1020 ++++++++++++++++++------------------------------ tests/read-non-seekable | 3 tests/testutils.c | 3 tests/virshtest.c | 4 5 files changed, 391 insertions(+), 641 deletions(-) Regards, Daniel diff -r 897248d2ee59 docs/testdomfc4.xml --- a/docs/testdomfc4.xml Sun Jun 22 17:59:45 2008 -0400 +++ b/docs/testdomfc4.xml Sun Jun 22 18:02:31 2008 -0400 @@ -2,7 +2,7 @@ <name>fc4</name> <uuid>EF86180145B911CB88E3AFBFE5370493</uuid> <os> - <type>linux</type> + <type>xen</type> <kernel>/boot/vmlinuz-2.6.15-1.43_FC5guest</kernel> <initrd>/boot/initrd-2.6.15-1.43_FC5guest.img</initrd> <root>/dev/sda1</root> diff -r 897248d2ee59 src/test.c --- a/src/test.c Sun Jun 22 17:59:45 2008 -0400 +++ b/src/test.c Sun Jun 22 18:02:31 2008 -0400 @@ -47,47 +47,8 @@ #include "capabilities.h" #include "memory.h" #include "network_conf.h" +#include "domain_conf.h" -/* Flags that determine the action to take on a shutdown or crash of a domain - */ -typedef enum { - VIR_DOMAIN_DESTROY = 1, /* destroy the domain */ - VIR_DOMAIN_RESTART = 2, /* restart the domain */ - VIR_DOMAIN_PRESERVE= 3, /* keep as is, need manual destroy, for debug */ - VIR_DOMAIN_RENAME_RESTART= 4/* restart under an new unique name */ -} virDomainRestart; - -struct _testDev { - char name[20]; - int mode; -}; -typedef struct _testDev testDev; -typedef struct _testDev *testDevPtr; - -#define MAX_DEVICES 10 - -struct _testDom { - int active; - int config; - int id; - char name[20]; - unsigned char uuid[VIR_UUID_BUFLEN]; - virDomainInfo info; - unsigned int maxVCPUs; - virDomainRestart onRestart; /* What to do at end of current shutdown procedure */ - virDomainRestart onReboot; - virDomainRestart onPoweroff; - virDomainRestart onCrash; - int numDevices; - testDev devices[MAX_DEVICES]; - int autostart; - unsigned int weight; -}; -typedef struct _testDom testDom; -typedef struct _testDom *testDomPtr; - -#define MAX_DOMAINS 20 -#define MAX_NETWORKS 20 #define MAX_CPUS 128 struct _testCell { @@ -103,9 +64,9 @@ struct _testConn { char path[PATH_MAX]; int nextDomID; + virCapsPtr caps; virNodeInfo nodeInfo; - int numDomains; - testDom domains[MAX_DOMAINS]; + virDomainObjPtr domains; virNetworkObjPtr networks; int numCells; testCell cells[MAX_CELLS]; @@ -115,6 +76,7 @@ #define TEST_MODEL "i686" #define TEST_MODEL_WORDSIZE 32 +#define TEST_EMULATOR "/usr/bin/test-hv" static const virNodeInfo defaultNodeInfo = { TEST_MODEL, @@ -128,17 +90,18 @@ }; #define GET_DOMAIN(dom, ret) \ - int domidx; \ testConnPtr privconn; \ - testDomPtr privdom; \ + virDomainObjPtr privdom; \ \ privconn = (testConnPtr)dom->conn->privateData; \ - if ((domidx = getDomainIndex(dom)) < 0) { \ - testError((dom)->conn, (dom), NULL, VIR_ERR_INVALID_ARG, \ - __FUNCTION__); \ - return (ret); \ - } \ - privdom = &privconn->domains[domidx]; + do { \ + if ((privdom = virDomainFindByName(privconn->domains, \ + (dom)->name)) == NULL) { \ + testError((dom)->conn, (dom), NULL, VIR_ERR_INVALID_ARG, \ + __FUNCTION__); \ + return (ret); \ + } \ + } while (0) #define GET_NETWORK(net, ret) \ testConnPtr privconn; \ @@ -154,7 +117,7 @@ } \ } while (0) -#define GET_CONNECTION(conn, ret) \ +#define GET_CONNECTION(conn) \ testConnPtr privconn; \ \ privconn = (testConnPtr)conn->privateData; @@ -177,230 +140,30 @@ errmsg, info, NULL, 0, 0, errmsg, info, 0); } -static int testRestartStringToFlag(const char *str) { - if (STREQ(str, "restart")) { - return VIR_DOMAIN_RESTART; - } else if (STREQ(str, "destroy")) { - return VIR_DOMAIN_DESTROY; - } else if (STREQ(str, "preserve")) { - return VIR_DOMAIN_PRESERVE; - } else if (STREQ(str, "rename-restart")) { - return VIR_DOMAIN_RENAME_RESTART; - } else { - return (0); - } -} -static const char *testRestartFlagToString(int flag) { - switch (flag) { - case VIR_DOMAIN_RESTART: - return "restart"; - case VIR_DOMAIN_DESTROY: - return "destroy"; - case VIR_DOMAIN_PRESERVE: - return "preserve"; - case VIR_DOMAIN_RENAME_RESTART: - return "rename-restart"; - } - return (NULL); -} +#define MAX_DOMAIN_XML_LEN 16000 +static virDomainObjPtr +testLoadDomainFromFile(virConnectPtr conn, + const char *filename) { + char *data; + virDomainDefPtr def; + GET_CONNECTION(conn); -static int testLoadDomain(virConnectPtr conn, - int domid, - xmlDocPtr xml) { - xmlNodePtr root = NULL; - xmlXPathContextPtr ctxt = NULL; - char *name = NULL; - unsigned char uuid[VIR_UUID_BUFLEN]; - struct timeval tv; - unsigned long memory = 0; - unsigned long maxMem = 0; - int nrVirtCpu; - char *str; - int handle = -1, i, ret; - long l; - virDomainRestart onReboot = VIR_DOMAIN_RESTART; - virDomainRestart onPoweroff = VIR_DOMAIN_DESTROY; - virDomainRestart onCrash = VIR_DOMAIN_RENAME_RESTART; - GET_CONNECTION(conn, -1); - - if (gettimeofday(&tv, NULL) < 0) { - testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("getting time of day")); - return (-1); + if (virFileReadAll(filename, MAX_DOMAIN_XML_LEN, &data) < 0) { + testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, + _("load domain definition file")); + return NULL; } - root = xmlDocGetRootElement(xml); - if ((root == NULL) || (!xmlStrEqual(root->name, BAD_CAST "domain"))) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("domain")); - goto error; + if ((def = virDomainDefParse(conn, privconn->caps, data, filename)) == NULL) { + VIR_FREE(data); + return NULL; } + VIR_FREE(data); - ctxt = xmlXPathNewContext(xml); - if (ctxt == NULL) { - testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("creating xpath context")); - goto error; - } - - name = virXPathString("string(/domain/name[1])", ctxt); - if (name == NULL) { - testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("domain name")); - goto error; - } - - str = virXPathString("string(/domain/uuid[1])", ctxt); - if (str == NULL) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("domain uuid")); - goto error; - } - if (virUUIDParse(str, uuid) < 0) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("domain uuid")); - goto error; - } - VIR_FREE(str); - - - ret = virXPathLong("string(/domain/memory[1])", ctxt, &l); - if (ret != 0) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("domain memory")); - goto error; - } - maxMem = l; - - ret = virXPathLong("string(/domain/currentMemory[1])", ctxt, &l); - if (ret == -1) { - memory = maxMem; - } else if (ret == -2) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("domain current memory")); - goto error; - } else { - memory = l; - } - - ret = virXPathLong("string(/domain/vcpu[1])", ctxt, &l); - if (ret == -1) { - nrVirtCpu = 1; - } else if (ret == -2) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("domain vcpus")); - goto error; - } else { - nrVirtCpu = l; - } - - str = virXPathString("string(/domain/on_reboot[1])", ctxt); - if (str != NULL) { - if (!(onReboot = testRestartStringToFlag(str))) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("domain reboot behaviour")); - VIR_FREE(str); - goto error; - } - VIR_FREE(str); - } - - str = virXPathString("string(/domain/on_poweroff[1])", ctxt); - if (str != NULL) { - if (!(onPoweroff = testRestartStringToFlag(str))) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("domain poweroff behaviour")); - VIR_FREE(str); - goto error; - } - VIR_FREE(str); - } - - str = virXPathString("string(/domain/on_crash[1])", ctxt); - if (str != NULL) { - if (!(onCrash = testRestartStringToFlag(str))) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("domain crash behaviour")); - VIR_FREE(str); - goto error; - } - VIR_FREE(str); - } - - for (i = 0 ; i < MAX_DOMAINS ; i++) { - if (!privconn->domains[i].active) { - handle = i; - break; - } - } - if (handle < 0) - goto error; - - privconn->domains[handle].active = 1; - privconn->domains[handle].id = domid; - strncpy(privconn->domains[handle].name, name, sizeof(privconn->domains[handle].name)-1); - privconn->domains[handle].name[sizeof(privconn->domains[handle].name)-1] = '\0'; - VIR_FREE(name); - name = NULL; - - if (memory > maxMem) - memory = maxMem; - - memmove(privconn->domains[handle].uuid, uuid, VIR_UUID_BUFLEN); - privconn->domains[handle].info.maxMem = maxMem; - privconn->domains[handle].info.memory = memory; - privconn->domains[handle].info.state = domid < 0 ? VIR_DOMAIN_SHUTOFF : VIR_DOMAIN_RUNNING; - privconn->domains[handle].info.nrVirtCpu = nrVirtCpu; - privconn->domains[handle].info.cpuTime = ((tv.tv_sec * 1000ll * 1000ll * 1000ll) + (tv.tv_usec * 1000ll)); - privconn->domains[handle].maxVCPUs = nrVirtCpu; - - privconn->domains[handle].onReboot = onReboot; - privconn->domains[handle].onPoweroff = onPoweroff; - privconn->domains[handle].onCrash = onCrash; - - xmlXPathFreeContext(ctxt); - return (handle); - - error: - xmlXPathFreeContext(ctxt); - VIR_FREE(name); - return (-1); -} - -static int testLoadDomainFromDoc(virConnectPtr conn, - int domid, - const char *doc) { - int ret; - xmlDocPtr xml; - if (!(xml = xmlReadDoc(BAD_CAST doc, "domain.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("domain")); - return (-1); - } - - ret = testLoadDomain(conn, domid, xml); - - xmlFreeDoc(xml); - - return (ret); -} - - -static int testLoadDomainFromFile(virConnectPtr conn, - int domid, - const char *filename) { - int ret, fd; - xmlDocPtr xml; - - if ((fd = open(filename, O_RDONLY)) < 0) { - testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("load domain definition file")); - return (-1); - } - - if (!(xml = xmlReadFd(fd, filename, NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("domain")); - return (-1); - } - - ret = testLoadDomain(conn, domid, xml); - - xmlFreeDoc(xml); - close(fd); - - return (ret); + return virDomainAssignDef(conn, &privconn->domains, + def); } @@ -411,7 +174,7 @@ const char *filename) { char *data; virNetworkDefPtr def; - GET_CONNECTION(conn, -1); + GET_CONNECTION(conn); if (virFileReadAll(filename, MAX_NETWORK_XML_LEN, &data) < 0) { testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("load network definition file")); @@ -428,6 +191,73 @@ def); } +static virCapsPtr +testBuildCapabilities(virConnectPtr conn) { + virCapsPtr caps; + virCapsGuestPtr guest; + const char *guest_types[] = { "hvm", "xen" }; + int i; + GET_CONNECTION(conn); + + if ((caps = virCapabilitiesNew(TEST_MODEL, 0, 0)) == NULL) + goto no_memory; + + if (virCapabilitiesAddHostFeature(caps, "pae") < 0) + goto no_memory; + if (virCapabilitiesAddHostFeature(caps ,"nonpae") < 0) + goto no_memory; + + for (i = 0; i < privconn->numCells; ++i) { + if (virCapabilitiesAddHostNUMACell(caps, i, privconn->cells[i].numCpus, + privconn->cells[i].cpus) < 0) + goto no_memory; + } + + for (i = 0; i < (sizeof(guest_types)/sizeof(guest_types[0])); ++i) { + if ((guest = virCapabilitiesAddGuest(caps, + guest_types[i], + TEST_MODEL, + TEST_MODEL_WORDSIZE, + TEST_EMULATOR, + NULL, + 0, + NULL)) == NULL) + goto no_memory; + + if (virCapabilitiesAddGuestDomain(guest, + "test", + NULL, + NULL, + 0, + NULL) == NULL) + goto no_memory; + + if (virCapabilitiesAddGuestFeature(guest, "pae", 1, 1) == NULL) + goto no_memory; + if (virCapabilitiesAddGuestFeature(guest ,"nonpae", 1, 1) == NULL) + goto no_memory; + } + + return caps; + +no_memory: + testError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL); + virCapabilitiesFree(caps); + return NULL; +} + + +static const char *defaultDomainXML = +"<domain type='test'>" +" <name>test</name>" +" <memory>8388608</memory>" +" <currentMemory>2097152</currentMemory>" +" <vcpu>2</vcpu>" +" <os>" +" <type>hvm</type>" +" </os>" +"</domain>"; + static const char *defaultNetworkXML = "<network>" " <name>default</name>" @@ -445,50 +275,23 @@ int u; struct timeval tv; testConnPtr privconn; - virNetworkDefPtr netdef; - virNetworkObjPtr netobj; + virDomainDefPtr domdef = NULL; + virDomainObjPtr domobj = NULL; + virNetworkDefPtr netdef = NULL; + virNetworkObjPtr netobj = NULL; if (VIR_ALLOC(privconn) < 0) { testError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, "testConn"); return VIR_DRV_OPEN_ERROR; } + conn->privateData = privconn; if (gettimeofday(&tv, NULL) < 0) { testError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("getting time of day")); - return VIR_DRV_OPEN_ERROR; + goto error; } memmove(&privconn->nodeInfo, &defaultNodeInfo, sizeof(defaultNodeInfo)); - - strcpy(privconn->path, "/default"); - privconn->nextDomID = 1; - privconn->numDomains = 1; - privconn->domains[0].active = 1; - privconn->domains[0].config = 1; - privconn->domains[0].id = privconn->nextDomID++; - privconn->domains[0].onReboot = VIR_DOMAIN_RESTART; - privconn->domains[0].onCrash = VIR_DOMAIN_RESTART; - privconn->domains[0].onPoweroff = VIR_DOMAIN_DESTROY; - strcpy(privconn->domains[0].name, "test"); - for (u = 0 ; u < VIR_UUID_BUFLEN ; u++) { - privconn->domains[0].uuid[u] = (u * 75)%255; - } - privconn->domains[0].info.maxMem = 8192 * 1024; - privconn->domains[0].info.memory = 2048 * 1024; - privconn->domains[0].info.state = VIR_DOMAIN_RUNNING; - privconn->domains[0].info.nrVirtCpu = 2; - privconn->domains[0].info.cpuTime = ((tv.tv_sec * 1000ll * 1000ll * 1000ll) + (tv.tv_usec * 1000ll)); - - - if (!(netdef = virNetworkDefParse(conn, defaultNetworkXML, "default.xml"))) { - return VIR_DRV_OPEN_ERROR; - } - if (!(netobj = virNetworkAssignDef(conn, &privconn->networks, netdef))) { - virNetworkDefFree(netdef); - return VIR_DRV_OPEN_ERROR; - } - netobj->active = 1; - netobj->persistent = 1; // Numa setup privconn->numCells = 2; @@ -500,8 +303,39 @@ privconn->cells[u % 2].cpus[(u / 2)] = u; } - conn->privateData = privconn; - return (VIR_DRV_OPEN_SUCCESS); + if (!(privconn->caps = testBuildCapabilities(conn))) + goto error; + + privconn->nextDomID = 1; + + if (!(domdef = virDomainDefParse(conn, privconn->caps, defaultDomainXML, "default.xml"))) + goto error; + if (!(domobj = virDomainAssignDef(conn, &privconn->domains, domdef))) { + virDomainDefFree(domdef); + goto error; + } + domobj->def->id = 1; + domobj->state = VIR_DOMAIN_RUNNING; + domobj->persistent = 1; + + if (!(netdef = virNetworkDefParse(conn, defaultNetworkXML, "default.xml"))) + goto error; + if (!(netobj = virNetworkAssignDef(conn, &privconn->networks, netdef))) { + virNetworkDefFree(netdef); + goto error; + } + + netobj->active = 1; + netobj->persistent = 1; + + return VIR_DRV_OPEN_SUCCESS; + +error: + virDomainObjFree(privconn->domains); + virNetworkObjFree(privconn->networks); + virCapabilitiesFree(privconn->caps); + VIR_FREE(privconn); + return VIR_DRV_OPEN_ERROR; } @@ -538,11 +372,17 @@ xmlNodePtr *domains, *networks = NULL; xmlXPathContextPtr ctxt = NULL; virNodeInfoPtr nodeInfo; + virDomainObjPtr dom; + virNetworkObjPtr net; testConnPtr privconn; if (VIR_ALLOC(privconn) < 0) { testError(NULL, NULL, NULL, VIR_ERR_NO_MEMORY, "testConn"); return VIR_DRV_OPEN_ERROR; } + conn->privateData = privconn; + + if (!(privconn->caps = testBuildCapabilities(conn))) + goto error; if ((fd = open(file, O_RDONLY)) < 0) { testError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("loading host definition file")); @@ -570,10 +410,7 @@ goto error; } - - conn->privateData = privconn; privconn->nextDomID = 1; - privconn->numDomains = 0; privconn->numCells = 0; strncpy(privconn->path, file, PATH_MAX-1); privconn->path[PATH_MAX-1] = '\0'; @@ -645,39 +482,35 @@ goto error; } + ret = virXPathNodeSet("/node/domain", ctxt, &domains); - if (ret < 0) { - testError(NULL, NULL, NULL, VIR_ERR_XML_ERROR, _("node domain list")); - goto error; + if (ret > 0) { + for (i = 0 ; i < ret ; i++) { + xmlChar *netFile = xmlGetProp(domains[i], BAD_CAST "file"); + char *absFile = testBuildFilename(file, (const char *)netFile); + VIR_FREE(netFile); + if (!absFile) { + testError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("resolving domain filename")); + goto error; + } + if ((dom = testLoadDomainFromFile(conn, absFile)) == NULL) { + VIR_FREE(absFile); + goto error; + } + dom->state = VIR_DOMAIN_RUNNING; + dom->def->id = privconn->nextDomID++; + dom->persistent = 1; + dom->configFile = absFile; + } + if (domains != NULL) { + VIR_FREE(domains); + domains = NULL; + } } - - for (i = 0 ; i < ret ; i++) { - xmlChar *domFile = xmlGetProp(domains[i], BAD_CAST "file"); - char *absFile = testBuildFilename(file, (const char *)domFile); - int domid = privconn->nextDomID++, handle; - VIR_FREE(domFile); - if (!absFile) { - testError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("resolving domain filename")); - goto error; - } - if ((handle = testLoadDomainFromFile(conn, domid, absFile)) < 0) { - VIR_FREE(absFile); - goto error; - } - privconn->domains[handle].config = 1; - VIR_FREE(absFile); - privconn->numDomains++; - } - if (domains != NULL) { - VIR_FREE(domains); - domains = NULL; - } - ret = virXPathNodeSet("/node/network", ctxt, &networks); if (ret > 0) { for (i = 0 ; i < ret ; i++) { - virNetworkObjPtr net; xmlChar *netFile = xmlGetProp(networks[i], BAD_CAST "file"); char *absFile = testBuildFilename(file, (const char *)netFile); VIR_FREE(netFile); @@ -705,32 +538,28 @@ error: xmlXPathFreeContext(ctxt); + xmlFreeDoc(xml); VIR_FREE(domains); VIR_FREE(networks); - if (xml) - xmlFreeDoc(xml); if (fd != -1) close(fd); + dom = privconn->domains; + while (dom) { + virDomainObjPtr tmp = dom->next; + virDomainObjFree(dom); + dom = tmp; + } + net = privconn->networks; + while (net) { + virNetworkObjPtr tmp = net->next; + virNetworkObjFree(net); + net = tmp; + } VIR_FREE(privconn); conn->privateData = NULL; return VIR_DRV_OPEN_ERROR; } -static int getDomainIndex(virDomainPtr domain) { - int i; - GET_CONNECTION(domain->conn, -1); - - for (i = 0 ; i < MAX_DOMAINS ; i++) { - if (domain->id >= 0) { - if (domain->id == privconn->domains[i].id) - return (i); - } else { - if (STREQ(domain->name, privconn->domains[i].name)) - return (i); - } - } - return (-1); -} static int testOpen(virConnectPtr conn, xmlURIPtr uri, @@ -772,7 +601,22 @@ static int testClose(virConnectPtr conn) { - GET_CONNECTION(conn, -1); + virDomainObjPtr dom; + virNetworkObjPtr net; + GET_CONNECTION(conn); + virCapabilitiesFree(privconn->caps); + dom = privconn->domains; + while (dom) { + virDomainObjPtr tmp = dom->next; + virDomainObjFree(dom); + dom = tmp; + } + net = privconn->networks; + while (net) { + virNetworkObjPtr tmp = net->next; + virNetworkObjFree(net); + net = tmp; + } VIR_FREE (privconn); conn->privateData = conn; return 0; @@ -806,7 +650,7 @@ static char * testGetURI (virConnectPtr conn) { char *uri; - GET_CONNECTION(conn, NULL); + GET_CONNECTION(conn); if (asprintf (&uri, "test://%s", privconn->path) == -1) { testError (conn, NULL, NULL, VIR_ERR_SYSTEM_ERROR, strerror (errno)); @@ -824,224 +668,139 @@ static int testNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) { - GET_CONNECTION(conn, -1); + GET_CONNECTION(conn); memcpy(info, &privconn->nodeInfo, sizeof(virNodeInfo)); return (0); } static char *testGetCapabilities (virConnectPtr conn) { - virCapsPtr caps; - virCapsGuestPtr guest; char *xml; - const char *guest_types[] = { "hvm", "xen" }; - int i; + GET_CONNECTION(conn); - GET_CONNECTION(conn, -1); - - if ((caps = virCapabilitiesNew(TEST_MODEL, 0, 0)) == NULL) - goto no_memory; - - if (virCapabilitiesAddHostFeature(caps, "pae") < 0) - goto no_memory; - if (virCapabilitiesAddHostFeature(caps ,"nonpae") < 0) - goto no_memory; - - for (i = 0; i < privconn->numCells; ++i) { - if (virCapabilitiesAddHostNUMACell(caps, i, privconn->cells[i].numCpus, - privconn->cells[i].cpus) < 0) - goto no_memory; + if ((xml = virCapabilitiesFormatXML(privconn->caps)) == NULL) { + testError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, NULL); + return NULL; } - for (i = 0; i < (sizeof(guest_types)/sizeof(guest_types[0])); ++i) { - - if ((guest = virCapabilitiesAddGuest(caps, - guest_types[i], - TEST_MODEL, - TEST_MODEL_WORDSIZE, - NULL, - NULL, - 0, - NULL)) == NULL) - goto no_memory; - - if (virCapabilitiesAddGuestDomain(guest, - "test", - NULL, - NULL, - 0, - NULL) == NULL) - goto no_memory; - - if (virCapabilitiesAddGuestFeature(guest, "pae", 1, 1) == NULL) - goto no_memory; - if (virCapabilitiesAddGuestFeature(guest ,"nonpae", 1, 1) == NULL) - goto no_memory; - } - - if ((xml = virCapabilitiesFormatXML(caps)) == NULL) - goto no_memory; - - virCapabilitiesFree(caps); - return xml; - - no_memory: - virCapabilitiesFree(caps); - testError(conn, NULL, NULL, VIR_ERR_NO_MEMORY, __FUNCTION__); - return NULL; } static int testNumOfDomains(virConnectPtr conn) { - int numActive = 0, i; - GET_CONNECTION(conn, -1); + int numActive = 0; + virDomainObjPtr dom; + GET_CONNECTION(conn); - for (i = 0 ; i < MAX_DOMAINS ; i++) { - if (!privconn->domains[i].active || - privconn->domains[i].info.state == VIR_DOMAIN_SHUTOFF) - continue; - numActive++; + dom = privconn->domains; + while (dom) { + if (virDomainIsActive(dom)) + numActive++; + dom = dom->next; } - return (numActive); + return numActive; } static virDomainPtr -testDomainCreateLinux(virConnectPtr conn, const char *xmlDesc, +testDomainCreateLinux(virConnectPtr conn, const char *xml, unsigned int flags ATTRIBUTE_UNUSED) { - int domid, handle = -1; - virDomainPtr dom; - GET_CONNECTION(conn, NULL); + virDomainPtr ret; + virDomainDefPtr def; + virDomainObjPtr dom; + GET_CONNECTION(conn); - if (xmlDesc == NULL) { - testError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); - return (NULL); - } + if ((def = virDomainDefParse(conn, privconn->caps, xml, NULL)) == NULL) + return NULL; - if (privconn->numDomains == MAX_DOMAINS) { - testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("too many domains")); - return (NULL); - } + if ((dom = virDomainAssignDef(conn, &privconn->domains, + def)) == NULL) + return NULL; + dom->state = VIR_DOMAIN_RUNNING; + dom->def->id = privconn->nextDomID++; - domid = privconn->nextDomID++; - if ((handle = testLoadDomainFromDoc(conn, domid, xmlDesc)) < 0) - return (NULL); - privconn->domains[handle].config = 0; - - dom = virGetDomain(conn, privconn->domains[handle].name, privconn->domains[handle].uuid); - if (dom == NULL) return NULL; - privconn->numDomains++; - return (dom); + ret = virGetDomain(conn, def->name, def->uuid); + ret->id = def->id; + return ret; } static virDomainPtr testLookupDomainByID(virConnectPtr conn, int id) { - virDomainPtr dom; - int i, idx = -1; - GET_CONNECTION(conn, NULL); + virDomainObjPtr dom = NULL; + virDomainPtr ret; + GET_CONNECTION(conn); - for (i = 0 ; i < MAX_DOMAINS ; i++) { - if (privconn->domains[i].active && - privconn->domains[i].id == id) { - idx = i; - break; - } + if ((dom = virDomainFindByID(privconn->domains, id)) == NULL) { + testError (conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL); + return NULL; } - if (idx < 0) { - testError (conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL); - return(NULL); - } - - dom = virGetDomain(conn, privconn->domains[idx].name, privconn->domains[idx].uuid); - if (dom == NULL) return NULL; - dom->id = id; - return (dom); + ret = virGetDomain(conn, dom->def->name, dom->def->uuid); + ret->id = dom->def->id; + return ret; } static virDomainPtr testLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid) { - virDomainPtr dom; - int i, idx = -1; - GET_CONNECTION(conn, NULL); + virDomainPtr ret; + virDomainObjPtr dom = NULL; + GET_CONNECTION(conn); - for (i = 0 ; i < MAX_DOMAINS ; i++) { - if (privconn->domains[i].active && - memcmp(uuid, privconn->domains[i].uuid, VIR_UUID_BUFLEN) == 0) { - idx = i; - break; - } - } - - if (idx < 0) { + if ((dom = virDomainFindByUUID(privconn->domains, uuid)) == NULL) { testError (conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL); return NULL; } - dom = virGetDomain(conn, privconn->domains[idx].name, privconn->domains[idx].uuid); - if (dom == NULL) return NULL; - dom->id = privconn->domains[idx].id; - - return dom; + ret = virGetDomain(conn, dom->def->name, dom->def->uuid); + ret->id = dom->def->id; + return ret; } static virDomainPtr testLookupDomainByName(virConnectPtr conn, const char *name) { - virDomainPtr dom; - int i, idx = -1; - GET_CONNECTION(conn, NULL); + virDomainPtr ret; + virDomainObjPtr dom = NULL; + GET_CONNECTION(conn); - for (i = 0 ; i < MAX_DOMAINS ; i++) { - if (privconn->domains[i].active && - STREQ(name, privconn->domains[i].name)) { - idx = i; - break; - } - } - - if (idx < 0) { + if ((dom = virDomainFindByName(privconn->domains, name)) == NULL) { testError (conn, NULL, NULL, VIR_ERR_NO_DOMAIN, NULL); return NULL; } - dom = virGetDomain(conn, privconn->domains[idx].name, privconn->domains[idx].uuid); - if (dom == NULL) return NULL; - dom->id = privconn->domains[idx].id; - - return dom; + ret = virGetDomain(conn, dom->def->name, dom->def->uuid); + ret->id = dom->def->id; + return ret; } static int testListDomains (virConnectPtr conn, int *ids, int maxids) { - int n, i; - GET_CONNECTION(conn, -1); + int n = 0; + virDomainObjPtr dom; + GET_CONNECTION(conn); - for (i = 0, n = 0 ; i < MAX_DOMAINS && n < maxids ; i++) { - if (privconn->domains[i].active && - privconn->domains[i].info.state != VIR_DOMAIN_SHUTOFF) { - ids[n++] = privconn->domains[i].id; - } + dom = privconn->domains; + while (dom && n < maxids) { + if (virDomainIsActive(dom)) + ids[n++] = dom->def->id; + dom = dom->next; } - return (n); + return n; } static int testDestroyDomain (virDomainPtr domain) { GET_DOMAIN(domain, -1); - if (privdom->config) { - privdom->info.state = VIR_DOMAIN_SHUTOFF; - privdom->id = -1; - domain->id = -1; - } else { - privdom->active = 0; + privdom->state = VIR_DOMAIN_SHUTOFF; + if (!privdom->persistent) { + virDomainRemoveInactive(&privconn->domains, + privdom); } return (0); } @@ -1050,12 +809,13 @@ { GET_DOMAIN(domain, -1); - if (privdom->info.state != VIR_DOMAIN_PAUSED) { - testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "domain not paused"); + if (privdom->state != VIR_DOMAIN_PAUSED) { + testError(domain->conn, domain, NULL, + VIR_ERR_INTERNAL_ERROR, _("domain not paused")); return -1; } - privdom->info.state = VIR_DOMAIN_RUNNING; + privdom->state = VIR_DOMAIN_RUNNING; return (0); } @@ -1063,13 +823,14 @@ { GET_DOMAIN(domain, -1); - if (privdom->info.state == VIR_DOMAIN_SHUTOFF || - privdom->info.state == VIR_DOMAIN_PAUSED) { - testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "domain not running"); + if (privdom->state == VIR_DOMAIN_SHUTOFF || + privdom->state == VIR_DOMAIN_PAUSED) { + testError(domain->conn, domain, NULL, + VIR_ERR_INTERNAL_ERROR, _("domain not running")); return -1; } - privdom->info.state = VIR_DOMAIN_PAUSED; + privdom->state = VIR_DOMAIN_PAUSED; return (0); } @@ -1077,52 +838,50 @@ { GET_DOMAIN(domain, -1); - if (privdom->info.state == VIR_DOMAIN_SHUTOFF) { + if (privdom->state == VIR_DOMAIN_SHUTOFF) { testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "domain not running"); return -1; } - privdom->info.state = VIR_DOMAIN_SHUTOFF; + privdom->state = VIR_DOMAIN_SHUTOFF; domain->id = -1; - privdom->id = -1; + privdom->def->id = -1; return (0); } /* Similar behaviour as shutdown */ -static int testRebootDomain (virDomainPtr domain, virDomainRestart action) +static int testRebootDomain (virDomainPtr domain, + unsigned int action ATTRIBUTE_UNUSED) { GET_DOMAIN(domain, -1); - if (!action) - action = VIR_DOMAIN_RESTART; - - privdom->info.state = VIR_DOMAIN_SHUTDOWN; - switch (action) { - case VIR_DOMAIN_DESTROY: - privdom->info.state = VIR_DOMAIN_SHUTOFF; + privdom->state = VIR_DOMAIN_SHUTDOWN; + switch (privdom->def->onReboot) { + case VIR_DOMAIN_LIFECYCLE_DESTROY: + privdom->state = VIR_DOMAIN_SHUTOFF; domain->id = -1; - privdom->id = -1; + privdom->def->id = -1; break; - case VIR_DOMAIN_RESTART: - privdom->info.state = VIR_DOMAIN_RUNNING; + case VIR_DOMAIN_LIFECYCLE_RESTART: + privdom->state = VIR_DOMAIN_RUNNING; break; - case VIR_DOMAIN_PRESERVE: - privdom->info.state = VIR_DOMAIN_SHUTOFF; + case VIR_DOMAIN_LIFECYCLE_PRESERVE: + privdom->state = VIR_DOMAIN_SHUTOFF; domain->id = -1; - privdom->id = -1; + privdom->def->id = -1; break; - case VIR_DOMAIN_RENAME_RESTART: - privdom->info.state = VIR_DOMAIN_RUNNING; + case VIR_DOMAIN_LIFECYCLE_RESTART_RENAME: + privdom->state = VIR_DOMAIN_RUNNING; break; default: - privdom->info.state = VIR_DOMAIN_SHUTOFF; + privdom->state = VIR_DOMAIN_SHUTOFF; domain->id = -1; - privdom->id = -1; + privdom->def->id = -1; break; } @@ -1140,13 +899,11 @@ return (-1); } - if (privdom->info.state == VIR_DOMAIN_SHUTOFF) { - privdom->info.cpuTime = 0; - privdom->info.memory = 0; - } else { - privdom->info.cpuTime = ((tv.tv_sec * 1000ll * 1000ll * 1000ll) + (tv.tv_usec * 1000ll)); - } - memcpy(info, &privdom->info, sizeof(virDomainInfo)); + info->state = privdom->state; + info->memory = privdom->def->memory; + info->maxMem = privdom->def->maxmem; + info->nrVirtCpu = privdom->def->vcpus; + info->cpuTime = ((tv.tv_sec * 1000ll * 1000ll * 1000ll) + (tv.tv_usec * 1000ll)); return (0); } @@ -1200,12 +957,10 @@ close(fd); return (-1); } - if (privdom->config) { - privdom->info.state = VIR_DOMAIN_SHUTOFF; - privdom->id = -1; - domain->id = -1; - } else { - privdom->active = 0; + privdom->state = VIR_DOMAIN_SHUTOFF; + if (!privdom->persistent) { + virDomainRemoveInactive(&privconn->domains, + privdom); } return 0; } @@ -1215,8 +970,10 @@ { char *xml; char magic[15]; - int fd, len, ret, domid; - GET_CONNECTION(conn, -1); + int fd, len; + virDomainDefPtr def; + virDomainObjPtr dom; + GET_CONNECTION(conn); if ((fd = open(path, O_RDONLY)) < 0) { testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, @@ -1260,10 +1017,19 @@ } xml[len] = '\0'; close(fd); - domid = privconn->nextDomID++; - ret = testLoadDomainFromDoc(conn, domid, xml); + + if ((def = virDomainDefParse(conn, privconn->caps, xml, NULL)) == NULL) { + VIR_FREE(xml); + return -1; + } VIR_FREE(xml); - return ret < 0 ? -1 : 0; + + if ((dom = virDomainAssignDef(conn, &privconn->domains, + def)) == NULL) + return -1; + dom->state = VIR_DOMAIN_RUNNING; + dom->def->id = privconn->nextDomID++; + return dom->def->id; } static int testDomainCoreDump(virDomainPtr domain, @@ -1290,12 +1056,10 @@ close(fd); return (-1); } - if (privdom->config) { - privdom->info.state = VIR_DOMAIN_SHUTOFF; - privdom->id = -1; - domain->id = -1; - } else { - privdom->active = 0; + privdom->state = VIR_DOMAIN_SHUTOFF; + if (!privdom->persistent) { + virDomainRemoveInactive(&privconn->domains, + privdom); } return 0; } @@ -1307,7 +1071,7 @@ static unsigned long testGetMaxMemory(virDomainPtr domain) { GET_DOMAIN(domain, -1); - return privdom->info.maxMem; + return privdom->def->maxmem; } static int testSetMaxMemory(virDomainPtr domain, @@ -1316,7 +1080,7 @@ GET_DOMAIN(domain, -1); /* XXX validate not over host memory wrt to other domains */ - privdom->info.maxMem = memory; + privdom->def->maxmem = memory; return (0); } @@ -1325,12 +1089,13 @@ { GET_DOMAIN(domain, -1); - if (memory > privdom->info.maxMem) { - testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG, __FUNCTION__); + if (memory > privdom->def->maxmem) { + testError(domain->conn, domain, NULL, + VIR_ERR_INVALID_ARG, __FUNCTION__); return (-1); } - privdom->info.memory = memory; + privdom->def->memory = memory; return (0); } @@ -1344,88 +1109,72 @@ return (-1); } - privdom->info.nrVirtCpu = nrCpus; + privdom->def->vcpus = nrCpus; return (0); } -static char *testDomainDumpXML(virDomainPtr domain, int flags ATTRIBUTE_UNUSED) +static char *testDomainDumpXML(virDomainPtr domain, int flags) { - virBuffer buf = VIR_BUFFER_INITIALIZER; - unsigned char *uuid; - char uuidstr[VIR_UUID_STRING_BUFLEN]; + virDomainDefPtr def; GET_DOMAIN(domain, NULL); - virBufferVSprintf(&buf, "<domain type='test' id='%d'>\n", domain->id); - virBufferVSprintf(&buf, " <name>%s</name>\n", domain->name); - uuid = domain->uuid; - virUUIDFormat(uuid, uuidstr); - virBufferVSprintf(&buf, " <uuid>%s</uuid>\n", uuidstr); - virBufferVSprintf(&buf, " <memory>%lu</memory>\n", privdom->info.maxMem); - virBufferVSprintf(&buf, " <vcpu>%d</vcpu>\n", privdom->info.nrVirtCpu); - virBufferVSprintf(&buf, " <on_reboot>%s</on_reboot>\n", testRestartFlagToString(privdom->onReboot)); - virBufferVSprintf(&buf, " <on_poweroff>%s</on_poweroff>\n", testRestartFlagToString(privdom->onPoweroff)); - virBufferVSprintf(&buf, " <on_crash>%s</on_crash>\n", testRestartFlagToString(privdom->onCrash)); + def = (flags & VIR_DOMAIN_XML_INACTIVE) && + privdom->newDef ? privdom->newDef : privdom->def; - virBufferAddLit(&buf, "</domain>\n"); - - if (virBufferError(&buf)) { - testError(domain->conn, domain, NULL, VIR_ERR_NO_MEMORY, __FUNCTION__); - return NULL; - } - - return virBufferContentAndReset(&buf); + return virDomainDefFormat(domain->conn, + def, + (flags & VIR_DOMAIN_XML_SECURE) ? 1 : 0); } static int testNumOfDefinedDomains(virConnectPtr conn) { - int numInactive = 0, i; - GET_CONNECTION(conn, -1); + int numInactive = 0; + virDomainObjPtr dom; + GET_CONNECTION(conn); - for (i = 0 ; i < MAX_DOMAINS ; i++) { - if (!privconn->domains[i].active || - privconn->domains[i].info.state != VIR_DOMAIN_SHUTOFF) - continue; - numInactive++; + dom = privconn->domains; + while (dom) { + if (!virDomainIsActive(dom)) + numInactive++; + dom = dom->next; } - return (numInactive); + return numInactive; } static int testListDefinedDomains(virConnectPtr conn, char **const names, int maxnames) { - int n = 0, i; - GET_CONNECTION(conn, -1); + int n = 0; + virDomainObjPtr dom; + GET_CONNECTION(conn); - for (i = 0, n = 0 ; i < MAX_DOMAINS && n < maxnames ; i++) { - if (privconn->domains[i].active && - privconn->domains[i].info.state == VIR_DOMAIN_SHUTOFF) { - names[n++] = strdup(privconn->domains[i].name); - } + dom = privconn->domains; + while (dom && n < maxnames) { + if (virDomainIsActive(dom)) + names[n++] = strdup(dom->def->name); + dom = dom->next; } - return (n); + return n; } static virDomainPtr testDomainDefineXML(virConnectPtr conn, - const char *doc) { - int handle; - xmlDocPtr xml; - GET_CONNECTION(conn, NULL); + const char *xml) { + virDomainPtr ret; + virDomainDefPtr def; + virDomainObjPtr dom; + GET_CONNECTION(conn); - if (!(xml = xmlReadDoc(BAD_CAST doc, "domain.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) { - testError(conn, NULL, NULL, VIR_ERR_XML_ERROR, _("domain")); - return (NULL); - } + if ((def = virDomainDefParse(conn, privconn->caps, xml, NULL)) == NULL) + return NULL; - handle = testLoadDomain(conn, -1, xml); - privconn->domains[handle].config = 1; + if ((dom = virDomainAssignDef(conn, &privconn->domains, + def)) == NULL) + return NULL; + dom->persistent = 1; + dom->def->id = -1; - xmlFreeDoc(xml); - - if (handle < 0) - return (NULL); - - return virGetDomain(conn, privconn->domains[handle].name, privconn->domains[handle].uuid); + ret = virGetDomain(conn, def->name, def->uuid); + ret->id = -1; + return ret; } static int testNodeGetCellsFreeMemory(virConnectPtr conn, @@ -1433,7 +1182,7 @@ int startCell, int maxCells) { int i, j; - GET_CONNECTION(conn, -1); + GET_CONNECTION(conn); if (startCell > privconn->numCells) { testError(conn, NULL, NULL, VIR_ERR_INVALID_ARG, @@ -1454,14 +1203,14 @@ static int testDomainCreate(virDomainPtr domain) { GET_DOMAIN(domain, -1); - if (privdom->info.state != VIR_DOMAIN_SHUTOFF) { + if (privdom->state != VIR_DOMAIN_SHUTOFF) { testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, _("Domain is already running")); return (-1); } - domain->id = privdom->id = privconn->nextDomID++; - privdom->info.state = VIR_DOMAIN_RUNNING; + domain->id = privdom->def->id = privconn->nextDomID++; + privdom->state = VIR_DOMAIN_RUNNING; return (0); } @@ -1469,13 +1218,15 @@ static int testDomainUndefine(virDomainPtr domain) { GET_DOMAIN(domain, -1); - if (privdom->info.state != VIR_DOMAIN_SHUTOFF) { + if (privdom->state != VIR_DOMAIN_SHUTOFF) { testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, _("Domain is still running")); return (-1); } - privdom->active = 0; + privdom->state = VIR_DOMAIN_SHUTOFF; + virDomainRemoveInactive(&privconn->domains, + privdom); return (0); } @@ -1521,7 +1272,9 @@ } strcpy(params[0].field, "weight"); params[0].type = VIR_DOMAIN_SCHED_FIELD_UINT; - params[0].value.ui = privdom->weight; + /* XXX */ + /*params[0].value.ui = privdom->weight;*/ + params[0].value.ui = 50; return 0; } @@ -1543,7 +1296,8 @@ testError(domain->conn, domain, NULL, VIR_ERR_INVALID_ARG, "type"); return (-1); } - privdom->weight = params[0].value.ui; + /* XXX */ + /*privdom->weight = params[0].value.ui;*/ return 0; } @@ -1568,7 +1322,7 @@ const unsigned char *uuid) { virNetworkObjPtr net = NULL; - GET_CONNECTION(conn, NULL); + GET_CONNECTION(conn); if ((net = virNetworkFindByUUID(privconn->networks, uuid)) == NULL) { testError (conn, NULL, NULL, VIR_ERR_NO_NETWORK, NULL); @@ -1582,7 +1336,7 @@ const char *name) { virNetworkObjPtr net = NULL; - GET_CONNECTION(conn, NULL); + GET_CONNECTION(conn); if ((net = virNetworkFindByName(privconn->networks, name)) == NULL) { testError (conn, NULL, NULL, VIR_ERR_NO_NETWORK, NULL); @@ -1596,7 +1350,7 @@ static int testNumNetworks(virConnectPtr conn) { int numActive = 0; virNetworkObjPtr net; - GET_CONNECTION(conn, -1); + GET_CONNECTION(conn); net = privconn->networks; while (net) { @@ -1610,7 +1364,7 @@ static int testListNetworks(virConnectPtr conn, char **const names, int nnames) { int n = 0; virNetworkObjPtr net; - GET_CONNECTION(conn, -1); + GET_CONNECTION(conn); net = privconn->networks; while (net && n < nnames) { @@ -1624,7 +1378,7 @@ static int testNumDefinedNetworks(virConnectPtr conn) { int numInactive = 0; virNetworkObjPtr net; - GET_CONNECTION(conn, -1); + GET_CONNECTION(conn); net = privconn->networks; while (net) { @@ -1638,7 +1392,7 @@ static int testListDefinedNetworks(virConnectPtr conn, char **const names, int nnames) { int n = 0; virNetworkObjPtr net; - GET_CONNECTION(conn, -1); + GET_CONNECTION(conn); net = privconn->networks; while (net && n < nnames) { @@ -1652,7 +1406,7 @@ static virNetworkPtr testNetworkCreate(virConnectPtr conn, const char *xml) { virNetworkDefPtr def; virNetworkObjPtr net; - GET_CONNECTION(conn, NULL); + GET_CONNECTION(conn); if ((def = virNetworkDefParse(conn, xml, NULL)) == NULL) return NULL; @@ -1668,7 +1422,7 @@ static virNetworkPtr testNetworkDefine(virConnectPtr conn, const char *xml) { virNetworkDefPtr def; virNetworkObjPtr net; - GET_CONNECTION(conn, NULL); + GET_CONNECTION(conn); if ((def = virNetworkDefParse(conn, xml, NULL)) == NULL) return NULL; diff -r 897248d2ee59 tests/read-non-seekable --- a/tests/read-non-seekable Sun Jun 22 17:59:45 2008 -0400 +++ b/tests/read-non-seekable Sun Jun 22 18:02:31 2008 -0400 @@ -31,6 +31,9 @@ <uuid>004b96e1-2d78-c30f-5aa5-000000000000</uuid> <memory>8388608</memory> <vcpu>2</vcpu> + <os> + <type>xen</type> + </os> <on_reboot>restart</on_reboot> <on_poweroff>destroy</on_poweroff> <on_crash>restart</on_crash> diff -r 897248d2ee59 tests/testutils.c --- a/tests/testutils.c Sun Jun 22 17:59:45 2008 -0400 +++ b/tests/testutils.c Sun Jun 22 18:02:31 2008 -0400 @@ -246,9 +246,6 @@ const char *actualStart = actual; const char *actualEnd = actual + (strlen(actual)-1); - if (testOOM < 2) - return 0; - if (!testDebug) return 0; diff -r 897248d2ee59 tests/virshtest.c --- a/tests/virshtest.c Sun Jun 22 17:59:45 2008 -0400 +++ b/tests/virshtest.c Sun Jun 22 18:02:31 2008 -0400 @@ -48,10 +48,6 @@ if (testFilterLine(actualData, filter) < 0) return -1; - if (getenv("DEBUG_TESTS")) { - printf("Expect %d '%s'\n", (int)strlen(expectData), expectData); - printf("Actual %d '%s'\n", (int)strlen(actualData), actualData); - } if (STRNEQ(expectData, actualData)) { virtTestDifference(stderr, expectData, actualData); return -1; -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Tue, Jun 24, 2008 at 05:03:16PM +0100, Daniel P. Berrange wrote:
This patch ports the Test driver over to the domain XML apis.
Basically the 'struct _testDom' is removed, and replaced by usage of the generic virDomainObjPtr and virDomainDefPtr objects. The XML parser and formatters are ripped out and replaced by calls to the generic APIs in domain_conf.h. Finally there is just alot of cleanup of all the individual domain driver implementations to adjust to the style of the new APIs used.
Big cleanup, less code and more functionalities, yay !
This actually dramatically increases the functionality of the test driver - previously its XML parser didn't support any of the <device> section in the XML. This should make it much more useful for advanced apps like virt-manager
and more advanced regression tests like for the bindings.
+ +static const char *defaultDomainXML = +"<domain type='test'>" +" <name>test</name>" +" <memory>8388608</memory>" +" <currentMemory>2097152</currentMemory>" +" <vcpu>2</vcpu>" +" <os>" +" <type>hvm</type>" +" </os>"
shouldn't we add a couple of device (one disk, one net) for good measure here ?
+"</domain>"; + [...] + if (ret > 0) { + for (i = 0 ; i < ret ; i++) { + xmlChar *netFile = xmlGetProp(domains[i], BAD_CAST "file"); + char *absFile = testBuildFilename(file, (const char *)netFile); + VIR_FREE(netFile); + if (!absFile) { + testError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("resolving domain filename")); + goto error; + } + if ((dom = testLoadDomainFromFile(conn, absFile)) == NULL) { + VIR_FREE(absFile); + goto error; + } + dom->state = VIR_DOMAIN_RUNNING; + dom->def->id = privconn->nextDomID++; + dom->persistent = 1; + dom->configFile = absFile; + } + if (domains != NULL) { + VIR_FREE(domains); + domains = NULL; + } }
One thing i'm wondering is if we couldn't do single instance XML definitions for tests including the domains, network, etc... just reusing the new routines to parse up the full set. that way a test could be defined as a single standalone file, we could also easilly get testing of the full set of parsing code just by using the test driver pointing to xml test instances. This just push the actual test configuration one step further, and actually I would not be too annnoyed to break some existing support in the test driver to gte that more complete and convenient coverage. What do you think ? +1 for the patch Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

On Thu, Jun 26, 2008 at 09:57:08AM -0400, Daniel Veillard wrote:
On Tue, Jun 24, 2008 at 05:03:16PM +0100, Daniel P. Berrange wrote:
One thing i'm wondering is if we couldn't do single instance XML definitions for tests including the domains, network, etc... just reusing the new routines to parse up the full set. that way a test could be defined as a single standalone file, we could also easilly get testing of the full set of parsing code just by using the test driver pointing to xml test instances. This just push the actual test configuration one step further, and actually I would not be too annnoyed to break some existing support in the test driver to gte that more complete and convenient coverage.
Yes, for feeding data into the test suite we could easily do that. The master definition file normally looks like this: <node> <!-- This file gives an example config for the mock 'test' backend driver to libvirt. This is intended to allow relible unit testing of applications using libvirt. To use this with virsh, run something like: virsh -connect test:////path/to/this/dir/testnode.xml nodeinfo --> <domain file="testdomfv0.xml"/> <domain file="testdomfc4.xml"/> <network file="testnetpriv.xml"/> <network file="testnetdef.xml"/> <cpu> <mhz>6000</mhz> <model>i986</model> <active>50</active> <nodes>4</nodes> <sockets>4</sockets> <cores>4</cores> <threads>2</threads> </cpu> <memory>8192000</memory> </node> With the API re-factoring you suggested we could easily have the network/domain data inline, rather than referencing an external file - or even support both options. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Thu, Jun 26, 2008 at 02:59:37PM +0100, Daniel P. Berrange wrote:
On Thu, Jun 26, 2008 at 09:57:08AM -0400, Daniel Veillard wrote:
On Tue, Jun 24, 2008 at 05:03:16PM +0100, Daniel P. Berrange wrote:
One thing i'm wondering is if we couldn't do single instance XML definitions for tests including the domains, network, etc... just reusing the new routines to parse up the full set. that way a test could be defined as a single standalone file, we could also easilly get testing of the full set of parsing code just by using the test driver pointing to xml test instances. This just push the actual test configuration one step further, and actually I would not be too annnoyed to break some existing support in the test driver to gte that more complete and convenient coverage.
Yes, for feeding data into the test suite we could easily do that. The master definition file normally looks like this:
<node> <!-- This file gives an example config for the mock 'test' backend driver to libvirt. This is intended to allow relible unit testing of applications using libvirt. To use this with virsh, run something like:
virsh -connect test:////path/to/this/dir/testnode.xml nodeinfo
--> <domain file="testdomfv0.xml"/> <domain file="testdomfc4.xml"/> <network file="testnetpriv.xml"/> <network file="testnetdef.xml"/>
<cpu> <mhz>6000</mhz> <model>i986</model> <active>50</active> <nodes>4</nodes> <sockets>4</sockets> <cores>4</cores> <threads>2</threads> </cpu> <memory>8192000</memory> </node>
With the API re-factoring you suggested we could easily have the network/domain data inline, rather than referencing an external file - or even support both options.
yes, one of the small things to fix is to make sure every place where we do XPath queries we need to use relative queries and reset the context node Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

"Daniel P. Berrange" <berrange@redhat.com> wrote:
This patch ports the Test driver over to the domain XML apis. ... docs/testdomfc4.xml | 2 src/test.c | 1020 ++++++++++++++++++------------------------------ tests/read-non-seekable | 3 tests/testutils.c | 3 tests/virshtest.c | 4 5 files changed, 391 insertions(+), 641 deletions(-) ...
Hi Dan, Nice work. This will make testing a lot easier. I've made a few suggestions, spotted some new bugs and a couple in moved code.
+static virCapsPtr +testBuildCapabilities(virConnectPtr conn) { + virCapsPtr caps; + virCapsGuestPtr guest; + const char *guest_types[] = { "hvm", "xen" };
you can sneak one more "const" in there: const char *const guest_types[] = { "hvm", "xen" };
+ int i; + GET_CONNECTION(conn); + + if ((caps = virCapabilitiesNew(TEST_MODEL, 0, 0)) == NULL) + goto no_memory; + + if (virCapabilitiesAddHostFeature(caps, "pae") < 0) + goto no_memory; + if (virCapabilitiesAddHostFeature(caps ,"nonpae") < 0) + goto no_memory; + + for (i = 0; i < privconn->numCells; ++i) { + if (virCapabilitiesAddHostNUMACell(caps, i, privconn->cells[i].numCpus, + privconn->cells[i].cpus) < 0) + goto no_memory; + } + + for (i = 0; i < (sizeof(guest_types)/sizeof(guest_types[0])); ++i) {
With ARRAY_CARDINALITY it's more readable: for (i = 0; i < ARRAY_CARDINALITY(guest_types); ++i) {
+ if ((guest = virCapabilitiesAddGuest(caps, + guest_types[i], + TEST_MODEL, + TEST_MODEL_WORDSIZE, + TEST_EMULATOR, + NULL, + 0, + NULL)) == NULL) + goto no_memory; ... @@ -538,11 +372,17 @@ xmlNodePtr *domains, *networks = NULL; xmlXPathContextPtr ctxt = NULL; virNodeInfoPtr nodeInfo; + virDomainObjPtr dom; + virNetworkObjPtr net; testConnPtr privconn; if (VIR_ALLOC(privconn) < 0) { testError(NULL, NULL, NULL, VIR_ERR_NO_MEMORY, "testConn"); return VIR_DRV_OPEN_ERROR; } + conn->privateData = privconn; + + if (!(privconn->caps = testBuildCapabilities(conn))) + goto error;
if ((fd = open(file, O_RDONLY)) < 0) { testError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("loading host definition file")); @@ -570,10 +410,7 @@ goto error; }
- - conn->privateData = privconn; privconn->nextDomID = 1; - privconn->numDomains = 0; privconn->numCells = 0; strncpy(privconn->path, file, PATH_MAX-1); privconn->path[PATH_MAX-1] = '\0'; @@ -645,39 +482,35 @@ goto error; }
+ ret = virXPathNodeSet("/node/domain", ctxt, &domains); - if (ret < 0) { - testError(NULL, NULL, NULL, VIR_ERR_XML_ERROR, _("node domain list")); - goto error; + if (ret > 0) { + for (i = 0 ; i < ret ; i++) { + xmlChar *netFile = xmlGetProp(domains[i], BAD_CAST "file");
This needs to handle xmlGetProp failure. [the bug was present before this change: just indented]
+ char *absFile = testBuildFilename(file, (const char *)netFile); + VIR_FREE(netFile); + if (!absFile) { + testError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("resolving domain filename")); + goto error; ... - for (i = 0 ; i < ret ; i++) { - xmlChar *domFile = xmlGetProp(domains[i], BAD_CAST "file"); - char *absFile = testBuildFilename(file, (const char *)domFile); - int domid = privconn->nextDomID++, handle; - VIR_FREE(domFile); - if (!absFile) { - testError(NULL, NULL, NULL, VIR_ERR_INTERNAL_ERROR, _("resolving domain filename")); - goto error; ... static virDomainPtr -testDomainCreateLinux(virConnectPtr conn, const char *xmlDesc, +testDomainCreateLinux(virConnectPtr conn, const char *xml, unsigned int flags ATTRIBUTE_UNUSED) { ...
- dom = virGetDomain(conn, privconn->domains[handle].name, privconn->domains[handle].uuid); - if (dom == NULL) return NULL; - privconn->numDomains++; - return (dom); + ret = virGetDomain(conn, def->name, def->uuid);
Handle virGetDomain failure.
+ ret->id = def->id; + return ret; }
static virDomainPtr testLookupDomainByID(virConnectPtr conn, int id) { ... + ret = virGetDomain(conn, dom->def->name, dom->def->uuid);
Likewise. Handle virGetDomain failure.
+ ret->id = dom->def->id; + return ret; }
static virDomainPtr testLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid) { ... + ret = virGetDomain(conn, dom->def->name, dom->def->uuid);
Likewise.
+ ret->id = dom->def->id; + return ret; }
static virDomainPtr testLookupDomainByName(virConnectPtr conn, const char *name) { ... + ret = virGetDomain(conn, dom->def->name, dom->def->uuid);
Likewise.
+ ret->id = dom->def->id; + return ret; } ... @@ -1215,8 +970,10 @@ { char *xml; char magic[15]; - int fd, len, ret, domid; - GET_CONNECTION(conn, -1); + int fd, len; + virDomainDefPtr def; + virDomainObjPtr dom; + GET_CONNECTION(conn);
if ((fd = open(path, O_RDONLY)) < 0) { testError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, @@ -1260,10 +1017,19 @@ } xml[len] = '\0'; close(fd); - domid = privconn->nextDomID++; - ret = testLoadDomainFromDoc(conn, domid, xml); + + if ((def = virDomainDefParse(conn, privconn->caps, xml, NULL)) == NULL) { + VIR_FREE(xml); + return -1; + } VIR_FREE(xml);
You can save a VIR_FREE: def = virDomainDefParse(conn, privconn->caps, xml, NULL); VIR_FREE(xml); if (def == NULL) return -1; ...
static int testListDefinedDomains(virConnectPtr conn, char **const names, int maxnames) { - int n = 0, i; - GET_CONNECTION(conn, -1); + int n = 0; + virDomainObjPtr dom; + GET_CONNECTION(conn);
- for (i = 0, n = 0 ; i < MAX_DOMAINS && n < maxnames ; i++) { - if (privconn->domains[i].active && - privconn->domains[i].info.state == VIR_DOMAIN_SHUTOFF) { - names[n++] = strdup(privconn->domains[i].name); - } + dom = privconn->domains; + while (dom && n < maxnames) { + if (virDomainIsActive(dom)) + names[n++] = strdup(dom->def->name);
This could be the same problem I pointed out in testListDefinedNetworks: when strdup fails, some caller may dereference the NULL pointer we record here. Either skip any NULL pointers as I suggested before, or return -1 to indicate the error (and update all callers).
+ dom = dom->next; } - return (n); + return n; }
static virDomainPtr testDomainDefineXML(virConnectPtr conn, ... - if (handle < 0) - return (NULL); - - return virGetDomain(conn, privconn->domains[handle].name, privconn->domains[handle].uuid); + ret = virGetDomain(conn, def->name, def->uuid);
Possible NULL deref.
+ ret->id = -1; + return ret; } ...
participants (3)
-
Daniel P. Berrange
-
Daniel Veillard
-
Jim Meyering