[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

2008/10/3 Daniel P. Berrange <berrange redhat com <mailto:berrange 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
we can drop openvzGetDefinedBridge and use one existing method
openvzReadConfigParam(veid, "#BRIDGE-ifname", value, sizeof(value));

openvzSetDefinedBridge can be simplified if create

openvzSetDefinedBridge() {
if not found

May be you will found better way.

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

|: 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 redhat com

Evgeniy Sokolov

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]