10:52 a.m.
From: Maxim Nestratov <mnestratov(a)parallels.com>
v2 change:
- rebased on recent network rework
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 network adapter device status to connected
parallels: make E1000 network adapter type default
parallels: switch off offline management feature
parallels: don't prevent domain define if VIR_DOMAIN_NET_TYPE_BRIDGE
10:52 a.m.
New subject: [libvirt] [PATCH 1/7] parallels: introduce and use string constants for network types and names
Signed-off-by: Maxim Nestratov <mnestratov(a)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 89a9a58..76cbb02 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);
@@ -2653,8 +2653,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
9:15 a.m.
New subject: [libvirt] [PATCH 1/7] parallels: introduce and use string constants for network types and names
On 13.03.2015 16:52, Maxim Nestratov wrote:
Signed-off-by: Maxim Nestratov <mnestratov(a)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;
How about introducing 'int parallelsNetworkTypeFromString(const char *)'
using our ENUM_DECL and ENUM_IMPL macros? This code could then be turned
into:
if ((def->forward.type = parallelNetworkTypeFromString(tmp)) < 0) {
parallelsParseError();
goto cleanup;
}
switch((virNetworkForwardType) def->forward.type) {
case VIR_NETWORK_FORWARD_BRIDGE:
if (parallelsGetBridgedNetInfo(def, jobj) < 0)
goto cleanup;
break;
case VIR_NETWORK_FORWARD_NONE:
if (parallelsGetHostOnlyNetInfo(def, def->name) < 0)
goto cleanup;
...
}
I find it more future proof then STREQ() spaghetti.
Michal
9:25 a.m.
New subject: [libvirt] [PATCH 1/7] parallels: introduce and use string constants for network types and names
17.03.2015 17:15, Michal Privoznik пишет:
On 13.03.2015 16:52, Maxim Nestratov wrote:
> Signed-off-by: Maxim Nestratov <mnestratov(a)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;
>
How about introducing 'int parallelsNetworkTypeFromString(const char *)'
using our ENUM_DECL and ENUM_IMPL macros? This code could then be turned
into:
if ((def->forward.type = parallelNetworkTypeFromString(tmp)) < 0) {
parallelsParseError();
goto cleanup;
}
switch((virNetworkForwardType) def->forward.type) {
case VIR_NETWORK_FORWARD_BRIDGE:
if (parallelsGetBridgedNetInfo(def, jobj) < 0)
goto cleanup;
break;
case VIR_NETWORK_FORWARD_NONE:
if (parallelsGetHostOnlyNetInfo(def, def->name) < 0)
goto cleanup;
...
}
I find it more future proof then STREQ() spaghetti.
Michal
Ok. Makes sence. I'll do this in the next series version.
9:56 a.m.
New subject: [libvirt] [PATCH 1/7] parallels: introduce and use string constants for network types and names
On 17.03.2015 15:25, Maxim Nestratov wrote:
17.03.2015 17:15, Michal Privoznik пишет:
> On 13.03.2015 16:52, Maxim Nestratov wrote:
>> Signed-off-by: Maxim Nestratov <mnestratov(a)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;
>>
> How about introducing 'int parallelsNetworkTypeFromString(const char *)'
> using our ENUM_DECL and ENUM_IMPL macros? This code could then be turned
> into:
>
> if ((def->forward.type = parallelNetworkTypeFromString(tmp)) < 0) {
> parallelsParseError();
> goto cleanup;
> }
>
> switch((virNetworkForwardType) def->forward.type) {
> case VIR_NETWORK_FORWARD_BRIDGE:
> if (parallelsGetBridgedNetInfo(def, jobj) < 0)
> goto cleanup;
> break;
> case VIR_NETWORK_FORWARD_NONE:
> if (parallelsGetHostOnlyNetInfo(def, def->name) < 0)
> goto cleanup;
> ...
> }
>
> I find it more future proof then STREQ() spaghetti.
>
> Michal
Ok. Makes sence. I'll do this in the next series version.
D'oh. Now that I'm going through the code again, this patch actually
makes sense and my suggestion doesn't. The problem is, initially I
thought that @tmp holds network type in string, while in fact it holds a
network's name. And it doesn't make much sense to create an enum for
that. Although, other patches in the series need some discussion before
I can push them.
Michal
10:52 a.m.
New subject: [libvirt] [PATCH 2/7] parallels: fix parallelsLoadNetworks
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(a)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..4dc7115 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 required to be configured completely */
+ if (!STREQ(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 required to be configured completely */
+ if (!STREQ(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;
+ return ret;
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
9:15 a.m.
New subject: [libvirt] [PATCH 2/7] parallels: fix parallelsLoadNetworks
On 13.03.2015 16:52, 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.
Signed-off-by: Maxim Nestratov <mnestratov(a)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..4dc7115 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 required to be configured completely */
s/networks required/networks are required/
+ if (!STREQ(def->name,
PARALLELS_REQUIRED_BRIDGED_NETWORK))
s/!STREQ/STRNEQ/
+ 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 required to be configured completely */
+ if (!STREQ(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;
+ return ret;
This doesn't make much sense. drop return, let it fall through cleanup,
and move virNetworkObjEndAPI() under the label. Of course, @def should
be set to NULL as soon as virNetworkAssignDef() succeeds.
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;
}
ACK with nits fixed.
Michal
10:52 a.m.
New subject: [libvirt] [PATCH 3/7] parallels: better bridge network interface support
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(a)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 76cbb02..f581fbb 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);
@@ -2625,10 +2635,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];
@@ -2653,10 +2665,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);
}
@@ -2669,10 +2710,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;
@@ -2844,7 +2909,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;
@@ -2904,8 +2970,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++) {
@@ -2931,6 +2997,9 @@ prlsdkDoApplyConfig(PRL_HANDLE sdkdom,
error:
VIR_FREE(mask);
+ for (i = 0; i < def->nnets; i++)
+ prlsdkDelNet(conn->privateData, def->nets[i]);
+
return -1;
}
@@ -2952,7 +3021,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);
@@ -2989,7 +3058,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;
@@ -3051,7 +3120,7 @@ prlsdkCreateCt(virConnectPtr conn, virDomainDefPtr def)
}
- ret = prlsdkDoApplyConfig(sdkdom, def);
+ ret = prlsdkDoApplyConfig(conn, sdkdom, def);
if (ret)
goto cleanup;
@@ -3070,6 +3139,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
9:14 a.m.
New subject: [libvirt] [PATCH 3/7] parallels: better bridge network interface support
On 13.03.2015 16:52, 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(a)parallels.com>
---
src/parallels/parallels_sdk.c | 99 +++++++++++++++++++++++++++++++++++-----
1 files changed, 86 insertions(+), 13 deletions(-)
ACK
Michal
10:52 a.m.
New subject: [libvirt] [PATCH 4/7] parallels: set network adapter device status to connected
when a new network adapter device is added
Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com>
---
src/parallels/parallels_sdk.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index f581fbb..9588163 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2653,7 +2653,7 @@ 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, 1);
prlsdkCheckRetGoto(pret, cleanup);
if (net->ifname) {
--
1.7.1
9:14 a.m.
New subject: [libvirt] [PATCH 4/7] parallels: set network adapter device status to connected
On 13.03.2015 16:52, Maxim Nestratov wrote:
when a new network adapter device is added
Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com>
---
src/parallels/parallels_sdk.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index f581fbb..9588163 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2653,7 +2653,7 @@ 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, 1);
prlsdkCheckRetGoto(pret, cleanup);
if (net->ifname) {
Why? net->linkstate contains the link state requested by user in XML. I
think we should honour user's configuration.
Michal
10:52 a.m.
New subject: [libvirt] [PATCH 5/7] parallels: make E1000 network adapter type default
Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com>
---
src/parallels/parallels_sdk.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 9588163..748a308 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2665,6 +2665,10 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, parallelsConnPtr
privconn, virDomainN
pret = PrlVmDevNet_SetMacAddress(sdknet, macstr);
prlsdkCheckRetGoto(pret, cleanup);
+ /* Other alternatives: PNT_VIRTIO, PNT_RTL */
+ pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_E1000);
+ 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
9:14 a.m.
New subject: [libvirt] [PATCH 5/7] parallels: make E1000 network adapter type default
On 13.03.2015 16:52, Maxim Nestratov wrote:
Signed-off-by: Maxim Nestratov <mnestratov(a)parallels.com>
---
src/parallels/parallels_sdk.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 9588163..748a308 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2665,6 +2665,10 @@ static int prlsdkAddNet(PRL_HANDLE sdkdom, parallelsConnPtr
privconn, virDomainN
pret = PrlVmDevNet_SetMacAddress(sdknet, macstr);
prlsdkCheckRetGoto(pret, cleanup);
+ /* Other alternatives: PNT_VIRTIO, PNT_RTL */
+ pret = PrlVmDevNet_SetAdapterType(sdknet, PNT_E1000);
+ 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);
I'd appreciate if this would go through XML. There's option for
hypervisor drivers to register a callback to fill in defaults after XML
is parsed. That's perfect place to set e1000 as default vNIC model (if
not already provided by user in XML). And here just set model requested
in XML (or throw an error if unsupported).
Michal
10:52 a.m.
New subject: [libvirt] [PATCH 6/7] parallels: switch off offline management feature
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(a)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 748a308..a0a2ba0 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -3062,6 +3062,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
9:14 a.m.
New subject: [libvirt] [PATCH 6/7] parallels: switch off offline management feature
On 13.03.2015 16:52, Maxim Nestratov wrote:
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(a)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 748a308..a0a2ba0 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -3062,6 +3062,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;
ACK, libvirt has it's own subsystem for managing firewall.
Michal
10:52 a.m.
New subject: [libvirt] [PATCH 7/7] parallels: don't prevent domain define if VIR_DOMAIN_NET_TYPE_BRIDGE
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 a0a2ba0..4c90a18 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2246,7 +2246,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
9:14 a.m.
New subject: [libvirt] [PATCH 7/7] parallels: don't prevent domain define if VIR_DOMAIN_NET_TYPE_BRIDGE
On 13.03.2015 16:52, Maxim Nestratov wrote:
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 a0a2ba0..4c90a18 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -2246,7 +2246,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."));
I really, really hate how the whole 'is this configuration supported'
thing is handled in parallels. Instead of enumerating what it does not
know, it should do it the other way round. Do a positive checking, if
configuration is supported. Then, even if we introduce yet another
device, or device attribute in other hypervisors, we don't need to
update parallels driver (which we permanently keep forgetting about).
ACK though.
Michal
12:09 p.m.
New subject: [libvirt] [PATCH 7/7] parallels: don't prevent domain define if VIR_DOMAIN_NET_TYPE_BRIDGE
17.03.2015 17:14, Michal Privoznik пишет:
On 13.03.2015 16:52, Maxim Nestratov wrote:
> 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 a0a2ba0..4c90a18 100644
> --- a/src/parallels/parallels_sdk.c
> +++ b/src/parallels/parallels_sdk.c
> @@ -2246,7 +2246,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."));
>
I really, really hate how the whole 'is this configuration supported'
thing is handled in parallels. Instead of enumerating what it does not
know, it should do it the other way round. Do a positive checking, if
configuration is supported. Then, even if we introduce yet another
device, or device attribute in other hypervisors, we don't need to
update parallels driver (which we permanently keep forgetting about).
ACK though.
Michal
Michal, I got your message and absolutely agree with you. We suffer from
this a lot and we'll rework this approach shortly.
Thank you for pointing this out.
Maxim
8:31 a.m.
New subject: [libvirt] [PATCH 7/7] parallels: don't prevent domain define if VIR_DOMAIN_NET_TYPE_BRIDGE
On 03/17/2015 05:14 PM, Michal Privoznik wrote:
On 13.03.2015 16:52, Maxim Nestratov wrote:
> 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 a0a2ba0..4c90a18 100644
> --- a/src/parallels/parallels_sdk.c
> +++ b/src/parallels/parallels_sdk.c
> @@ -2246,7 +2246,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."));
>
I really, really hate how the whole 'is this configuration supported'
thing is handled in parallels. Instead of enumerating what it does not
know, it should do it the other way round. Do a positive checking, if
configuration is supported. Then, even if we introduce yet another
device, or device attribute in other hypervisors, we don't need to
update parallels driver (which we permanently keep forgetting about).
I also don't like, that this function will fail, if some unsupported
option is provided. It was long time ago when Peter told me, that I
should report error, if there are some unsupported parameters:
https://www.redhat.com/archives/libvir-list/2012-July/msg00602.html.
It should be possible to define a domain if some unsupported parameters
are provided, which wouldn't affect domain behaviour very much.
ACK though.
Michal
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
Dmitry Guryanov
3538
days inactive
3543
days old
18 comments
3 participants
participants (3)
-
Dmitry Guryanov -
Maxim Nestratov -
Michal Privoznik