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

2008/9/29 Evgeniy V. Sokolov <evg@parallels.com>
This behaviour contradicts with description found in docs (in which <source> tag specify interface in host, not in container). I think, the previous bridge must be specified as <interface type='bridge'> <mac address='00:16:3e:34:21:9e'/> - <source bridge='eth10'/> + <target dev='eth10'/> </interface>
You are right. It is bug.
Well, here is the patch, that fixes that behaviour.
Your patch is half of fix. It require to fix openvzReadNetworkConf() in openvz_conf.c.
Here is a new one. Check it, please.

2008/9/29 Evgeniy V. Sokolov <evg@parallels.com <mailto:evg@parallels.com>>
This behaviour contradicts with description found in docs (in which <source> tag specify interface in host, not in container). I think, the previous bridge must be specified as <interface type='bridge'> <mac address='00:16:3e:34:21:9e'/> - <source bridge='eth10'/> + <target dev='eth10'/> </interface>
You are right. It is bug.
Well, here is the patch, that fixes that behaviour.
Your patch is half of fix. It require to fix openvzReadNetworkConf() in openvz_conf.c.
Here is a new one. Check it, please.
Patch looks good. But currently it will work strange because of all drivers (except OpenVZ) in libvirt require specified <source bridge=""/> - it is generic. With patch you need to specify both <source bridge='eth10'/> - to satisfy common requirements <target dev='eth10'/> - to create network

On Mon, Sep 29, 2008 at 04:11:39PM +0400, Evgeniy Sokolov wrote:
2008/9/29 Evgeniy V. Sokolov <evg@parallels.com <mailto:evg@parallels.com>>
This behaviour contradicts with description found in docs (in which <source> tag specify interface in host, not in container). I think, the previous bridge must be specified as <interface type='bridge'> <mac address='00:16:3e:34:21:9e'/> - <source bridge='eth10'/> + <target dev='eth10'/> </interface>
You are right. It is bug.
Well, here is the patch, that fixes that behaviour.
Your patch is half of fix. It require to fix openvzReadNetworkConf() in openvz_conf.c.
Here is a new one. Check it, please.
Patch looks good. But currently it will work strange because of all drivers (except OpenVZ) in libvirt require specified <source bridge=""/> - it is generic. With patch you need to specify both <source bridge='eth10'/> - to satisfy common requirements <target dev='eth10'/> - to create network
That is not right. The <target> element *must* be optional when creating a new domain. If omitted, the driver must generated a suitable target dev according to its desired naming scheme. Only the <source> element can be compulsory 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/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On Mon, Sep 29, 2008 at 04:11:39PM +0400, Evgeniy Sokolov wrote:
2008/9/29 Evgeniy V. Sokolov <evg@parallels.com <mailto:evg@parallels.com>>
This behaviour contradicts with description found in docs (in which <source> tag specify interface in host, not in container). I think, the previous bridge must be specified as <interface type='bridge'> <mac address='00:16:3e:34:21:9e'/> - <source bridge='eth10'/> + <target dev='eth10'/> </interface>
You are right. It is bug.
Well, here is the patch, that fixes that behaviour.
Your patch is half of fix. It require to fix openvzReadNetworkConf() in openvz_conf.c.
Here is a new one. Check it, please. Patch looks good. But currently it will work strange because of all drivers (except OpenVZ) in libvirt require specified <source bridge=""/> - it is generic. With patch you need to specify both <source bridge='eth10'/> - to satisfy common requirements <target dev='eth10'/> - to create network
That is not right. The <target> element *must* be optional when creating a new domain. If omitted, the driver must generated a suitable target dev according to its desired naming scheme. Only the <source> element can be compulsory
Agree with you. You say about "how should it be". I say about "how is it now". Currenty OpenVZ driver don't generate device name - my mistake.
Daniel.

