[libvirt] [PATCH] qemu: handle -1 for pid in qemuDomainGetMachineName

Nikolay Shirokovskiy posted 1 patch 6 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1504170104-841798-1-git-send-email-nshirokovskiy@virtuozzo.com
src/qemu/qemu_domain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[libvirt] [PATCH] qemu: handle -1 for pid in qemuDomainGetMachineName
Posted by Nikolay Shirokovskiy 6 years, 7 months ago
We call qemuDomainGetMachineName on domain start. On first
start (after daemon start) pid is 0 and virSystemdGetMachineNameByPID
don't get called. But after domain shutting down pid became -1 so
on next start virSystemdGetMachineNameByPID is called and returned an error.
Error is ignored so it is not critical. But at least on my system
(systemd-219 with extra patches) systemd-machined is crashed on
this request.

This behaviour is triggered by eaf2c9f89.
---
 src/qemu/qemu_domain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e580a5b..fe44d36 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9696,7 +9696,7 @@ qemuDomainGetMachineName(virDomainObjPtr vm)
     virQEMUDriverPtr driver = priv->driver;
     char *ret = NULL;
 
-    if (vm->pid) {
+    if (vm->pid > 0) {
         ret = virSystemdGetMachineNameByPID(vm->pid);
         if (!ret)
             virResetLastError();
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: handle -1 for pid in qemuDomainGetMachineName
Posted by Daniel P. Berrange 6 years, 7 months ago
On Thu, Aug 31, 2017 at 12:01:44PM +0300, Nikolay Shirokovskiy wrote:
> We call qemuDomainGetMachineName on domain start. On first
> start (after daemon start) pid is 0 and virSystemdGetMachineNameByPID
> don't get called. But after domain shutting down pid became -1 so
> on next start virSystemdGetMachineNameByPID is called and returned an error.
> Error is ignored so it is not critical. But at least on my system
> (systemd-219 with extra patches) systemd-machined is crashed on
> this request.
> 
> This behaviour is triggered by eaf2c9f89.
> ---
>  src/qemu/qemu_domain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index e580a5b..fe44d36 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -9696,7 +9696,7 @@ qemuDomainGetMachineName(virDomainObjPtr vm)
>      virQEMUDriverPtr driver = priv->driver;
>      char *ret = NULL;
>  
> -    if (vm->pid) {
> +    if (vm->pid > 0) {
>          ret = virSystemdGetMachineNameByPID(vm->pid);
>          if (!ret)
>              virResetLastError();

Reviewed-by: Daniel P. Berrange <berrange@redhat.com>


Worth pushing for this release too IMHO

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: handle -1 for pid in qemuDomainGetMachineName
Posted by Michal Privoznik 6 years, 7 months ago
On 08/31/2017 11:29 AM, Daniel P. Berrange wrote:
> On Thu, Aug 31, 2017 at 12:01:44PM +0300, Nikolay Shirokovskiy wrote:
>> We call qemuDomainGetMachineName on domain start. On first
>> start (after daemon start) pid is 0 and virSystemdGetMachineNameByPID
>> don't get called. But after domain shutting down pid became -1 so
>> on next start virSystemdGetMachineNameByPID is called and returned an error.
>> Error is ignored so it is not critical. But at least on my system
>> (systemd-219 with extra patches) systemd-machined is crashed on
>> this request.
>>

Well, you need to fix your systemd too since anybody can issue the call
with pid = -1 ;-)

>> This behaviour is triggered by eaf2c9f89.
>> ---
>>  src/qemu/qemu_domain.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index e580a5b..fe44d36 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -9696,7 +9696,7 @@ qemuDomainGetMachineName(virDomainObjPtr vm)
>>      virQEMUDriverPtr driver = priv->driver;
>>      char *ret = NULL;
>>  
>> -    if (vm->pid) {
>> +    if (vm->pid > 0) {
>>          ret = virSystemdGetMachineNameByPID(vm->pid);
>>          if (!ret)
>>              virResetLastError();
> 
> Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
> 
> 
> Worth pushing for this release too IMHO

Yep. I've just pushed this. Thanks,

Michal

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