On Thursday 27 May 2010, Stefan Berger wrote:
> +static int
> +getPortProfileStatus(struct nlattr **tb, int32_t vf, uint16_t *status)
> +{
> + int rc = 1;
> + const char *msg = NULL;
> + struct nlattr *tb2[IFLA_VF_PORT_MAX + 1],
> + *tb3[IFLA_PORT_MAX+1];
> +
> + if (vf == PORT_SELF_VF) {
> + if (tb[IFLA_PORT_SELF]) {
> + if (nla_parse_nested(tb3, IFLA_PORT_MAX, tb[IFLA_PORT_SELF],
> + ifla_port_policy)) {
> + msg = _("error parsing nested IFLA_VF_PORT part");
> + goto err_exit;
> + }
> + }
> + } else {
> + if (tb[IFLA_VF_PORTS]) {
> + if (nla_parse_nested(tb2, IFLA_VF_PORT_MAX, tb[IFLA_VF_PORTS],
> + ifla_vf_ports_policy)) {
> + msg = _("error parsing nested IFLA_VF_PORTS part");
> + goto err_exit;
> + }
> + if (tb2[IFLA_VF_PORT]) {
> + if (nla_parse_nested(tb3, IFLA_PORT_MAX, tb2[IFLA_VF_PORT],
> + ifla_port_policy)) {
> + msg = _("error parsing nested IFLA_VF_PORT part");
> + goto err_exit;
> + }
> + }
> + }
> + }
There may be multiple IFLA_VF_PORT attributes in the IFLA_VF_PORTS list,
so you cannot do nla_parse_nested. I think this should be nla_for_each_attr
instead, and compare the uuid to the one you expect.
> + memcpy(ifla_vf_mac.mac, macaddr, 6);
> +
> + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VFINFO_LIST,
> + NULL, 0);
> + if (!rta ||
> + !(vfinfolist = nlAppend(nlm, sizeof(nlmsgbuf),
> + rtattbuf, rta->rta_len)))
> + goto buffer_too_small;
> +
> + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_INFO,
> + NULL, 0);
> + if (!rta ||
> + !(vfinfo = nlAppend(nlm, sizeof(nlmsgbuf),
> + rtattbuf, rta->rta_len)))
> + goto buffer_too_small;
> +
> + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_MAC,
> + &ifla_vf_mac, sizeof(ifla_vf_mac));
> + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
> + goto buffer_too_small;
> +
> + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_VLAN,
> + &ifla_vf_vlan, sizeof(ifla_vf_vlan));
> +
> + if (!rta || !nlAppend(nlm, sizeof(nlmsgbuf), rtattbuf, rta->rta_len))
> + goto buffer_too_small;
> +
> + vfinfo->rta_len = (char *)nlm + nlm->nlmsg_len - (char *)vfinfo;
> +
> + vfinfolist->rta_len = (char *)nlm + nlm->nlmsg_len -
> + (char *)vfinfolist;
> + }
This part looks good now.
> + if (vf == PORT_SELF_VF) {
> + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_PORT_SELF, NULL, 0);
> + } else {
> + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_PORTS, NULL, 0);
> + if (!rta ||
> + !(vfports = nlAppend(nlm, sizeof(nlmsgbuf),
> + rtattbuf, rta->rta_len)))
> + goto buffer_too_small;
> +
> + /* begin nesting vfports */
> + rta = rtattrCreate(rtattbuf, sizeof(rtattbuf), IFLA_VF_PORT, NULL, 0);
> + }
But this still goes down the IFLA_PORT_SELF route because you pass PORT_SELF_VF
even for nltarget_kernel==false, where it makes no sense.
Maybe make the above
if (vf == PORT_SELF_VF && nltarget_kernel)
> + if (vf != PORT_SELF_VF) {
> + /* end nesting of vfports */
> + vfports->rta_len = (char *)nlm + nlm->nlmsg_len - (char *)vfports;
> + }
Here too.
> + if (nltarget_kernel) {
> + if (nlComm(nlm, &recvbuf, &recvbuflen) < 0)
> + return -1;
> + } else {
> + if (nlCommWaitSuccess(nlm, RTMGRP_LINK, &recvbuf, &recvbuflen,
> + 5 * MICROSEC_PER_SEC) < 0)
> + return -1;
> + }
I don't understand this part yet. Do we need this difference?
> + while (--repeats >= 0) {
> + rc = link_dump(nltarget_kernel, NULL, ifindex, tb, &recvbuf);
> + if (rc)
> + goto err_exit;
> + rc = getPortProfileStatus(tb, vf, &status);
> + if (rc == 0) {
> + if (status == PORT_PROFILE_RESPONSE_SUCCESS ||
> + status == PORT_VDP_RESPONSE_SUCCESS) {
> + break;
> + } else if (status == PORT_PROFILE_RESPONSE_INPROGRESS) {
> + // keep trying...
> + } else {
> + virReportSystemError(EINVAL,
> + _("error %d during port-profile setlink on ifindex %d"),
> + status, ifindex);
> + rc = 1;
> + break;
> + }
Hmm, we seem to be missing an INPROGRESS status for Qbg. Any suggestions
what we should return there? Should we possibly just leave out
IFLA_PORT_RESPONSE in order to signal INPROGRESS, as in not clear yet?
> + rc = doPortProfileOpCommon(nltarget_kernel,
> + physdev_ifname, physdev_ifindex,
> + macaddr,
> + vlanid,
> + NULL,
> + &portVsi,
> + virtPort->u.virtPort8021Qbg.instanceID,
> + NULL,
> + PORT_SELF_VF,
> + op);
This is where we pass PORT_SELF_VF together with nltarget_kernel=false,
as mentioned above.
Arnd
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list