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