[PATCH] virt-admin: Fix segfault when libvirtd dies
vshAdmCatchDisconnect requires non-NULL structure vshControl for
getting connection name (stored at opaque), but
virAdmConnectRegisterCloseCallback at vshAdmConnect called it
with NULL.
Signed-off-by: Adam Julis
---
tools/virt-admin.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index 37bc6fe4f0..0766032e4a 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -112,7 +112,7 @@ vshAdmConnect(vshControl *ctl, unsigned int flags)
return -1;
} else {
if (virAdmConnectRegisterCloseCallback(priv->conn,
vshAdmCatchDisconnect,
- NULL, NULL) < 0)
+ ctl, NULL) < 0)
vshError(ctl, "%s", _("Unable to register disconnect callback"));
if (priv->wantReconnect)
--
2.43.2
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] virt-admin: Fix segfault when libvirtd dies
On 3/19/24 12:02, Adam Julis wrote:
> vshAdmCatchDisconnect requires non-NULL structure vshControl for
> getting connection name (stored at opaque), but
> virAdmConnectRegisterCloseCallback at vshAdmConnect called it
> with NULL.
>
> Signed-off-by: Adam Julis
> ---
> tools/virt-admin.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/virt-admin.c b/tools/virt-admin.c
> index 37bc6fe4f0..0766032e4a 100644
> --- a/tools/virt-admin.c
> +++ b/tools/virt-admin.c
> @@ -112,7 +112,7 @@ vshAdmConnect(vshControl *ctl, unsigned int flags)
> return -1;
> } else {
> if (virAdmConnectRegisterCloseCallback(priv->conn,
> vshAdmCatchDisconnect,
> - NULL, NULL) < 0)
> + ctl, NULL) < 0)
> vshError(ctl, "%s", _("Unable to register disconnect callback"));
>
> if (priv->wantReconnect)
Hi,
if that is the case I would then expect that
virAdmConnectRegisterCloseCallback() needs to also be updated with:
+virCheckNonNullArgGoto(opaque, error);
or something like that. Is there a reason why it isn't? We better catch the
error early if the API is used wrongly.
Claudio
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] virt-admin: Fix segfault when libvirtd dies
On Tue, Mar 19, 2024 at 14:34:08 +0100, Claudio Fontana wrote:
> On 3/19/24 12:02, Adam Julis wrote:
> > vshAdmCatchDisconnect requires non-NULL structure vshControl for
> > getting connection name (stored at opaque), but
> > virAdmConnectRegisterCloseCallback at vshAdmConnect called it
> > with NULL.
> >
> > Signed-off-by: Adam Julis
> > ---
> > tools/virt-admin.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/virt-admin.c b/tools/virt-admin.c
> > index 37bc6fe4f0..0766032e4a 100644
> > --- a/tools/virt-admin.c
> > +++ b/tools/virt-admin.c
> > @@ -112,7 +112,7 @@ vshAdmConnect(vshControl *ctl, unsigned int flags)
> > return -1;
> > } else {
> > if (virAdmConnectRegisterCloseCallback(priv->conn,
> > vshAdmCatchDisconnect,
> > - NULL, NULL) < 0)
> > + ctl, NULL) < 0)
> > vshError(ctl, "%s", _("Unable to register disconnect
> > callback"));
> >
> > if (priv->wantReconnect)
>
> Hi,
>
> if that is the case I would then expect that
> virAdmConnectRegisterCloseCallback() needs to also be updated with:
>
> +virCheckNonNullArgGoto(opaque, error);
>
> or something like that. Is there a reason why it isn't? We better catch the
> error early if the API is used wrongly.
Well, vshAdmCatchDisconnect requires opaque to be non-NULL, but other
callbacks registered with virAdmConnectRegisterCloseCallback may not
need any opaque data.
Jirka
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] virt-admin: Fix segfault when libvirtd dies
On a Tuesday in 2024, Adam Julis wrote: vshAdmCatchDisconnect requires non-NULL structure vshControl for getting connection name (stored at opaque), but virAdmConnectRegisterCloseCallback at vshAdmConnect called it with NULL. Signed-off-by: Adam Julis --- tools/virt-admin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] virt-admin: Fix segfault when libvirtd dies
On 3/19/24 15:58, Jiri Denemark wrote:
> On Tue, Mar 19, 2024 at 14:34:08 +0100, Claudio Fontana wrote:
>> On 3/19/24 12:02, Adam Julis wrote:
>>> vshAdmCatchDisconnect requires non-NULL structure vshControl for
>>> getting connection name (stored at opaque), but
>>> virAdmConnectRegisterCloseCallback at vshAdmConnect called it
>>> with NULL.
>>>
>>> Signed-off-by: Adam Julis ---
>>> tools/virt-admin.c | 2 +- 1 file changed, 1 insertion(+), 1
>>> deletion(-)
>>>
>>> diff --git a/tools/virt-admin.c b/tools/virt-admin.c index
>>> 37bc6fe4f0..0766032e4a 100644 --- a/tools/virt-admin.c +++
>>> b/tools/virt-admin.c @@ -112,7 +112,7 @@ vshAdmConnect(vshControl
>>> *ctl, unsigned int flags) return -1; } else { if
>>> (virAdmConnectRegisterCloseCallback(priv->conn,
>>> vshAdmCatchDisconnect, -
>>> NULL, NULL) < 0) +
>>> ctl, NULL) < 0) vshError(ctl, "%s", _("Unable to register
>>> disconnect callback"));
>>>
>>> if (priv->wantReconnect)
>>
>> Hi,
>>
>> if that is the case I would then expect that
>> virAdmConnectRegisterCloseCallback() needs to also be updated
>> with:
>>
>> +virCheckNonNullArgGoto(opaque, error);
>>
>> or something like that. Is there a reason why it isn't? We better
>> catch the error early if the API is used wrongly.
>
> Well, vshAdmCatchDisconnect requires opaque to be non-NULL, but
> other callbacks registered with virAdmConnectRegisterCloseCallback
> may not need any opaque data.
>
Fair enough.
Reviewed-by: Claudio Fontana
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org
