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