Re: [PATCHv2 4/9] util: utility functions for virNetDevVPortProfile

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This looks good to me.

Acked-by: Kyle Mestery <kmestery@xxxxxxxxx>

On Aug 14, 2012, at 2:04 AM, Laine Stump wrote:

> This patch adds three utility functions that operate on
> virNetDevVPortProfile objects.
> 
> * virNetDevVPortProfileCheckComplete() - verifies that all attributes
>    required for the type of the given virtport are specified.
> 
> * virNetDevVPortProfileCheckNoExtras() - verifies that there are no
>    attributes specified which are inappropriate for the type of the
>    given virtport.
> 
> * virNetDevVPortProfileMerge3() - merges 3 virtports into a single,
>    newly allocated virtport. If any attributes are specified in
>    more than one of the three sources, and do not exactly match,
>    an error is logged and the function fails.
> 
> These new functions depend on new fields in the virNetDevVPortProfile
> object that keep track of whether or not each attribute was
> specified. Since the higher level parse function doesn't yet set those
> fields, these functions are not actually usable yet (but that's okay,
> because they also aren't yet used - all of that functionality comes in
> a later patch.)
> 
> Note that these three functions return 0 on success and -1 on
> failure. This may seem odd for the first two Check functions, since
> they could also easily return true/false, but since they actually log
> an error when the requested condition isn't met (and should result in
> a failure of the calling function), I thought 0/-1 was more
> appropriate.
> ---
> src/libvirt_private.syms         |   3 +
> src/util/virnetdevvportprofile.c | 313 +++++++++++++++++++++++++++++++++++++++
> src/util/virnetdevvportprofile.h |  17 ++-
> 3 files changed, 332 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index c023dbf..89fb1f4 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1427,8 +1427,11 @@ virNetDevVethDelete;
> 
> # virnetdevvportprofile.h
> virNetDevVPortProfileAssociate;
> +virNetDevVPortProfileCheckComplete;
> +virNetDevVPortProfileCheckNoExtras;
> virNetDevVPortProfileDisassociate;
> virNetDevVPortProfileEqual;
> +virNetDevVPortProfileMerge3;
> virNetDevVPortProfileOpTypeFromString;
> virNetDevVPortProfileOpTypeToString;
> 
> diff --git a/src/util/virnetdevvportprofile.c b/src/util/virnetdevvportprofile.c
> index 6db04f7..e686fd9 100644
> --- a/src/util/virnetdevvportprofile.c
> +++ b/src/util/virnetdevvportprofile.c
> @@ -120,6 +120,319 @@ virNetDevVPortProfileEqual(virNetDevVPortProfilePtr a, virNetDevVPortProfilePtr
>     return true;
> }
> 
> +/* virNetDevVPortProfileCheckComplete() checks that all attributes
> + * required for the type of virtport are specified. When
> + * generateMissing is true, any missing attribute that can be
> + * autogenerated, will be (instanceid, interfaceid). If virtport ==
> + * NULL or virtPortType == NONE, then the result is always 0
> + * (success). If a required attribute is missing, an error is logged
> + * and -1 is returned.
> + */
> +int
> +virNetDevVPortProfileCheckComplete(virNetDevVPortProfilePtr virtport,
> +                                   bool generateMissing)
> +{
> +    const char *missing = NULL;
> +
> +    if (!virtport || virtport->virtPortType == VIR_NETDEV_VPORT_PROFILE_NONE)
> +        return 0;
> +
> +    switch (virtport->virtPortType) {
> +    case VIR_NETDEV_VPORT_PROFILE_8021QBG:
> +        if (!virtport->managerID_specified) {
> +            missing = "managerid";
> +        } else if (!virtport->typeID_specified) {
> +            missing = "typeid";
> +        } else if (!virtport->typeIDVersion_specified) {
> +            missing = "typeidversion";
> +        } else if (!virtport->instanceID_specified) {
> +            if (generateMissing) {
> +                if (virUUIDGenerate(virtport->instanceID) < 0) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                   _("cannot generate a random uuid for instanceid"));
> +                    return -1;
> +                }
> +                virtport->instanceID_specified = true;
> +            } else {
> +                missing = "instanceid";
> +            }
> +        }
> +        break;
> +
> +    case VIR_NETDEV_VPORT_PROFILE_8021QBH:
> +        if (!virtport->profileID[0])
> +            missing = "profileid";
> +        break;
> +
> +    case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
> +        /* profileid is optional for openvswitch */
> +        if (!virtport->interfaceID_specified) {
> +            if (generateMissing) {
> +                if (virUUIDGenerate(virtport->interfaceID) < 0) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                                   _("cannot generate a random uuid for interfaceid"));
> +                    return -1;
> +                }
> +                virtport->interfaceID_specified = true;
> +            } else {
> +                missing = "interfaceid";
> +            }
> +        }
> +        break;
> +    }
> +
> +    if (missing) {
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       _("missing %s in <virtualport type='%s'>"), missing,
> +                       virNetDevVPortTypeToString(virtport->virtPortType));
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +/* virNetDevVPortProfileCheckNoExtras() checks that there are no
> + * attributes specified in this virtport that are inappropriate for
> + * the type. if virtport == NULL or virtPortType == NONE, then the
> + * result is always 0 (success). If an extra attribute is present,
> + * an error is logged and -1 is returned.
> + */
> +int
> +virNetDevVPortProfileCheckNoExtras(virNetDevVPortProfilePtr virtport)
> +{
> +    const char *extra = NULL;
> +
> +    if (!virtport || virtport->virtPortType == VIR_NETDEV_VPORT_PROFILE_NONE)
> +        return 0;
> +
> +    switch (virtport->virtPortType) {
> +    case VIR_NETDEV_VPORT_PROFILE_8021QBG:
> +        if (virtport->profileID[0])
> +            extra = "profileid";
> +        else if (virtport->interfaceID_specified)
> +            extra = "interfaceid";
> +        break;
> +
> +    case VIR_NETDEV_VPORT_PROFILE_8021QBH:
> +        if (virtport->managerID_specified)
> +            extra = "managerid";
> +        else if (virtport->typeID_specified)
> +            extra = "typeid";
> +        else if (virtport->typeIDVersion_specified)
> +            extra = "typeidversion";
> +        else if (virtport->instanceID_specified)
> +            extra = "instanceid";
> +        else if (virtport->interfaceID_specified)
> +            extra = "interfaceid";
> +        break;
> +
> +    case VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH:
> +        if (virtport->managerID_specified)
> +            extra = "managerid";
> +        else if (virtport->typeID_specified)
> +            extra = "typeid";
> +        else if (virtport->typeIDVersion_specified)
> +            extra = "typeidversion";
> +        else if (virtport->instanceID_specified)
> +            extra = "instanceid";
> +        break;
> +    }
> +
> +    if (extra) {
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       _("extra %s unsupported in <virtualport type='%s'>"),
> +                       extra,
> +                       virNetDevVPortTypeToString(virtport->virtPortType));
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +/* virNetDevVPortProfileMerge() - merge the attributes in mods into
> + * orig. If anything that is set in mods has already been set in orig
> + * *and doesn't match*, log an error and return -1, otherwise return 0.
> + */
> +static int
> +virNetDevVPortProfileMerge(virNetDevVPortProfilePtr orig,
> +                           virNetDevVPortProfilePtr mods)
> +{
> +    enum virNetDevVPortProfile otype;
> +
> +    if (!orig || !mods)
> +        return 0;
> +
> +    otype = orig->virtPortType;
> +
> +    if (mods->virtPortType != VIR_NETDEV_VPORT_PROFILE_NONE) {
> +        if (otype != VIR_NETDEV_VPORT_PROFILE_NONE &&
> +            otype != mods->virtPortType) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("attempt to merge virtualports "
> +                             "with mismatched types (%s and %s)"),
> +                           virNetDevVPortTypeToString(otype),
> +                           virNetDevVPortTypeToString(mods->virtPortType));
> +            return -1;
> +        }
> +        otype = orig->virtPortType = mods->virtPortType;
> +    }
> +
> +    if (mods->managerID_specified &&
> +        (otype == VIR_NETDEV_VPORT_PROFILE_8021QBG ||
> +         otype == VIR_NETDEV_VPORT_PROFILE_NONE)) {
> +        if (orig->managerID_specified &&
> +            (orig->managerID != mods->managerID)) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("attempt to merge virtualports "
> +                             "with mismatched managerids (%d and %d)"),
> +                           orig->managerID, mods->managerID);
> +            return -1;
> +        }
> +        orig->managerID = mods->managerID;
> +        orig->managerID_specified = true;
> +    }
> +
> +    if (mods->typeID_specified &&
> +        (otype == VIR_NETDEV_VPORT_PROFILE_8021QBG ||
> +         otype == VIR_NETDEV_VPORT_PROFILE_NONE)) {
> +        if (orig->typeID_specified &&
> +            (orig->typeID != mods->typeID)) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("attempt to merge virtualports "
> +                             "with mismatched typeids (%d and %d)"),
> +                           orig->typeID, mods->typeID);
> +            return -1;
> +        }
> +        orig->typeID = mods->typeID;
> +        orig->typeID_specified = true;
> +    }
> +
> +    if (mods->typeIDVersion_specified &&
> +        (otype == VIR_NETDEV_VPORT_PROFILE_8021QBG ||
> +         otype == VIR_NETDEV_VPORT_PROFILE_NONE)) {
> +        if (orig->typeIDVersion_specified &&
> +            (orig->typeIDVersion != mods->typeIDVersion)) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("attempt to merge virtualports with "
> +                             "mismatched typeidversions (%d and %d)"),
> +                           orig->typeIDVersion, mods->typeIDVersion);
> +            return -1;
> +        }
> +        orig->typeIDVersion = mods->typeIDVersion;
> +        orig->typeIDVersion_specified = true;
> +    }
> +
> +    if (mods->instanceID_specified &&
> +        (otype == VIR_NETDEV_VPORT_PROFILE_8021QBG ||
> +         otype == VIR_NETDEV_VPORT_PROFILE_NONE)) {
> +        if (orig->instanceID_specified &&
> +            memcmp(orig->instanceID, mods->instanceID,
> +                   sizeof(orig->instanceID))) {
> +            char origuuid[VIR_UUID_STRING_BUFLEN];
> +            char modsuuid[VIR_UUID_STRING_BUFLEN];
> +
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("attempt to merge virtualports with "
> +                             "mismatched instanceids ('%s' and '%s')"),
> +                           virUUIDFormat(orig->instanceID, origuuid),
> +                           virUUIDFormat(mods->instanceID, modsuuid));
> +            return -1;
> +        }
> +        memcpy(orig->instanceID, mods->instanceID, sizeof(orig->instanceID));
> +        orig->instanceID_specified = true;
> +    }
> +
> +    if (mods->interfaceID_specified &&
> +        (otype == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH ||
> +         otype == VIR_NETDEV_VPORT_PROFILE_NONE)) {
> +        if (orig->interfaceID_specified &&
> +            memcmp(orig->interfaceID, mods->interfaceID,
> +                   sizeof(orig->interfaceID))) {
> +            char origuuid[VIR_UUID_STRING_BUFLEN];
> +            char modsuuid[VIR_UUID_STRING_BUFLEN];
> +
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("attempt to merge virtualports with "
> +                             "mismatched interfaceids ('%s' and '%s')"),
> +                           virUUIDFormat(orig->interfaceID, origuuid),
> +                           virUUIDFormat(mods->interfaceID, modsuuid));
> +            return -1;
> +        }
> +        memcpy(orig->interfaceID, mods->interfaceID, sizeof(orig->interfaceID));
> +        orig->interfaceID_specified = true;
> +    }
> +
> +    if (mods->profileID[0] &&
> +        (otype == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH ||
> +         otype == VIR_NETDEV_VPORT_PROFILE_8021QBH ||
> +         otype == VIR_NETDEV_VPORT_PROFILE_NONE)) {
> +        if (orig->profileID[0] &&
> +            STRNEQ(orig->profileID, mods->profileID)) {
> +            virReportError(VIR_ERR_XML_ERROR,
> +                           _("attempt to merge virtualports with "
> +                             "mismatched profileids ('%s' and '%s')"),
> +                           orig->profileID, mods->profileID);
> +            return -1;
> +        }
> +        if (virStrcpyStatic(orig->profileID, mods->profileID)) {
> +            /* this should never happen - it indicates mods->profileID
> +             * isn't properly null terminated. */
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("corrupted profileid string"));
> +            return -1;
> +        }
> +    }
> +    return 0;
> +}
> +
> +/* virNetDevVPortProfileMerge3() - create a new virNetDevVPortProfile
> + * that is a combination of the three input profiles. fromInterface is
> + * highest priority and fromPortgroup is lowest. As lower priority
> + * objects' attributes are merged in, if the attribute is unset in the
> + * result object, it is set from the lower priority object, but if it
> + * is already set in the result and the lower priority object wants to
> + * change it, that is an error.
> + */
> +
> +int virNetDevVPortProfileMerge3(virNetDevVPortProfilePtr *result,
> +                                virNetDevVPortProfilePtr fromInterface,
> +                                virNetDevVPortProfilePtr fromNetwork,
> +                                virNetDevVPortProfilePtr fromPortgroup)
> +{
> +    int ret = -1;
> +    *result = NULL;
> +
> +    if ((!fromInterface || (fromInterface->virtPortType == VIR_NETDEV_VPORT_PROFILE_NONE)) &&
> +        (!fromNetwork   || (fromNetwork->virtPortType   == VIR_NETDEV_VPORT_PROFILE_NONE)) &&
> +        (!fromPortgroup || (fromPortgroup->virtPortType == VIR_NETDEV_VPORT_PROFILE_NONE))) {
> +        return 0;
> +    }
> +
> +    /* at least one of the source profiles is non-empty */
> +    if (VIR_ALLOC(*result) < 0) {
> +        virReportOOMError();
> +        return ret;
> +    }
> +
> +    /* start with the interface's profile. There are no pointers in a
> +     * virtualPortProfile, so a shallow copy is sufficient.
> +     */
> +    if (fromInterface)
> +        **result = *fromInterface;
> +
> +    if (virNetDevVPortProfileMerge(*result, fromNetwork) < 0)
> +        goto error;
> +    if (virNetDevVPortProfileMerge(*result, fromPortgroup) < 0)
> +        goto error;
> +
> +    ret = 0;
> +
> +error:
> +    if (ret < 0)
> +        VIR_FREE(*result);
> +    return ret;
> +}
> +
> 
> #if WITH_VIRTUALPORT
> 
> diff --git a/src/util/virnetdevvportprofile.h b/src/util/virnetdevvportprofile.h
> index 3c16bfe..d23c284 100644
> --- a/src/util/virnetdevvportprofile.h
> +++ b/src/util/virnetdevvportprofile.h
> @@ -58,18 +58,24 @@ VIR_ENUM_DECL(virNetDevVPortProfileOp)
> typedef struct _virNetDevVPortProfile virNetDevVPortProfile;
> typedef virNetDevVPortProfile *virNetDevVPortProfilePtr;
> struct _virNetDevVPortProfile {
> -    enum virNetDevVPortProfile   virtPortType;
> +    int           virtPortType; /* enum virNetDevVPortProfile */
>     /* these members are used when virtPortType == 802.1Qbg */
>     uint8_t       managerID;
> +    bool          managerID_specified;
>     uint32_t      typeID; /* 24 bit valid */
> +    bool          typeID_specified;
>     uint8_t       typeIDVersion;
> +    bool          typeIDVersion_specified;
>     unsigned char instanceID[VIR_UUID_BUFLEN];
> +    bool          instanceID_specified;
> 
>     /* this member is used when virtPortType == 802.1Qbh|openvswitch */
> +    /* this is a null-terminated character string */
>     char          profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX];
> 
>     /* this member is used when virtPortType == openvswitch */
>     unsigned char interfaceID[VIR_UUID_BUFLEN];
> +    bool          interfaceID_specified;
>     /* NB - if virtPortType == NONE, any/all of the items could be used */
> };
> 
> @@ -77,6 +83,15 @@ struct _virNetDevVPortProfile {
> bool virNetDevVPortProfileEqual(virNetDevVPortProfilePtr a,
>                                 virNetDevVPortProfilePtr b);
> 
> +int virNetDevVPortProfileCheckComplete(virNetDevVPortProfilePtr virtport,
> +                                       bool generateMissing);
> +int virNetDevVPortProfileCheckNoExtras(virNetDevVPortProfilePtr virtport);
> +
> +int virNetDevVPortProfileMerge3(virNetDevVPortProfilePtr *result,
> +                                virNetDevVPortProfilePtr fromInterface,
> +                                virNetDevVPortProfilePtr fromNetwork,
> +                                virNetDevVPortProfilePtr fromPortgroup);
> +
> int virNetDevVPortProfileAssociate(const char *ifname,
>                                    const virNetDevVPortProfilePtr virtPort,
>                                    const virMacAddrPtr macaddr,
> -- 
> 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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]