Re: [PATCH v3 2/3] util: Get rid of virFileFlock()

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.

-- Andrea Bolognani / Red Hat / Virtualization