Re: [libvirt] [PATCH 1/3] vz: support type=bridge network interface type correctly

[Maxim Nestratov]

27-Oct-16 16:58, Maxim Nestratov пишет:


24-Oct-16 09:46, Nikolay Shirokovskiy пишет:



On 21.10.2016 18:20, Maxim Nestratov wrote:

07-Sep-16 14:00, Nikolay Shirokovskiy пишет:


On 05.09.2016 19:39, Maxim Nestratov wrote:

Recently, libprlsdk got a separate flag PNA_BRIDGE corresponding to
type=bridge libvirt network interfaces. Let's use it and get rid of
all workarounds previously added to  support it.

Signed-off-by: Maxim Nestratov 
---
   src/vz/vz_sdk.c | 100 

   1 file changed, 14 insertions(+), 86 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f2a5c96..933b222 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1004,27 +1004,19 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, 
virDomainNetDefPtr net, bool isCt)
PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) < 0)
   goto cleanup;
   } else {
-char *netid = NULL;
-
-if (!(netid =
+char *netid =
prlsdkGetStringParamVar(PrlVmDevNet_GetVirtualNetworkId,
-  netAdapter)))
-goto cleanup;
+  netAdapter);
   -/*
- * 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 (STRNEQ(netid, PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) {
+if (emulatedType == PNA_BRIDGE) {
   net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
-net->data.network.name = netid;
+if (netid)
+net->data.bridge.brname = netid;
   } else {
   net->type = VIR_DOMAIN_NET_TYPE_NETWORK;
-net->data.bridge.brname = netid;
+if (netid)
+net->data.network.name = netid;

not sure why check for not NULL here and above,

Assuming that I fix this and below, I have your ACK on the first two patches, 
right?

Maxim

Yes.

I just realized you removed NULL check on PrlVmDevNet_GetVirtualNetworkId. NULL 
is not a error case?

Nikolay


Yes, it is not, as it can be changed later and we don't want to prevent such 
domains to be listed.

Maxim



Pushed now.

Maxim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] vz: support type=bridge network interface type correctly

[Maxim Nestratov]

24-Oct-16 09:46, Nikolay Shirokovskiy пишет:



On 21.10.2016 18:20, Maxim Nestratov wrote:

07-Sep-16 14:00, Nikolay Shirokovskiy пишет:


On 05.09.2016 19:39, Maxim Nestratov wrote:

Recently, libprlsdk got a separate flag PNA_BRIDGE corresponding to
type=bridge libvirt network interfaces. Let's use it and get rid of
all workarounds previously added to  support it.

Signed-off-by: Maxim Nestratov 
---
   src/vz/vz_sdk.c | 100 

   1 file changed, 14 insertions(+), 86 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f2a5c96..933b222 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1004,27 +1004,19 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, 
virDomainNetDefPtr net, bool isCt)
  PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) < 0)
   goto cleanup;
   } else {
-char *netid = NULL;
-
-if (!(netid =
+char *netid =
 prlsdkGetStringParamVar(PrlVmDevNet_GetVirtualNetworkId,
-  netAdapter)))
-goto cleanup;
+  netAdapter);
   -/*
- * 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 (STRNEQ(netid, PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) {
+if (emulatedType == PNA_BRIDGE) {
   net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
-net->data.network.name = netid;
+if (netid)
+net->data.bridge.brname = netid;
   } else {
   net->type = VIR_DOMAIN_NET_TYPE_NETWORK;
-net->data.bridge.brname = netid;
+if (netid)
+net->data.network.name = netid;

not sure why check for not NULL here and above,

Assuming that I fix this and below, I have your ACK on the first two patches, 
right?

Maxim

Yes.

I just realized you removed NULL check on PrlVmDevNet_GetVirtualNetworkId. NULL 
is not a error case?

Nikolay


Yes, it is not, as it can be changed later and we don't want to prevent such 
domains to be listed.

Maxim

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] vz: support type=bridge network interface type correctly

[Nikolay Shirokovskiy]

On 21.10.2016 18:20, Maxim Nestratov wrote:
> 07-Sep-16 14:00, Nikolay Shirokovskiy пишет:
> 
>>
>> On 05.09.2016 19:39, Maxim Nestratov wrote:
>>> Recently, libprlsdk got a separate flag PNA_BRIDGE corresponding to
>>> type=bridge libvirt network interfaces. Let's use it and get rid of
>>> all workarounds previously added to  support it.
>>>
>>> Signed-off-by: Maxim Nestratov 
>>> ---
>>>   src/vz/vz_sdk.c | 100 
>>> 
>>>   1 file changed, 14 insertions(+), 86 deletions(-)
>>>
>>> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
>>> index f2a5c96..933b222 100644
>>> --- a/src/vz/vz_sdk.c
>>> +++ b/src/vz/vz_sdk.c
>>> @@ -1004,27 +1004,19 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, 
>>> virDomainNetDefPtr net, bool isCt)
>>>  PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) < 0)
>>>   goto cleanup;
>>>   } else {
>>> -char *netid = NULL;
>>> -
>>> -if (!(netid =
>>> +char *netid =
>>> prlsdkGetStringParamVar(PrlVmDevNet_GetVirtualNetworkId,
>>> -  netAdapter)))
>>> -goto cleanup;
>>> +  netAdapter);
>>>   -/*
>>> - * 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 (STRNEQ(netid, PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) {
>>> +if (emulatedType == PNA_BRIDGE) {
>>>   net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
>>> -net->data.network.name = netid;
>>> +if (netid)
>>> +net->data.bridge.brname = netid;
>>>   } else {
>>>   net->type = VIR_DOMAIN_NET_TYPE_NETWORK;
>>> -net->data.bridge.brname = netid;
>>> +if (netid)
>>> +net->data.network.name = netid;
>> not sure why check for not NULL here and above,
> 
> Assuming that I fix this and below, I have your ACK on the first two patches, 
> right?
> 
> Maxim

Yes. 

I just realized you removed NULL check on PrlVmDevNet_GetVirtualNetworkId. NULL 
is not a error case?

Nikolay

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] vz: support type=bridge network interface type correctly

[Maxim Nestratov]

07-Sep-16 14:00, Nikolay Shirokovskiy пишет:



On 05.09.2016 19:39, Maxim Nestratov wrote:

Recently, libprlsdk got a separate flag PNA_BRIDGE corresponding to
type=bridge libvirt network interfaces. Let's use it and get rid of
all workarounds previously added to  support it.

Signed-off-by: Maxim Nestratov 
---
  src/vz/vz_sdk.c | 100 
  1 file changed, 14 insertions(+), 86 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f2a5c96..933b222 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1004,27 +1004,19 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, 
virDomainNetDefPtr net, bool isCt)
 PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) < 0)
  goto cleanup;
  } else {
-char *netid = NULL;
-
-if (!(netid =
+char *netid =
prlsdkGetStringParamVar(PrlVmDevNet_GetVirtualNetworkId,
-  netAdapter)))
-goto cleanup;
+  netAdapter);
  
-/*

- * 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 (STRNEQ(netid, PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) {
+if (emulatedType == PNA_BRIDGE) {
  net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
-net->data.network.name = netid;
+if (netid)
+net->data.bridge.brname = netid;
  } else {
  net->type = VIR_DOMAIN_NET_TYPE_NETWORK;
-net->data.bridge.brname = netid;
+if (netid)
+net->data.network.name = netid;

not sure why check for not NULL here and above,


Assuming that I fix this and below, I have your ACK on the first two patches, 
right?

Maxim



1.) now i see my mistake introduced in 3a82c04c, setting net->data need to be 
swapped)


  }
-

here is my usual pedantic comment


  }
  
  if (!isCt) {

[snip]


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] vz: support type=bridge network interface type correctly

[Nikolay Shirokovskiy]

On 05.09.2016 19:39, Maxim Nestratov wrote:
> Recently, libprlsdk got a separate flag PNA_BRIDGE corresponding to
> type=bridge libvirt network interfaces. Let's use it and get rid of
> all workarounds previously added to  support it.
> 
> Signed-off-by: Maxim Nestratov 
> ---
>  src/vz/vz_sdk.c | 100 
> 
>  1 file changed, 14 insertions(+), 86 deletions(-)
> 
> diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
> index f2a5c96..933b222 100644
> --- a/src/vz/vz_sdk.c
> +++ b/src/vz/vz_sdk.c
> @@ -1004,27 +1004,19 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, 
> virDomainNetDefPtr net, bool isCt)
> PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) < 0)
>  goto cleanup;
>  } else {
> -char *netid = NULL;
> -
> -if (!(netid =
> +char *netid =
>prlsdkGetStringParamVar(PrlVmDevNet_GetVirtualNetworkId,
> -  netAdapter)))
> -goto cleanup;
> +  netAdapter);
>  
> -/*
> - * 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 (STRNEQ(netid, PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) {
> +if (emulatedType == PNA_BRIDGE) {
>  net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
> -net->data.network.name = netid;
> +if (netid)
> +net->data.bridge.brname = netid;
>  } else {
>  net->type = VIR_DOMAIN_NET_TYPE_NETWORK;
> -net->data.bridge.brname = netid;
> +if (netid)
> +net->data.network.name = netid;

not sure why check for not NULL here and above,

1.) now i see my mistake introduced in 3a82c04c, setting net->data need to be 
swapped)

>  }
> -

here is my usual pedantic comment

>  }
>  
>  if (!isCt) {

[snip]

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/3] vz: support type=bridge network interface type correctly

[Maxim Nestratov]

Recently, libprlsdk got a separate flag PNA_BRIDGE corresponding to
type=bridge libvirt network interfaces. Let's use it and get rid of
all workarounds previously added to  support it.

Signed-off-by: Maxim Nestratov 
---
 src/vz/vz_sdk.c | 100 
 1 file changed, 14 insertions(+), 86 deletions(-)

diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f2a5c96..933b222 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1004,27 +1004,19 @@ prlsdkGetNetInfo(PRL_HANDLE netAdapter, 
virDomainNetDefPtr net, bool isCt)
PARALLELS_DOMAIN_ROUTED_NETWORK_NAME) < 0)
 goto cleanup;
 } else {
-char *netid = NULL;
-
-if (!(netid =
+char *netid =
   prlsdkGetStringParamVar(PrlVmDevNet_GetVirtualNetworkId,
-  netAdapter)))
-goto cleanup;
+  netAdapter);
 
-/*
- * 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 (STRNEQ(netid, PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME)) {
+if (emulatedType == PNA_BRIDGE) {
 net->type = VIR_DOMAIN_NET_TYPE_BRIDGE;
-net->data.network.name = netid;
+if (netid)
+net->data.bridge.brname = netid;
 } else {
 net->type = VIR_DOMAIN_NET_TYPE_NETWORK;
-net->data.bridge.brname = netid;
+if (netid)
+net->data.network.name = netid;
 }
-
 }
 
 if (!isCt) {
@@ -3175,16 +3167,14 @@ static int prlsdkConfigureGateways(PRL_HANDLE sdknet, 
virDomainNetDefPtr net)
 return ret;
 }
 
-static int prlsdkConfigureNet(vzDriverPtr driver,
-  virDomainObjPtr dom,
+static int prlsdkConfigureNet(vzDriverPtr driver ATTRIBUTE_UNUSED,
+  virDomainObjPtr dom ATTRIBUTE_UNUSED,
   PRL_HANDLE sdkdom,
   virDomainNetDefPtr net,
   bool isCt, bool create)
 {
 PRL_RESULT pret;
 PRL_HANDLE sdknet = PRL_INVALID_HANDLE;
-PRL_HANDLE vnet = PRL_INVALID_HANDLE;
-PRL_HANDLE job = PRL_INVALID_HANDLE;
 PRL_HANDLE addrlist = PRL_INVALID_HANDLE;
 size_t i;
 int ret = -1;
@@ -3291,35 +3281,17 @@ static int prlsdkConfigureNet(vzDriverPtr driver,
 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);
+} else {
+pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGED_NETWORK);
 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();
-prlsdkCheckRetGoto(pret, cleanup);
-
-pret = PrlVirtNet_SetNetworkId(vnet, net->data.bridge.brname);
-prlsdkCheckRetGoto(pret, cleanup);
 
-pret = PrlVirtNet_SetNetworkType(vnet, PVN_BRIDGED_ETHERNET);
-prlsdkCheckRetGoto(pret, cleanup);
-
-job = PrlSrv_AddVirtualNetwork(driver->server,
-   vnet,
-   PRL_USE_VNET_NAME_FOR_BRIDGE_NAME);
-if (PRL_FAILED(pret = waitDomainJob(job, dom)))
-goto cleanup;
+} else if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
 
-pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGED_ETHERNET);
+pret = PrlVmDev_SetEmulatedType(sdknet, PNA_BRIDGE);
 prlsdkCheckRetGoto(pret, cleanup);
 
 pret = PrlVmDevNet_SetVirtualNetworkId(sdknet, 
net->data.bridge.brname);
@@ -3334,40 +3306,10 @@ static int prlsdkConfigureNet(vzDriverPtr driver,
  cleanup:
 VIR_FREE(addrstr);
 PrlHandle_Free(addrlist);
-PrlHandle_Free(vnet);
 PrlHandle_Free(sdknet);
 return ret;
 }
 
-static void
-prlsdkCleanupBridgedNet(vzDriverPtr driver,
-virDomainObjPtr dom,
-virDomainNetDefPtr net)
-{
-PRL_RESULT pret;
-PRL_HANDLE vnet = PRL_INVALID_HANDLE;
-PRL_HANDLE job = PRL_INVALID_HANDLE;
-
-if (net->type !=