Re: [libvirt] [PATCH] openvz: swap <source bridge=...> with <target dev=...>

["Anton Protopopov" <aspsk2@xxxxxxxxx>]

I do not undesrtand how it will simplify parsing: the iterator
in parsing is an interface name, not bridge name. I attached a patch, so
you will see how I do think about it :) (this patch includes all
discussed changes)

My point of view is to use small count common methods to manipulate config parameters. That is don't use array of differend methods for different types of data. In current case we can transform format to be used common functions.

For example:
if we will use format
#BRIDGE-ifname=<>
we can drop openvzGetDefinedBridge and use one existing method
openvzReadConfigParam(veid, "#BRIDGE-ifname", value, sizeof(value));

openvzSetDefinedBridge can be simplified if create
openvzAppendParamToConfig

openvzSetDefinedBridge() {
openvzReadConfigParam()
if not found
       openvzAppendParamToConfig()
return
}

 

I done the following:

- Add function openvzAppendConfigParam(veid, param, value)
  This function simply appends a string `param="value"` to config file.
- Rewrite functions openvz{Get,Set}DefinedBridge to use that function

I think, we need to rewrite {Get,Set}UUID functions in the same manner.

+        case VIR_DOMAIN_NET_TYPE_BRIDGE:
+            veth = net->ifname;
+            bridge = net->data.bridge.brname;
+            if (rc = brAddInterface(brctl, bridge, veth)) {
+                openvzError(conn, VIR_ERR_INTERNAL_ERROR,
+                         _("failed to add %s device to %s: %s"),
+                         veth, bridge, strerror(rc));
+                goto exit;
+            }
+            break;
It will be good to check veth & bridge for NULL. Potentially it may happens when config is broken.

Done. If veth or bridge is NULL, then openvzError is raised
and one iteration of loop is passed.

@@ -602,6 +713,12 @@ openvzDomainCreate(virDomainPtr dom)
        return -1;
    }

+    if (openvzSetBridges(dom->conn, vm->def->name, vm->def->nets) < 0) {
+        openvzError(dom->conn, VIR_ERR_INTERNAL_ERROR,
+                _("Could not configure bridges"));
+        return -1;
+    }
+
Also, we need to set bridges in openvzDomainReboot method.

Done.

+    if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
+        static int vnetNo = 0;

Libvirt may be use as library in applications. If some will call create 2 containers, then first container will have eth0...ethN
second will have ethN+1...

Write function openvzGenerateContainerVethName.
When called, it will search for interface names in $veid.conf.
If there are ethN,...,ethM, it will return name "ethL",
where L = 1+max(N,...,M)

Attachment: libvirt_openvz_ifaces_names_and_macs.patch
Description: Binary data

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