[...]
>
> diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
> index 102fd85c1..cca9d81a4 100644
> --- a/src/util/virhostdev.c
> +++ b/src/util/virhostdev.c
> @@ -534,6 +534,10 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev,
> int ret = -1;
> int vf = -1;
> bool port_profile_associate = false;
> + virMacAddrPtr MAC = NULL;
> + virMacAddrPtr adminMAC = NULL;
> + virNetDevVlanPtr vlan = NULL;
> +
>
> /* This is only needed for PCI devices that have been defined
> * using <interface type='hostdev'>. For all others, it is a NOP.
> @@ -559,62 +563,92 @@ virHostdevRestoreNetConfig(virDomainHostdevDefPtr hostdev,
> NULL,
> port_profile_associate);
> } else {
> - virMacAddrPtr MAC = NULL;
> - virMacAddrPtr adminMAC = NULL;
> - virNetDevVlanPtr vlan = NULL;
> -
> - ret = virNetDevReadNetConfig(linkdev, vf, stateDir, &adminMAC, &vlan, &MAC);
> - if (ret < 0 && oldStateDir)
> - ret = virNetDevReadNetConfig(linkdev, vf, oldStateDir,
> - &adminMAC, &vlan, &MAC);
> -
> - if (ret < 0) {
> - /* see if the config was saved using the PF's "port 2"
> - * netdev for the file name.
> - */
> - VIR_FREE(linkdev);
> + /* we need to try 3 different places for the config file:
> + * 1) ${stateDir}/${PF}_vf${vf}
> + * This is almost always where the saved config is
> + *
> + * 2) ${oldStateDir/${PF}_vf${vf}
> + * saved config is only here if this machine was running a
> + * (by now *very*) old version of libvirt that saved the
> + * file in a different directory
> + *
> + * 3) ${stateDir}${PF[1]}_vf${VF}
> + * PF[1] means "the netdev for port 2 of the PF device", and
> + * is only valid when the PF is a Mellanox dual port NIC with
> + * a VF that was created in "single port" mode.
> + *
> + * NB: if virNetDevReadNetConfig() returns < 0, then it found
> + * the file, but there was a problem, so we should
> + * immediately return an error to our caller. If it returns
> + * 0, but all of the interesting stuff is NULL, that means
> + * the file wasn't found, so we can/should check other
> + * locations for it.
> + */
>
> - if (virHostdevNetDevice(hostdev, 1, &linkdev, &vf) >= 0) {
> - ret = virNetDevReadNetConfig(linkdev, vf, stateDir,
> - &adminMAC, &vlan, &MAC);
> - }
> + /* 1) standard location */
> + if (virNetDevReadNetConfig(linkdev, vf, stateDir,
> + &adminMAC, &vlan, &MAC) < 0) {
> + goto cleanup;
> }
>
> - if (ret == 0) {
> - /* if a MAC was stored for the VF, we should now restore
> - * that as the adminMAC. We have to do it this way because
> - * the VF is still not bound to the host's net driver, so
> - * we can't directly set its MAC (and even after it is
> - * re-bound to the host net driver, it will still have its
> - * "administratively set" flag on, and that prohibits the
> - * VF's net driver from directly setting the MAC
> - * anyway). But it we set the desired VF MAC as the "admin
> - * MAC" *now*, then when the VF is re-bound to the host
> - * net driver (which will happen soon after returning from
> - * this function), that adminMAC will be set (by the PF)
> - * as the VF's new initial MAC.
> - *
> - * If no MAC was stored for the VF, that means it wasn't
> - * bound to a net driver before we used it anyway, so the
> - * adminMAC is all we have, and we can just restore it
> - * directly.
> - */
> - if (MAC) {
> - VIR_FREE(adminMAC);
> - adminMAC = MAC;
> - MAC = NULL;
> + /* 2) "old" (pre-1.2.3 circa 2014) location - whenever we get
> + * to the point that nobody will ever upgrade directly from
> + * 1.2.3 (or older) directly to current libvirt, we can
> + * eliminate this clause
> + **/
> + if (!(adminMAC || vlan || MAC) && oldStateDir &&
> + virNetDevReadNetConfig(linkdev, vf, oldStateDir,
> + &adminMAC, &vlan, &MAC) < 0) {
> + goto cleanup;
> + }
> +
> + /* 3) try using the PF's "port 2" netdev as the name of the
> + * config file
> + */
> + if (!(adminMAC || vlan || MAC)) {
> + VIR_FREE(linkdev);
> +
> + if (virHostdevNetDevice(hostdev, 1, &linkdev, &vf) < 0 ||
> + virNetDevReadNetConfig(linkdev, vf, stateDir,
> + &adminMAC, &vlan, &MAC) < 0) {
> + goto cleanup;
> }
> + }
>
> - ignore_value(virNetDevSetNetConfig(linkdev, vf,
> - adminMAC, vlan, MAC, true));
> + /* if a MAC was stored for the VF, we should now restore
> + * that as the adminMAC. We have to do it this way because
> + * the VF is still not bound to the host's net driver, so
> + * we can't directly set its MAC (and even after it is
> + * re-bound to the host net driver, it will still have its
> + * "administratively set" flag on, and that prohibits the
> + * VF's net driver from directly setting the MAC
> + * anyway). But it we set the desired VF MAC as the "admin
> + * MAC" *now*, then when the VF is re-bound to the host
> + * net driver (which will happen soon after returning from
> + * this function), that adminMAC will be set (by the PF)
> + * as the VF's new initial MAC.
> + *
> + * If no MAC was stored for the VF, that means it wasn't
> + * bound to a net driver before we used it anyway, so the
> + * adminMAC is all we have, and we can just restore it
> + * directly.
> + */
> + if (MAC) {
> + VIR_FREE(adminMAC);
> + adminMAC = MAC;
> + MAC = NULL;
> }
>
> - VIR_FREE(MAC);
> - VIR_FREE(adminMAC);
> - virNetDevVlanFree(vlan);
> + ignore_value(virNetDevSetNetConfig(linkdev, vf,
> + adminMAC, vlan, MAC, true));
> }
>
> + ret = 0;
Coverity notes that in the first half of the if statement :
ret = virHostdevNetConfigVirtPortProfile(...)
However, this overwrites that.
John
> + cleanup:
> VIR_FREE(linkdev);
> + VIR_FREE(MAC);
> + VIR_FREE(adminMAC);
> + virNetDevVlanFree(vlan);
>
> return ret;
> }
[...]
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
