[Qemu-devel] [PATCH v2] ivshmem-server: Clean up shmem on shutdown

Jan Kiszka posted 1 patch 4 years, 8 months ago
Test checkpatch passed
Test s390x passed
Test asan passed
Test docker-mingw@fedora passed
Test FreeBSD passed
Test docker-clang@ubuntu passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/d938a62c-7538-9d2b-cc0a-13b240ab9141@web.de
contrib/ivshmem-server/ivshmem-server.c | 6 ++++++
1 file changed, 6 insertions(+)
[Qemu-devel] [PATCH v2] ivshmem-server: Clean up shmem on shutdown
Posted by Jan Kiszka 4 years, 8 months ago
From: Jan Kiszka <jan.kiszka@siemens.com>

So far, the server leaves the posix shared memory object behind when
terminating, requiring the user to explicitly remove it in order to
start a new instance.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

Changes in v2:
 - respect use_shm_open
 - also clean up in ivshmem_server_start error path

 contrib/ivshmem-server/ivshmem-server.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/contrib/ivshmem-server/ivshmem-server.c b/contrib/ivshmem-server/ivshmem-server.c
index 77f97b209c..88daee812d 100644
--- a/contrib/ivshmem-server/ivshmem-server.c
+++ b/contrib/ivshmem-server/ivshmem-server.c
@@ -353,6 +353,9 @@ ivshmem_server_start(IvshmemServer *server)
 err_close_sock:
     close(sock_fd);
 err_close_shm:
+    if (server->use_shm_open) {
+        shm_unlink(server->shm_path);
+    }
     close(shm_fd);
     return -1;
 }
@@ -370,6 +373,9 @@ ivshmem_server_close(IvshmemServer *server)
     }

     unlink(server->unix_sock_path);
+    if (server->use_shm_open) {
+        shm_unlink(server->shm_path);
+    }
     close(server->sock_fd);
     close(server->shm_fd);
     server->sock_fd = -1;
--
2.16.4

Re: [Qemu-devel] [PATCH v2] ivshmem-server: Clean up shmem on shutdown
Posted by Claudio Fontana 4 years, 8 months ago
On 8/5/19 7:54 AM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> So far, the server leaves the posix shared memory object behind when
> terminating, requiring the user to explicitly remove it in order to
> start a new instance.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> Changes in v2:
>  - respect use_shm_open
>  - also clean up in ivshmem_server_start error path
> 
>  contrib/ivshmem-server/ivshmem-server.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/contrib/ivshmem-server/ivshmem-server.c b/contrib/ivshmem-server/ivshmem-server.c
> index 77f97b209c..88daee812d 100644
> --- a/contrib/ivshmem-server/ivshmem-server.c
> +++ b/contrib/ivshmem-server/ivshmem-server.c
> @@ -353,6 +353,9 @@ ivshmem_server_start(IvshmemServer *server)
>  err_close_sock:
>      close(sock_fd);
>  err_close_shm:
> +    if (server->use_shm_open) {
> +        shm_unlink(server->shm_path);
> +    }
>      close(shm_fd);
>      return -1;
>  }
> @@ -370,6 +373,9 @@ ivshmem_server_close(IvshmemServer *server)
>      }
> 
>      unlink(server->unix_sock_path);
> +    if (server->use_shm_open) {
> +        shm_unlink(server->shm_path);
> +    }
>      close(server->sock_fd);
>      close(server->shm_fd);
>      server->sock_fd = -1;
> --
> 2.16.4
> 
> 

Reviewed-by: Claudio Fontana <claudio.fontana@suse.com>


Re: [PATCH v2] ivshmem-server: Clean up shmem on shutdown
Posted by Jan Kiszka 4 years, 5 months ago
On 06.08.19 15:01, Claudio Fontana wrote:
> On 8/5/19 7:54 AM, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> So far, the server leaves the posix shared memory object behind when
>> terminating, requiring the user to explicitly remove it in order to
>> start a new instance.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> Changes in v2:
>>   - respect use_shm_open
>>   - also clean up in ivshmem_server_start error path
>>
>>   contrib/ivshmem-server/ivshmem-server.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/contrib/ivshmem-server/ivshmem-server.c b/contrib/ivshmem-server/ivshmem-server.c
>> index 77f97b209c..88daee812d 100644
>> --- a/contrib/ivshmem-server/ivshmem-server.c
>> +++ b/contrib/ivshmem-server/ivshmem-server.c
>> @@ -353,6 +353,9 @@ ivshmem_server_start(IvshmemServer *server)
>>   err_close_sock:
>>       close(sock_fd);
>>   err_close_shm:
>> +    if (server->use_shm_open) {
>> +        shm_unlink(server->shm_path);
>> +    }
>>       close(shm_fd);
>>       return -1;
>>   }
>> @@ -370,6 +373,9 @@ ivshmem_server_close(IvshmemServer *server)
>>       }
>>
>>       unlink(server->unix_sock_path);
>> +    if (server->use_shm_open) {
>> +        shm_unlink(server->shm_path);
>> +    }
>>       close(server->sock_fd);
>>       close(server->shm_fd);
>>       server->sock_fd = -1;
>> --
>> 2.16.4
>>
>>
> 
> Reviewed-by: Claudio Fontana <claudio.fontana@suse.com>
> 
> 
> 

