[libvirt] [PATCH v4 12/23] lock_daemon_dispatch: Check for ownerPid rather than ownerId

Michal Privoznik posted 23 patches 6 years, 8 months ago
[libvirt] [PATCH v4 12/23] lock_daemon_dispatch: Check for ownerPid rather than ownerId
Posted by Michal Privoznik 6 years, 8 months ago
At the beginning of each dispatch function we check if owner
attributes were registered (these consist of ID, UUID, PID and
name). The check then consists of checking if ID is not zero.
This is not going to work with
VIR_LOCK_MANAGER_OBJECT_TYPE_DAEMON which doesn't set ID. Switch
to setting PID which is available for both cases.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/locking/lock_daemon_dispatch.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/locking/lock_daemon_dispatch.c b/src/locking/lock_daemon_dispatch.c
index a683ad3d6b..36a2462592 100644
--- a/src/locking/lock_daemon_dispatch.c
+++ b/src/locking/lock_daemon_dispatch.c
@@ -68,7 +68,7 @@ virLockSpaceProtocolDispatchAcquireResource(virNetServerPtr server ATTRIBUTE_UNU
         goto cleanup;
     }
 
-    if (!priv->ownerId) {
+    if (!priv->ownerPid) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                        _("lock owner details have not been registered"));
         goto cleanup;
@@ -129,7 +129,7 @@ virLockSpaceProtocolDispatchCreateResource(virNetServerPtr server ATTRIBUTE_UNUS
         goto cleanup;
     }
 
-    if (!priv->ownerId) {
+    if (!priv->ownerPid) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                        _("lock owner details have not been registered"));
         goto cleanup;
@@ -178,7 +178,7 @@ virLockSpaceProtocolDispatchDeleteResource(virNetServerPtr server ATTRIBUTE_UNUS
         goto cleanup;
     }
 
-    if (!priv->ownerId) {
+    if (!priv->ownerPid) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                        _("lock owner details have not been registered"));
         goto cleanup;
@@ -227,7 +227,7 @@ virLockSpaceProtocolDispatchNew(virNetServerPtr server ATTRIBUTE_UNUSED,
         goto cleanup;
     }
 
-    if (!priv->ownerId) {
+    if (!priv->ownerPid) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                        _("lock owner details have not been registered"));
         goto cleanup;
@@ -282,7 +282,7 @@ virLockSpaceProtocolDispatchRegister(virNetServerPtr server ATTRIBUTE_UNUSED,
         goto cleanup;
     }
 
-    if (!args->owner.id) {
+    if (!args->owner.pid) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                        _("lock owner details have not been registered"));
         goto cleanup;
@@ -329,7 +329,7 @@ virLockSpaceProtocolDispatchReleaseResource(virNetServerPtr server ATTRIBUTE_UNU
         goto cleanup;
     }
 
-    if (!priv->ownerId) {
+    if (!priv->ownerPid) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                        _("lock owner details have not been registered"));
         goto cleanup;
@@ -379,7 +379,7 @@ virLockSpaceProtocolDispatchRestrict(virNetServerPtr server ATTRIBUTE_UNUSED,
         goto cleanup;
     }
 
-    if (!priv->ownerId) {
+    if (!priv->ownerPid) {
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                        _("lock owner details have not been registered"));
         goto cleanup;
-- 
2.16.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 12/23] lock_daemon_dispatch: Check for ownerPid rather than ownerId
Posted by John Ferlan 6 years, 7 months ago

On 09/10/2018 05:36 AM, Michal Privoznik wrote:
> At the beginning of each dispatch function we check if owner
> attributes were registered (these consist of ID, UUID, PID and
> name). The check then consists of checking if ID is not zero.
> This is not going to work with
> VIR_LOCK_MANAGER_OBJECT_TYPE_DAEMON which doesn't set ID. Switch
> to setting PID which is available for both cases.

It would if daemon used @pid for ID ;-)

> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/locking/lock_daemon_dispatch.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 

Looking through a bit of history and came across an oh shinola moment, see:

commit id ee3eddb332

You may need to implement what I suggest above and move on from there.

