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

[Evgeniy V. Sokolov]

> 2008/10/3 Daniel P. Berrange <berrange at redhat.com 
> <mailto:berrange at redhat.com>>
> 
>     On Fri, Oct 03, 2008 at 04:04:06PM +0400, Evgeniy Sokolov wrote:
>      > >
>      > >P.S. Now I am working on that:
>      > >
>      > >
>      > >
>      > >        P.S. Are someone going to implement
>      > >             <interface type='bridge'>
>      > >                ...
>      > >                 <source bridge="...">
>      > >                ...
>      > >              </interface>
>      > >        part of openvz driver? :)
>      > >
>      > >    I plan to implement it in a month.
>      > >    It will be fine if you are ready to develop the feature.
>      > >
>      > >
>      > >I'm using $veid.conf to store information about bridge device in the
>      > >following manner:
>      > >for interface <ifname>, therhe will be a line
>      > >   #BRIGDE(<ifname>): <bridge name>
>      > >for example,
>      > >   #BRIDGE(veth101.0): virbr1
>      > >
>      > >Do you agree with that behaviour?
>      > I think we can simplify format to simplify parsing.
>      > #BRIDGE="virbr1:veth101.0,veth101.1,veth101.2"
> 
> 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
}

May be you will found better way.

> 
> 
+        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.

@@ -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.
> 
>     There's no need to store  veth101.0, veth101.2 - they are automatically
>     generated & not intended to be stable across restarts. It is merely
> 
> No, there is. They are generated only once --- when VE container is created.
> After that they are stored in the config file.
>  
> 
> 
>     neccessary to persist the name of the bridge (or virtual network) to
>     which they are attached, and the MAC address.
> 
>     Daniel
>     --
>     |: Red Hat, Engineering, London   -o-  
>     http://people.redhat.com/berrange/ :|
>     |: http://libvirt.org  -o-  http://virt-manager.org  -o-
>      http://ovirt.org :|
>     |: http://autobuild.org       -o-        
>     http://search.cpan.org/~danberr/ <http://search.cpan.org/%7Edanberr/> :|
>     |: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742
>     7D3B 9505 :|
> 
> 
> 
> ------------------------------------------------------------------------
> 
> --
> Libvir-list mailing list
> Libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


-- 
Evgeniy Sokolov
Parallels