On Wed, Jul 29, 2020 at 02:20:50PM +0200, Andrea Bolognani wrote:
On Wed, 2020-07-29 at 13:43 +0200, Martin Kletzander wrote:
+++ b/src/util/virresctrl.c @@ -456,6 +456,8 @@ VIR_ONCE_GLOBAL_INIT(virResctrl); static int virResctrlLockWrite(void) { +#ifndef WIN32 + int fd = open(SYSFS_RESCTRL_PATH, O_RDONLY | O_CLOEXEC);
if (fd < 0) { @@ -463,13 +465,20 @@ virResctrlLockWrite(void) return -1; }
- if (virFileFlock(fd, true, false) < 0) { + if (flock(fd, LOCK_EX) < 0) { virReportSystemError(errno, "%s", _("Cannot lock resctrl")); VIR_FORCE_CLOSE(fd); return -1; }
return fd; + +#else /* WIN32 */ + + virReportSystemError(ENOSYS, "%s", _("Cannot lock resctrl")); + return -1; + +#endif }
@@ -484,10 +493,14 @@ virResctrlUnlock(int fd) if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, "%s", _("Cannot close resctrl"));
+#ifndef WIN32 + /* Trying to save the already broken */ - if (virFileFlock(fd, false, false) < 0) + if (flock(fd, LOCK_UN) < 0) virReportSystemError(errno, "%s", _("Cannot unlock resctrl"));
+#endif + return -1; }
So in the end you decided to go for the nuclear option :D
I'm okay with the approach, but I would prefer if you stubbed out the functions completely, eg.
#ifndef WIN32
static int virResctrlLockWrite(void) { /* do stuff */ }
static int virResctrlUnlock(int fd) { /* do stuff */ }
#else
static int virResctrlLockWrite(void) { virReportSystemError(ENOSYS, "%s", _("resctrl locking is not supported " "on this platform")); return -1; }
static int virResctrlUnlock(int fd) { virReportSystemError(ENOSYS, "%s", _("resctrl locking is not supported " "on this platform")); return -1; }
#endif
Also, since AFAIU resctrl is Linux-only, perhaps a better preprocessor guard would be
#ifdef __linux__
so that we (correctly) stub the functions out on FreeBSD and macOS too.
Well, in case they get any similar feature (since it is actually Intel-only AFAIK) I just wanted to stub it out only for platforms where flock(2) is not available.
-- Andrea Bolognani / Red Hat / Virtualization