On Wed, Sep 17, 2008 at 08:28:56PM +0400, Evgeniy Sokolov wrote:

I attached patch without hunk which was commited by Daniel. Please commit if you are agree with it.

A few comments inline..

Index: openvz_conf.c =================================================================== RCS file: /data/cvs/libvirt/src/openvz_conf.c,v retrieving revision 1.38 diff -u -p -r1.38 openvz_conf.c --- openvz_conf.c 5 Sep 2008 15:00:14 -0000 1.38 +++ openvz_conf.c 8 Sep 2008 11:38:48 -0000 @@ -151,6 +151,148 @@ char *openvzMacToString(const unsigned c return strdup(str); }

+/*parse MAC from view: 00:18:51:8F:D9:F3 + return -1 - error + 0 - OK +*/ +int openvzParseMac(const char *macaddr, unsigned char *mac) +{ + int ret; + ret = sscanf((const char *)macaddr, "%02X:%02X:%02X:%02X:%02X:%02X", + (unsigned int*)&mac[0], + (unsigned int*)&mac[1], + (unsigned int*)&mac[2], + (unsigned int*)&mac[3], + (unsigned int*)&mac[4], + (unsigned int*)&mac[5]) ; + if (ret == 6) + return 0; + + return -1; +} + +virDomainNetDefPtr +openvzReadNetworkConf(virConnectPtr conn, int veid) { + int ret; + virDomainNetDefPtr net = NULL; + virDomainNetDefPtr new_net; + char temp[4096]; + char *token, *saveptr = NULL; + + /*parse routing network configuration* + * Sample from config: + * IP_ADDRESS="1.1.1.1 1.1.1.2" + * splited IPs by space + */ + ret = openvzReadConfigParam(veid, "IP_ADDRESS", temp, sizeof(temp)); + if (ret < 0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Cound not read 'IP_ADDRESS' from config for container %d"), + veid); + goto error; + } else if (ret > 0) { + token = strtok_r(temp, " ", &saveptr); + while (token != NULL) { + new_net = NULL; + if (VIR_ALLOC(new_net) < 0) { + openvzError(conn, VIR_ERR_NO_MEMORY, _("virDomainNetDefPtr")); + goto error; + } + new_net->next = net; + net = new_net; + + net->type = VIR_DOMAIN_NET_TYPE_ETHERNET; + net->data.ethernet.ipaddr = strdup(token); + + token = strtok_r(NULL, " ", &saveptr); + } + } + + /*parse bridge devices*/ + /*Sample from config: + *NETIF="ifname=eth10,mac=00:18:51:C1:05:EE,host_ifname=veth105.10,host_mac=00:18:51:8F:D9:F3" + *devices splited by ';' + */ + ret = openvzReadConfigParam(veid, "NETIF", temp, sizeof(temp)); + if (ret < 0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Cound not read 'NETIF' from config for container %d"), + veid); + goto error; + } else if (ret > 0) { + token = strtok_r(temp, ";", &saveptr); + while (token != NULL) { + /*add new device to list*/ + new_net = NULL; + if (VIR_ALLOC(new_net) < 0) { + openvzError(conn, VIR_ERR_NO_MEMORY, _("virDomainNetDefPtr")); + goto error; + } + new_net->next = net; + net = new_net; + + net->type = VIR_DOMAIN_NET_TYPE_BRIDGE; + + char *p = token, *next = token; + char cpy_temp[32]; + int len; + + /*parse string*/ + do { + while (*next != '\0' && *next != ',') next++; + if (STREQLEN("ifname=", p, 7)) {

You can optionally use STRPREFIX(p, "ifname=") for this kind of scenario

+ p += 7; + len = next - p; + if (len > 16) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Too long network device name")); + goto error; + } + strncpy(cpy_temp, p, len); + cpy_temp[len] = '\0'; + net->data.bridge.brname = strdup(cpy_temp); + + if (net->data.bridge.brname == NULL) { + openvzError(conn, VIR_ERR_NO_MEMORY, _("Can't strdup")); + goto error; + }

Why do you need to use the intermediate buffer for the value ? It would be a little simpler todo something like len = next - p; if (VIR_ALLOC_N(net->data.bridge.brname, len+1) < 0) goto no_memory; strncpy(net->data.bridge.brname, p, len); net->data.bridge.brname[len] = '\0'; So you're not artifically restricting the max length of the network name.

+ + } else if (STREQLEN("host_ifname=", p, 12)) { + p += 12; + //skip in libvirt + } else if (STREQLEN("mac=", p, 4)) { + p += 4; + len = next - p; + if (len != 17) { //should be 17 + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Wrong length MAC address")); + goto error; + } + strncpy(cpy_temp, p, len); + cpy_temp[len] = '\0'; + if (openvzParseMac(cpy_temp, net->mac)<0) { + openvzError(conn, VIR_ERR_INTERNAL_ERROR, + _("Wrong MAC address")); + goto error; + } + } else if (STREQLEN("host_mac=", p, 9)) { + p += 9; + //skip in libvirt + } + p = ++next; + } while (p < token + strlen(token)); + + token = strtok_r(NULL, ";", &saveptr); + } + } + + return net;

no_memory: openvzError(conn, VIR_ERR_NO_MEMORY, NULL); /* fallthrough to error: */

+error: + virDomainNetDefFree(net); + return NULL; +} + + /* Free all memory associated with a openvz_driver structure */ void openvzFreeDriver(struct openvz_driver *driver) @@ -243,6 +385,8 @@ int openvzLoadDomains(struct openvz_driv

/* XXX load rest of VM config data .... */

+ dom->def->nets = openvzReadNetworkConf(NULL, veid); + if (prev) { prev->next = dom; } else { Index: openvz_conf.h =================================================================== RCS file: /data/cvs/libvirt/src/openvz_conf.h,v retrieving revision 1.12 diff -u -p -r1.12 openvz_conf.h --- openvz_conf.h 5 Sep 2008 14:10:58 -0000 1.12 +++ openvz_conf.h 8 Sep 2008 11:38:48 -0000 @@ -60,6 +60,9 @@ void openvzFreeDriver(struct openvz_driv int strtoI(const char *str); int openvzCheckEmptyMac(const unsigned char *mac); char *openvzMacToString(const unsigned char *mac); +int openvzParseMac(const char *macaddr, unsigned char *mac); int openvzSetDefinedUUID(int vpsid, unsigned char *uuid); +virDomainNetDefPtr openvzReadNetworkConf(virConnectPtr conn, int veid);

Since this API is only ever called from within openvz_conf.c, there's no need to have it in the header. Just make it a static function in openvz_conf.c 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 :|