On 2021-01-07 at 10:52, Shi Lei wrote:
On 2021-01-06 at 18:21, Michal Privoznik wrote:
On 1/6/21 3:40 AM, Shi Lei wrote:
Extract common code as helper function virNetlinkTalk, then simplify the functions virNetlink[DumpLink|NewLink|DelLink|GetNeighbor].
- default: - goto malformed_resp; + if (resp->nlmsg_type != NLMSG_DONE) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed netlink response message")); + return -1; }
This is not correct IMO. The way that control flows through this function before your patch is (I'm trying to create virbr0 using 'virsh net-start default'):
1) virNetlinkCommand() suceeds, 2) resp->nlmsg_type == NLMSG_ERROR even though the bridge was created, 3) err->error is 0 hence we hit 'break' and fall out of the switch and ..
return 0;
Yes. So the result ((resp->nlmsg_type == NLMSG_ERROR) && (err->error == 0)) should be regarded as a success.
I feel like that this piece of code can be like:
if (((resp->nlmsg_type != NLMSG_ERROR) || (*error != 0)) && (resp->nlmsg_type != NLMSG_DONE) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed netlink response message")); return -1; }
return 0;
This piece of code can be simplified as : if (resp->nlmsg_type != NLMSG_ERROR && resp->nlmsg_type != NLMSG_DONE) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed netlink response message")); return -1; } When we reach here, virNetlinkTalk have returned 0. And if (resp->nlmsg_type == NLMSG_ERROR), then (*error) must be 0. Shi Lei