[dpdk-dev] [PATCH] vhost: unlink existing file for server mode

[dpdk-dev] [PATCH] vhost: unlink existing file for server mode

[Zhiyong Yang]

Vhost-user startup will fail based on server mode, if the specified
socket file has already existed. The patch introduces function
unlink() to remove the possible existing file.

Cc: yliu@fridaylinux.org
Cc: maxime.coquelin@redhat.com

Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
---
 lib/librte_vhost/socket.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
index 6e3857e7a..324a24f4e 100644
--- a/lib/librte_vhost/socket.c
+++ b/lib/librte_vhost/socket.c
@@ -315,6 +315,7 @@ vhost_user_start_server(struct vhost_user_socket *vsocket)
 	int fd = vsocket->socket_fd;
 	const char *path = vsocket->path;
 
+	unlink(path);
 	ret = bind(fd, (struct sockaddr *)&vsocket->un, sizeof(vsocket->un));
 	if (ret < 0) {
 		RTE_LOG(ERR, VHOST_CONFIG,
-- 
2.13.3

Re: [dpdk-dev] vhost: unlink existing file for server mode

[Ilya Maximets]

Oh.

It's such a game: twice a year someone sends this patch to mail list.

I have another one for you:
* Find all the patches equal to this one in archives.
* Read all the discussions.
* Come back if you have some new ideas, not already discussed many times here.

Sorry for my sarcasm.
NACK for this, as usual.

Best regards, Ilya Maximets.

On 02.02.2018 11:39, Zhiyong Yang wrote:
> Vhost-user startup will fail based on server mode, if the specified
> socket file has already existed. The patch introduces function
> unlink() to remove the possible existing file.
> 
> Cc: yliu@fridaylinux.org
> Cc: maxime.coquelin@redhat.com
> 
> Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
> ---
>  lib/librte_vhost/socket.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> index 6e3857e7a..324a24f4e 100644
> --- a/lib/librte_vhost/socket.c
> +++ b/lib/librte_vhost/socket.c
> @@ -315,6 +315,7 @@ vhost_user_start_server(struct vhost_user_socket *vsocket)
>  	int fd = vsocket->socket_fd;
>  	const char *path = vsocket->path;
>  
> +	unlink(path);
>  	ret = bind(fd, (struct sockaddr *)&vsocket->un, sizeof(vsocket->un));
>  	if (ret < 0) {
>  		RTE_LOG(ERR, VHOST_CONFIG,
> 

Re: [dpdk-dev] vhost: unlink existing file for server mode

[Richardson, Bruce]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ilya Maximets
> Sent: Friday, February 2, 2018 3:30 PM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>; dev@dpdk.org
> Cc: yliu@fridaylinux.org; maxime.coquelin@redhat.com
> Subject: Re: [dpdk-dev] vhost: unlink existing file for server mode
> 
> Oh.
> 
> It's such a game: twice a year someone sends this patch to mail list.
> 
> I have another one for you:
> * Find all the patches equal to this one in archives.
> * Read all the discussions.
> * Come back if you have some new ideas, not already discussed many times
> here.
> 
> Sorry for my sarcasm.
> NACK for this, as usual.
> 
> Best regards, Ilya Maximets.

Surely the fact of repeated patches is an indication that this should be
explicitly called out at appropriately places in the code via comments.
Far easier to provide people the info in the code they are changing than
expecting them to trawl through historical mailing list entries.

Regards,
/Bruce

> 
> On 02.02.2018 11:39, Zhiyong Yang wrote:
> > Vhost-user startup will fail based on server mode, if the specified
> > socket file has already existed. The patch introduces function
> > unlink() to remove the possible existing file.
> >
> > Cc: yliu@fridaylinux.org
> > Cc: maxime.coquelin@redhat.com
> >
> > Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
> > ---
> >  lib/librte_vhost/socket.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
> > index 6e3857e7a..324a24f4e 100644
> > --- a/lib/librte_vhost/socket.c
> > +++ b/lib/librte_vhost/socket.c
> > @@ -315,6 +315,7 @@ vhost_user_start_server(struct vhost_user_socket
> *vsocket)
> >  	int fd = vsocket->socket_fd;
> >  	const char *path = vsocket->path;
> >
> > +	unlink(path);
> >  	ret = bind(fd, (struct sockaddr *)&vsocket->un, sizeof(vsocket-
> >un));
> >  	if (ret < 0) {
> >  		RTE_LOG(ERR, VHOST_CONFIG,
> >

Re: [dpdk-dev] vhost: unlink existing file for server mode

[Ilya Maximets]

On 02.02.2018 18:38, Richardson, Bruce wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ilya Maximets
>> Sent: Friday, February 2, 2018 3:30 PM
>> To: Yang, Zhiyong <zhiyong.yang@intel.com>; dev@dpdk.org
>> Cc: yliu@fridaylinux.org; maxime.coquelin@redhat.com
>> Subject: Re: [dpdk-dev] vhost: unlink existing file for server mode
>>
>> Oh.
>>
>> It's such a game: twice a year someone sends this patch to mail list.
>>
>> I have another one for you:
>> * Find all the patches equal to this one in archives.
>> * Read all the discussions.
>> * Come back if you have some new ideas, not already discussed many times
>> here.
>>
>> Sorry for my sarcasm.
>> NACK for this, as usual.
>>
>> Best regards, Ilya Maximets.
> 
> Surely the fact of repeated patches is an indication that this should be
> explicitly called out at appropriately places in the code via comments.
> Far easier to provide people the info in the code they are changing than
> expecting them to trawl through historical mailing list entries.
> 
> Regards,
> /Bruce
> 

Yes, you're right. We just discussed the situation locally in the office
and came to the exactly same conclusion. We definitely need the comment
here to prevent future unlink related patches.

>>
>> On 02.02.2018 11:39, Zhiyong Yang wrote:
>>> Vhost-user startup will fail based on server mode, if the specified
>>> socket file has already existed. The patch introduces function
>>> unlink() to remove the possible existing file.
>>>
>>> Cc: yliu@fridaylinux.org
>>> Cc: maxime.coquelin@redhat.com
>>>
>>> Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
>>> ---
>>>  lib/librte_vhost/socket.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/lib/librte_vhost/socket.c b/lib/librte_vhost/socket.c
>>> index 6e3857e7a..324a24f4e 100644
>>> --- a/lib/librte_vhost/socket.c
>>> +++ b/lib/librte_vhost/socket.c
>>> @@ -315,6 +315,7 @@ vhost_user_start_server(struct vhost_user_socket
>> *vsocket)
>>>  	int fd = vsocket->socket_fd;
>>>  	const char *path = vsocket->path;
>>>
>>> +	unlink(path);
>>>  	ret = bind(fd, (struct sockaddr *)&vsocket->un, sizeof(vsocket-
>>> un));
>>>  	if (ret < 0) {
>>>  		RTE_LOG(ERR, VHOST_CONFIG,
>>>