10:42 a.m.
On 06/04/2013 03:44 AM, Alvaro Polo wrote:
OpenVZ was accessing ethernet data to obtain the guest iface name
regardless the domain is configured to use ethernet or bridged
networking. This prevented the guest network interface to be rightly
named for bridged networking.
---
src/openvz/openvz_driver.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index c8081ce..db738a4 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -815,6 +815,7 @@ openvzDomainSetNetwork(virConnectPtr conn, const char *vpsid,
char host_macaddr[VIR_MAC_STRING_BUFLEN];
struct openvz_driver *driver = conn->privateData;
virCommandPtr cmd = NULL;
+ char *guest_ifname = NULL;
if (net == NULL)
return 0;
@@ -840,11 +841,15 @@ openvzDomainSetNetwork(virConnectPtr conn, const char *vpsid,
virBuffer buf = VIR_BUFFER_INITIALIZER;
int veid = openvzGetVEID(vpsid);
- /* if user doesn't specify guest interface name,
- * then we need to generate it */
- if (net->data.ethernet.dev == NULL) {
- net->data.ethernet.dev = openvzGenerateContainerVethName(veid);
- if (net->data.ethernet.dev == NULL) {
+ /* if net is ethernet and the user has specified guest interface name,
+ * let's use it; otherwise generate a new one */
+ if (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET &&
+ net->data.ethernet.dev != NULL) {
[1] I know this code was pushed, but see note below
+ if (VIR_STRDUP(guest_ifname, net->data.ethernet.dev)
== -1)
+ goto cleanup;
+ } else {
+ guest_ifname = openvzGenerateContainerVethName(veid);
+ if (guest_ifname == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Could not generate eth name for
container"));
goto cleanup;
@@ -862,7 +867,7 @@ openvzDomainSetNetwork(virConnectPtr conn, const char *vpsid,
}
}
- virBufferAdd(&buf, net->data.ethernet.dev, -1); /* Guest dev */
+ virBufferAdd(&buf, guest_ifname, -1); /* Guest dev */
virBufferAsprintf(&buf, ",%s", macaddr); /* Guest dev mac */
virBufferAsprintf(&buf, ",%s", net->ifname); /* Host dev */
virBufferAsprintf(&buf, ",%s", host_macaddr); /* Host dev mac */
@@ -871,7 +876,7 @@ openvzDomainSetNetwork(virConnectPtr conn, const char *vpsid,
if (driver->version >= VZCTL_BRIDGE_MIN_VERSION) {
virBufferAsprintf(&buf, ",%s",
net->data.bridge.brname); /* Host bridge */
} else {
- virBufferAsprintf(configBuf, "ifname=%s",
net->data.ethernet.dev);
+ virBufferAsprintf(configBuf, "ifname=%s", guest_ifname);
virBufferAsprintf(configBuf, ",mac=%s", macaddr); /* Guest dev
mac */
virBufferAsprintf(configBuf, ",host_ifname=%s",
net->ifname); /* Host dev */
virBufferAsprintf(configBuf, ",host_mac=%s", host_macaddr); /*
Host dev mac */
@@ -895,6 +900,7 @@ openvzDomainSetNetwork(virConnectPtr conn, const char *vpsid,
cleanup:
virCommandFree(cmd);
+ VIR_FREE(guest_ifname);
return rc;
}
[1] My nightly Coverity run had the following complaint as a result of this patch:
818 char *guest_ifname = NULL;
819
(1) Event cond_false: Condition "net == NULL", taking false branch
820 if (net == NULL)
821 return 0;
(2) Event cond_false: Condition "vpsid == NULL", taking false branch
822 if (vpsid == NULL) {
823 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
824 _("Container ID is not specified"));
825 return -1;
(3) Event if_end: End of if statement
826 }
827
(4) Event cond_true: Condition "net->type != VIR_DOMAIN_NET_TYPE_BRIDGE",
taking true branch
(5) Event cond_false: Condition "net->type != VIR_DOMAIN_NET_TYPE_ETHERNET",
taking false branch
828 if (net->type != VIR_DOMAIN_NET_TYPE_BRIDGE &&
829 net->type != VIR_DOMAIN_NET_TYPE_ETHERNET)
830 return 0;
831
832 cmd = virCommandNewArgList(VZCTL, "--quiet", "set", vpsid,
NULL);
833
834 virMacAddrFormat(&net->mac, macaddr);
835 virDomainNetGenerateMAC(driver->xmlopt, &host_mac);
836 virMacAddrFormat(&host_mac, host_macaddr);
837
(6) Event cond_false: Condition "net->type == VIR_DOMAIN_NET_TYPE_BRIDGE",
taking false branch
(7) Event cond_true: Condition "net->type == VIR_DOMAIN_NET_TYPE_ETHERNET",
taking true branch
(8) Event cond_true: Condition "net->data.ethernet.ipaddr == NULL", taking
true branch
838 if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
839 (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET &&
840 net->data.ethernet.ipaddr == NULL)) {
841 virBuffer buf = VIR_BUFFER_INITIALIZER;
842 int veid = openvzGetVEID(vpsid);
843
844 /* if net is ethernet and the user has specified guest interface name,
845 * let's use it; otherwise generate a new one */
(9) Event cond_true: Condition "net->type == VIR_DOMAIN_NET_TYPE_ETHERNET",
taking true branch
(10) Event cond_false: Condition "net->data.ethernet.dev != NULL", taking
false branch
(12) Event var_compare_op: Comparing "net->data.ethernet.dev" to null
implies that "net->data.ethernet.dev" might be null.
Also see events: [var_deref_model]
846 if (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET &&
847 net->data.ethernet.dev != NULL) {
848 if (VIR_STRDUP(guest_ifname, net->data.ethernet.dev) == -1)
849 goto cleanup;
(11) Event else_branch: Reached else branch
850 } else {
851 guest_ifname = openvzGenerateContainerVethName(veid);
(13) Event cond_false: Condition "guest_ifname == NULL", taking false branch
852 if (guest_ifname == NULL) {
853 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
854 _("Could not generate eth name for
container"));
855 goto cleanup;
(14) Event if_end: End of if statement
856 }
857 }
858
859 /* if user doesn't specified host interface name,
860 * than we need to generate it */
(15) Event cond_true: Condition "net->ifname == NULL", taking true branch
861 if (net->ifname == NULL) {
(16) Event var_deref_model: Passing null pointer "net->data.ethernet.dev" to
function "openvzGenerateVethName(int, char *)", which dereferences it.
[details]
Also see events: [var_compare_op]
862 net->ifname = openvzGenerateVethName(veid,
net->data.ethernet.dev);
863 if (net->ifname == NULL) {
864 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
865 _("Could not generate veth name"));
866 goto cleanup;
867 }
868 }
869
If I'm reading the code correctly, it seems replacing
'net->data.ethernet.dev'
with 'guest_ifname' at line 862 will be the right solution. Doing so makes
Coverity happy. Previously the code guarenteed data.ethernet.dev was filled
in with the openvzGenerateContainerVethName() result.
John