2008/9/29 Evgeniy Sokolov <evg@openvz.org>
On Mon, Sep 29, 2008 at 04:11:39PM +0400, Evgeniy Sokolov wrote:
2008/9/29 Evgeniy V. Sokolov <evg@parallels.com <mailto:
evg@parallels.com>>
This behaviour contradicts with description found in docs (in which <source> tag specify interface in host, not in container). I think, the previous bridge must be specified as <interface type='bridge'> <mac address='00:16:3e:34:21:9e'/> - <source bridge='eth10'/> + <target dev='eth10'/> </interface>
You are right. It is bug.
Well, here is the patch, that fixes that behaviour.
Your patch is half of fix. It require to fix openvzReadNetworkConf() in openvz_conf.c.
Here is a new one. Check it, please.
Patch looks good. But currently it will work strange because of all drivers (except OpenVZ) in libvirt require specified <source bridge=""/> - it is generic. With patch you need to specify both <source bridge='eth10'/> - to satisfy common requirements <target dev='eth10'/> - to create network
That is not right. The <target> element *must* be optional when creating a new domain. If omitted, the driver must generated a suitable target dev according to its desired naming scheme. Only the <source> element can be compulsory
Agree with you. You say about "how should it be". I say about "how is it now". Currenty OpenVZ driver don't generate device name - my mistake.
Is vnet$i (where $i is the number of automatically generated interface name for this container) appropriate?
Daniel.

On Tue, Sep 30, 2008 at 04:57:15PM +0400, Anton Protopopov wrote:
2008/9/29 Evgeniy Sokolov <evg@openvz.org>
On Mon, Sep 29, 2008 at 04:11:39PM +0400, Evgeniy Sokolov wrote:
2008/9/29 Evgeniy V. Sokolov <evg@parallels.com <mailto: Patch looks good. But currently it will work strange because of all drivers (except OpenVZ) in libvirt require specified <source bridge=""/> - it is generic. With patch you need to specify both <source bridge='eth10'/> - to satisfy common requirements <target dev='eth10'/> - to create network
That is not right. The <target> element *must* be optional when creating a new domain. If omitted, the driver must generated a suitable target dev according to its desired naming scheme. Only the <source> element can be compulsory
Agree with you. You say about "how should it be". I say about "how is it now". Currenty OpenVZ driver don't generate device name - my mistake.
Is vnet$i (where $i is the number of automatically generated interface name for this container) appropriate?
Possibly, though its not terribly consistent so far. Xen - vifNNN.MMM (NNN dom ID, MMM = nic number) QEMU - vnetNNN (NNN = global host nic number) LXC - vethNNN (MMM = global host nic number) We can't change the format Xen uses, but it could be desirable to make LXC, QEMU and OpenVZ all use vnetNNN scheme by default for auto assigned nics names on the host side. 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/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Possibly, though its not terribly consistent so far.
Xen - vifNNN.MMM (NNN dom ID, MMM = nic number) QEMU - vnetNNN (NNN = global host nic number) LXC - vethNNN (MMM = global host nic number)
And how these drivers choose NNN? Do QEMU driver and LXC driver use the same NNN?
We can't change the format Xen uses, but it could be desirable to make LXC, QEMU and OpenVZ all use vnetNNN scheme by default for auto assigned nics names on the host side.
Now openvz driver chooses not interface name in host, but interface name in container. (If you specify contaner-ifname, say, vnet4, you get host-ifname veth$veid.4.) So first we need to decide: is container-ifname derivative from host-ifname or vice versa :)
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 :|

