[libvirt] [PATCH 0/3] vz: nettype=bridge and other corrections
Maxim Nestratov (3): vz: support type=bridge network interface type correctly vz: remove Bridged network name and rename Routed vz: add MIGRATION_V3 capability src/vz/vz_driver.c | 1 + src/vz/vz_sdk.c | 100 ++++++++--------------------------------------------- src/vz/vz_utils.h | 3 +- 3 files changed, 16 insertions(+), 88 deletions(-) -- 2.4.11
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 <mnestratov@virtuozzo.com> --- 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(&vnet); - 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 != VIR_DOMAIN_NET_TYPE_BRIDGE) - return; - - pret = PrlVirtNet_Create(&vnet); - prlsdkCheckRetGoto(pret, cleanup); - - pret = PrlVirtNet_SetNetworkId(vnet, net->data.network.name); - prlsdkCheckRetGoto(pret, cleanup); - - job = PrlSrv_DeleteVirtualNetwork(driver->server, vnet, 0); - ignore_value(waitDomainJob(job, dom)); - - /* As far as waitDomainJob finally calls virReportErrorHelper - * and we are not going to report it, reset it expicitly*/ - virResetLastError(); - - cleanup: - PrlHandle_Free(vnet); -} - static PRL_HANDLE prlsdkFindNetByMAC(PRL_HANDLE sdkdom, virMacAddrPtr mac) { @@ -3608,7 +3550,7 @@ prlsdkAttachDevice(vzDriverPtr driver, } int -prlsdkDetachDevice(vzDriverPtr driver, +prlsdkDetachDevice(vzDriverPtr driver ATTRIBUTE_UNUSED, virDomainObjPtr dom, virDomainDeviceDefPtr dev) { @@ -3643,8 +3585,6 @@ prlsdkDetachDevice(vzDriverPtr driver, if (sdkdev == PRL_INVALID_HANDLE) goto cleanup; - prlsdkCleanupBridgedNet(driver, dom, dev->data.net); - pret = PrlVmDev_Remove(sdkdev); prlsdkCheckRetGoto(pret, cleanup); @@ -3957,11 +3897,6 @@ prlsdkDoApplyConfig(vzDriverPtr driver, if (prlsdkRemoveBootDevices(sdkdom) < 0) goto error; - if (dom) { - for (i = 0; i < dom->def->nnets; i++) - prlsdkCleanupBridgedNet(driver, dom, dom->def->nets[i]); - } - for (i = 0; i < def->nnets; i++) { if (prlsdkConfigureNet(driver, dom, sdkdom, def->nets[i], IS_CT(def), true) < 0) @@ -4010,9 +3945,6 @@ prlsdkDoApplyConfig(vzDriverPtr driver, error: VIR_FREE(mask); - for (i = 0; i < def->nnets; i++) - prlsdkCleanupBridgedNet(driver, dom, def->nets[i]); - return -1; } @@ -4251,7 +4183,6 @@ prlsdkUnregisterDomain(vzDriverPtr driver, virDomainObjPtr dom, unsigned int fla { vzDomObjPtr privdom = dom->privateData; PRL_HANDLE job; - size_t i; virDomainSnapshotObjListPtr snapshots = NULL; VIRTUAL_MACHINE_STATE domainState; int ret = -1; @@ -4288,9 +4219,6 @@ prlsdkUnregisterDomain(vzDriverPtr driver, virDomainObjPtr dom, unsigned int fla if (PRL_FAILED(waitDomainJob(job, dom))) goto cleanup; - for (i = 0; i < dom->def->nnets; i++) - prlsdkCleanupBridgedNet(driver, dom, dom->def->nets[i]); - prlsdkSendEvent(driver, dom, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); -- 2.4.11
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 <mnestratov@virtuozzo.com> --- 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]
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 <mnestratov@virtuozzo.com> --- 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]
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 <mnestratov@virtuozzo.com> --- 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
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 <mnestratov@virtuozzo.com> --- 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
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 <mnestratov@virtuozzo.com> --- 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
It's funny, but Routed network name was incorrect. We should use host-routed instead. --- src/vz/vz_utils.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index 4b407ec..9e02fe0 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -47,8 +47,7 @@ _("no domain with matching uuid '%s'"), uuidstr); \ } while (0) -# define PARALLELS_DOMAIN_ROUTED_NETWORK_NAME "Routed" -# define PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME "Bridged" +# define PARALLELS_DOMAIN_ROUTED_NETWORK_NAME "host-routed" # define VIRTUOZZO_VER_7 ((unsigned long) 7000000) struct _vzCapabilities { -- 2.4.11
On 05.09.2016 19:39, Maxim Nestratov wrote:
It's funny, but Routed network name was incorrect. We should use host-routed instead. --- src/vz/vz_utils.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/vz/vz_utils.h b/src/vz/vz_utils.h index 4b407ec..9e02fe0 100644 --- a/src/vz/vz_utils.h +++ b/src/vz/vz_utils.h @@ -47,8 +47,7 @@ _("no domain with matching uuid '%s'"), uuidstr); \ } while (0)
-# define PARALLELS_DOMAIN_ROUTED_NETWORK_NAME "Routed" -# define PARALLELS_DOMAIN_BRIDGED_NETWORK_NAME "Bridged" +# define PARALLELS_DOMAIN_ROUTED_NETWORK_NAME "host-routed" # define VIRTUOZZO_VER_7 ((unsigned long) 7000000)
struct _vzCapabilities {
ACK, by the way I would move removing "Bridged" to previous commit
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 819dad7..b971add 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -3071,6 +3071,7 @@ vzConnectSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature) return -1; switch (feature) { + case VIR_DRV_FEATURE_MIGRATION_V3: case VIR_DRV_FEATURE_MIGRATION_PARAMS: case VIR_DRV_FEATURE_MIGRATION_P2P: return 1; -- 2.4.11
On 05.09.2016 19:39, Maxim Nestratov wrote:
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/vz/vz_driver.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 819dad7..b971add 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -3071,6 +3071,7 @@ vzConnectSupportsFeature(virConnectPtr conn ATTRIBUTE_UNUSED, int feature) return -1;
switch (feature) { + case VIR_DRV_FEATURE_MIGRATION_V3: case VIR_DRV_FEATURE_MIGRATION_PARAMS: case VIR_DRV_FEATURE_MIGRATION_P2P: return 1;
this imply we have implemented domainMigrateBegin3 etc, that is v3 without params suffix
participants (2)
-
Maxim Nestratov -
Nikolay Shirokovskiy