[libvirt] [PATCH 3/4] virnetdevvportprofile: Changes to support portprofiles for hostdevs
Laine Stump
laine at laine.org
Mon Mar 5 19:16:49 UTC 2012
I encountered two conflicts when I rebased this patch to upstream. Noted
in the comments.
On 03/04/2012 10:15 PM, Roopa Prabhu wrote:
> From: Roopa Prabhu <roprabhu at cisco.com>
>
> This patch includes the following changes
> - removes some netlink functions which are now available in virnetdev.c
> - Adds a vf argument to all port profile functions
>
> For 802.1Qbh devices, the port profile calls can use a vf argument if
> passed by the caller. If the vf argument is -1 it will try to derive the vf
> if the device passed is a virtual function.
>
> For 802.1Qbg devices, This patch introduces a null check for the device
> argument because during port profile assignment on a hostdev, this argument
> can be null. Stefan CC'ed for comments
>
> Signed-off-by: Roopa Prabhu <roprabhu at cisco.com>
> ---
> src/qemu/qemu_migration.c | 2
> src/util/virnetdevmacvlan.c | 6 +
> src/util/virnetdevvportprofile.c | 203 +++++---------------------------------
> src/util/virnetdevvportprofile.h | 8 +
> 4 files changed, 42 insertions(+), 177 deletions(-)
>
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 7df2d4f..b80ed69 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -2605,6 +2605,7 @@ qemuMigrationVPAssociatePortProfiles(virDomainDefPtr def) {
> virDomainNetGetActualVirtPortProfile(net),
> net->mac,
> virDomainNetGetActualDirectDev(net),
> + -1,
> def->uuid,
> VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH) < 0)
> goto err_exit;
> @@ -2622,6 +2623,7 @@ err_exit:
> virDomainNetGetActualVirtPortProfile(net),
> net->mac,
> virDomainNetGetActualDirectDev(net),
> + -1,
> VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH));
> }
> }
> diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
> index 25d0846..498a2a0 100644
> --- a/src/util/virnetdevmacvlan.c
> +++ b/src/util/virnetdevmacvlan.c
> @@ -486,6 +486,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
> uint32_t macvtapMode;
> const char *cr_ifname;
> int ret;
> + int vf = -1;
>
> macvtapMode = modeMap[mode];
>
> @@ -547,6 +548,7 @@ create_name:
> virtPortProfile,
> macaddress,
> linkdev,
> + vf,
> vmuuid, vmOp) < 0) {
> rc = -1;
> goto link_del_exit;
> @@ -597,6 +599,7 @@ disassociate_exit:
> virtPortProfile,
> macaddress,
> linkdev,
> + vf,
> vmOp));
>
> link_del_exit:
> @@ -624,6 +627,8 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname,
> char *stateDir)
> {
> int ret = 0;
> + int vf = -1;
> +
> if (mode == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) {
> ignore_value(virNetDevRestoreMacAddress(linkdev, stateDir));
> }
> @@ -633,6 +638,7 @@ int virNetDevMacVLanDeleteWithVPortProfile(const char *ifname,
> virtPortProfile,
> macaddr,
> linkdev,
> + vf,
> VIR_NETDEV_VPORT_PROFILE_OP_DESTROY) < 0)
> ret = -1;
> if (virNetDevMacVLanDelete(ifname) < 0)
> diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c
> index 67fd7bc..8d9e906 100644
> --- a/src/util/virnetdevvportprofile.c
> +++ b/src/util/virnetdevvportprofile.c
> @@ -126,11 +126,6 @@ static struct nla_policy ifla_port_policy[IFLA_PORT_MAX + 1] =
> {
> [IFLA_PORT_RESPONSE] = { .type = NLA_U16 },
> };
> -static struct nla_policy ifla_policy[IFLA_MAX + 1] =
> -{
> - [IFLA_VF_PORTS] = { .type = NLA_NESTED },
> -};
> -
>
> static uint32_t
> virNetDevVPortProfileGetLldpadPid(void) {
> @@ -164,126 +159,6 @@ virNetDevVPortProfileGetLldpadPid(void) {
> return pid;
> }
>
> -
> -/**
> - * virNetDevVPortProfileLinkDump:
> - *
> - * @ifname: The name of the interface; only use if ifindex < 0
> - * @ifindex: The interface index; may be < 0 if ifname is given
> - * @nltarget_kernel: whether to send the message to the kernel or another
> - * process
> - * @nlattr: pointer to a pointer of netlink attributes that will contain
> - * the results
> - * @recvbuf: Pointer to the buffer holding the returned netlink response
> - * message; free it, once not needed anymore
> - * @getPidFunc: Pointer to a function that will be invoked if the kernel
> - * is not the target of the netlink message but it is to be
> - * sent to another process.
> - *
> - * Get information about an interface given its name or index.
> - *
> - * Returns 0 on success, -1 on fatal error.
> - */
> -static int
> -virNetDevVPortProfileLinkDump(const char *ifname, int ifindex, bool nltarget_kernel,
> - struct nlattr **tb, unsigned char **recvbuf,
> - uint32_t (*getPidFunc)(void))
> -{
> - int rc = 0;
> - struct nlmsghdr *resp;
> - struct nlmsgerr *err;
> - struct ifinfomsg ifinfo = {
> - .ifi_family = AF_UNSPEC,
> - .ifi_index = ifindex
> - };
> - unsigned int recvbuflen;
> - uint32_t pid = 0;
> - struct nl_msg *nl_msg;
> -
> - *recvbuf = NULL;
> -
> - nl_msg = nlmsg_alloc_simple(RTM_GETLINK, NLM_F_REQUEST);
> - if (!nl_msg) {
> - virReportOOMError();
> - return -1;
> - }
> -
> - if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
> - goto buffer_too_small;
> -
> - if (ifindex < 0 && ifname) {
> - if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0)
> - goto buffer_too_small;
> - }
> -
> - if (!nltarget_kernel) {
> - pid = getPidFunc();
> - if (pid == 0) {
> - rc = -1;
> - goto cleanup;
> - }
> - }
> -
> - if (virNetlinkCommand(nl_msg, recvbuf, &recvbuflen, pid) < 0) {
> - rc = -1;
> - goto cleanup;
> - }
> -
> - if (recvbuflen < NLMSG_LENGTH(0) || *recvbuf == NULL)
> - goto malformed_resp;
> -
> - resp = (struct nlmsghdr *)*recvbuf;
> -
> - switch (resp->nlmsg_type) {
> - case NLMSG_ERROR:
> - err = (struct nlmsgerr *)NLMSG_DATA(resp);
> - if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
> - goto malformed_resp;
> -
> - if (err->error) {
> - virReportSystemError(-err->error,
> - _("error dumping %s (%d) interface"),
> - ifname, ifindex);
> - rc = -1;
> - }
> - break;
> -
> - case GENL_ID_CTRL:
> - case NLMSG_DONE:
> - if (nlmsg_parse(resp, sizeof(struct ifinfomsg),
> - tb, IFLA_MAX, ifla_policy)) {
> - goto malformed_resp;
> - }
> - break;
> -
> - default:
> - goto malformed_resp;
> - }
> -
> - if (rc != 0)
> - VIR_FREE(*recvbuf);
> -
> -cleanup:
> - nlmsg_free(nl_msg);
> -
> - return rc;
> -
> -malformed_resp:
> - nlmsg_free(nl_msg);
> -
> - virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("malformed netlink response message"));
> - VIR_FREE(*recvbuf);
> - return -1;
> -
> -buffer_too_small:
> - nlmsg_free(nl_msg);
> -
> - virNetDevError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("allocated netlink buffer is too small"));
> - return -1;
> -}
> -
> /**
> * virNetDevVPortProfileGetStatus:
> *
> @@ -607,7 +482,7 @@ virNetDevVPortProfileGetNthParent(const char *ifname, int ifindex, unsigned int
> return -1;
>
> while (!end && i <= nthParent) {
> - rc = virNetDevVPortProfileLinkDump(ifname, ifindex, true, tb, &recvbuf, NULL);
> + rc = virNetDevLinkDump(ifname, ifindex, true, tb, &recvbuf, NULL);
> if (rc < 0)
> break;
>
> @@ -676,8 +551,8 @@ virNetDevVPortProfileOpCommon(const char *ifname, int ifindex,
> }
>
> while (--repeats >= 0) {
> - rc = virNetDevVPortProfileLinkDump(NULL, ifindex, nltarget_kernel, tb,
> - &recvbuf, virNetDevVPortProfileGetLldpadPid);
> + rc = virNetDevLinkDump(NULL, ifindex, nltarget_kernel, tb,
> + &recvbuf, virNetDevVPortProfileGetLldpadPid);
> if (rc < 0)
> goto err_exit;
>
> @@ -750,6 +625,7 @@ virNetDevVPortProfileGetPhysdevAndVlan(const char *ifname, int *root_ifindex, ch
> static int
> virNetDevVPortProfileOp8021Qbg(const char *ifname,
> const unsigned char *macaddr,
> + int vf,
> const virNetDevVPortProfilePtr virtPort,
> enum virNetDevVPortProfileLinkOp virtPortOp)
> {
> @@ -763,7 +639,11 @@ virNetDevVPortProfileOp8021Qbg(const char *ifname,
> int vlanid;
> int physdev_ifindex = 0;
> char physdev_ifname[IFNAMSIZ] = { 0, };
> - int vf = PORT_SELF_VF;
> +
> + if (!ifname)
> + return -1;
> +
> + vf = PORT_SELF_VF;
>
> if (virNetDevVPortProfileGetPhysdevAndVlan(ifname, &physdev_ifindex, physdev_ifname,
> &vlanid) < 0) {
> @@ -811,46 +691,11 @@ err_exit:
> return rc;
> }
>
> -
> -static int
> -virNetDevVPortProfileGetPhysfnDev(const char *linkdev,
> - int32_t *vf,
> - char **physfndev)
> -{
> - int rc = -1;
> -
> - if (virNetDevIsVirtualFunction(linkdev) == 1) {
> - /* if linkdev is SR-IOV VF, then set vf = VF index */
> - /* and set linkdev = PF device */
> -
> - rc = virNetDevGetPhysicalFunction(linkdev, physfndev);
> - if (!rc)
> - rc = virNetDevGetVirtualFunctionIndex(*physfndev, linkdev, vf);
> - } else {
> -
> - /* Not SR-IOV VF: physfndev is linkdev and VF index
> - * refers to linkdev self
> - */
> -
> - *vf = PORT_SELF_VF;
> - *physfndev = strdup(linkdev);
> - if (!*physfndev) {
> - virReportOOMError();
> - goto err_exit;
> - }
> - rc = 0;
> - }
> -
> -err_exit:
> -
> - return rc;
> -}
> -
> -
> /* Returns 0 on success, -1 on general failure, and -2 on timeout */
> static int
> virNetDevVPortProfileOp8021Qbh(const char *ifname,
> const unsigned char *macaddr,
> + int32_t vf,
> const virNetDevVPortProfilePtr virtPort,
> const unsigned char *vm_uuid,
> enum virNetDevVPortProfileLinkOp virtPortOp)
> @@ -858,14 +703,20 @@ virNetDevVPortProfileOp8021Qbh(const char *ifname,
> int rc = 0;
> char *physfndev = NULL;
> unsigned char hostuuid[VIR_UUID_BUFLEN];
> - int32_t vf;
> bool nltarget_kernel = true;
> int ifindex;
> int vlanid = -1;
>
> - rc = virNetDevVPortProfileGetPhysfnDev(ifname, &vf, &physfndev);
> - if (rc < 0)
> - goto err_exit;
> + if (vf == -1 && virNetDevIsVirtualFunction(ifname) == 1) {
It's kind of bothersome that virNetDevIsVirtualFunction() can return an
error, but we're ignoring it here.
Perhaps it should go something like this:
bool is_vf = false;
if (vf == -1) {
int isvf_ret = virNetDevIstVirtualFunction(ifname)
if (isvf_ret == -1)
goto cleanup; // yep - change its name :-)
is_vf = !!isvf_ret;
}
if (is_vf) {
blah blah blah
> + if (virNetDevGetVirtualFunctionInfo(ifname, &physfndev, &vf) < 0)
> + goto err_exit;
> + } else {
> + physfndev = strdup(ifname);
> + if (!physfndev) {
> + virReportOOMError();
> + goto err_exit;
> + }
> + }
>
> rc = virNetDevGetIndex(physfndev, &ifindex);
> if (rc < 0)
> @@ -953,13 +804,14 @@ virNetDevVPortProfileAssociate(const char *macvtap_ifname,
> const virNetDevVPortProfilePtr virtPort,
> const unsigned char *macvtap_macaddr,
> const char *linkdev,
> + int vf,
> const unsigned char *vmuuid,
> enum virNetDevVPortProfileOp vmOp)
> {
> int rc = 0;
>
> VIR_DEBUG("Associating port profile '%p' on link device '%s'",
> - virtPort, macvtap_ifname);
> + virtPort, (macvtap_ifname ? macvtap_ifname : linkdev));
>
> VIR_DEBUG("%s: VM OPERATION: %s", __FUNCTION__, virNetDevVPortProfileOpTypeToString(vmOp));
>
> @@ -974,14 +826,14 @@ virNetDevVPortProfileAssociate(const char *macvtap_ifname,
>
> case VIR_NETDEV_VPORT_PROFILE_8021QBG:
> rc = virNetDevVPortProfileOp8021Qbg(macvtap_ifname, macvtap_macaddr,
> - virtPort,
> + vf, virtPort,
> (vmOp == VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START)
> ? VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE
> : VIR_NETDEV_VPORT_PROFILE_LINK_OP_ASSOCIATE);
> break;
>
> case VIR_NETDEV_VPORT_PROFILE_8021QBH:
> - rc = virNetDevVPortProfileOp8021Qbh(linkdev, macvtap_macaddr,
> + rc = virNetDevVPortProfileOp8021Qbh(linkdev, macvtap_macaddr, vf,
> virtPort, vmuuid,
> (vmOp == VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START)
> ? VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE_RR
> @@ -1014,6 +866,7 @@ virNetDevVPortProfileDisassociate(const char *macvtap_ifname,
> const virNetDevVPortProfilePtr virtPort,
> const unsigned char *macvtap_macaddr,
> const char *linkdev,
> + int vf,
> enum virNetDevVPortProfileOp vmOp)
> {
> int rc = 0;
> @@ -1033,7 +886,7 @@ virNetDevVPortProfileDisassociate(const char *macvtap_ifname,
> break;
>
> case VIR_NETDEV_VPORT_PROFILE_8021QBG:
> - rc = virNetDevVPortProfileOp8021Qbg(macvtap_ifname, macvtap_macaddr,
> + rc = virNetDevVPortProfileOp8021Qbg(macvtap_ifname, macvtap_macaddr, vf,
> virtPort,
> VIR_NETDEV_VPORT_PROFILE_LINK_OP_DISASSOCIATE);
> break;
> @@ -1043,7 +896,7 @@ virNetDevVPortProfileDisassociate(const char *macvtap_ifname,
> if (vmOp == VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_FINISH)
> break;
> ignore_value(virNetDevSetOnline(linkdev, false));
> - rc = virNetDevVPortProfileOp8021Qbh(linkdev, macvtap_macaddr,
> + rc = virNetDevVPortProfileOp8021Qbh(linkdev, macvtap_macaddr, vf,
> virtPort, NULL,
> VIR_NETDEV_VPORT_PROFILE_LINK_OP_DISASSOCIATE);
> break;
> @@ -1057,6 +910,7 @@ int virNetDevVPortProfileAssociate(const char *macvtap_ifname ATTRIBUTE_UNUSED,
> const virNetDevVPortProfilePtr virtPort ATTRIBUTE_UNUSED,
> const unsigned char *macvtap_macaddr ATTRIBUTE_UNUSED,
> const char *linkdev ATTRIBUTE_UNUSED,
> + int vf,
> const unsigned char *vmuuid ATTRIBUTE_UNUSED,
> enum virNetDevVPortProfileOp vmOp ATTRIBUTE_UNUSED)
> {
> @@ -1069,6 +923,7 @@ int virNetDevVPortProfileDisassociate(const char *macvtap_ifname ATTRIBUTE_UNUSE
> const virNetDevVPortProfilePtr virtPort ATTRIBUTE_UNUSED,
> const unsigned char *macvtap_macaddr ATTRIBUTE_UNUSED,
> const char *linkdev ATTRIBUTE_UNUSED,
> + int vf,
> enum virNetDevVPortProfileOp vmOp ATTRIBUTE_UNUSED)
> {
> virReportSystemError(ENOSYS, "%s",
> diff --git a/src/util/virnetdevvportprofile.h b/src/util/virnetdevvportprofile.h
> index ec2d06d..5c5dcf1 100644
> --- a/src/util/virnetdevvportprofile.h
> +++ b/src/util/virnetdevvportprofile.h
> @@ -85,17 +85,19 @@ int virNetDevVPortProfileAssociate(const char *ifname,
> const virNetDevVPortProfilePtr virtPort,
> const unsigned char *macaddr,
> const char *linkdev,
> + int vf,
> const unsigned char *vmuuid,
> enum virNetDevVPortProfileOp vmOp)
> - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
> - ATTRIBUTE_NONNULL(5) ATTRIBUTE_RETURN_CHECK;
> + ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(6)
> + ATTRIBUTE_RETURN_CHECK;
This function (virNetDevVPortProfileAssociate()) has a new final
argument "setlink_only". It was properly merged in most places, but the
change to ATTRIBUTE macros here caused a conflict. There was a call to
that function failed to merge, and that of course didn't get the new
arg. Unfortunately, I did "git commit --amend" in my local tree, so I no
longer have the info about *which* call to the function, but it leap
right out at you :-)
>
> int virNetDevVPortProfileDisassociate(const char *ifname,
> const virNetDevVPortProfilePtr virtPort,
> const unsigned char *macaddr,
> const char *linkdev,
> + int vf,
> enum virNetDevVPortProfileOp vmOp)
> - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
> + ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
> ATTRIBUTE_RETURN_CHECK;
Otherwise this looks fine.
More information about the libvir-list
mailing list