On Fri, Jan 22, 2016 at 12:52:27PM -0500, Laine Stump wrote:
> This patch creates two bitmaps, one for macvlan devicenames and one
s/devicenames/device names/
[...]
> diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
[...]
> +/**
> + * virNetDevMacVLanReserveName:
> + *
> + * @name: already-known name of device
> + * @quietFail: don't log an error if this name is already in-use
> + *
> + * Extract the device type and id from a macvtap/macvlan device name
> + * and mark the appropriate position as in-use in the appropriate
> + * bitmap.
> + *
> + * returns 0 on success, -1 on failure, -2 if the name doesn't fit the auto-pattern
Long line, would be nice to wrap it. And the return 0 isn't true, because
virNetDevMacVLanReserveID() returns ID on success.
> + */
> +int
> +virNetDevMacVLanReserveName(const char *name, bool quietFail)
> +{
> + unsigned int id;
> + unsigned int flags = 0;
> + const char *idstr = NULL;
> +
> + if (virNetDevMacVLanInitialize() < 0)
> + return -1;
> +
> + if (STRPREFIX(name, MACVTAP_NAME_PREFIX)) {
> + idstr = name + strlen(MACVTAP_NAME_PREFIX);
> + flags |= VIR_NETDEV_MACVLAN_CREATE_WITH_TAP;
> + } else if (STRPREFIX(name, MACVLAN_NAME_PREFIX)) {
> + idstr = name + strlen(MACVLAN_NAME_PREFIX);
> + } else {
> + return -2;
> + }
> +
> + if (virStrToLong_ui(idstr, NULL, 10, &id) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("couldn't get id value from macvtap device name %s"),
> + name);
> + return -1;
> + }
> + return virNetDevMacVLanReserveID(id, flags, quietFail, false);
> +}
[...]
> const char *type = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
> "macvtap" : "macvlan";
> - const char *prefix = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
> - MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX;
Maybe use the MACV(TAP|LAN)_NAME_PREFIX instead of the strings for *type.
> const char *pattern = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
> MACVTAP_NAME_PATTERN : MACVLAN_NAME_PATTERN;
> - int c, rc;
> + int rc, reservedID = -1;
> char ifname[IFNAMSIZ];
> int retries, do_retry = 0;
> uint32_t macvtapMode;
> - const char *cr_ifname = NULL;
> + const char *ifnameCreated = NULL;
> int ret;
> int vf = -1;
> bool vnet_hdr = flags & VIR_NETDEV_MACVLAN_VNET_HDR;
[...]
> + retries = MACVLAN_MAX_ID + 1;
> + while (!ifnameCreated && retries) {
> virMutexLock(&virNetDevMacVLanCreateMutex);
> - for (c = 0; c < 8192; c++) {
> - snprintf(ifname, sizeof(ifname), pattern, c);
> - if ((ret = virNetDevExists(ifname)) < 0) {
> - virMutexUnlock(&virNetDevMacVLanCreateMutex);
> + reservedID = virNetDevMacVLanReserveID(reservedID, flags, false, true);
> + if (reservedID < 0) {
> + virMutexUnlock(&virNetDevMacVLanCreateMutex);
> + return -1;
> + }
> + snprintf(ifname, sizeof(ifname), pattern, reservedID);
Since you're changing this, snprintf returns -1 in case of error.
ACK with the issues fixed.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list