On 2020-12-08 at 07:42, Laine Stump wrote:
>On 12/4/20 2:01 AM, Shi Lei wrote:
>> Simplify GenerateName/ReserveName for netdevtap by using common
>> functions.
>>
>> Signed-off-by: Shi Lei <shi_lei@xxxxxxxxxxxxxx>
>> ---
>> src/util/virnetdevtap.c | 58 +++--------------------------------------
>> 1 file changed, 4 insertions(+), 54 deletions(-)
>>
>> diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
>> index 198607b5..38b2f171 100644
>> --- a/src/util/virnetdevtap.c
>> +++ b/src/util/virnetdevtap.c
>> @@ -55,9 +55,6 @@
>>
>> VIR_LOG_INIT("util.netdevtap");
>>
>> -virMutex virNetDevTapCreateMutex = VIR_MUTEX_INITIALIZER;
>> -static int virNetDevTapLastID = -1; /* not "unsigned" because callers use %d */
>> -
>>
>> /**
>> * virNetDevTapReserveName:
>> @@ -72,25 +69,7 @@ static int virNetDevTapLastID = -1; /* not "unsigned" because callers use %d */
>> void
>> virNetDevTapReserveName(const char *name)
>> {
>> - unsigned int id;
>> - const char *idstr = NULL;
>> -
>> -
>> - if (STRPREFIX(name, VIR_NET_GENERATED_TAP_PREFIX)) {
>> -
>> - VIR_INFO("marking device in use: '%s'", name);
>> -
>> - idstr = name + strlen(VIR_NET_GENERATED_TAP_PREFIX);
>> -
>> - if (virStrToLong_ui(idstr, NULL, 10, &id) >= 0) {
>> - virMutexLock(&virNetDevTapCreateMutex);
>> -
>> - if (virNetDevTapLastID < (int)id)
>> - virNetDevTapLastID = id;
>> -
>> - virMutexUnlock(&virNetDevTapCreateMutex);
>> - }
>> - }
>> + virNetDevReserveName(name, VIR_NET_DEV_GEN_NAME_TAP, true);
>
>
>I think we can just call virNetDevReserveName() directly, rather than
>keeping virNetDevTapReserveName() as a go-between...
>
Okay.
>
>
>> }
>>
>>
>> @@ -199,36 +178,7 @@ virNetDevTapGetRealDeviceName(char *ifname G_GNUC_UNUSED)
>> static int
>> virNetDevTapGenerateName(char **ifname)
>> {
>> - int id;
>> - double maxIDd = pow(10, IFNAMSIZ - 1 - strlen(VIR_NET_GENERATED_TAP_PREFIX));
>> - int maxID = INT_MAX;
>> - int attempts = 0;
>> -
>> - if (maxIDd <= (double)INT_MAX)
>> - maxID = (int)maxIDd;
>> -
>> - do {
>> - g_autofree char *try = NULL;
>> -
>> - id = ++virNetDevTapLastID;
>> -
>> - /* reset before overflow */
>> - if (virNetDevTapLastID >= maxID)
>> - virNetDevTapLastID = -1;
>> -
>> - try = g_strdup_printf(*ifname, id);
>> -
>> - if (!virNetDevExists(try)) {
>> - g_free(*ifname);
>> - *ifname = g_steal_pointer(&try);
>> - return 0;
>> - }
>> - } while (++attempts < 10000);
>> -
>> - virReportError(VIR_ERR_INTERNAL_ERROR,
>> - _("no unused %s names available"),
>> - VIR_NET_GENERATED_TAP_PREFIX);
>> - return -1;
>> + return virNetDevGenerateName(ifname, VIR_NET_DEV_GEN_NAME_TAP);
>
>
>Same here.
Okay.
>
>
>> }
>>
>>
>> @@ -263,7 +213,7 @@ int virNetDevTapCreate(char **ifname,
>> int ret = -1;
>> int fd = -1;
>>
>> - virMutexLock(&virNetDevTapCreateMutex);
>> + virNetDevLockGenName(VIR_NET_DEV_GEN_NAME_TAP);
>>
>> /* if ifname is "vnet%d", then auto-generate a name for the new
>> * device (the kernel could do this for us, but has a bad habit of
>
>
>The code around here is going to need to be updated to take advantage of
>the "g_strdup_printf(*ifname, id)" in your new Generate function. (as a
>matter of fact, the functions that call virNetDevTapCreate() should
>probably also be updated to stop them from adding in "vnet%d" when
>ifname is empty - we can just let it remain empty until the call to
>virNetDevGenerateName().
Okay. I'll find out those functions and prevent them from doing such things.
>
>
>> @@ -333,7 +283,7 @@ int virNetDevTapCreate(char **ifname,
>> ret = 0;
>>
>> cleanup:
>> - virMutexUnlock(&virNetDevTapCreateMutex);
>> + virNetDevUnlockGenName(VIR_NET_DEV_GEN_NAME_TAP);
>> if (ret < 0) {
>> VIR_FORCE_CLOSE(fd);
>> while (i--)
>
>
