[libvirt] [PATCH 3/3] networking API for hostonly networks in VirtualBox driver in libvirt

[Daniel P. Berrange]

On Wed, May 06, 2009 at 06:12:21PM +0200, Pritesh Kothari wrote:
> +
> +static virNetworkPtr vboxNetworkCreateXML(virConnectPtr conn, const char *xml) {
> +    vboxGlobalData *data  = conn->privateData;
> +    virNetworkDefPtr def  = NULL;
> +    virNetworkPtr ret     = NULL;
> +    nsID *iid             = NULL;
> +    char *networkNameUtf8 = NULL;
> +    int i = 0;
> +
> +    if ((def = virNetworkDefParseString(conn, xml)) == NULL)
> +        goto cleanup;
> +
> +    if (VIR_ALLOC(iid) < 0) {
> +        virReportOOMError(conn);
> +        goto cleanup;
> +    }
> +
> +    if (VIR_ALLOC_N(networkNameUtf8, sizeof("HostInterfaceNetworking-") +
> +                                     sizeof(def->name) + 1) < 0) {

Fairly sure that should be 'strlen()' there.

> +        virReportOOMError(conn);
> +        goto cleanup;
> +    }
> +
> +    strcpy (networkNameUtf8, "HostInterfaceNetworking-");
> +    strcat (networkNameUtf8, def->name);

That said, how about  just using virAsprintf(&nmetworkNameUtf8..
instead of alloc+strcpy

> +
> +    nsIDFromChar(iid, def->uuid);
> +
> +    DEBUG("Network Name: %s", def->name);
> +    DEBUG("Network UUID: "
> +          "{%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x}",
> +          (unsigned)iid->m0,    (unsigned)iid->m1,
> +          (unsigned)iid->m2,    (unsigned)iid->m3[0],
> +          (unsigned)iid->m3[1], (unsigned)iid->m3[2],
> +          (unsigned)iid->m3[3], (unsigned)iid->m3[4],
> +          (unsigned)iid->m3[5], (unsigned)iid->m3[6],
> +          (unsigned)iid->m3[7]);
> +    DEBUG("bridge : %s", def->bridge);
> +    DEBUG("domain : %s", def->domain);
> +    DEBUG("forwardType : %d", def->forwardType);
> +    DEBUG("forwardDev : %s", def->forwardDev);
> +    DEBUG("ipAddress : %s", def->ipAddress);
> +    DEBUG("netmask : %s", def->netmask);
> +    DEBUG("network : %s", def->network);
> +    for (i = 0; i < def->nranges; i++) {
> +        DEBUG("DHCP(%d) start: %s", i, def->ranges[i].start);
> +        DEBUG("DHCP(%d) end  : %s", i, def->ranges[i].end);
> +    }
> +    for (i = 0; i < def->nhosts; i++) {
> +        DEBUG("DHCP Host(%d) mac : %s", i, def->hosts[i].mac);
> +        DEBUG("DHCP Host(%d) name: %s", i, def->hosts[i].name);
> +        DEBUG("DHCP Host(%d) ip  : %s", i, def->hosts[i].ip);
> +    }

This is rather overkill, since we already log the entire XML document 
passed into the public API which has all this info

> +
> +static virNetworkPtr vboxNetworkDefineXML(virConnectPtr conn, const char *xml) {
> +    vboxGlobalData *data  = conn->privateData;
> +    virNetworkDefPtr def  = NULL;
> +    virNetworkPtr ret     = NULL;
> +    nsID *iid             = NULL;
> +    char *networkNameUtf8 = NULL;
> +    int i = 0;
> +
> +    /* vboxNetworkDefineXML() is not exactly "network definition"
> +     * as the network is up and running, only the DHCP server is off,
> +     * so you can always assign static IP and get the network running.
> +     */
> +    if ((def = virNetworkDefParseString(conn, xml)) == NULL)
> +        goto cleanup;
> +
> +    if (VIR_ALLOC(iid) < 0) {
> +        virReportOOMError(conn);
> +        goto cleanup;
> +    }
> +
> +    if (VIR_ALLOC_N(networkNameUtf8, sizeof("HostInterfaceNetworking-") +
> +                                     sizeof(def->name) + 1) < 0) {
> +        virReportOOMError(conn);
> +        goto cleanup;
> +    }
> +
> +    strcpy (networkNameUtf8, "HostInterfaceNetworking-");
> +    strcat (networkNameUtf8, def->name);

Same note here, as before, and is most other methods that follow


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 :|