Re: [libvirt] [PATCHv2 2/9] util: eliminate union in virNetDevVPortProfile

virNetDevVPortProfile has (had) a type field that can be set to one of several values, and a union of several structs, one for each type. When a domain's interface object is of type "network", the domain config may not know beforehand which type of virtualport is going to be provided in the actual device handed down from the network driver at runtime, but may want to set some values in the virtualport that may or may not be used, depending on the type. To support this usage, this patch replaces the union of structs with toplevel fields in the struct, making it possible for all of the fields to be set at the same time. --- src/conf/netdev_vport_profile_conf.c | 38 ++++++++++++++++++------------------ src/util/virnetdevopenvswitch.c | 8 ++++---- src/util/virnetdevvportprofile.c | 24 +++++++++++------------ src/util/virnetdevvportprofile.h | 27 ++++++++++++------------- 4 files changed, 47 insertions(+), 50 deletions(-) diff --git a/src/conf/netdev_vport_profile_conf.c b/src/conf/netdev_vport_profile_conf.c index d008042..31ee9b4 100644 --- a/src/conf/netdev_vport_profile_conf.c +++ b/src/conf/netdev_vport_profile_conf.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2009-2011 Red Hat, Inc. + * Copyright (C) 2009-2012 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -100,7 +100,7 @@ virNetDevVPortProfileParse(xmlNodePtr node) goto error; } - virtPort->u.virtPort8021Qbg.managerID = (uint8_t)val; + virtPort->managerID = (uint8_t)val; if (virStrToLong_ui(virtPortTypeID, NULL, 0, &val)) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -114,7 +114,7 @@ virNetDevVPortProfileParse(xmlNodePtr node) goto error; } - virtPort->u.virtPort8021Qbg.typeID = (uint32_t)val; + virtPort->typeID = (uint32_t)val; if (virStrToLong_ui(virtPortTypeIDVersion, NULL, 0, &val)) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -128,17 +128,17 @@ virNetDevVPortProfileParse(xmlNodePtr node) goto error; } - virtPort->u.virtPort8021Qbg.typeIDVersion = (uint8_t)val; + virtPort->typeIDVersion = (uint8_t)val; if (virtPortInstanceID != NULL) { if (virUUIDParse(virtPortInstanceID, - virtPort->u.virtPort8021Qbg.instanceID)) { + virtPort->instanceID)) { virReportError(VIR_ERR_XML_ERROR, "%s", _("cannot parse instanceid parameter as a uuid")); goto error; } } else { - if (virUUIDGenerate(virtPort->u.virtPort8021Qbg.instanceID)) { + if (virUUIDGenerate(virtPort->instanceID)) { virReportError(VIR_ERR_XML_ERROR, "%s", _("cannot generate a random uuid for instanceid")); goto error; @@ -156,7 +156,7 @@ virNetDevVPortProfileParse(xmlNodePtr node) case VIR_NETDEV_VPORT_PROFILE_8021QBH: if (virtPortProfileID != NULL) { - if (virStrcpyStatic(virtPort->u.virtPort8021Qbh.profileID, + if (virStrcpyStatic(virtPort->profileID, virtPortProfileID) != NULL) { virtPort->virtPortType = VIR_NETDEV_VPORT_PROFILE_8021QBH; } else { @@ -173,13 +173,13 @@ virNetDevVPortProfileParse(xmlNodePtr node) case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH: if (virtPortInterfaceID != NULL) { if (virUUIDParse(virtPortInterfaceID, - virtPort->u.openvswitch.interfaceID)) { + virtPort->interfaceID)) { virReportError(VIR_ERR_XML_ERROR, "%s", _("cannot parse interfaceid parameter as a uuid")); goto error; } } else { - if (virUUIDGenerate(virtPort->u.openvswitch.interfaceID)) { + if (virUUIDGenerate(virtPort->interfaceID)) { virReportError(VIR_ERR_XML_ERROR, "%s", _("cannot generate a random uuid for interfaceid")); goto error; @@ -187,14 +187,14 @@ virNetDevVPortProfileParse(xmlNodePtr node) } /* profileid is not mandatory for Open vSwitch */ if (virtPortProfileID != NULL) { - if (virStrcpyStatic(virtPort->u.openvswitch.profileID, + if (virStrcpyStatic(virtPort->profileID, virtPortProfileID) == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("profileid parameter too long")); goto error; } } else { - virtPort->u.openvswitch.profileID[0] = '\0'; + virtPort->profileID[0] = '\0'; } break; @@ -234,33 +234,33 @@ virNetDevVPortProfileFormat(virNetDevVPortProfilePtr virtPort, switch (virtPort->virtPortType) { case VIR_NETDEV_VPORT_PROFILE_8021QBG: - virUUIDFormat(virtPort->u.virtPort8021Qbg.instanceID, + virUUIDFormat(virtPort->instanceID, uuidstr); virBufferAsprintf(buf, " <parameters managerid='%d' typeid='%d' " "typeidversion='%d' instanceid='%s'/>\n", - virtPort->u.virtPort8021Qbg.managerID, - virtPort->u.virtPort8021Qbg.typeID, - virtPort->u.virtPort8021Qbg.typeIDVersion, + virtPort->managerID, + virtPort->typeID, + virtPort->typeIDVersion, uuidstr); break; case VIR_NETDEV_VPORT_PROFILE_8021QBH: virBufferAsprintf(buf, " <parameters profileid='%s'/>\n", - virtPort->u.virtPort8021Qbh.profileID); + virtPort->profileID); break; case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH: - virUUIDFormat(virtPort->u.openvswitch.interfaceID, + virUUIDFormat(virtPort->interfaceID, uuidstr); - if (virtPort->u.openvswitch.profileID[0] == '\0') { + if (virtPort->profileID[0] == '\0') { virBufferAsprintf(buf, " <parameters interfaceid='%s'/>\n", uuidstr); } else { virBufferAsprintf(buf, " <parameters interfaceid='%s' " "profileid='%s'/>\n", uuidstr, - virtPort->u.openvswitch.profileID); + virtPort->profileID); } break; diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 7e0beef..b57532b 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -59,7 +59,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, char *vmid_ex_id = NULL; virMacAddrFormat(macaddr, macaddrstr); - virUUIDFormat(ovsport->u.openvswitch.interfaceID, ifuuidstr); + virUUIDFormat(ovsport->interfaceID, ifuuidstr); virUUIDFormat(vmuuid, vmuuidstr); if (virAsprintf(&attachedmac_ex_id, "external-ids:attached-mac=\"%s\"", @@ -71,14 +71,14 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, if (virAsprintf(&vmid_ex_id, "external-ids:vm-id=\"%s\"", vmuuidstr) < 0) goto out_of_memory; - if (ovsport->u.openvswitch.profileID[0] != '\0') { + if (ovsport->profileID[0] != '\0') { if (virAsprintf(&profile_ex_id, "external-ids:port-profile=\"%s\"", - ovsport->u.openvswitch.profileID) < 0) + ovsport->profileID) < 0) goto out_of_memory; } cmd = virCommandNew(OVSVSCTL); - if (ovsport->u.openvswitch.profileID[0] == '\0') { + if (ovsport->profileID[0] == '\0') { virCommandAddArgList(cmd, "--", "--may-exist", "add-port", brname, ifname, "--", "set", "Interface", ifname, attachedmac_ex_id, diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c index 506240b..af9151e 100644 --- a/src/util/virnetdevvportprofile.c +++ b/src/util/virnetdevvportprofile.c @@ -95,15 +95,15 @@ virNetDevVPortProfileEqual(virNetDevVPortProfilePtr a, virNetDevVPortProfilePtr break; case VIR_NETDEV_VPORT_PROFILE_8021QBG: - if (a->u.virtPort8021Qbg.managerID != b->u.virtPort8021Qbg.managerID || - a->u.virtPort8021Qbg.typeID != b->u.virtPort8021Qbg.typeID || - a->u.virtPort8021Qbg.typeIDVersion != b->u.virtPort8021Qbg.typeIDVersion || - memcmp(a->u.virtPort8021Qbg.instanceID, b->u.virtPort8021Qbg.instanceID, VIR_UUID_BUFLEN) != 0) + if (a->managerID != b->managerID || + a->typeID != b->typeID || + a->typeIDVersion != b->typeIDVersion || + memcmp(a->instanceID, b->instanceID, VIR_UUID_BUFLEN) != 0) return false; break; case VIR_NETDEV_VPORT_PROFILE_8021QBH: - if (STRNEQ(a->u.virtPort8021Qbh.profileID, b->u.virtPort8021Qbh.profileID)) + if (STRNEQ(a->profileID, b->profileID)) return false; break; @@ -637,8 +637,8 @@ virNetDevVPortProfileOp8021Qbg(const char *ifname, int rc = -1; int op = PORT_REQUEST_ASSOCIATE; struct ifla_port_vsi portVsi = { - .vsi_mgr_id = virtPort->u.virtPort8021Qbg.managerID, - .vsi_type_version = virtPort->u.virtPort8021Qbg.typeIDVersion, + .vsi_mgr_id = virtPort->managerID, + .vsi_type_version = virtPort->typeIDVersion, }; bool nltarget_kernel = false; int vlanid; @@ -658,9 +658,9 @@ virNetDevVPortProfileOp8021Qbg(const char *ifname, if (vlanid < 0) vlanid = 0; - portVsi.vsi_type_id[2] = virtPort->u.virtPort8021Qbg.typeID >> 16; - portVsi.vsi_type_id[1] = virtPort->u.virtPort8021Qbg.typeID >> 8; - portVsi.vsi_type_id[0] = virtPort->u.virtPort8021Qbg.typeID; + portVsi.vsi_type_id[2] = virtPort->typeID >> 16; + portVsi.vsi_type_id[1] = virtPort->typeID >> 8; + portVsi.vsi_type_id[0] = virtPort->typeID; switch (virtPortOp) { case VIR_NETDEV_VPORT_PROFILE_LINK_OP_PREASSOCIATE: @@ -684,7 +684,7 @@ virNetDevVPortProfileOp8021Qbg(const char *ifname, vlanid, NULL, &portVsi, - virtPort->u.virtPort8021Qbg.instanceID, + virtPort->instanceID, NULL, vf, op, @@ -749,7 +749,7 @@ virNetDevVPortProfileOp8021Qbh(const char *ifname, nltarget_kernel, macaddr, vlanid, - virtPort->u.virtPort8021Qbh.profileID, + virtPort->profileID, NULL, vm_uuid, hostuuid, diff --git a/src/util/virnetdevvportprofile.h b/src/util/virnetdevvportprofile.h index f33da18..3c16bfe 100644 --- a/src/util/virnetdevvportprofile.h +++ b/src/util/virnetdevvportprofile.h @@ -59,21 +59,18 @@ typedef struct _virNetDevVPortProfile virNetDevVPortProfile; typedef virNetDevVPortProfile *virNetDevVPortProfilePtr; struct _virNetDevVPortProfile { enum virNetDevVPortProfile virtPortType; - union { - struct { - uint8_t managerID; - uint32_t typeID; /* 24 bit valid */ - uint8_t typeIDVersion; - unsigned char instanceID[VIR_UUID_BUFLEN]; - } virtPort8021Qbg; - struct { - char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX]; - } virtPort8021Qbh; - struct { - unsigned char interfaceID[VIR_UUID_BUFLEN]; - char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX]; - } openvswitch; - } u; + /* these members are used when virtPortType == 802.1Qbg */ + uint8_t managerID; + uint32_t typeID; /* 24 bit valid */ + uint8_t typeIDVersion; + unsigned char instanceID[VIR_UUID_BUFLEN]; + + /* this member is used when virtPortType == 802.1Qbh|openvswitch */ + char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX]; + + /* this member is used when virtPortType == openvswitch */ + unsigned char interfaceID[VIR_UUID_BUFLEN]; + /* NB - if virtPortType == NONE, any/all of the items could be used */ }; -- 1.7.11.2 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list