On Tue, Sep 30, 2008 at 05:57:20PM +0400, Anton Protopopov wrote:
Possibly, though its not terribly consistent so far.
Xen - vifNNN.MMM (NNN dom ID, MMM = nic number) QEMU - vnetNNN (NNN = global host nic number) LXC - vethNNN (MMM = global host nic number)
And how these drivers choose NNN? Do QEMU driver and LXC driver use the same NNN?
Start from vnet0 and look for the first available name.
We can't change the format Xen uses, but it could be desirable to make LXC, QEMU and OpenVZ all use vnetNNN scheme by default for auto assigned nics names on the host side.
Now openvz driver chooses not interface name in host, but interface name in container.
The interface name inside the container is not something that is expressed in the libvirt XML. That is left upto the guest OS to decide, and not something libvirt needs to care about. We only care about configuration from the host side. 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/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2008/9/30 Daniel P. Berrange <berrange@redhat.com>
On Tue, Sep 30, 2008 at 05:57:20PM +0400, Anton Protopopov wrote:
Possibly, though its not terribly consistent so far.
Xen - vifNNN.MMM (NNN dom ID, MMM = nic number) QEMU - vnetNNN (NNN = global host nic number) LXC - vethNNN (MMM = global host nic number)
And how these drivers choose NNN? Do QEMU driver and LXC driver use the same NNN?
Start from vnet0 and look for the first available name.
We can't change the format Xen uses, but it could be desirable to make LXC, QEMU and OpenVZ all use vnetNNN scheme by default for auto
assigned
nics names on the host side.
Now openvz driver chooses not interface name in host, but interface name in container.
The interface name inside the container is not something that is expressed in the libvirt XML. That is left upto the guest OS to decide, and not something libvirt needs to care about.
In OpenVZ case you _must_ to decide what name the interface will have inside the container. The command line for ading an interface in OpenVZ looks like # vzctl set $veid --ifname_add ifname[mac, host_ifname, host_mac] The only mandatory field is the name of _container interface_ The possible solution is the following: * absolutely ignore the <target dev=".."> in openvz XML description * Set (ifname, host_ifname) of the interface to (eth${N}, veth${ID}.${N}), where ID is the container id and N is the interface number within that container. It is simplifies saving the bridge name and adding the interface to the bridge too, for example, by adding the following line to the $veid.conf: #BRIDGE$N: <name of bridge for interface # $N >
We only care about configuration from the host side.
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 :|

On Wed, Oct 01, 2008 at 10:33:49AM +0400, Anton Protopopov wrote:
2008/9/30 Daniel P. Berrange <berrange@redhat.com>
On Tue, Sep 30, 2008 at 05:57:20PM +0400, Anton Protopopov wrote:
Now openvz driver chooses not interface name in host, but interface name
in
container.
The interface name inside the container is not something that is expressed in the libvirt XML. That is left upto the guest OS to decide, and not something libvirt needs to care about.
In OpenVZ case you _must_ to decide what name the interface will have inside the container. The command line for ading an interface in OpenVZ looks like # vzctl set $veid --ifname_add ifname[mac, host_ifname, host_mac] The only mandatory field is the name of _container interface_
If openvz kernel requries specification of a NIC name for inside the container, then this is an implemntation detail that the libvirt OpenVZ driver will have to deal with. It can auto-assign them to be eth0, eth1, etc. This is not exposed in the libvirt XML though, since it is not relevant to the host admin, only apps inside the guest.
* absolutely ignore the <target dev=".."> in openvz XML description
As I said before this needs to reflect the name of the interface on the host side. It can be ignored when creating a guest, since for the majority of uses cases it can be safely auto-generated. It must be filled in when dumping XML for a guest, so the host admin knows which NIC in the host corresponds to the guest. 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/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

If openvz kernel requries specification of a NIC name for inside the container, then this is an implemntation detail that the libvirt OpenVZ driver will have to deal with. It can auto-assign them to be eth0, eth1, etc. This is not exposed in the libvirt XML though, since it is not relevant to the host admin, only apps inside the guest.
* absolutely ignore the <target dev=".."> in openvz XML description
As I said before this needs to reflect the name of the interface on the host side. It can be ignored when creating a guest, since for the majority of uses cases it can be safely auto-generated. It must be filled in when dumping XML for a guest, so the host admin knows which NIC in the host corresponds to the guest.
Here is the patch, that implements the following behaviour * interface name inside container is automatically generated and equals ethN, where N is the number of that interface within current domain * mac address of that interface inside container is generated automatically by function openvzGenerateMac * if <target dev=""> is specified, use it; otherwise, use the default openvz name, i.e., vethN.M for interface ethM in container with veid N * if <mac address='...'> specified, use it; otherwise, vzctl will generate it automatically * <target dev> and <mac address> are (re)stored (from)to $veid.conf Anton