Markus, would you take this?

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v2] ivshmem-server: Clean up shmem on shutdown
Posted by Markus Armbruster 4 years, 5 months ago
Jan Kiszka <jan.kiszka@siemens.com> writes:

> On 06.08.19 15:01, Claudio Fontana wrote:
>> On 8/5/19 7:54 AM, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> So far, the server leaves the posix shared memory object behind when
>>> terminating, requiring the user to explicitly remove it in order to
>>> start a new instance.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>
>>> Changes in v2:
>>>   - respect use_shm_open
>>>   - also clean up in ivshmem_server_start error path
>>>
>>>   contrib/ivshmem-server/ivshmem-server.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/contrib/ivshmem-server/ivshmem-server.c b/contrib/ivshmem-server/ivshmem-server.c
>>> index 77f97b209c..88daee812d 100644
>>> --- a/contrib/ivshmem-server/ivshmem-server.c
>>> +++ b/contrib/ivshmem-server/ivshmem-server.c
>>> @@ -353,6 +353,9 @@ ivshmem_server_start(IvshmemServer *server)
>>>   err_close_sock:
>>>       close(sock_fd);
>>>   err_close_shm:
>>> +    if (server->use_shm_open) {
>>> +        shm_unlink(server->shm_path);
>>> +    }
>>>       close(shm_fd);
>>>       return -1;
>>>   }
>>> @@ -370,6 +373,9 @@ ivshmem_server_close(IvshmemServer *server)
>>>       }
>>>
>>>       unlink(server->unix_sock_path);
>>> +    if (server->use_shm_open) {
>>> +        shm_unlink(server->shm_path);
>>> +    }
>>>       close(server->sock_fd);
>>>       close(server->shm_fd);
>>>       server->sock_fd = -1;
>>> --
>>> 2.16.4
>>>
>>>
>>
>> Reviewed-by: Claudio Fontana <claudio.fontana@suse.com>
>
> Markus, would you take this?

ivshmem has no maintainer.  I you need me to serve as a pull request
monkey of last resort, I can do that.  However, for this one,
qemu-trivial (cc'ed) should do.


Re: [PATCH v2] ivshmem-server: Clean up shmem on shutdown
Posted by Laurent Vivier 4 years, 5 months ago
Le 08/11/2019 à 16:14, Markus Armbruster a écrit :
> Jan Kiszka <jan.kiszka@siemens.com> writes:
> 
>> On 06.08.19 15:01, Claudio Fontana wrote:
>>> On 8/5/19 7:54 AM, Jan Kiszka wrote:
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> So far, the server leaves the posix shared memory object behind when
>>>> terminating, requiring the user to explicitly remove it in order to
>>>> start a new instance.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>>
>>>> Changes in v2:
>>>>   - respect use_shm_open
>>>>   - also clean up in ivshmem_server_start error path
>>>>
>>>>   contrib/ivshmem-server/ivshmem-server.c | 6 ++++++
>>>>   1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/contrib/ivshmem-server/ivshmem-server.c b/contrib/ivshmem-server/ivshmem-server.c
>>>> index 77f97b209c..88daee812d 100644
>>>> --- a/contrib/ivshmem-server/ivshmem-server.c
>>>> +++ b/contrib/ivshmem-server/ivshmem-server.c
>>>> @@ -353,6 +353,9 @@ ivshmem_server_start(IvshmemServer *server)
>>>>   err_close_sock:
>>>>       close(sock_fd);
>>>>   err_close_shm:
>>>> +    if (server->use_shm_open) {
>>>> +        shm_unlink(server->shm_path);
>>>> +    }
>>>>       close(shm_fd);
>>>>       return -1;
>>>>   }
>>>> @@ -370,6 +373,9 @@ ivshmem_server_close(IvshmemServer *server)
>>>>       }
>>>>
>>>>       unlink(server->unix_sock_path);
>>>> +    if (server->use_shm_open) {
>>>> +        shm_unlink(server->shm_path);
>>>> +    }
>>>>       close(server->sock_fd);
>>>>       close(server->shm_fd);
>>>>       server->sock_fd = -1;
>>>> --
>>>> 2.16.4
>>>>
>>>>
>>>
>>> Reviewed-by: Claudio Fontana <claudio.fontana@suse.com>
>>
>> Markus, would you take this?
> 
> ivshmem has no maintainer.  I you need me to serve as a pull request
> monkey of last resort, I can do that.  However, for this one,
> qemu-trivial (cc'ed) should do.
> 
> 

Applied to my trivial-patches branch.

Thanks,
Laurent