John

(shinola is a reference to a movie called "The Jerk" and the main
character not knowing sh!t from shinola)

> diff --git a/src/locking/lock_daemon_dispatch.c b/src/locking/lock_daemon_dispatch.c
> index a683ad3d6b..36a2462592 100644
> --- a/src/locking/lock_daemon_dispatch.c
> +++ b/src/locking/lock_daemon_dispatch.c
> @@ -68,7 +68,7 @@ virLockSpaceProtocolDispatchAcquireResource(virNetServerPtr server ATTRIBUTE_UNU
>          goto cleanup;
>      }
>  
> -    if (!priv->ownerId) {
> +    if (!priv->ownerPid) {
>          virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>                         _("lock owner details have not been registered"));
>          goto cleanup;
> @@ -129,7 +129,7 @@ virLockSpaceProtocolDispatchCreateResource(virNetServerPtr server ATTRIBUTE_UNUS
>          goto cleanup;
>      }
>  
> -    if (!priv->ownerId) {
> +    if (!priv->ownerPid) {
>          virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>                         _("lock owner details have not been registered"));
>          goto cleanup;
> @@ -178,7 +178,7 @@ virLockSpaceProtocolDispatchDeleteResource(virNetServerPtr server ATTRIBUTE_UNUS
>          goto cleanup;
>      }
>  
> -    if (!priv->ownerId) {
> +    if (!priv->ownerPid) {
>          virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>                         _("lock owner details have not been registered"));
>          goto cleanup;
> @@ -227,7 +227,7 @@ virLockSpaceProtocolDispatchNew(virNetServerPtr server ATTRIBUTE_UNUSED,
>          goto cleanup;
>      }
>  
> -    if (!priv->ownerId) {
> +    if (!priv->ownerPid) {
>          virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>                         _("lock owner details have not been registered"));
>          goto cleanup;
> @@ -282,7 +282,7 @@ virLockSpaceProtocolDispatchRegister(virNetServerPtr server ATTRIBUTE_UNUSED,
>          goto cleanup;
>      }
>  
> -    if (!args->owner.id) {
> +    if (!args->owner.pid) {
>          virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>                         _("lock owner details have not been registered"));
>          goto cleanup;
> @@ -329,7 +329,7 @@ virLockSpaceProtocolDispatchReleaseResource(virNetServerPtr server ATTRIBUTE_UNU
>          goto cleanup;
>      }
>  
> -    if (!priv->ownerId) {
> +    if (!priv->ownerPid) {
>          virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>                         _("lock owner details have not been registered"));
>          goto cleanup;
> @@ -379,7 +379,7 @@ virLockSpaceProtocolDispatchRestrict(virNetServerPtr server ATTRIBUTE_UNUSED,
>          goto cleanup;
>      }
>  
> -    if (!priv->ownerId) {
> +    if (!priv->ownerPid) {
>          virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>                         _("lock owner details have not been registered"));
>          goto cleanup;
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 12/23] lock_daemon_dispatch: Check for ownerPid rather than ownerId
Posted by Michal Privoznik 6 years, 7 months ago
On 09/17/2018 08:40 PM, John Ferlan wrote:
> 
> 
> On 09/10/2018 05:36 AM, Michal Privoznik wrote:
>> At the beginning of each dispatch function we check if owner
>> attributes were registered (these consist of ID, UUID, PID and
>> name). The check then consists of checking if ID is not zero.
>> This is not going to work with
>> VIR_LOCK_MANAGER_OBJECT_TYPE_DAEMON which doesn't set ID. Switch
>> to setting PID which is available for both cases.
> 
> It would if daemon used @pid for ID ;-)
> 
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/locking/lock_daemon_dispatch.c | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
> 
> Looking through a bit of history and came across an oh shinola moment, see:
> 
> commit id ee3eddb332
> 
> You may need to implement what I suggest above and move on from there.

Ah, this is a very good point. I'll have
VIR_LOCK_MANAGER_OBJECT_TYPE_DAEMON set id to PID.

This one should be dropped then.

Michal

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