2008/10/1 Anton Protopopov <aspsk2@gmail.com>
If openvz kernel requries specification of a NIC name for inside the
container, then this is an implemntation detail that the libvirt OpenVZ driver will have to deal with. It can auto-assign them to be eth0, eth1, etc. This is not exposed in the libvirt XML though, since it is not relevant to the host admin, only apps inside the guest.
* absolutely ignore the <target dev=".."> in openvz XML description
As I said before this needs to reflect the name of the interface on the host side. It can be ignored when creating a guest, since for the majority of uses cases it can be safely auto-generated. It must be filled in when dumping XML for a guest, so the host admin knows which NIC in the host corresponds to the guest.
Here is the patch, that implements the following behaviour
* interface name inside container is automatically generated and equals ethN, where N is the number of that interface within current domain * mac address of that interface inside container is generated automatically by function openvzGenerateMac * if <target dev=""> is specified, use it; otherwise, use the default openvz name, i.e., vethN.M for interface ethM in container with veid N * if <mac address='...'> specified, use it; otherwise, vzctl will generate it automatically * <target dev> and <mac address> are (re)stored (from)to $veid.conf
Anton
Actually, I have some doubts about that patch: the functions openvzGenerateMac and openvzGenerateVethName are taken from vzctl sources and slightly changed then. The problem is that libvirt uses LGPL, while vzctl uses GPL. So you can't apply that patch, right? Even if it is good :)

Hi. I rewrite functions taken from vzctl in the new patch, so this:
Here is the patch, that implements the following behaviour
* interface name inside container is automatically generated and equals ethN, where N is the number of that interface within current domain * mac address of that interface inside container is generated automatically by function openvzGenerateMac * if <target dev=""> is specified, use it; otherwise, use the default openvz name, i.e., vethN.M for interface ethM in container with veid N * if <mac address='...'> specified, use it; otherwise, vzctl will generate it automatically * <target dev> and <mac address> are (re)stored (from)to $veid.conf
is true, while that:
the functions openvzGenerateMac and openvzGenerateVethName are taken from vzctl sources and slightly changed then
is not true anymore.

