[libvirt] [PATCH] avoid using deprecated udev logging functions

In systemd >= 218, the udev_set_log_fn method has been marked deprecated and turned into a no-op. Nothing in the udev client library will print to stderr by default anymore, so we can just stop installing a logging hook for new enough udev. --- m4/virt-udev.m4 | 7 +++++++ src/node_device/node_device_udev.c | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/m4/virt-udev.m4 b/m4/virt-udev.m4 index 55673bf..0d785c4 100644 --- a/m4/virt-udev.m4 +++ b/m4/virt-udev.m4 @@ -24,6 +24,13 @@ AC_DEFUN([LIBVIRT_CHECK_UDEV],[ if test "$with_udev" = "yes" && test "$with_pciaccess" != "yes" ; then AC_MSG_ERROR([You must install the pciaccess module to build with udev]) fi + + if test "$with_udev" = "yes" ; then + PKG_CHECK_EXISTS(libudev >= 218, [with_udev_logging=no], [with_udev_logging=yes]) + if test "$with_udev_logging" = "yes" ; then + AC_DEFINE_UNQUOTED([HAVE_UDEV_LOGGING], 1, [whether libudev logging can be used]) + fi + fi ]) AC_DEFUN([LIBVIRT_RESULT_UDEV],[ diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 343ef85..1a35c07 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -340,7 +340,7 @@ static int udevGenerateDeviceName(struct udev_device *device, return ret; } - +#if HAVE_UDEV_LOGGING typedef void (*udevLogFunctionPtr)(struct udev *udev, int priority, const char *file, @@ -373,6 +373,7 @@ udevLogFunction(struct udev *udev ATTRIBUTE_UNUSED, VIR_FREE(format); } +#endif static int udevTranslatePCIIds(unsigned int vendor, @@ -1759,8 +1760,10 @@ static int nodeStateInitialize(bool privileged, * its return value. */ udev = udev_new(); +#if HAVE_UDEV_LOGGING /* cast to get rid of missing-format-attribute warning */ udev_set_log_fn(udev, (udevLogFunctionPtr) udevLogFunction); +#endif priv->udev_monitor = udev_monitor_new_from_netlink(udev, "udev"); if (priv->udev_monitor == NULL) { -- 2.1.0

On 12/15/2014 08:33 AM, Daniel P. Berrange wrote:
In systemd >= 218, the udev_set_log_fn method has been marked deprecated and turned into a no-op. Nothing in the udev client library will print to stderr by default anymore, so we can just stop installing a logging hook for new enough udev. --- m4/virt-udev.m4 | 7 +++++++ src/node_device/node_device_udev.c | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/m4/virt-udev.m4 b/m4/virt-udev.m4 index 55673bf..0d785c4 100644 --- a/m4/virt-udev.m4 +++ b/m4/virt-udev.m4 @@ -24,6 +24,13 @@ AC_DEFUN([LIBVIRT_CHECK_UDEV],[ if test "$with_udev" = "yes" && test "$with_pciaccess" != "yes" ; then AC_MSG_ERROR([You must install the pciaccess module to build with udev]) fi + + if test "$with_udev" = "yes" ; then + PKG_CHECK_EXISTS(libudev >= 218, [with_udev_logging=no], [with_udev_logging=yes])
Style: I'd write [libudev >= 218] (that is, add [] quoting), since autoconf recommends quoting all arguments. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Mon, Dec 15, 2014 at 03:33:13PM +0000, Daniel P. Berrange wrote:
In systemd >= 218, the udev_set_log_fn method has been marked deprecated and turned into a no-op. Nothing in the udev client library will print to stderr by default anymore, so we can just stop installing a logging hook for new enough udev.
This looks somehow reversed. Why should we log for libudev < 218 if we deem the messages superfluous for >= 218? I'm probably missing something obvious. If not we should remove the messages completely to be consitent. Cheers, -- Guido

On 12/16/2014 03:08 PM, Guido Günther wrote:
On Mon, Dec 15, 2014 at 03:33:13PM +0000, Daniel P. Berrange wrote:
In systemd >= 218, the udev_set_log_fn method has been marked deprecated and turned into a no-op. Nothing in the udev client library will print to stderr by default anymore, so we can just stop installing a logging hook for new enough udev.
This looks somehow reversed. Why should we log for libudev < 218 if we deem the messages superfluous for >= 218? I'm probably missing something obvious. If not we should remove the messages completely to be consitent.
If I understand correctly, the older udev _requires_ us to supply a logging alternative (because the default is noisy); the newer udev is silent by default and therefore deprecated the function used to install our logging alternative. So the patch looked correct - for old versions, do what we have always done to stay quiet, for new versions, avoid the deprecated function and we stay quiet by default. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Tue, Dec 16, 2014 at 03:28:33PM -0700, Eric Blake wrote:
On 12/16/2014 03:08 PM, Guido Günther wrote:
On Mon, Dec 15, 2014 at 03:33:13PM +0000, Daniel P. Berrange wrote:
In systemd >= 218, the udev_set_log_fn method has been marked deprecated and turned into a no-op. Nothing in the udev client library will print to stderr by default anymore, so we can just stop installing a logging hook for new enough udev.
This looks somehow reversed. Why should we log for libudev < 218 if we deem the messages superfluous for >= 218? I'm probably missing something obvious. If not we should remove the messages completely to be consitent.
If I understand correctly, the older udev _requires_ us to supply a logging alternative (because the default is noisy); the newer udev is silent by default and therefore deprecated the function used to install our logging alternative. So the patch looked correct - for old versions, do what we have always done to stay quiet, for new versions, avoid the deprecated function and we stay quiet by default.
Yeah, rereading the code made it clear. Thanks for the clarification. Cheers, -- Guido
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
Guido Günther