virnetdev.c: Use g_auto*()

While I'm at it, use more g_autofree and g_autoptr() in this
file. This also fixes a possible mem-leak in
virNetDevGetVirtualFunctions().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This commit is contained in:
Michal Privoznik 2020-04-19 07:24:40 +02:00
parent 423664a6e9
commit 281f445b6f

View File

@ -505,17 +505,16 @@ int virNetDevSetMTUFromDevice(const char *ifname,
*/ */
int virNetDevSetNamespace(const char *ifname, pid_t pidInNs) int virNetDevSetNamespace(const char *ifname, pid_t pidInNs)
{ {
int ret = -1; g_autofree char *pid = NULL;
char *pid = NULL; g_autofree char *phy = NULL;
char *phy = NULL; g_autofree char *phy_path = NULL;
char *phy_path = NULL;
int len; int len;
pid = g_strdup_printf("%lld", (long long) pidInNs); pid = g_strdup_printf("%lld", (long long) pidInNs);
/* The 802.11 wireless devices only move together with their PHY. */ /* The 802.11 wireless devices only move together with their PHY. */
if (virNetDevSysfsFile(&phy_path, ifname, "phy80211/name") < 0) if (virNetDevSysfsFile(&phy_path, ifname, "phy80211/name") < 0)
goto cleanup; return -1;
if ((len = virFileReadAllQuiet(phy_path, 1024, &phy)) <= 0) { if ((len = virFileReadAllQuiet(phy_path, 1024, &phy)) <= 0) {
/* Not a wireless device. */ /* Not a wireless device. */
@ -525,7 +524,7 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs)
argv[5] = pid; argv[5] = pid;
if (virRun(argv, NULL) < 0) if (virRun(argv, NULL) < 0)
goto cleanup; return -1;
} else { } else {
const char *argv[] = { const char *argv[] = {
@ -538,15 +537,10 @@ int virNetDevSetNamespace(const char *ifname, pid_t pidInNs)
argv[2] = phy; argv[2] = phy;
argv[5] = pid; argv[5] = pid;
if (virRun(argv, NULL) < 0) if (virRun(argv, NULL) < 0)
goto cleanup; return -1;
} }
ret = 0; return 0;
cleanup:
VIR_FREE(phy_path);
VIR_FREE(phy);
VIR_FREE(pid);
return ret;
} }
#if defined(SIOCSIFNAME) && defined(HAVE_STRUCT_IFREQ) #if defined(SIOCSIFNAME) && defined(HAVE_STRUCT_IFREQ)
@ -913,25 +907,21 @@ int virNetDevGetIndex(const char *ifname G_GNUC_UNUSED,
int int
virNetDevGetMaster(const char *ifname, char **master) virNetDevGetMaster(const char *ifname, char **master)
{ {
int ret = -1; g_autofree void *nlData = NULL;
void *nlData = NULL;
struct nlattr *tb[IFLA_MAX + 1] = {NULL, }; struct nlattr *tb[IFLA_MAX + 1] = {NULL, };
*master = NULL; *master = NULL;
if (virNetlinkDumpLink(ifname, -1, &nlData, tb, 0, 0) < 0) if (virNetlinkDumpLink(ifname, -1, &nlData, tb, 0, 0) < 0)
goto cleanup; return -1;
if (tb[IFLA_MASTER]) { if (tb[IFLA_MASTER]) {
if (!(*master = virNetDevGetName(*(int *)RTA_DATA(tb[IFLA_MASTER])))) if (!(*master = virNetDevGetName(*(int *)RTA_DATA(tb[IFLA_MASTER]))))
goto cleanup; return -1;
} }
VIR_DEBUG("IFLA_MASTER for %s is %s", ifname, *master ? *master : "(none)"); VIR_DEBUG("IFLA_MASTER for %s is %s", ifname, *master ? *master : "(none)");
ret = 0; return 0;
cleanup:
VIR_FREE(nlData);
return ret;
} }
@ -1090,57 +1080,44 @@ virNetDevSysfsDeviceFile(char **pf_sysfs_device_link, const char *ifname,
static bool static bool
virNetDevIsPCIDevice(const char *devpath) virNetDevIsPCIDevice(const char *devpath)
{ {
char *subsys_link = NULL; g_autofree char *subsys_link = NULL;
char *abs_path = NULL; g_autofree char *abs_path = NULL;
g_autofree char *subsys = NULL; g_autofree char *subsys = NULL;
bool ret = false;
subsys_link = g_strdup_printf("%s/subsystem", devpath); subsys_link = g_strdup_printf("%s/subsystem", devpath);
if (!virFileExists(subsys_link)) if (!virFileExists(subsys_link))
goto cleanup; return false;
if (virFileResolveLink(subsys_link, &abs_path) < 0) { if (virFileResolveLink(subsys_link, &abs_path) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to resolve device subsystem symlink %s"), _("Unable to resolve device subsystem symlink %s"),
subsys_link); subsys_link);
goto cleanup; return false;
} }
subsys = g_path_get_basename(abs_path); subsys = g_path_get_basename(abs_path);
ret = STRPREFIX(subsys, "pci"); return STRPREFIX(subsys, "pci");
cleanup:
VIR_FREE(subsys_link);
VIR_FREE(abs_path);
return ret;
} }
static virPCIDevicePtr static virPCIDevicePtr
virNetDevGetPCIDevice(const char *devName) virNetDevGetPCIDevice(const char *devName)
{ {
char *vfSysfsDevicePath = NULL; g_autofree char *vfSysfsDevicePath = NULL;
virPCIDeviceAddressPtr vfPCIAddr = NULL; g_autoptr(virPCIDeviceAddress) vfPCIAddr = NULL;
virPCIDevicePtr vfPCIDevice = NULL;
if (virNetDevSysfsFile(&vfSysfsDevicePath, devName, "device") < 0) if (virNetDevSysfsFile(&vfSysfsDevicePath, devName, "device") < 0)
goto cleanup; return NULL;
if (!virNetDevIsPCIDevice(vfSysfsDevicePath)) if (!virNetDevIsPCIDevice(vfSysfsDevicePath))
goto cleanup; return NULL;
vfPCIAddr = virPCIGetDeviceAddressFromSysfsLink(vfSysfsDevicePath); vfPCIAddr = virPCIGetDeviceAddressFromSysfsLink(vfSysfsDevicePath);
if (!vfPCIAddr) if (!vfPCIAddr)
goto cleanup; return NULL;
vfPCIDevice = virPCIDeviceNew(vfPCIAddr->domain, vfPCIAddr->bus, return virPCIDeviceNew(vfPCIAddr->domain, vfPCIAddr->bus,
vfPCIAddr->slot, vfPCIAddr->function); vfPCIAddr->slot, vfPCIAddr->function);
cleanup:
VIR_FREE(vfSysfsDevicePath);
VIR_FREE(vfPCIAddr);
return vfPCIDevice;
} }
@ -1162,25 +1139,20 @@ int
virNetDevGetPhysPortID(const char *ifname, virNetDevGetPhysPortID(const char *ifname,
char **physPortID) char **physPortID)
{ {
int ret = -1; g_autofree char *physPortIDFile = NULL;
char *physPortIDFile = NULL;
*physPortID = NULL; *physPortID = NULL;
if (virNetDevSysfsFile(&physPortIDFile, ifname, "phys_port_id") < 0) if (virNetDevSysfsFile(&physPortIDFile, ifname, "phys_port_id") < 0)
goto cleanup; return -1;
/* a failure to read just means the driver doesn't support /* a failure to read just means the driver doesn't support
* phys_port_id, so set success now and ignore the return from * phys_port_id, so set success now and ignore the return from
* virFileReadAllQuiet(). * virFileReadAllQuiet().
*/ */
ret = 0;
ignore_value(virFileReadAllQuiet(physPortIDFile, 1024, physPortID)); ignore_value(virFileReadAllQuiet(physPortIDFile, 1024, physPortID));
return 0;
cleanup:
VIR_FREE(physPortIDFile);
return ret;
} }
@ -1203,10 +1175,8 @@ virNetDevGetVirtualFunctions(const char *pfname,
{ {
int ret = -1; int ret = -1;
size_t i; size_t i;
char *pf_sysfs_device_link = NULL; g_autofree char *pf_sysfs_device_link = NULL;
char *pci_sysfs_device_link = NULL; g_autofree char *pfPhysPortID = NULL;
char *pciConfigAddr = NULL;
char *pfPhysPortID = NULL;
*virt_fns = NULL; *virt_fns = NULL;
*n_vfname = 0; *n_vfname = 0;
@ -1226,6 +1196,9 @@ virNetDevGetVirtualFunctions(const char *pfname,
goto cleanup; goto cleanup;
for (i = 0; i < *n_vfname; i++) { for (i = 0; i < *n_vfname; i++) {
g_autofree char *pciConfigAddr = NULL;
g_autofree char *pci_sysfs_device_link = NULL;
if (!(pciConfigAddr = virPCIDeviceAddressAsString((*virt_fns)[i]))) if (!(pciConfigAddr = virPCIDeviceAddressAsString((*virt_fns)[i])))
goto cleanup; goto cleanup;
@ -1248,13 +1221,14 @@ virNetDevGetVirtualFunctions(const char *pfname,
cleanup: cleanup:
if (ret < 0) { if (ret < 0) {
VIR_FREE(*vfname); virStringListFreeCount(*vfname, *n_vfname);
for (i = 0; i < *n_vfname; i++)
VIR_FREE((*virt_fns)[i]);
VIR_FREE(*virt_fns); VIR_FREE(*virt_fns);
*vfname = NULL;
*n_vfname = 0;
} }
VIR_FREE(pfPhysPortID);
VIR_FREE(pf_sysfs_device_link);
VIR_FREE(pci_sysfs_device_link);
VIR_FREE(pciConfigAddr);
return ret; return ret;
} }
@ -1270,17 +1244,12 @@ virNetDevGetVirtualFunctions(const char *pfname,
int int
virNetDevIsVirtualFunction(const char *ifname) virNetDevIsVirtualFunction(const char *ifname)
{ {
char *if_sysfs_device_link = NULL; g_autofree char *if_sysfs_device_link = NULL;
int ret = -1;
if (virNetDevSysfsFile(&if_sysfs_device_link, ifname, "device") < 0) if (virNetDevSysfsFile(&if_sysfs_device_link, ifname, "device") < 0)
return ret; return -1;
ret = virPCIIsVirtualFunction(if_sysfs_device_link); return virPCIIsVirtualFunction(if_sysfs_device_link);
VIR_FREE(if_sysfs_device_link);
return ret;
} }
/** /**
@ -1298,25 +1267,18 @@ int
virNetDevGetVirtualFunctionIndex(const char *pfname, const char *vfname, virNetDevGetVirtualFunctionIndex(const char *pfname, const char *vfname,
int *vf_index) int *vf_index)
{ {
char *pf_sysfs_device_link = NULL, *vf_sysfs_device_link = NULL; g_autofree char *pf_sysfs_device_link = NULL;
int ret = -1; g_autofree char *vf_sysfs_device_link = NULL;
if (virNetDevSysfsFile(&pf_sysfs_device_link, pfname, "device") < 0) if (virNetDevSysfsFile(&pf_sysfs_device_link, pfname, "device") < 0)
return ret; return -1;
if (virNetDevSysfsFile(&vf_sysfs_device_link, vfname, "device") < 0) { if (virNetDevSysfsFile(&vf_sysfs_device_link, vfname, "device") < 0)
VIR_FREE(pf_sysfs_device_link); return -1;
return ret;
}
ret = virPCIGetVirtualFunctionIndex(pf_sysfs_device_link, return virPCIGetVirtualFunctionIndex(pf_sysfs_device_link,
vf_sysfs_device_link, vf_sysfs_device_link,
vf_index); vf_index);
VIR_FREE(pf_sysfs_device_link);
VIR_FREE(vf_sysfs_device_link);
return ret;
} }
/** /**
@ -1332,19 +1294,18 @@ virNetDevGetVirtualFunctionIndex(const char *pfname, const char *vfname,
int int
virNetDevGetPhysicalFunction(const char *ifname, char **pfname) virNetDevGetPhysicalFunction(const char *ifname, char **pfname)
{ {
char *physfn_sysfs_path = NULL; g_autofree char *physfn_sysfs_path = NULL;
char *vfPhysPortID = NULL; g_autofree char *vfPhysPortID = NULL;
int ret = -1;
if (virNetDevGetPhysPortID(ifname, &vfPhysPortID) < 0) if (virNetDevGetPhysPortID(ifname, &vfPhysPortID) < 0)
goto cleanup; return -1;
if (virNetDevSysfsDeviceFile(&physfn_sysfs_path, ifname, "physfn") < 0) if (virNetDevSysfsDeviceFile(&physfn_sysfs_path, ifname, "physfn") < 0)
goto cleanup; return -1;
if (virPCIGetNetName(physfn_sysfs_path, 0, if (virPCIGetNetName(physfn_sysfs_path, 0,
vfPhysPortID, pfname) < 0) { vfPhysPortID, pfname) < 0) {
goto cleanup; return -1;
} }
if (!*pfname) { if (!*pfname) {
@ -1353,14 +1314,10 @@ virNetDevGetPhysicalFunction(const char *ifname, char **pfname)
virReportError(VIR_ERR_INTERNAL_ERROR, virReportError(VIR_ERR_INTERNAL_ERROR,
_("The PF device for VF %s has no network device name"), _("The PF device for VF %s has no network device name"),
ifname); ifname);
goto cleanup; return -1;
} }
ret = 0; return 0;
cleanup:
VIR_FREE(vfPhysPortID);
VIR_FREE(physfn_sysfs_path);
return ret;
} }
@ -1384,17 +1341,16 @@ virNetDevGetPhysicalFunction(const char *ifname, char **pfname)
int int
virNetDevPFGetVF(const char *pfname, int vf, char **vfname) virNetDevPFGetVF(const char *pfname, int vf, char **vfname)
{ {
char *virtfnName = NULL; g_autofree char *virtfnName = NULL;
char *virtfnSysfsPath = NULL; g_autofree char *virtfnSysfsPath = NULL;
char *pfPhysPortID = NULL; g_autofree char *pfPhysPortID = NULL;
int ret = -1;
/* a VF may have multiple "ports", each one having its own netdev, /* a VF may have multiple "ports", each one having its own netdev,
* and each netdev having a different phys_port_id. Be sure we get * and each netdev having a different phys_port_id. Be sure we get
* the VF netdev with a phys_port_id matchine that of pfname * the VF netdev with a phys_port_id matchine that of pfname
*/ */
if (virNetDevGetPhysPortID(pfname, &pfPhysPortID) < 0) if (virNetDevGetPhysPortID(pfname, &pfPhysPortID) < 0)
goto cleanup; return -1;
virtfnName = g_strdup_printf("virtfn%d", vf); virtfnName = g_strdup_printf("virtfn%d", vf);
@ -1402,7 +1358,7 @@ virNetDevPFGetVF(const char *pfname, int vf, char **vfname)
* e.g. "/sys/class/net/enp2s0f0/virtfn3" * e.g. "/sys/class/net/enp2s0f0/virtfn3"
*/ */
if (virNetDevSysfsDeviceFile(&virtfnSysfsPath, pfname, virtfnName) < 0) if (virNetDevSysfsDeviceFile(&virtfnSysfsPath, pfname, virtfnName) < 0)
goto cleanup; return -1;
/* and this gets the netdev name associated with it, which is a /* and this gets the netdev name associated with it, which is a
* directory entry in [virtfnSysfsPath]/net, * directory entry in [virtfnSysfsPath]/net,
@ -1411,14 +1367,7 @@ virNetDevPFGetVF(const char *pfname, int vf, char **vfname)
* isn't bound to a netdev driver, it won't have a netdev name, * isn't bound to a netdev driver, it won't have a netdev name,
* and vfname will be NULL). * and vfname will be NULL).
*/ */
ret = virPCIGetNetName(virtfnSysfsPath, 0, pfPhysPortID, vfname); return virPCIGetNetName(virtfnSysfsPath, 0, pfPhysPortID, vfname);
cleanup:
VIR_FREE(virtfnName);
VIR_FREE(virtfnSysfsPath);
VIR_FREE(pfPhysPortID);
return ret;
} }
@ -1559,7 +1508,7 @@ virNetDevSetVfConfig(const char *ifname, int vf,
{ {
int rc = -1; int rc = -1;
char macstr[VIR_MAC_STRING_BUFLEN]; char macstr[VIR_MAC_STRING_BUFLEN];
struct nlmsghdr *resp = NULL; g_autofree struct nlmsghdr *resp = NULL;
struct nlmsgerr *err; struct nlmsgerr *err;
unsigned int recvbuflen = 0; unsigned int recvbuflen = 0;
struct nl_msg *nl_msg; struct nl_msg *nl_msg;
@ -1671,7 +1620,6 @@ virNetDevSetVfConfig(const char *ifname, int vf,
vlanid, rc < 0 ? "Fail" : "Success"); vlanid, rc < 0 ? "Fail" : "Success");
nlmsg_free(nl_msg); nlmsg_free(nl_msg);
VIR_FREE(resp);
return rc; return rc;
malformed_resp: malformed_resp:
@ -1743,20 +1691,14 @@ static int
virNetDevGetVfConfig(const char *ifname, int vf, virMacAddrPtr mac, virNetDevGetVfConfig(const char *ifname, int vf, virMacAddrPtr mac,
int *vlanid) int *vlanid)
{ {
int rc = -1; g_autofree void *nlData = NULL;
void *nlData = NULL;
struct nlattr *tb[IFLA_MAX + 1] = {NULL, }; struct nlattr *tb[IFLA_MAX + 1] = {NULL, };
int ifindex = -1; int ifindex = -1;
rc = virNetlinkDumpLink(ifname, ifindex, &nlData, tb, 0, 0); if (virNetlinkDumpLink(ifname, ifindex, &nlData, tb, 0, 0) < 0)
if (rc < 0) return -1;
goto cleanup;
rc = virNetDevParseVfConfig(tb, vf, mac, vlanid); return virNetDevParseVfConfig(tb, vf, mac, vlanid);
cleanup:
VIR_FREE(nlData);
return rc;
} }
@ -1813,16 +1755,15 @@ virNetDevSaveNetConfig(const char *linkdev, int vf,
const char *stateDir, const char *stateDir,
bool saveVlan) bool saveVlan)
{ {
int ret = -1;
const char *pfDevName = NULL; const char *pfDevName = NULL;
char *pfDevOrig = NULL; g_autofree char *pfDevOrig = NULL;
char *vfDevOrig = NULL; g_autofree char *vfDevOrig = NULL;
virMacAddr oldMAC; virMacAddr oldMAC;
char MACStr[VIR_MAC_STRING_BUFLEN]; char MACStr[VIR_MAC_STRING_BUFLEN];
int oldVlanTag = -1; int oldVlanTag = -1;
char *filePath = NULL; g_autofree char *filePath = NULL;
char *fileStr = NULL; g_autofree char *fileStr = NULL;
virJSONValuePtr configJSON = NULL; g_autoptr(virJSONValue) configJSON = NULL;
if (vf >= 0) { if (vf >= 0) {
/* linkdev is the PF */ /* linkdev is the PF */
@ -1830,7 +1771,7 @@ virNetDevSaveNetConfig(const char *linkdev, int vf,
/* linkdev should get the VF's netdev name (or NULL if none) */ /* linkdev should get the VF's netdev name (or NULL if none) */
if (virNetDevPFGetVF(pfDevName, vf, &vfDevOrig) < 0) if (virNetDevPFGetVF(pfDevName, vf, &vfDevOrig) < 0)
goto cleanup; return -1;
linkdev = vfDevOrig; linkdev = vfDevOrig;
saveVlan = true; saveVlan = true;
@ -1842,7 +1783,7 @@ virNetDevSaveNetConfig(const char *linkdev, int vf,
*/ */
if (virNetDevGetVirtualFunctionInfo(linkdev, &pfDevOrig, &vf) < 0) if (virNetDevGetVirtualFunctionInfo(linkdev, &pfDevOrig, &vf) < 0)
goto cleanup; return -1;
pfDevName = pfDevOrig; pfDevName = pfDevOrig;
} }
@ -1861,7 +1802,7 @@ virNetDevSaveNetConfig(const char *linkdev, int vf,
* explicitly enable the PF in the host system network config. * explicitly enable the PF in the host system network config.
*/ */
if (virNetDevGetOnline(pfDevName, &pfIsOnline) < 0) if (virNetDevGetOnline(pfDevName, &pfIsOnline) < 0)
goto cleanup; return -1;
if (!pfIsOnline) { if (!pfIsOnline) {
virReportError(VIR_ERR_INTERNAL_ERROR, virReportError(VIR_ERR_INTERNAL_ERROR,
@ -1870,7 +1811,7 @@ virNetDevSaveNetConfig(const char *linkdev, int vf,
"change host network config to put the " "change host network config to put the "
"PF online."), "PF online."),
vf, pfDevName); vf, pfDevName);
goto cleanup; return -1;
} }
} }
@ -1886,7 +1827,7 @@ virNetDevSaveNetConfig(const char *linkdev, int vf,
/* get admin MAC and vlan tag */ /* get admin MAC and vlan tag */
if (virNetDevGetVfConfig(pfDevName, vf, &oldMAC, &oldVlanTag) < 0) if (virNetDevGetVfConfig(pfDevName, vf, &oldMAC, &oldVlanTag) < 0)
goto cleanup; return -1;
if (virJSONValueObjectAppendString(configJSON, if (virJSONValueObjectAppendString(configJSON,
VIR_NETDEV_KEYNAME_ADMIN_MAC, VIR_NETDEV_KEYNAME_ADMIN_MAC,
@ -1894,7 +1835,7 @@ virNetDevSaveNetConfig(const char *linkdev, int vf,
virJSONValueObjectAppendNumberInt(configJSON, virJSONValueObjectAppendNumberInt(configJSON,
VIR_NETDEV_KEYNAME_VLAN_TAG, VIR_NETDEV_KEYNAME_VLAN_TAG,
oldVlanTag) < 0) { oldVlanTag) < 0) {
goto cleanup; return -1;
} }
} else { } else {
@ -1903,33 +1844,26 @@ virNetDevSaveNetConfig(const char *linkdev, int vf,
if (linkdev) { if (linkdev) {
if (virNetDevGetMAC(linkdev, &oldMAC) < 0) if (virNetDevGetMAC(linkdev, &oldMAC) < 0)
goto cleanup; return -1;
/* for interfaces with no pfDevName (i.e. not a VF, this will /* for interfaces with no pfDevName (i.e. not a VF, this will
* be the only value in the file. * be the only value in the file.
*/ */
if (virJSONValueObjectAppendString(configJSON, VIR_NETDEV_KEYNAME_MAC, if (virJSONValueObjectAppendString(configJSON, VIR_NETDEV_KEYNAME_MAC,
virMacAddrFormat(&oldMAC, MACStr)) < 0) virMacAddrFormat(&oldMAC, MACStr)) < 0)
goto cleanup; return -1;
} }
if (!(fileStr = virJSONValueToString(configJSON, true))) if (!(fileStr = virJSONValueToString(configJSON, true)))
goto cleanup; return -1;
if (virFileWriteStr(filePath, fileStr, O_CREAT|O_TRUNC|O_WRONLY) < 0) { if (virFileWriteStr(filePath, fileStr, O_CREAT|O_TRUNC|O_WRONLY) < 0) {
virReportSystemError(errno, _("Unable to preserve mac/vlan tag " virReportSystemError(errno, _("Unable to preserve mac/vlan tag "
"for device = %s, vf = %d"), linkdev, vf); "for device = %s, vf = %d"), linkdev, vf);
goto cleanup; return -1;
} }
ret = 0; return 0;
cleanup:
VIR_FREE(pfDevOrig);
VIR_FREE(vfDevOrig);
VIR_FREE(filePath);
VIR_FREE(fileStr);
virJSONValueFree(configJSON);
return ret;
} }
@ -1963,11 +1897,11 @@ virNetDevReadNetConfig(const char *linkdev, int vf,
{ {
int ret = -1; int ret = -1;
const char *pfDevName = NULL; const char *pfDevName = NULL;
char *pfDevOrig = NULL; g_autofree char *pfDevOrig = NULL;
char *vfDevOrig = NULL; g_autofree char *vfDevOrig = NULL;
char *filePath = NULL; g_autofree char *filePath = NULL;
char *fileStr = NULL; g_autofree char *fileStr = NULL;
virJSONValuePtr configJSON = NULL; g_autoptr(virJSONValue) configJSON = NULL;
const char *MACStr = NULL; const char *MACStr = NULL;
const char *adminMACStr = NULL; const char *adminMACStr = NULL;
int vlanTag = -1; int vlanTag = -1;
@ -2132,11 +2066,6 @@ virNetDevReadNetConfig(const char *linkdev, int vf,
VIR_FREE(*vlan); VIR_FREE(*vlan);
} }
VIR_FREE(pfDevOrig);
VIR_FREE(vfDevOrig);
VIR_FREE(filePath);
VIR_FREE(fileStr);
virJSONValueFree(configJSON);
return ret; return ret;
} }
@ -2166,13 +2095,12 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
const virMacAddr *MAC, const virMacAddr *MAC,
bool setVlan) bool setVlan)
{ {
int ret = -1;
char MACStr[VIR_MAC_STRING_BUFLEN]; char MACStr[VIR_MAC_STRING_BUFLEN];
const char *pfDevName = NULL; const char *pfDevName = NULL;
char *pfDevOrig = NULL; g_autofree char *pfDevOrig = NULL;
char *vfDevOrig = NULL; g_autofree char *vfDevOrig = NULL;
int vlanTag = -1; int vlanTag = -1;
virPCIDevicePtr vfPCIDevice = NULL; g_autoptr(virPCIDevice) vfPCIDevice = NULL;
if (vf >= 0) { if (vf >= 0) {
/* linkdev is the PF */ /* linkdev is the PF */
@ -2180,7 +2108,7 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
/* linkdev should get the VF's netdev name (or NULL if none) */ /* linkdev should get the VF's netdev name (or NULL if none) */
if (virNetDevPFGetVF(pfDevName, vf, &vfDevOrig) < 0) if (virNetDevPFGetVF(pfDevName, vf, &vfDevOrig) < 0)
goto cleanup; return -1;
linkdev = vfDevOrig; linkdev = vfDevOrig;
@ -2191,7 +2119,7 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
*/ */
if (virNetDevGetVirtualFunctionInfo(linkdev, &pfDevOrig, &vf)) if (virNetDevGetVirtualFunctionInfo(linkdev, &pfDevOrig, &vf))
goto cleanup; return -1;
pfDevName = pfDevOrig; pfDevName = pfDevOrig;
} }
@ -2204,14 +2132,14 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("admin MAC can only be set for SR-IOV VFs, but " _("admin MAC can only be set for SR-IOV VFs, but "
"%s is not a VF"), linkdev); "%s is not a VF"), linkdev);
goto cleanup; return -1;
} }
if (vlan) { if (vlan) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("vlan can only be set for SR-IOV VFs, but " _("vlan can only be set for SR-IOV VFs, but "
"%s is not a VF"), linkdev); "%s is not a VF"), linkdev);
goto cleanup; return -1;
} }
} else { } else {
@ -2220,14 +2148,14 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("vlan trunking is not supported " _("vlan trunking is not supported "
"by SR-IOV network devices")); "by SR-IOV network devices"));
goto cleanup; return -1;
} }
if (!setVlan) { if (!setVlan) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("vlan tag set for interface %s but " _("vlan tag set for interface %s but "
"caller requested it not be set")); "caller requested it not be set"));
goto cleanup; return -1;
} }
vlanTag = vlan->tag[0]; vlanTag = vlan->tag[0];
@ -2245,7 +2173,7 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
_("VF %d of PF '%s' is not bound to a net driver, " _("VF %d of PF '%s' is not bound to a net driver, "
"so its MAC address cannot be set to %s"), "so its MAC address cannot be set to %s"),
vf, pfDevName, virMacAddrFormat(MAC, MACStr)); vf, pfDevName, virMacAddrFormat(MAC, MACStr));
goto cleanup; return -1;
} }
setMACrc = virNetDevSetMACInternal(linkdev, MAC, !!pfDevOrig); setMACrc = virNetDevSetMACInternal(linkdev, MAC, !!pfDevOrig);
@ -2256,7 +2184,7 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
/* if pfDevOrig == NULL, this isn't a VF, so we've failed */ /* if pfDevOrig == NULL, this isn't a VF, so we've failed */
if (!pfDevOrig || if (!pfDevOrig ||
(errno != EADDRNOTAVAIL && errno != EPERM)) (errno != EADDRNOTAVAIL && errno != EPERM))
goto cleanup; return -1;
/* Otherwise this is a VF, and virNetDevSetMAC failed with /* Otherwise this is a VF, and virNetDevSetMAC failed with
* EADDRNOTAVAIL/EPERM, which could be due to the * EADDRNOTAVAIL/EPERM, which could be due to the
@ -2270,18 +2198,18 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
if (virNetDevSetVfConfig(pfDevName, vf, if (virNetDevSetVfConfig(pfDevName, vf,
MAC, vlanTag, &allowRetry) < 0) { MAC, vlanTag, &allowRetry) < 0) {
goto cleanup; return -1;
} }
/* admin MAC is set, now we need to construct a virPCIDevice /* admin MAC is set, now we need to construct a virPCIDevice
* object so we can call virPCIDeviceRebind() * object so we can call virPCIDeviceRebind()
*/ */
if (!(vfPCIDevice = virNetDevGetPCIDevice(linkdev))) if (!(vfPCIDevice = virNetDevGetPCIDevice(linkdev)))
goto cleanup; return -1;
/* Rebind the device. This should set the proper MAC address */ /* Rebind the device. This should set the proper MAC address */
if (virPCIDeviceRebind(vfPCIDevice) < 0) if (virPCIDeviceRebind(vfPCIDevice) < 0)
goto cleanup; return -1;
/* Wait until virNetDevGetIndex for the VF netdev returns success. /* Wait until virNetDevGetIndex for the VF netdev returns success.
* This indicates that the device is ready to be used. If we don't * This indicates that the device is ready to be used. If we don't
@ -2333,22 +2261,17 @@ virNetDevSetNetConfig(const char *linkdev, int vf,
* with the "locally administered" bit set. * with the "locally administered" bit set.
*/ */
if (!allowRetry) if (!allowRetry)
goto cleanup; return -1;
allowRetry = false; allowRetry = false;
if (virNetDevSetVfConfig(pfDevName, vf, if (virNetDevSetVfConfig(pfDevName, vf,
&altZeroMAC, vlanTag, &allowRetry) < 0) { &altZeroMAC, vlanTag, &allowRetry) < 0) {
goto cleanup; return -1;
} }
} }
} }
ret = 0; return 0;
cleanup:
VIR_FREE(pfDevOrig);
VIR_FREE(vfDevOrig);
virPCIDeviceFree(vfPCIDevice);
return ret;
} }
@ -2428,28 +2351,27 @@ int
virNetDevGetLinkInfo(const char *ifname, virNetDevGetLinkInfo(const char *ifname,
virNetDevIfLinkPtr lnk) virNetDevIfLinkPtr lnk)
{ {
int ret = -1; g_autofree char *path = NULL;
char *path = NULL; g_autofree char *buf = NULL;
char *buf = NULL;
char *tmp; char *tmp;
int tmp_state; int tmp_state;
unsigned int tmp_speed; unsigned int tmp_speed;
if (virNetDevSysfsFile(&path, ifname, "operstate") < 0) if (virNetDevSysfsFile(&path, ifname, "operstate") < 0)
goto cleanup; return -1;
if (virFileReadAll(path, 1024, &buf) < 0) { if (virFileReadAll(path, 1024, &buf) < 0) {
virReportSystemError(errno, virReportSystemError(errno,
_("unable to read: %s"), _("unable to read: %s"),
path); path);
goto cleanup; return -1;
} }
if (!(tmp = strchr(buf, '\n'))) { if (!(tmp = strchr(buf, '\n'))) {
virReportError(VIR_ERR_INTERNAL_ERROR, virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse: %s"), _("Unable to parse: %s"),
buf); buf);
goto cleanup; return -1;
} }
*tmp = '\0'; *tmp = '\0';
@ -2460,7 +2382,7 @@ virNetDevGetLinkInfo(const char *ifname,
virReportError(VIR_ERR_INTERNAL_ERROR, virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse: %s"), _("Unable to parse: %s"),
buf); buf);
goto cleanup; return -1;
} }
lnk->state = tmp_state; lnk->state = tmp_state;
@ -2471,26 +2393,23 @@ virNetDevGetLinkInfo(const char *ifname,
* speed if that's the case. */ * speed if that's the case. */
if (lnk->state != VIR_NETDEV_IF_STATE_UP) { if (lnk->state != VIR_NETDEV_IF_STATE_UP) {
lnk->speed = 0; lnk->speed = 0;
ret = 0; return 0;
goto cleanup;
} }
VIR_FREE(path); VIR_FREE(path);
VIR_FREE(buf); VIR_FREE(buf);
if (virNetDevSysfsFile(&path, ifname, "speed") < 0) if (virNetDevSysfsFile(&path, ifname, "speed") < 0)
goto cleanup; return -1;
if (virFileReadAllQuiet(path, 1024, &buf) < 0) { if (virFileReadAllQuiet(path, 1024, &buf) < 0) {
/* Some devices doesn't report speed, in which case we get EINVAL */ /* Some devices doesn't report speed, in which case we get EINVAL */
if (errno == EINVAL) { if (errno == EINVAL)
ret = 0; return 0;
goto cleanup;
}
virReportSystemError(errno, virReportSystemError(errno,
_("unable to read: %s"), _("unable to read: %s"),
path); path);
goto cleanup; return -1;
} }
if (virStrToLong_ui(buf, &tmp, 10, &tmp_speed) < 0 || if (virStrToLong_ui(buf, &tmp, 10, &tmp_speed) < 0 ||
@ -2498,16 +2417,12 @@ virNetDevGetLinkInfo(const char *ifname,
virReportError(VIR_ERR_INTERNAL_ERROR, virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse: %s"), _("Unable to parse: %s"),
buf); buf);
goto cleanup; return -1;
} }
lnk->speed = tmp_speed; lnk->speed = tmp_speed;
ret = 0; return 0;
cleanup:
VIR_FREE(buf);
VIR_FREE(path);
return ret;
} }
#else #else
@ -2707,9 +2622,9 @@ static int virNetDevGetMcastList(const char *ifname,
virNetDevMcastListPtr mcast) virNetDevMcastListPtr mcast)
{ {
char *cur = NULL; char *cur = NULL;
char *buf = NULL; g_autofree char *buf = NULL;
char *next = NULL; char *next = NULL;
int ret = -1, len; int len;
g_autoptr(virNetDevMcastEntry) entry = NULL; g_autoptr(virNetDevMcastEntry) entry = NULL;
mcast->entries = NULL; mcast->entries = NULL;
@ -2717,35 +2632,31 @@ static int virNetDevGetMcastList(const char *ifname,
/* Read entire multicast table into memory */ /* Read entire multicast table into memory */
if ((len = virFileReadAll(PROC_NET_DEV_MCAST, MAX_MCAST_SIZE, &buf)) <= 0) if ((len = virFileReadAll(PROC_NET_DEV_MCAST, MAX_MCAST_SIZE, &buf)) <= 0)
goto cleanup; return -1;
cur = buf; cur = buf;
while (cur) { while (cur) {
if (!entry && VIR_ALLOC(entry) < 0) if (!entry && VIR_ALLOC(entry) < 0)
goto cleanup; return -1;
next = strchr(cur, '\n'); next = strchr(cur, '\n');
if (next) if (next)
next++; next++;
if (virNetDevParseMcast(cur, entry)) if (virNetDevParseMcast(cur, entry))
goto cleanup; return -1;
/* Only return global multicast MAC addresses for /* Only return global multicast MAC addresses for
* specified interface */ * specified interface */
if (entry->global && STREQ(ifname, entry->name)) { if (entry->global && STREQ(ifname, entry->name)) {
if (VIR_APPEND_ELEMENT(mcast->entries, mcast->nentries, entry)) if (VIR_APPEND_ELEMENT(mcast->entries, mcast->nentries, entry))
goto cleanup; return -1;
} else { } else {
memset(entry, 0, sizeof(virNetDevMcastEntry)); memset(entry, 0, sizeof(virNetDevMcastEntry));
} }
cur = next && ((next - buf) < len) ? next : NULL; cur = next && ((next - buf) < len) ? next : NULL;
} }
ret = 0; return 0;
cleanup:
VIR_FREE(buf);
return ret;
} }
@ -2883,10 +2794,8 @@ static int
virNetDevRDMAFeature(const char *ifname, virNetDevRDMAFeature(const char *ifname,
virBitmapPtr *out) virBitmapPtr *out)
{ {
char *eth_devpath = NULL; g_autofree char *eth_devpath = NULL;
char *ib_devpath = NULL; g_autofree char *eth_res_buf = NULL;
char *eth_res_buf = NULL;
char *ib_res_buf = NULL;
DIR *dirp = NULL; DIR *dirp = NULL;
struct dirent *dp; struct dirent *dp;
int ret = -1; int ret = -1;
@ -2910,24 +2819,20 @@ virNetDevRDMAFeature(const char *ifname,
goto cleanup; goto cleanup;
while (virDirRead(dirp, &dp, SYSFS_INFINIBAND_DIR) > 0) { while (virDirRead(dirp, &dp, SYSFS_INFINIBAND_DIR) > 0) {
ib_devpath = g_strdup_printf(SYSFS_INFINIBAND_DIR "%s/device/resource", g_autofree char *ib_res_buf = NULL;
dp->d_name); g_autofree char *ib_devpath = g_strdup_printf(SYSFS_INFINIBAND_DIR "%s/device/resource",
dp->d_name);
if (virFileReadAll(ib_devpath, RESOURCE_FILE_LEN, &ib_res_buf) > 0 && if (virFileReadAll(ib_devpath, RESOURCE_FILE_LEN, &ib_res_buf) > 0 &&
STREQ(eth_res_buf, ib_res_buf)) { STREQ(eth_res_buf, ib_res_buf)) {
ignore_value(virBitmapSetBit(*out, VIR_NET_DEV_FEAT_RDMA)); ignore_value(virBitmapSetBit(*out, VIR_NET_DEV_FEAT_RDMA));
break; break;
} }
VIR_FREE(ib_devpath);
VIR_FREE(ib_res_buf);
} }
ret = 0; ret = 0;
cleanup: cleanup:
VIR_DIR_CLOSE(dirp); VIR_DIR_CLOSE(dirp);
VIR_FREE(eth_devpath);
VIR_FREE(ib_devpath);
VIR_FREE(eth_res_buf);
VIR_FREE(ib_res_buf);
return ret; return ret;
} }
@ -3068,7 +2973,7 @@ virNetDevGetFamilyId(const char *family_name,
uint32_t *family_id) uint32_t *family_id)
{ {
struct nl_msg *nl_msg = NULL; struct nl_msg *nl_msg = NULL;
struct nlmsghdr *resp = NULL; g_autofree struct nlmsghdr *resp = NULL;
struct genlmsghdr gmsgh = { struct genlmsghdr gmsgh = {
.cmd = CTRL_CMD_GETFAMILY, .cmd = CTRL_CMD_GETFAMILY,
.version = DEVLINK_GENL_VERSION, .version = DEVLINK_GENL_VERSION,
@ -3112,7 +3017,6 @@ virNetDevGetFamilyId(const char *family_name,
cleanup: cleanup:
nlmsg_free(nl_msg); nlmsg_free(nl_msg);
VIR_FREE(resp);
return ret; return ret;
} }
@ -3132,17 +3036,17 @@ virNetDevSwitchdevFeature(const char *ifname,
virBitmapPtr *out) virBitmapPtr *out)
{ {
struct nl_msg *nl_msg = NULL; struct nl_msg *nl_msg = NULL;
struct nlmsghdr *resp = NULL; g_autofree struct nlmsghdr *resp = NULL;
unsigned int recvbuflen; unsigned int recvbuflen;
struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {NULL, }; struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {NULL, };
virPCIDevicePtr pci_device_ptr = NULL; g_autoptr(virPCIDevice) pci_device_ptr = NULL;
struct genlmsghdr gmsgh = { struct genlmsghdr gmsgh = {
.cmd = DEVLINK_CMD_ESWITCH_GET, .cmd = DEVLINK_CMD_ESWITCH_GET,
.version = DEVLINK_GENL_VERSION, .version = DEVLINK_GENL_VERSION,
.reserved = 0, .reserved = 0,
}; };
const char *pci_name; const char *pci_name;
char *pfname = NULL; g_autofree char *pfname = NULL;
int is_vf = -1; int is_vf = -1;
int ret = -1; int ret = -1;
uint32_t family_id; uint32_t family_id;
@ -3161,10 +3065,8 @@ virNetDevSwitchdevFeature(const char *ifname,
pci_device_ptr = pfname ? virNetDevGetPCIDevice(pfname) : pci_device_ptr = pfname ? virNetDevGetPCIDevice(pfname) :
virNetDevGetPCIDevice(ifname); virNetDevGetPCIDevice(ifname);
/* No PCI device, then no feature bit to check/add */ /* No PCI device, then no feature bit to check/add */
if (pci_device_ptr == NULL) { if (pci_device_ptr == NULL)
ret = 0; return 0;
goto cleanup;
}
if ((rv = virNetDevGetFamilyId(DEVLINK_GENL_NAME, &family_id)) <= 0) if ((rv = virNetDevGetFamilyId(DEVLINK_GENL_NAME, &family_id)) <= 0)
return rv; return rv;
@ -3205,9 +3107,6 @@ virNetDevSwitchdevFeature(const char *ifname,
cleanup: cleanup:
nlmsg_free(nl_msg); nlmsg_free(nl_msg);
virPCIDeviceFree(pci_device_ptr);
VIR_FREE(resp);
VIR_FREE(pfname);
return ret; return ret;
} }
# else # else
@ -3248,7 +3147,7 @@ virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap,
int fd, int fd,
struct ifreq *ifr) struct ifreq *ifr)
{ {
struct ethtool_gfeatures *g_cmd; g_autofree struct ethtool_gfeatures *g_cmd = NULL;
if (VIR_ALLOC_VAR(g_cmd, if (VIR_ALLOC_VAR(g_cmd,
struct ethtool_get_features_block, GFEATURES_SIZE) < 0) struct ethtool_get_features_block, GFEATURES_SIZE) < 0)
@ -3258,7 +3157,6 @@ virNetDevGetEthtoolGFeatures(virBitmapPtr bitmap,
g_cmd->size = GFEATURES_SIZE; g_cmd->size = GFEATURES_SIZE;
if (virNetDevGFeatureAvailable(fd, ifr, g_cmd)) if (virNetDevGFeatureAvailable(fd, ifr, g_cmd))
ignore_value(virBitmapSetBit(bitmap, VIR_NET_DEV_FEAT_TXUDPTNL)); ignore_value(virBitmapSetBit(bitmap, VIR_NET_DEV_FEAT_TXUDPTNL));
VIR_FREE(g_cmd);
return 0; return 0;
} }
# else # else
@ -3456,8 +3354,7 @@ int virNetDevSetCoalesce(const char *ifname,
int int
virNetDevRunEthernetScript(const char *ifname, const char *script) virNetDevRunEthernetScript(const char *ifname, const char *script)
{ {
virCommandPtr cmd; g_autoptr(virCommand) cmd = NULL;
int ret;
/* Not a bug! Previously we did accept script="" as a NO-OP. */ /* Not a bug! Previously we did accept script="" as a NO-OP. */
if (STREQ(script, "")) if (STREQ(script, ""))
@ -3471,8 +3368,5 @@ virNetDevRunEthernetScript(const char *ifname, const char *script)
#endif #endif
virCommandAddEnvPassCommon(cmd); virCommandAddEnvPassCommon(cmd);
ret = virCommandRun(cmd, NULL); return virCommandRun(cmd, NULL);
virCommandFree(cmd);
return ret;
} }