[libvirt] [PATCH v4 0/7] bridge network support enhancement and other network fixes
v2-v4 change: Addressed Michal Privoznik's comments Maxim Nestratov (7): parallels: introduce and use string constants for network types and names parallels: fix parallelsLoadNetworks parallels: better bridge network interface support parallels: set correct network adapter device state parallels: make E1000 network adapter model default parallels: switch off offline management feature parallels: don't prevent domain define if VIR_DOMAIN_NET_TYPE_BRIDGE
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_network.c | 6 +++--- src/parallels/parallels_sdk.c | 6 +++--- src/parallels/parallels_utils.h | 8 +++++++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c index 1d3b694..bb7ec5e 100644 --- a/src/parallels/parallels_network.c +++ b/src/parallels/parallels_network.c @@ -211,12 +211,12 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj) goto cleanup; } - if (STREQ(tmp, "bridged")) { + if (STREQ(tmp, PARALLELS_BRIDGED_NETWORK_TYPE)) { def->forward.type = VIR_NETWORK_FORWARD_BRIDGE; if (parallelsGetBridgedNetInfo(def, jobj) < 0) goto cleanup; - } else if (STREQ(tmp, "host-only")) { + } else if (STREQ(tmp, PARALLELS_HOSTONLY_NETWORK_TYPE)) { def->forward.type = VIR_NETWORK_FORWARD_NONE; if (parallelsGetHostOnlyNetInfo(def, def->name) < 0) @@ -248,7 +248,7 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn) def->forward.type = VIR_NETWORK_FORWARD_ROUTE; - if (VIR_STRDUP(def->name, PARALLELS_ROUTED_NETWORK_NAME) < 0) + if (VIR_STRDUP(def->name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) < 0) goto cleanup; if (virUUIDParse(PARALLELS_ROUTED_NETWORK_UUID, def->uuid) < 0) { diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index a0b62cd..d3fb5d4 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -708,7 +708,7 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt) * always up */ net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP; if (VIR_STRDUP(net->data.network.name, - PARALLELS_ROUTED_NETWORK_NAME) < 0) + PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) < 0) goto cleanup; return 0; } @@ -727,7 +727,7 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt) if (emulatedType == PNA_ROUTED) { if (VIR_STRDUP(net->data.network.name, - PARALLELS_ROUTED_NETWORK_NAME) < 0) + PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) < 0) goto cleanup; } else { pret = PrlVmDevNet_GetVirtualNetworkId(netAdapter, NULL, &buflen); @@ -2658,8 +2658,8 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, virDomainNetDefPtr net) pret = PrlVmDevNet_SetMacAddress(sdknet, macstr); prlsdkCheckRetGoto(pret, cleanup); - if (STREQ(net->data.network.name, PARALLELS_ROUTED_NETWORK_NAME)) { pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED); + if (STREQ(net->data.network.name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) { prlsdkCheckRetGoto(pret, cleanup); } else { pret = PrlVmDevNet_SetVirtualNetworkId(sdknet, net->data.network.name); diff --git a/src/parallels/parallels_utils.h b/src/parallels/parallels_utils.h index 394548a..0f29374 100644 --- a/src/parallels/parallels_utils.h +++ b/src/parallels/parallels_utils.h @@ -47,7 +47,13 @@ _("no domain with matching uuid '%s'"), uuidstr); \ } while (0) -# define PARALLELS_ROUTED_NETWORK_NAME "Routed" +# define PARALLELS_DOMAIN_ROUTED_NETWORK_NAME "Routed" +# define PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME "Bridged" + +# define PARALLELS_REQUIRED_HOSTONLY_NETWORK "Host-Only" +# define PARALLELS_HOSTONLY_NETWORK_TYPE "host-only" +# define PARALLELS_REQUIRED_BRIDGED_NETWORK "Bridged" +# define PARALLELS_BRIDGED_NETWORK_TYPE "bridged" struct _parallelsConn { virMutex lock; -- 1.7.1
Don't fail initialization of parallels driver if parallelsLoadNetwork fails for optional networks. This can happen when some of them are added manually and configured incompletely. PCS requires only two networks created automatically (named Host-Only and Bridged), others are optional and their incompletenes can be ignored. Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_network.c | 43 +++++++++++++++++++++++------------- 1 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c index bb7ec5e..1d0ee1c 100644 --- a/src/parallels/parallels_network.c +++ b/src/parallels/parallels_network.c @@ -180,9 +180,10 @@ static int parallelsGetHostOnlyNetInfo(virNetworkDefPtr def, const char *name) return ret; } -static virNetworkObjPtr +static int parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj) { + int ret = -1; virNetworkObjPtr net; virNetworkDefPtr def; const char *tmp; @@ -214,13 +215,25 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj) if (STREQ(tmp, PARALLELS_BRIDGED_NETWORK_TYPE)) { def->forward.type = VIR_NETWORK_FORWARD_BRIDGE; - if (parallelsGetBridgedNetInfo(def, jobj) < 0) + if (parallelsGetBridgedNetInfo(def, jobj) < 0) { + + /* Only mandatory networks are required to be configured completely */ + if (STRNEQ(def->name, PARALLELS_REQUIRED_BRIDGED_NETWORK)) + ret = 0; + goto cleanup; + } } else if (STREQ(tmp, PARALLELS_HOSTONLY_NETWORK_TYPE)) { def->forward.type = VIR_NETWORK_FORWARD_NONE; - if (parallelsGetHostOnlyNetInfo(def, def->name) < 0) + if (parallelsGetHostOnlyNetInfo(def, def->name) < 0) { + + /* Only mandatory networks are required to be configured completely */ + if (STRNEQ(def->name, PARALLELS_REQUIRED_HOSTONLY_NETWORK)) + ret = 0; + goto cleanup; + } } else { parallelsParseError(); goto cleanup; @@ -230,14 +243,16 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj) goto cleanup; net->active = 1; net->autostart = 1; - return net; + virNetworkObjEndAPI(&net); + ret = 0; + def = NULL; cleanup: virNetworkDefFree(def); - return NULL; + return ret; } -static virNetworkObjPtr +static int parallelsAddRoutedNetwork(parallelsConnPtr privconn) { virNetworkObjPtr net = NULL; @@ -264,18 +279,18 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn) } net->active = 1; net->autostart = 1; + virNetworkObjEndAPI(&net); - return net; + return 0; cleanup: virNetworkDefFree(def); - return NULL; + return -1; } static int parallelsLoadNetworks(parallelsConnPtr privconn) { virJSONValuePtr jobj, jobj2; - virNetworkObjPtr net = NULL; int ret = -1; int count; size_t i; @@ -300,22 +315,18 @@ static int parallelsLoadNetworks(parallelsConnPtr privconn) goto cleanup; } - net = parallelsLoadNetwork(privconn, jobj2); - if (!net) + ret = parallelsLoadNetwork(privconn, jobj2); + if (!ret) goto cleanup; - else - virNetworkObjEndAPI(&net); - } - if (!(net = parallelsAddRoutedNetwork(privconn))) + if (!(ret = parallelsAddRoutedNetwork(privconn))) goto cleanup; ret = 0; cleanup: virJSONValueFree(jobj); - virNetworkObjEndAPI(&net); return ret; } -- 1.7.1
On 18.03.2015 09:33, Maxim Nestratov wrote:
Don't fail initialization of parallels driver if parallelsLoadNetwork fails for optional networks. This can happen when some of them are added manually and configured incompletely. PCS requires only two networks created automatically (named Host-Only and Bridged), others are optional and their incompletenes can be ignored.
s/incompletenes/incompleteness/
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_network.c | 43 +++++++++++++++++++++++------------- 1 files changed, 27 insertions(+), 16 deletions(-)
diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c index bb7ec5e..1d0ee1c 100644 --- a/src/parallels/parallels_network.c +++ b/src/parallels/parallels_network.c @@ -180,9 +180,10 @@ static int parallelsGetHostOnlyNetInfo(virNetworkDefPtr def, const char *name) return ret; }
-static virNetworkObjPtr +static int parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj) { + int ret = -1; virNetworkObjPtr net; virNetworkDefPtr def; const char *tmp; @@ -214,13 +215,25 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj) if (STREQ(tmp, PARALLELS_BRIDGED_NETWORK_TYPE)) { def->forward.type = VIR_NETWORK_FORWARD_BRIDGE;
- if (parallelsGetBridgedNetInfo(def, jobj) < 0) + if (parallelsGetBridgedNetInfo(def, jobj) < 0) { + + /* Only mandatory networks are required to be configured completely */ + if (STRNEQ(def->name, PARALLELS_REQUIRED_BRIDGED_NETWORK)) + ret = 0; + goto cleanup; + } } else if (STREQ(tmp, PARALLELS_HOSTONLY_NETWORK_TYPE)) { def->forward.type = VIR_NETWORK_FORWARD_NONE;
- if (parallelsGetHostOnlyNetInfo(def, def->name) < 0) + if (parallelsGetHostOnlyNetInfo(def, def->name) < 0) { + + /* Only mandatory networks are required to be configured completely */ + if (STRNEQ(def->name, PARALLELS_REQUIRED_HOSTONLY_NETWORK)) + ret = 0; + goto cleanup; + } } else { parallelsParseError(); goto cleanup; @@ -230,14 +243,16 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj) goto cleanup; net->active = 1; net->autostart = 1; - return net; + virNetworkObjEndAPI(&net); + ret = 0; + def = NULL;
So if everything goes well, zero is returned ...
cleanup: virNetworkDefFree(def); - return NULL; + return ret; }
-static virNetworkObjPtr +static int parallelsAddRoutedNetwork(parallelsConnPtr privconn) { virNetworkObjPtr net = NULL; @@ -264,18 +279,18 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn) } net->active = 1; net->autostart = 1; + virNetworkObjEndAPI(&net);
- return net; + return 0;
cleanup: virNetworkDefFree(def);
OUCH! Not to be seen in this context, but if virNetworkAssignDef() called a few lines above fails, it'll call virNetworkDefFree() and goto 'cleanup' subsequently where @def is freed the second time.
- return NULL; + return -1; }
static int parallelsLoadNetworks(parallelsConnPtr privconn) { virJSONValuePtr jobj, jobj2; - virNetworkObjPtr net = NULL; int ret = -1; int count; size_t i; @@ -300,22 +315,18 @@ static int parallelsLoadNetworks(parallelsConnPtr privconn) goto cleanup; }
- net = parallelsLoadNetwork(privconn, jobj2); - if (!net) + ret = parallelsLoadNetwork(privconn, jobj2); + if (!ret)
... so this should have been (ret) instead of (!ret).
goto cleanup; - else - virNetworkObjEndAPI(&net); - }
- if (!(net = parallelsAddRoutedNetwork(privconn))) + if (!(ret = parallelsAddRoutedNetwork(privconn)))
Again, parallelsAddRoutedNetwork() returns zero on success.
goto cleanup;
ret = 0;
cleanup: virJSONValueFree(jobj); - virNetworkObjEndAPI(&net); return ret; }
I'm squashing this in: diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c index 1d0ee1c..8cc0582 100644 --- a/src/parallels/parallels_network.c +++ b/src/parallels/parallels_network.c @@ -184,7 +184,7 @@ static int parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj) { int ret = -1; - virNetworkObjPtr net; + virNetworkObjPtr net = NULL; virNetworkDefPtr def; const char *tmp; /* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */ @@ -241,13 +241,13 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj) if (!(net = virNetworkAssignDef(privconn->networks, def, false))) goto cleanup; + def = NULL; net->active = 1; net->autostart = 1; - virNetworkObjEndAPI(&net); ret = 0; - def = NULL; cleanup: + virNetworkObjEndAPI(&net); virNetworkDefFree(def); return ret; } @@ -273,10 +273,9 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn) } def->uuid_specified = 1; - if (!(net = virNetworkAssignDef(privconn->networks, def, false))) { - virNetworkDefFree(def); + if (!(net = virNetworkAssignDef(privconn->networks, def, false))) goto cleanup; - } + net->active = 1; net->autostart = 1; virNetworkObjEndAPI(&net); @@ -315,12 +314,11 @@ static int parallelsLoadNetworks(parallelsConnPtr privconn) goto cleanup; } - ret = parallelsLoadNetwork(privconn, jobj2); - if (!ret) + if (parallelsLoadNetwork(privconn, jobj2) < 0) goto cleanup; } - if (!(ret = parallelsAddRoutedNetwork(privconn))) + if (parallelsAddRoutedNetwork(privconn) < 0) goto cleanup; ret = 0; Michal
In order to support 'bridge' network adapters in parallels driver we need to plug our veth devices into corresponding linux bridges. We are going to do this by reusing our abstraction of Virtual Networks in terms of PCS. On a domain creation, we create a new Virtual Network naming it with the same name as a source bridge for each network interface. Having done this, we plug PCS veth interfaces created with names of target dev into specified bridges using our standard PCS procedures Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_sdk.c | 99 +++++++++++++++++++++++++++++++++++----- 1 files changed, 86 insertions(+), 13 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index d3fb5d4..58c52a8 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -691,9 +691,6 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt) /* use device name, shown by prlctl as target device * for identifying network adapter in virDomainDefineXML */ - pret = PrlVmDev_GetIndex(netAdapter, &netAdapterIndex); - prlsdkCheckRetGoto(pret, cleanup); - pret = PrlVmDevNet_GetHostInterfaceName(netAdapter, NULL, &buflen); prlsdkCheckRetGoto(pret, cleanup); @@ -703,6 +700,9 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt) pret = PrlVmDevNet_GetHostInterfaceName(netAdapter, net->ifname, &buflen); prlsdkCheckRetGoto(pret, cleanup); + pret = PrlVmDev_GetIndex(netAdapter, &netAdapterIndex); + prlsdkCheckRetGoto(pret, cleanup); + if (isCt && netAdapterIndex == (PRL_UINT32) -1) { /* venet devices don't have mac address and * always up */ @@ -740,6 +740,16 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt) net->data.network.name, &buflen); prlsdkCheckRetGoto(pret, cleanup); + + /* + * We use VIR_DOMAIN_NET_TYPE_NETWORK for all network adapters + * except those whose Virtual Network Id differ from Parallels + * predefined ones such as PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME + * and PARALLELS_DONAIN_ROUTED_NETWORK_NAME + */ + if (!STREQ(net->data.network.name, PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) + net->type = VIR_DOMAIN_NET_TYPE_BRIDGE; + } pret = PrlVmDev_IsConnected(netAdapter, &isConnected); @@ -2630,10 +2640,12 @@ static const char * prlsdkFormatMac(virMacAddrPtr mac, char *macstr) return macstr; } -static int prlsdkAddNet(PRL_HANDLE sdkdom, virDomainNetDefPtr net) +static int prlsdkAddNet(PRL_HANDLE sdkdom, parallelsConnPtr privconn, virDomainNetDefPtr net) { PRL_RESULT pret; PRL_HANDLE sdknet = PRL_INVALID_HANDLE; + PRL_HANDLE vnet = PRL_INVALID_HANDLE; + PRL_HANDLE job = PRL_INVALID_HANDLE; int ret = -1; char macstr[PRL_MAC_STRING_BUFNAME]; @@ -2658,10 +2670,39 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, virDomainNetDefPtr net) pret = PrlVmDevNet_SetMacAddress(sdknet, macstr); prlsdkCheckRetGoto(pret, cleanup); - pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED); - if (STREQ(net->data.network.name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) { + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { + if (STREQ(net->data.network.name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) { + pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED); + prlsdkCheckRetGoto(pret, cleanup); + } else if (STREQ(net->data.network.name, PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) { + pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGED_ETHERNET); + prlsdkCheckRetGoto(pret, cleanup); + + pret = PrlVmDevNet_SetVirtualNetworkId(sdknet, net->data.network.name); + prlsdkCheckRetGoto(pret, cleanup); + } + } else if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { + /* + * For this type of adapter we create a new + * Virtual Network assuming that bridge with given name exists + * Failing creating this means domain creation failure + */ + pret = PrlVirtNet_Create(&vnet); prlsdkCheckRetGoto(pret, cleanup); - } else { + + pret = PrlVirtNet_SetNetworkId(vnet, net->data.network.name); + prlsdkCheckRetGoto(pret, cleanup); + + pret = PrlVirtNet_SetNetworkType(vnet, PVN_BRIDGED_ETHERNET); + prlsdkCheckRetGoto(pret, cleanup); + + job = PrlSrv_AddVirtualNetwork(privconn->server, vnet, 0); + if (PRL_FAILED(pret = waitJob(job, privconn->jobTimeout))) + goto cleanup; + + pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGED_ETHERNET); + prlsdkCheckRetGoto(pret, cleanup); + pret = PrlVmDevNet_SetVirtualNetworkId(sdknet, net->data.network.name); prlsdkCheckRetGoto(pret, cleanup); } @@ -2674,10 +2715,34 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, virDomainNetDefPtr net) ret = 0; cleanup: + PrlHandle_Free(vnet); PrlHandle_Free(sdknet); return ret; } +static void prlsdkDelNet(parallelsConnPtr privconn, virDomainNetDefPtr net) +{ + PRL_RESULT pret; + PRL_HANDLE vnet = PRL_INVALID_HANDLE; + PRL_HANDLE job = PRL_INVALID_HANDLE; + + if (net->type != VIR_DOMAIN_NET_TYPE_BRIDGE) + return; + + pret = PrlVirtNet_Create(&vnet); + prlsdkCheckRetGoto(pret, cleanup); + + pret = PrlVirtNet_SetNetworkId(vnet, net->data.network.name); + prlsdkCheckRetGoto(pret, cleanup); + + PrlSrv_DeleteVirtualNetwork(privconn->server, vnet, 0); + if (PRL_FAILED(pret = waitJob(job, privconn->jobTimeout))) + goto cleanup; + + cleanup: + PrlHandle_Free(vnet); +} + static int prlsdkAddDisk(PRL_HANDLE sdkdom, virDomainDiskDefPtr disk, bool bootDisk) { PRL_RESULT pret; @@ -2849,7 +2914,8 @@ prlsdkAddFS(PRL_HANDLE sdkdom, virDomainFSDefPtr fs) return ret; } static int -prlsdkDoApplyConfig(PRL_HANDLE sdkdom, +prlsdkDoApplyConfig(virConnectPtr conn, + PRL_HANDLE sdkdom, virDomainDefPtr def) { PRL_RESULT pret; @@ -2924,8 +2990,8 @@ prlsdkDoApplyConfig(PRL_HANDLE sdkdom, } for (i = 0; i < def->nnets; i++) { - if (prlsdkAddNet(sdkdom, def->nets[i]) < 0) - goto error; + if (prlsdkAddNet(sdkdom, conn->privateData, def->nets[i]) < 0) + goto error; } for (i = 0; i < def->ndisks; i++) { @@ -2951,6 +3017,9 @@ prlsdkDoApplyConfig(PRL_HANDLE sdkdom, error: VIR_FREE(mask); + for (i = 0; i < def->nnets; i++) + prlsdkDelNet(conn->privateData, def->nets[i]); + return -1; } @@ -2972,7 +3041,7 @@ prlsdkApplyConfig(virConnectPtr conn, if (PRL_FAILED(waitJob(job, privconn->jobTimeout))) return -1; - ret = prlsdkDoApplyConfig(sdkdom, new); + ret = prlsdkDoApplyConfig(conn, sdkdom, new); if (ret == 0) { job = PrlVm_CommitEx(sdkdom, PVCF_DETACH_HDD_BUNDLE); @@ -3009,7 +3078,7 @@ prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def) pret = PrlVmCfg_SetDefaultConfig(sdkdom, srvconf, PVS_GUEST_VER_LIN_REDHAT, 0); prlsdkCheckRetGoto(pret, cleanup); - ret = prlsdkDoApplyConfig(sdkdom, def); + ret = prlsdkDoApplyConfig(conn, sdkdom, def); if (ret) goto cleanup; @@ -3071,7 +3140,7 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def) } - ret = prlsdkDoApplyConfig(sdkdom, def); + ret = prlsdkDoApplyConfig(conn, sdkdom, def); if (ret) goto cleanup; @@ -3090,6 +3159,10 @@ prlsdkUnregisterDomain(parallelsConnPtr privconn, virDomainObjPtr dom) { parallelsDomObjPtr privdom = dom->privateData; PRL_HANDLE job; + size_t i; + + for (i = 0; i < dom->def->nnets; i++) + prlsdkDelNet(privconn, dom->def->nets[i]); job = PrlVm_Unreg(privdom->sdkdom); if (PRL_FAILED(waitJob(job, privconn->jobTimeout))) -- 1.7.1
On 18.03.2015 09:33, Maxim Nestratov wrote:
In order to support 'bridge' network adapters in parallels driver we need to plug our veth devices into corresponding linux bridges. We are going to do this by reusing our abstraction of Virtual Networks in terms of PCS. On a domain creation, we create a new Virtual Network naming it with the same name as a source bridge for each network interface. Having done this, we plug PCS veth interfaces created with names of target dev into specified bridges using our standard PCS procedures
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_sdk.c | 99 +++++++++++++++++++++++++++++++++++----- 1 files changed, 86 insertions(+), 13 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index d3fb5d4..58c52a8 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -691,9 +691,6 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt)
/* use device name, shown by prlctl as target device * for identifying network adapter in virDomainDefineXML */ - pret = PrlVmDev_GetIndex(netAdapter, &netAdapterIndex); - prlsdkCheckRetGoto(pret, cleanup); - pret = PrlVmDevNet_GetHostInterfaceName(netAdapter, NULL, &buflen); prlsdkCheckRetGoto(pret, cleanup);
@@ -703,6 +700,9 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt) pret = PrlVmDevNet_GetHostInterfaceName(netAdapter, net->ifname, &buflen); prlsdkCheckRetGoto(pret, cleanup);
+ pret = PrlVmDev_GetIndex(netAdapter, &netAdapterIndex); + prlsdkCheckRetGoto(pret, cleanup); + if (isCt && netAdapterIndex == (PRL_UINT32) -1) { /* venet devices don't have mac address and * always up */ @@ -740,6 +740,16 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, virDomainNetDefPtr net, bool isCt) net->data.network.name, &buflen); prlsdkCheckRetGoto(pret, cleanup); + + /* + * We use VIR_DOMAIN_NET_TYPE_NETWORK for all network adapters + * except those whose Virtual Network Id differ from Parallels + * predefined ones such as PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME + * and PARALLELS_DONAIN_ROUTED_NETWORK_NAME + */ + if (!STREQ(net->data.network.name, PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME))
s/!STREQ/STRNEQ/ I'll fix that when pushing. Michal
when a new network adapter device is added Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_sdk.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 58c52a8..d699613 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -2658,7 +2658,9 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, parallelsConnPtr privconn, virDomainN pret = PrlVmDev_SetEnabled(sdknet, 1); prlsdkCheckRetGoto(pret, cleanup); - pret = PrlVmDev_SetConnected(sdknet, net->linkstate); + pret = PrlVmDev_SetConnected(sdknet, net->linkstate != + VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN); + prlsdkCheckRetGoto(pret, cleanup); if (net->ifname) { -- 1.7.1
On 18.03.2015 09:33, Maxim Nestratov wrote:
when a new network adapter device is added
s/device/link/ in $SUBJ.
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_sdk.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 58c52a8..d699613 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -2658,7 +2658,9 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, parallelsConnPtr privconn, virDomainN pret = PrlVmDev_SetEnabled(sdknet, 1); prlsdkCheckRetGoto(pret, cleanup);
- pret = PrlVmDev_SetConnected(sdknet, net->linkstate); + pret = PrlVmDev_SetConnected(sdknet, net->linkstate != + VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN); + prlsdkCheckRetGoto(pret, cleanup);
if (net->ifname) {
Michal
and set adapter model specified in xml Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_driver.c | 7 ++++++- src/parallels/parallels_sdk.c | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 1 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 1a6d6e6..807c1b1 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -168,13 +168,18 @@ parallelsDomainDefPostParse(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; } - static int parallelsDomainDeviceDefPostParse(virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED, const virDomainDef *def ATTRIBUTE_UNUSED, virCapsPtr caps ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { + if (dev->type == VIR_DOMAIN_DEVICE_NET && + (dev->data.net->type == VIR_DOMAIN_NET_TYPE_NETWORK || + dev->data.net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) && + !dev->data.net->model) + return VIR_STRDUP(dev->data.net->model, "e1000"); + return 0; } diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index d699613..8deebd4 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -2672,6 +2672,21 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, parallelsConnPtr privconn, virDomainN pret = PrlVmDevNet_SetMacAddress(sdknet, macstr); prlsdkCheckRetGoto(pret, cleanup); + if (STREQ(net->model, "rtl8139")) + pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_RTL); + else if (STREQ(net->model, "e1000")) + pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_E1000); + else if (STREQ(net->model, "virtio")) + pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_VIRTIO); + else { + + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Specified network adapter model is not " + "supported by Parallels Cloud Server.")); + goto cleanup; + } + prlsdkCheckRetGoto(pret, cleanup); + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) { if (STREQ(net->data.network.name, PARALLELS_DOMAIN_ROUTED_NETWORK_NAME)) { pret = PrlVmDev_SetEmulatedType(sdknet, PNA_ROUTED); -- 1.7.1
On 18.03.2015 09:33, Maxim Nestratov wrote:
and set adapter model specified in xml
Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_driver.c | 7 ++++++- src/parallels/parallels_sdk.c | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 1 deletions(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 1a6d6e6..807c1b1 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -168,13 +168,18 @@ parallelsDomainDefPostParse(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; }
- static int parallelsDomainDeviceDefPostParse(virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED,
@dev is now clearly used.
const virDomainDef *def ATTRIBUTE_UNUSED, virCapsPtr caps ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { + if (dev->type == VIR_DOMAIN_DEVICE_NET && + (dev->data.net->type == VIR_DOMAIN_NET_TYPE_NETWORK || + dev->data.net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) && + !dev->data.net->model) + return VIR_STRDUP(dev->data.net->model, "e1000"); + return 0; }
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index d699613..8deebd4 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -2672,6 +2672,21 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, parallelsConnPtr privconn, virDomainN pret = PrlVmDevNet_SetMacAddress(sdknet, macstr); prlsdkCheckRetGoto(pret, cleanup);
+ if (STREQ(net->model, "rtl8139")) + pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_RTL); + else if (STREQ(net->model, "e1000")) + pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_E1000); + else if (STREQ(net->model, "virtio")) + pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_VIRTIO); + else {
We have this rule, that if one arm of if-else is in curly braces, the other one has to be too. Even if it is a oneliner.
+
Whitespace.
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Specified network adapter model is not " + "supported by Parallels Cloud Server.")); + goto cleanup; + } + prlsdkCheckRetGoto(pret, cleanup); +
Awesome! The code is much cleaner now. Michal
which is on by default when a new VM/CT is created. We should do this because this feature can't be controlled by libvirt now and it sets up some iptables rules. So it's better to do this to avoid potential conflict of different set of rules or to avoid unexpected behavior. Signed-off-by: Maxim Nestratov <mnestratov@parallels.com> --- src/parallels/parallels_sdk.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 8deebd4..3140757 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -3095,6 +3095,9 @@ prlsdkCreateVm(virConnectPtr conn, virDomainDefPtr def) pret = PrlVmCfg_SetDefaultConfig(sdkdom, srvconf, PVS_GUEST_VER_LIN_REDHAT, 0); prlsdkCheckRetGoto(pret, cleanup); + pret = PrlVmCfg_SetOfflineManagementEnabled(sdkdom, 0); + prlsdkCheckRetGoto(pret, cleanup); + ret = prlsdkDoApplyConfig(conn, sdkdom, def); if (ret) goto cleanup; -- 1.7.1
network adapter is used --- src/parallels/parallels_sdk.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c index 3140757..1a448f1 100644 --- a/src/parallels/parallels_sdk.c +++ b/src/parallels/parallels_sdk.c @@ -2251,7 +2251,8 @@ static int prlsdkCheckSerialUnsupportedParams(virDomainChrDefPtr chr) static int prlsdkCheckNetUnsupportedParams(virDomainNetDefPtr net) { - if (net->type != VIR_DOMAIN_NET_TYPE_NETWORK) { + if (net->type != VIR_DOMAIN_NET_TYPE_NETWORK && + net->type != VIR_DOMAIN_NET_TYPE_BRIDGE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Specified network adapter type is not " "supported by Parallels Cloud Server.")); -- 1.7.1
On 18.03.2015 09:33, Maxim Nestratov wrote:
v2-v4 change: Addressed Michal Privoznik's comments
Maxim Nestratov (7): parallels: introduce and use string constants for network types and names parallels: fix parallelsLoadNetworks parallels: better bridge network interface support parallels: set correct network adapter device state parallels: make E1000 network adapter model default parallels: switch off offline management feature parallels: don't prevent domain define if VIR_DOMAIN_NET_TYPE_BRIDGE
I've fixed all the nits I've found, ACKed and pushed whole series. One more thing though, look at the rest of our commit messages. They usually consist of small subject, and start freshly with new sentence. For some reason we don't like if the sentence form subject continues at the start of commit message. Michal
18.03.2015 20:34, Michal Privoznik пишет:
I've fixed all the nits I've found, ACKed and pushed whole series.
Thank you!
One more thing though, look at the rest of our commit messages. They usually consist of small subject, and start freshly with new sentence. For some reason we don't like if the sentence form subject continues at the start of commit message.
Michal
Sure. Will do next time.
participants (2)
-
Maxim Nestratov -
Michal Privoznik