That's the way it should've been all the time.  It was originally the case, but
then the rework to virFileFlock() made the function ambiguous when it was
created in commit 5a0a5f7fb5f5, and due to that it was misused in commit
657ddeff2313 and since then the lock being taken was shared rather than
exclusive.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 src/util/virresctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index 3563fc560db5..9b78a6cb159b 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -463,7 +463,7 @@ virResctrlLockWrite(void)
         return -1;
     }
 
-    if (virFileFlock(fd, true, true) < 0) {
+    if (virFileFlock(fd, true, false) < 0) {
         virReportSystemError(errno, "%s", _("Cannot lock resctrl"));
         VIR_FORCE_CLOSE(fd);
         return -1;
-- 
2.28.0

On Wed, 2020-07-29 at 13:43 +0200, Martin Kletzander wrote:
> That's the way it should've been all the time.  It was originally the case, but
> then the rework to virFileFlock() made the function ambiguous when it was
> created in commit 5a0a5f7fb5f5, and due to that it was misused in commit
> 657ddeff2313 and since then the lock being taken was shared rather than
> exclusive.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  src/util/virresctrl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Consider this

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

and safe for freeze, but honestly this issue has been around for more
than two years at this point so I'm not entirely convinced it can't
just wait for the merge window to open again. Up to you.

-- 
Andrea Bolognani / Red Hat / Virtualization

On Wed, Jul 29, 2020 at 02:09:20PM +0200, Andrea Bolognani wrote:
>On Wed, 2020-07-29 at 13:43 +0200, Martin Kletzander wrote:
>> That's the way it should've been all the time.  It was originally the case, but
>> then the rework to virFileFlock() made the function ambiguous when it was
>> created in commit 5a0a5f7fb5f5, and due to that it was misused in commit
>> 657ddeff2313 and since then the lock being taken was shared rather than
>> exclusive.
>>
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>>  src/util/virresctrl.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
>Consider this
>
>  Reviewed-by: Andrea Bolognani <abologna@redhat.com>
>
>and safe for freeze, but honestly this issue has been around for more
>than two years at this point so I'm not entirely convinced it can't
>just wait for the merge window to open again. Up to you.
>

I agree and I'm fine with that.

>--
>Andrea Bolognani / Red Hat / Virtualization
>

On a Wednesday in 2020, Andrea Bolognani wrote:
>On Wed, 2020-07-29 at 13:43 +0200, Martin Kletzander wrote:
>> That's the way it should've been all the time.  It was originally the case, but
>> then the rework to virFileFlock() made the function ambiguous when it was
>> created in commit 5a0a5f7fb5f5, and due to that it was misused in commit
>> 657ddeff2313 and since then the lock being taken was shared rather than
>> exclusive.
>>
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>>  src/util/virresctrl.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
>Consider this
>
>  Reviewed-by: Andrea Bolognani <abologna@redhat.com>
>
>and safe for freeze, but honestly this issue has been around for more
>than two years at this point so I'm not entirely convinced it can't
>just wait for the merge window to open again. Up to you.

The merge window is open. We are in a feature freeze, meaning we are
free to push bug fixes. Possibly trivial typos in the CI configuration.

A freeze meaing "we do not push anything at all" is pointless.

Jano

>
>-- 
>Andrea Bolognani / Red Hat / Virtualization
>