Hi.
I rewrite functions taken from vzctl in the new patch, so this:
Here is the patch, that implements the following behaviour
* interface name inside container is automatically generated and equals ethN, where N is the number of that interface within current domain * mac address of that interface inside container is generated automatically by function openvzGenerateMac * if <target dev=""> is specified, use it; otherwise, use the default openvz name, i.e., vethN.M for interface ethM in container with veid N * if <mac address='...'> specified, use it; otherwise, vzctl will generate it automatically * <target dev> and <mac address> are (re)stored (from)to $veid.conf
is true, while that:
the functions openvzGenerateMac and openvzGenerateVethName are taken from vzctl sources and slightly changed then
is not true anymore.
+static char * +openvzGenerateMac(void) +{ + char mac[6] = { + 0x52, + 0x54, + 0x00, How did you get 0x52, 0x54, 0x00? + 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... Other looks good.

2008/10/3 Evgeniy Sokolov <evg@openvz.org>
Hi.
I rewrite functions taken from vzctl in the new patch, so this:
Here is the patch, that implements the following behaviour
* interface name inside container is automatically generated and equals ethN, where N is the number of that interface within current domain * mac address of that interface inside container is generated automatically by function openvzGenerateMac * if <target dev=""> is specified, use it; otherwise, use the default openvz name, i.e., vethN.M for interface ethM in container with veid N * if <mac address='...'> specified, use it; otherwise, vzctl will generate it automatically * <target dev> and <mac address> are (re)stored (from)to $veid.conf
is true, while that:
the functions openvzGenerateMac and openvzGenerateVethName are taken from vzctl sources and slightly changed then
is not true anymore.
+static char * +openvzGenerateMac(void) +{ + char mac[6] = { + 0x52, + 0x54, + 0x00, How did you get 0x52, 0x54, 0x00?
This is the same as in virDomainNetRandomMAC() function, see the source code for it.
+ 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...
OK, I will try to fix it.
Other looks good.
Actually, there is a bug :( When we generate host interface name, we need to save it in net->ifname. 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?

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" In this case we may use openvzReadConfigParam(101, "#BRIDGE", value, 1024)

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"
There's no need to store veth101.0, veth101.2 - they are automatically generated & not intended to be stable across restarts. It is merely 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/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2008/10/3 Daniel P. Berrange <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)
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 :|

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 #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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Evgeniy Sokolov Parallels

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)

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
Patch looks good now.
I think, we need to rewrite {Get,Set}UUID functions in the same manner.
Yes, good idea.

Hi all, 2008/10/8 Evgeniy Sokolov <evg@openvz.org>
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
Patch looks good now.
So, what about that patch? Will it be commited or there are reasons against it? (Or maybe nobody has seen it yet? :))

On Wed, Oct 08, 2008 at 01:02:44PM +0400, Anton Protopopov wrote:
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));
This idea is not good - if someone starts the container outside of libvirt using 'vzctl start 101', then the bridge config will not be activated. The latest openvz code has explicit support in its configuration file for setting the bridge device name, via a 'bridge=br0' attribute in the NETIF= config setting. This can also be specified wehn invoking the vzctl --netif_add command: http://wiki.openvz.org/Virtual_Ethernet_device#Making_a_bridged_veth-device_... So, if using openvz > 3.0.22, we should use this config param to record the bridge and not invent our own. To make upgrades easy, we should also use this config syntax for version of openvz <= 3.0.22, and simply document that they need to create a helper script /usr/sbin/vznetaddbr to make it work - they can even just grab the helper script straight from newer openvz code. eg http://git.openvz.org/?p=vzctl;a=blob;f=bin/vznetaddbr.in;h=81975b3275c7e60b... 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/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

2008/10/14 Daniel P. Berrange <berrange@redhat.com>
On Wed, Oct 08, 2008 at 01:02:44PM +0400, Anton Protopopov wrote:
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));
This idea is not good - if someone starts the container outside of libvirt using 'vzctl start 101', then the bridge config will not be activated.
The latest openvz code has explicit support in its configuration file for setting the bridge device name, via a 'bridge=br0' attribute in the NETIF= config setting. This can also be specified wehn invoking the vzctl --netif_add command:
http://wiki.openvz.org/Virtual_Ethernet_device#Making_a_bridged_veth-device_...
So, if using openvz > 3.0.22, we should use this config param to record the bridge and not invent our own.
OK, I will try to implement this in libvirt
To make upgrades easy, we should also use this config syntax for version of openvz <= 3.0.22, and simply document that they need to create a helper script /usr/sbin/vznetaddbr to make it work - they can even just grab the helper script straight from newer openvz code. eg
http://git.openvz.org/?p=vzctl;a=blob;f=bin/vznetaddbr.in;h=81975b3275c7e60b...
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 :|

2008/10/14 Anton Protopopov <aspsk2@gmail.com>
2008/10/14 Daniel P. Berrange <berrange@redhat.com>
On Wed, Oct 08, 2008 at 01:02:44PM +0400, Anton Protopopov wrote:
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));
This idea is not good - if someone starts the container outside of libvirt using 'vzctl start 101', then the bridge config will not be activated.
The latest openvz code has explicit support in its configuration file for setting the bridge device name, via a 'bridge=br0' attribute in the NETIF= config setting. This can also be specified wehn invoking the vzctl --netif_add command:
http://wiki.openvz.org/Virtual_Ethernet_device#Making_a_bridged_veth-device_...
So, if using openvz > 3.0.22, we should use this config param to record the bridge and not invent our own.
OK, I will try to implement this in libvirt
Oops, someone already did it.. :)
To make upgrades easy, we should also use this config syntax for version of openvz <= 3.0.22, and simply document that they need to create a helper script /usr/sbin/vznetaddbr to make it work - they can even just grab the helper script straight from newer openvz code. eg
http://git.openvz.org/?p=vzctl;a=blob;f=bin/vznetaddbr.in;h=81975b3275c7e60b...
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 :|
participants (4)
-
Anton Protopopov
-
Daniel P. Berrange
-
Evgeniy Sokolov
-
Evgeniy V. Sokolov