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

Re: [XEN PATCH v2 2/5] xen: export get_free_port


  • To: Stefano Stabellini <sstabellini@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Fri, 14 Jan 2022 08:02:13 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=n0gnMEQOIpPKl4jmI1uGgU/Ts53WMgX8AJj1qhT/W2s=; b=W7GaAWue0CzXl+6Wnnpf9FYAVmR75NhKqnp8i3r4NNhSJFDV0HQgm+Tx6xp5x18mi3ZdL840Rxhtesal/qB70mhzslthPHCXEOlbrYXxdqtf/rsnFDqP3fHWyu1Wn08D+YGA8VrjWR6tB7CW/za1iVigdG6gv6sRapWMwFK2fRSQLJ4Q8P9QY2vT6cwmcn6JffXISp46nuXufwaqfB9Xp5puCnXB+YN0rzCuEmfV+mf2et5rsO82P/ZHecNg33Q3VB/Njd7us6nFqm7DmXQiqy3uYS7fzlCrZCnsVjO33AcKcD0zo/KPLbVLqmYoP+dbL5qVz9l3AWmct77Z0IqsPA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=jF3Jg3fgKsVheaBHyZLyzo2RlLsR0danD1Cl6kXhU/zuzTo+YwRMR2p7/aeT/7XSCfmPp6DFw7CdIA1aRxN4oJfWs1ZHlh9d68xE/1Xr2uVPo0x8w3LgKaWBRQW3OQXWKIKpFkCz73Y/1/WJIQcgEDhuvChphw9honywlvqCjdZOSt7CeDs6OfI45XBkh5OrxOA/Yyr5hJ29o9Di+PGmO2qMowzmunfzuFKcsjv6kQMiq+kT3PU6LGXpR6DQoA2pOa6oqBkHCykSYxJKeRDJjj+FzKymXkt6T6ZTzrPK0RnnIRPYBeBWthl00CBIWCiO5MQizFLUVPUeb9R6PtEJIg==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: jgross@xxxxxxxx, Bertrand.Marquis@xxxxxxx, julien@xxxxxxx, Volodymyr_Babchuk@xxxxxxxx, Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Fri, 14 Jan 2022 07:02:51 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 14.01.2022 02:20, Stefano Stabellini wrote:
> On Thu, 13 Jan 2022, Jan Beulich wrote:
>> On 13.01.2022 01:58, Stefano Stabellini wrote:
>>> --- a/xen/common/event_channel.c
>>> +++ b/xen/common/event_channel.c
>>> @@ -232,7 +232,7 @@ int evtchn_allocate_port(struct domain *d, 
>>> evtchn_port_t port)
>>>      return 0;
>>>  }
>>>  
>>> -static int get_free_port(struct domain *d)
>>> +int get_free_port(struct domain *d)
>>
>> The name of the function isn't really suitable for being non-static.
>> Can't we fold its functionality back into evtchn_allocate_port() (or
>> the other way around, depending on the perspective you want to take)
>> in case the caller passes in port 0? (Btw., it is imo wrong for the
>> loop over ports to start at 0, when it is part of the ABI that port
>> 0 is always invalid. evtchn_init() also better wouldn't depend on it
>> being the only party to successfully invoke the function getting back
>> port 0.)
> 
> I agree that "get_free_port" is not a great name for a non-static
> function. Also, it should be noted that for the sake of this patch
> series I could just call evtchn_allocate_port(d, 1) given that it is the
> first evtchn to be allocated. So maybe, that's the best way forward?
> 
> 
> To address your specific suggestion, in my opinion it would be best to
> have two different functions to allocate a new port:
> - one with a specific evtchn_port_t port parameter
> - one without it (meaning: "I don't care about the number")
> 
> Folding the functionality of "give me any number" when 0 is passed to
> evtchn_allocate_port() doesn't seem to be an improvement to the API to
> me.

I view it the other way around - that way the function would actually
start matching its name. So far it's more like marking a given port
number as in use, rather than allocating.

> That said, I am still OK with making the suggested change if that's what
> you prefer.

Given experience, hoping for others to voice an opinion isn't likely
to become reality.

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.