1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] util:Fix with process number and pid file do not match

dubo163 posted 1 patch 5 years, 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1530529695-36951-2-git-send-email-dubo163@126.com
Test syntax-check passed
src/util/virpidfile.c | 6 ++++++
1 file changed, 6 insertions(+)
[libvirt] [PATCH] util:Fix with process number and pid file do not match
Posted by dubo163 5 years, 10 months ago 
From: dubobo <dubobo@didichuxing.com>

the libvirtd pid file is not match the os process pid number
which is smaller than before.

this would be exist if the libvirtd process coredump or the os
process was killed which the next pid number is smaller.

you can be also edit the pid file to write the longer number than
before,then restart the libvirtd service.

Signed-off-by: dubobo <dubobo@didichuxing.com>
---
 src/util/virpidfile.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c
index 58ab29f..8b0ff99 100644
--- a/src/util/virpidfile.c
+++ b/src/util/virpidfile.c
@@ -445,6 +445,12 @@ int virPidFileAcquirePath(const char *path,
     }
 
     snprintf(pidstr, sizeof(pidstr), "%lld", (long long) pid);
+    if (ftruncate(fd, 0) < 0) {
+        VIR_FORCE_CLOSE(fd);
+        return -1;
+    }
+
+    lseek(fd, 0, SEEK_SET);
 
     if (safewrite(fd, pidstr, strlen(pidstr)) < 0) {
         virReportSystemError(errno,
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util:Fix with process number and pid file do not match
Posted by Michal Prívozník 5 years, 9 months ago 
On 07/02/2018 01:08 PM, dubo163 wrote:
> From: dubobo <dubobo@didichuxing.com>
> 
> the libvirtd pid file is not match the os process pid number
> which is smaller than before.
> 
> this would be exist if the libvirtd process coredump or the os
> process was killed which the next pid number is smaller.
> 
> you can be also edit the pid file to write the longer number than
> before,then restart the libvirtd service.
> 
> Signed-off-by: dubobo <dubobo@didichuxing.com>

I'm sorry, but this has to be your legal name, which I believe dubobo is
not. Also as I was pointed out earlier, the name of the author of the
patch has to be legal name.

> ---
>  src/util/virpidfile.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c
> index 58ab29f..8b0ff99 100644
> --- a/src/util/virpidfile.c
> +++ b/src/util/virpidfile.c
> @@ -445,6 +445,12 @@ int virPidFileAcquirePath(const char *path,
>      }
>  
>      snprintf(pidstr, sizeof(pidstr), "%lld", (long long) pid);
> +    if (ftruncate(fd, 0) < 0) {
> +        VIR_FORCE_CLOSE(fd);
> +        return -1;

So if ftruncate() fails, caller sees -1 but no error message. This is
not nice because users then have no idea what went wrong. All they see
is a failed attempt to start libvirtd. We need virReportSystemError() here.

> +    }
> +
> +    lseek(fd, 0, SEEK_SET);

This is pretty useless. Since open() nothing was written to/read from
the pidfile. So we don't really need to seek in it.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util:Fix with process number and pid file do not match
Posted by Chen Hanxiao 5 years, 9 months ago 
At 2018-07-03 15:36:19, "Michal Prívozník" <mprivozn@redhat.com> wrote:
>On 07/02/2018 01:08 PM, dubo163 wrote:
>> From: dubobo <dubobo@didichuxing.com>
>> 
>> the libvirtd pid file is not match the os process pid number
>> which is smaller than before.
>> 
>> this would be exist if the libvirtd process coredump or the os
>> process was killed which the next pid number is smaller.
>> 
>> you can be also edit the pid file to write the longer number than
>> before,then restart the libvirtd service.
>> 
>> Signed-off-by: dubobo <dubobo@didichuxing.com>
>
>I'm sorry, but this has to be your legal name, which I believe dubobo is
>not. Also as I was pointed out earlier, the name of the author of the
>patch has to be legal name.

Guess that a space needed between family name and given name.
Such as "du bobo" , "Du Bobo" or "Bobo Du"

As a Chinese, the author's name is  a common given name.
One of my friend had the name with that same pronounciation : )

Regards,
- Chen

>
>> ---
>>  src/util/virpidfile.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c
>> index 58ab29f..8b0ff99 100644
>> --- a/src/util/virpidfile.c
>> +++ b/src/util/virpidfile.c
>> @@ -445,6 +445,12 @@ int virPidFileAcquirePath(const char *path,
>>      }
>>  
>>      snprintf(pidstr, sizeof(pidstr), "%lld", (long long) pid);
>> +    if (ftruncate(fd, 0) < 0) {
>> +        VIR_FORCE_CLOSE(fd);
>> +        return -1;
>
>So if ftruncate() fails, caller sees -1 but no error message. This is
>not nice because users then have no idea what went wrong. All they see
>is a failed attempt to start libvirtd. We need virReportSystemError() here.
>
>> +    }
>> +
>> +    lseek(fd, 0, SEEK_SET);
>
>This is pretty useless. Since open() nothing was written to/read from
>the pidfile. So we don't really need to seek in it.
>
>Michal
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util:Fix with process number and pid file do not match
Posted by Michal Prívozník 5 years, 9 months ago 
On 07/04/2018 04:36 AM, Chen Hanxiao wrote:
> 
> At 2018-07-03 15:36:19, "Michal Prívozník" <mprivozn@redhat.com> wrote:
>> On 07/02/2018 01:08 PM, dubo163 wrote:
>>> From: dubobo <dubobo@didichuxing.com>
>>>
>>> the libvirtd pid file is not match the os process pid number
>>> which is smaller than before.
>>>
>>> this would be exist if the libvirtd process coredump or the os
>>> process was killed which the next pid number is smaller.
>>>
>>> you can be also edit the pid file to write the longer number than
>>> before,then restart the libvirtd service.
>>>
>>> Signed-off-by: dubobo <dubobo@didichuxing.com>
>>
>> I'm sorry, but this has to be your legal name, which I believe dubobo is
>> not. Also as I was pointed out earlier, the name of the author of the
>> patch has to be legal name.
> 
> Guess that a space needed between family name and given name.
> Such as "du bobo" , "Du Bobo" or "Bobo Du"

Ah okay. So let me know which one you prefer and I will fix it before
pushing.

Miahl

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util:Fix with process number and pid file do not match
Posted by 杜 博 5 years, 9 months ago 
Bobo Du
在 2018-07-04 12:44:22，"Michal Prívozník" <mprivozn@redhat.com> 写道：
>On 07/04/2018 04:36 AM, Chen Hanxiao wrote:
>> 
>> At 2018-07-03 15:36:19, "Michal Prívozník" <mprivozn@redhat.com> wrote:
>>> On 07/02/2018 01:08 PM, dubo163 wrote:
>>>> From: dubobo <dubobo@didichuxing.com>
>>>>
>>>> the libvirtd pid file is not match the os process pid number
>>>> which is smaller than before.
>>>>
>>>> this would be exist if the libvirtd process coredump or the os
>>>> process was killed which the next pid number is smaller.
>>>>
>>>> you can be also edit the pid file to write the longer number than
>>>> before,then restart the libvirtd service.
>>>>
>>>> Signed-off-by: dubobo <dubobo@didichuxing.com>
>>>
>>> I'm sorry, but this has to be your legal name, which I believe dubobo is
>>> not. Also as I was pointed out earlier, the name of the author of the
>>> patch has to be legal name.
>> 
>> Guess that a space needed between family name and given name.
>> Such as "du bobo" , "Du Bobo" or "Bobo Du"
>
>Ah okay. So let me know which one you prefer and I will fix it before
>pushing.
>
>Miahl

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util:Fix with process number and pid file do not match
Posted by 杜 博 5 years, 9 months ago 
Hi， I Have do it in the last git email sending with correct author name.
在 2018-07-04 12:48:21，"杜 博" <dubo163@126.com> 写道：
>Bobo Du
>在 2018-07-04 12:44:22，"Michal Prívozník" <mprivozn@redhat.com> 写道：
>>On 07/04/2018 04:36 AM, Chen Hanxiao wrote:
>>> 
>>> At 2018-07-03 15:36:19, "Michal Prívozník" <mprivozn@redhat.com> wrote:
>>>> On 07/02/2018 01:08 PM, dubo163 wrote:
>>>>> From: dubobo <dubobo@didichuxing.com>
>>>>>
>>>>> the libvirtd pid file is not match the os process pid number
>>>>> which is smaller than before.
>>>>>
>>>>> this would be exist if the libvirtd process coredump or the os
>>>>> process was killed which the next pid number is smaller.
>>>>>
>>>>> you can be also edit the pid file to write the longer number than
>>>>> before,then restart the libvirtd service.
>>>>>
>>>>> Signed-off-by: dubobo <dubobo@didichuxing.com>
>>>>
>>>> I'm sorry, but this has to be your legal name, which I believe dubobo is
>>>> not. Also as I was pointed out earlier, the name of the author of the
>>>> patch has to be legal name.
>>> 
>>> Guess that a space needed between family name and given name.
>>> Such as "du bobo" , "Du Bobo" or "Bobo Du"
>>
>>Ah okay. So let me know which one you prefer and I will fix it before
>>pushing.
>>
>>Miahl

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.