[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/1] Added Networking API to change/create/etc. hostonly/internal network in VirtualBox



On Wed, Apr 29, 2009 at 01:14:07PM +0200, Pritesh Kothari wrote:
> Hi All:
> 
> [PATCH 0/1]: Contains sample xml file showing features supported so far.
> [PATCH 1/1]: Contains the patch for adding Networking API to 
> change/create/etc. hostonly/internal network in VirtualBox

First up, now I see the way this works out, I think this is the correct
approach for hostonly networks, but I think I was wrong about internal
networking - your original proposal for the 'internal' network worked
better.

So, just remove the 'TODO internal network' bits from this patch and
go back to adding a type='internal' to the domain_conf.c parser for
<interface> tags.

> diff --git a/src/network_conf.c b/src/network_conf.c
> index b4da3fb..b4e0b39 100644
> --- a/src/network_conf.c
> +++ b/src/network_conf.c
> @@ -50,7 +50,11 @@ VIR_ENUM_DECL(virNetworkForward)
>  
>  VIR_ENUM_IMPL(virNetworkForward,
>                VIR_NETWORK_FORWARD_LAST,
> -              "none", "nat", "route" )
> +              "none",
> +              "nat",
> +              "route",
> +              "hostonly",
> +              "internal")
>  
>  #define virNetworkReportError(conn, code, fmt...)                            \
>          virReportErrorHelper(conn, VIR_FROM_NETWORK, code, __FILE__,       \
> diff --git a/src/network_conf.h b/src/network_conf.h
> index 365d469..32db7c2 100644
> --- a/src/network_conf.h
> +++ b/src/network_conf.h
> @@ -36,6 +36,8 @@ enum virNetworkForwardType {
>      VIR_NETWORK_FORWARD_NONE   = 0,
>      VIR_NETWORK_FORWARD_NAT,
>      VIR_NETWORK_FORWARD_ROUTE,
> +    VIR_NETWORK_FORWARD_HOSTONLY,
> +    VIR_NETWORK_FORWARD_INTERNAL,
>  
>      VIR_NETWORK_FORWARD_LAST,
>  };

We shouldn't change these two files - just use VIR_NETWORK_FORWARD_NONE
for the hostonly network.



> +static int vboxNumOfNetworks(virConnectPtr conn) {
> +    vboxGlobalData *data = conn->privateData;
> +    int numActive = 0;
> +
> +    /* TODO: "internal" networks are just strings and
> +     * thus can't do much with them
> +     */
> +    if (data->vboxObj) {
> +        IHost *host = NULL;
> +
> +        data->vboxObj->vtbl->GetHost(data->vboxObj, &host);
> +        if (host) {
> +            int i = 0;
> +            PRUint32 networkInterfacesSize = 0;
> +            IHostNetworkInterface **networkInterfaces = NULL;
> +
> +            host->vtbl->GetNetworkInterfaces(host, &networkInterfacesSize, &networkInterfaces);
> +
> +            for (i = 0; i < networkInterfacesSize; i++) {
> +                if (networkInterfaces[i]) {
> +                    PRUint32 interfaceType = 0;
> +
> +                    networkInterfaces[i]->vtbl->GetInterfaceType(networkInterfaces[i], &interfaceType);
> +                    if (interfaceType == HostNetworkInterfaceType_HostOnly)
> +                        numActive++;
> +
> +                    networkInterfaces[i]->vtbl->nsisupports.Release((nsISupports *) networkInterfaces[i]);
> +                }
> +            }
> +
> +            host->vtbl->nsisupports.Release((nsISupports *) host);
> +        }
> +    }
> +
> +    DEBUG("numActive: %d", numActive);
> +    return numActive;
> +}
> +
> +static int vboxListNetworks(virConnectPtr conn, char **const names, int nnames) {
> +    vboxGlobalData *data = conn->privateData;
> +    int numActive = 0;
> +
> +    /* TODO: "internal" networks are just strings and
> +     * thus can't do much with them
> +     */
> +    if (data->vboxObj) {
> +        IHost *host = NULL;
> +
> +        data->vboxObj->vtbl->GetHost(data->vboxObj, &host);
> +        if (host) {
> +            int i = 0;
> +            PRUint32 networkInterfacesSize = 0;
> +            IHostNetworkInterface **networkInterfaces = NULL;
> +
> +            host->vtbl->GetNetworkInterfaces(host, &networkInterfacesSize, &networkInterfaces);
> +
> +            for (i = 0; (numActive < nnames) && (i < networkInterfacesSize); i++) {
> +                if (networkInterfaces[i]) {
> +                    PRUint32 interfaceType = 0;
> +
> +                    networkInterfaces[i]->vtbl->GetInterfaceType(networkInterfaces[i], &interfaceType);
> +
> +                    if (interfaceType == HostNetworkInterfaceType_HostOnly) {
> +                        char *nameUtf8       = NULL;
> +                        PRUnichar *nameUtf16 = NULL;
> +
> +                        networkInterfaces[i]->vtbl->GetName(networkInterfaces[i], &nameUtf16);
> +                        data->pFuncs->pfnUtf16ToUtf8(nameUtf16, &nameUtf8);
> +
> +                        DEBUG("nnames[%d]: %s", numActive, nameUtf8);
> +                        names[numActive++] = strdup(nameUtf8);
> +
> +                        data->pFuncs->pfnUtf8Free(nameUtf8);
> +                        data->pFuncs->pfnUtf16Free(nameUtf16);
> +                    }
> +                }
> +            }
> +
> +            for (i = 0; i < networkInterfacesSize; i++) {
> +                if (networkInterfaces[i]) {
> +                    networkInterfaces[i]->vtbl->nsisupports.Release((nsISupports *) networkInterfaces[i]);
> +                }
> +            }
> +
> +            host->vtbl->nsisupports.Release((nsISupports *) host);
> +        }
> +    }
> +
> +    return numActive;
> +}

Trying this out on my test box, it appears that the network interface 
for the hostonly network is kept offline, until a guest is started.
We should match this in the virNetwork API driver impl here. So if the
interface is offline, report it in  an impl of  vboxListDefinedNetworks
and vboxNumOfDefinedNetworks, instead of these 2.

This would make the 'virsh net-list --inactive' operation work.



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]