This looks good to me.
Acked-by: Kyle Mestery <kmestery@xxxxxxxxx>
On Aug 14, 2012, at 2:04 AM, Laine Stump wrote:
> 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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list