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