From nobody Thu May 15 13:41:28 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1508334804322196.59505359026264; Wed, 18 Oct 2017 06:53:24 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4C3265F7AC; Wed, 18 Oct 2017 13:53:23 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 147BF4388; Wed, 18 Oct 2017 13:53:23 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id D1C6762CF1; Wed, 18 Oct 2017 13:53:22 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v9IDrLA9009843 for ; Wed, 18 Oct 2017 09:53:21 -0400 Received: by smtp.corp.redhat.com (Postfix) id 3FC189D352; Wed, 18 Oct 2017 13:53:21 +0000 (UTC) Received: from beluga.usersys.redhat.com (unknown [10.43.2.166]) by smtp.corp.redhat.com (Postfix) with ESMTP id CAD294388; Wed, 18 Oct 2017 13:53:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 4C3265F7AC Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=libvir-list-bounces@redhat.com From: Erik Skultety To: libvir-list@redhat.com Date: Wed, 18 Oct 2017 15:52:25 +0200 Message-Id: <34d95f5775f02c964563164b0b65ceec037e9d5e.1508333637.git.eskultet@redhat.com> In-Reply-To: References: In-Reply-To: References: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-loop: libvir-list@redhat.com Cc: Erik Skultety Subject: [libvirt] [PATCH v6 3/9] nodedev: udev: Convert udev private data to a lockable object X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Wed, 18 Oct 2017 13:53:23 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Since there's going to be a worker thread which needs to have some data protected by a lock, the whole code would just simply get unnecessary complex, since two sets of locks would be necessary, driver lock (for udev monitor and event handle) and a mutex protecting thread-local data. Given the future thread will need to access the udev monitor socket as well, why not protect everything with a single lock, even better, by converting the driver's private data to a lockable object, we get the automatic object disposal feature for free. Signed-off-by: Erik Skultety --- src/node_device/node_device_udev.c | 134 +++++++++++++++++++++++----------= ---- src/node_device/node_device_udev.h | 3 - 2 files changed, 85 insertions(+), 52 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_devi= ce_udev.c index 8314b3834..a7b628153 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -53,11 +53,65 @@ VIR_LOG_INIT("node_device.node_device_udev"); # define TYPE_RAID 12 #endif =20 -struct _udevPrivate { +typedef struct _udevEventData udevEventData; +typedef udevEventData *udevEventDataPtr; + +struct _udevEventData { + virObjectLockable parent; + struct udev_monitor *udev_monitor; int watch; }; =20 +static virClassPtr udevEventDataClass; + +static void +udevEventDataDispose(void *obj) +{ + struct udev *udev =3D NULL; + udevEventDataPtr priv =3D obj; + + if (priv->watch !=3D -1) + virEventRemoveHandle(priv->watch); + + if (!priv->udev_monitor) + return; + + udev =3D udev_monitor_get_udev(priv->udev_monitor); + udev_monitor_unref(priv->udev_monitor); + udev_unref(udev); +} + + +static int +udevEventDataOnceInit(void) +{ + if (!(udevEventDataClass =3D virClassNew(virClassForObjectLockable(), + "udevEventData", + sizeof(udevEventData), + udevEventDataDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(udevEventData) + +static udevEventDataPtr +udevEventDataNew(void) +{ + udevEventDataPtr ret =3D NULL; + + if (udevEventDataInitialize() < 0) + return NULL; + + if (!(ret =3D virObjectLockableNew(udevEventDataClass))) + return NULL; + + ret->watch =3D -1; + return ret; +} + =20 static bool udevHasDeviceProperty(struct udev_device *dev, @@ -1562,39 +1616,18 @@ udevPCITranslateDeinit(void) static int nodeStateCleanup(void) { - udevPrivate *priv =3D NULL; - struct udev_monitor *udev_monitor =3D NULL; - struct udev *udev =3D NULL; - if (!driver) return -1; =20 nodeDeviceLock(); =20 + virObjectUnref(driver->privateData); virObjectUnref(driver->nodeDeviceEventState); =20 - priv =3D driver->privateData; - - if (priv) { - if (priv->watch !=3D -1) - virEventRemoveHandle(priv->watch); - - udev_monitor =3D DRV_STATE_UDEV_MONITOR(driver); - - if (udev_monitor !=3D NULL) { - udev =3D udev_monitor_get_udev(udev_monitor); - udev_monitor_unref(udev_monitor); - } - } - - if (udev !=3D NULL) - udev_unref(udev); - virNodeDeviceObjListFree(driver->devs); nodeDeviceUnlock(); virMutexDestroy(&driver->lock); VIR_FREE(driver); - VIR_FREE(priv); =20 udevPCITranslateDeinit(); return 0; @@ -1618,16 +1651,17 @@ udevHandleOneDevice(struct udev_device *device) } =20 =20 +/* the caller must be holding the udevEventData object lock prior to calli= ng + * this function + */ static bool -udevEventMonitorSanityCheck(struct udev_monitor *udev_monitor, +udevEventMonitorSanityCheck(udevEventDataPtr priv, int fd) { int rc =3D -1; =20 - rc =3D udev_monitor_get_fd(udev_monitor); + rc =3D udev_monitor_get_fd(priv->udev_monitor); if (fd !=3D rc) { - udevPrivate *priv =3D driver->privateData; - virReportError(VIR_ERR_INTERNAL_ERROR, _("File descriptor returned by udev %d does not " "match node device file descriptor %d"), @@ -1653,15 +1687,19 @@ udevEventHandleCallback(int watch ATTRIBUTE_UNUSED, int events ATTRIBUTE_UNUSED, void *data ATTRIBUTE_UNUSED) { + udevEventDataPtr priv =3D driver->privateData; struct udev_device *device =3D NULL; - struct udev_monitor *udev_monitor =3D NULL; =20 - udev_monitor =3D DRV_STATE_UDEV_MONITOR(driver); + virObjectLock(priv); =20 - if (!udevEventMonitorSanityCheck(udev_monitor, fd)) + if (!udevEventMonitorSanityCheck(priv, fd)) { + virObjectUnlock(priv); return; + } + + device =3D udev_monitor_receive_device(priv->udev_monitor); + virObjectUnlock(priv); =20 - device =3D udev_monitor_receive_device(udev_monitor); if (device =3D=3D NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("udev_monitor_receive_device returned NULL")); @@ -1678,12 +1716,13 @@ udevEventHandleCallback(int watch ATTRIBUTE_UNUSED, static void udevGetDMIData(virNodeDevCapSystemPtr syscap) { + udevEventDataPtr priv =3D driver->privateData; struct udev *udev =3D NULL; struct udev_device *device =3D NULL; virNodeDevCapSystemHardwarePtr hardware =3D &syscap->hardware; virNodeDevCapSystemFirmwarePtr firmware =3D &syscap->firmware; =20 - udev =3D udev_monitor_get_udev(DRV_STATE_UDEV_MONITOR(driver)); + udev =3D udev_monitor_get_udev(priv->udev_monitor); =20 device =3D udev_device_new_from_syspath(udev, DMI_DEVPATH); if (device =3D=3D NULL) { @@ -1794,34 +1833,26 @@ nodeStateInitialize(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { - udevPrivate *priv =3D NULL; + udevEventDataPtr priv =3D NULL; struct udev *udev =3D NULL; =20 - if (VIR_ALLOC(priv) < 0) + if (VIR_ALLOC(driver) < 0) return -1; =20 - priv->watch =3D -1; - - if (VIR_ALLOC(driver) < 0) { - VIR_FREE(priv); - return -1; - } - if (virMutexInit(&driver->lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to initialize mutex")); - VIR_FREE(priv); VIR_FREE(driver); return -1; } =20 - driver->privileged =3D privileged; + nodeDeviceLock(); + + if (!(driver->devs =3D virNodeDeviceObjListNew()) || + !(priv =3D udevEventDataNew())) + goto unlock; + driver->privateData =3D priv; - nodeDeviceLock(); - - if (!(driver->devs =3D virNodeDeviceObjListNew())) - goto unlock; - driver->nodeDeviceEventState =3D virObjectEventStateNew(); =20 if (udevPCITranslateInit(privileged) < 0) @@ -1838,8 +1869,10 @@ nodeStateInitialize(bool privileged, udev_set_log_fn(udev, (udevLogFunctionPtr) udevLogFunction); #endif =20 + virObjectLock(priv); + priv->udev_monitor =3D udev_monitor_new_from_netlink(udev, "udev"); - if (priv->udev_monitor =3D=3D NULL) { + if (!priv->udev_monitor) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("udev_monitor_new_from_netlink returned NULL")); goto unlock; @@ -1874,6 +1907,7 @@ nodeStateInitialize(bool privileged, if (udevSetupSystemDev() !=3D 0) goto unlock; =20 + virObjectUnlock(priv); nodeDeviceUnlock(); =20 /* Populate with known devices */ @@ -1887,6 +1921,8 @@ nodeStateInitialize(bool privileged, return -1; =20 unlock: + if (priv) + virObjectUnlock(priv); nodeDeviceUnlock(); goto cleanup; } diff --git a/src/node_device/node_device_udev.h b/src/node_device/node_devi= ce_udev.h index 9a07ab77e..f15e5204c 100644 --- a/src/node_device/node_device_udev.h +++ b/src/node_device/node_device_udev.h @@ -23,9 +23,6 @@ #include #include =20 -typedef struct _udevPrivate udevPrivate; - #define SYSFS_DATA_SIZE 4096 -#define DRV_STATE_UDEV_MONITOR(ds) (((udevPrivate *)((ds)->privateData))->= udev_monitor) #define DMI_DEVPATH "/sys/devices/virtual/dmi/id" #define DMI_DEVPATH_FALLBACK "/sys/class/dmi/id" --=20 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list