[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH v2 02/23] hw/core/qdev: Add qdev_warn_deprecated_function

From: BALATON Zoltan
Subject: Re: [RFC PATCH v2 02/23] hw/core/qdev: Add qdev_warn_deprecated_function_used() helper
Date: Sat, 4 Jul 2020 18:22:22 +0200 (CEST)
User-agent: Alpine 2.22 (BSF 395 2020-01-19)

On Sat, 4 Jul 2020, Philippe Mathieu-Daudé wrote:
When built with --enable-qdev-deprecation-warning, calling
qdev_warn_deprecated_function_used() will emit a warning such:

 $ qemu-system-arm -M verdex ...
 qemu-system-arm: warning: use of deprecated non-qdev/non-qom code in 
 qemu-system-arm: warning: use of deprecated non-qdev/non-qom code in 
 qemu-system-arm: warning: use of deprecated non-qdev/non-qom code in 

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
I'd rather use --enable-qdev-debug suggested here:
configure                    |  8 ++++++++
include/hw/qdev-deprecated.h | 26 ++++++++++++++++++++++++++
hw/core/qdev.c               |  8 ++++++++
3 files changed, 42 insertions(+)
create mode 100644 include/hw/qdev-deprecated.h

diff --git a/configure b/configure
index 8a65240d4a..aac3dc0767 100755
--- a/configure
+++ b/configure
@@ -441,6 +441,7 @@ edk2_blobs="no"
@@ -1124,6 +1125,8 @@ for opt do
  --enable-qom-cast-debug) qom_cast_debug="yes"
+  --enable-qdev-deprecation-warning) qdev_deprecation_warning="yes"
+  ;;
  --disable-virtfs) virtfs="no"
  --enable-virtfs) virtfs="yes"
@@ -1915,6 +1918,7 @@ disabled with --disable-FEATURE, default is enabled if 
  virglrenderer   virgl rendering support
  xfsctl          xfsctl support
  qom-cast-debug  cast debugging support
+  qdev-deprecation-warning display qdev deprecation warnings
  tools           build qemu-io, qemu-nbd and qemu-img tools
  vxhs            Veritas HyperScale vDisk backend support
  bochs           bochs image format support
@@ -6966,6 +6970,7 @@ echo "gcov enabled      $gcov"
echo "TPM support       $tpm"
echo "libssh support    $libssh"
echo "QOM debugging     $qom_cast_debug"
+echo "QDEV deprecation warnings $qdev_deprecation_warning"
echo "Live block migration $live_block_migration"
echo "lzo support       $lzo"
echo "snappy support    $snappy"
@@ -7594,6 +7599,9 @@ fi
if test "$qom_cast_debug" = "yes" ; then
  echo "CONFIG_QOM_CAST_DEBUG=y" >> $config_host_mak
+if test "$qdev_deprecation_warning" = "yes" ; then
+  echo "CONFIG_QDEV_DEPRECATION_WARNING=y" >> $config_host_mak
if test "$rbd" = "yes" ; then
  echo "CONFIG_RBD=m" >> $config_host_mak
  echo "RBD_CFLAGS=$rbd_cflags" >> $config_host_mak
diff --git a/include/hw/qdev-deprecated.h b/include/hw/qdev-deprecated.h
new file mode 100644
index 0000000000..b815f62dae
--- /dev/null
+++ b/include/hw/qdev-deprecated.h
@@ -0,0 +1,26 @@
+ * QEMU QOM qdev deprecation helpers
+ *
+ * Copyright (c) 2020 Red Hat, Inc.
+ *
+ * Author:
+ *   Philippe Mathieu-Daudé <philmd@redhat.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+ * qdev_warn_deprecated_function_used:
+ *
+ * Display a warning that deprecated code is used.
+ */
+#define qdev_warn_deprecated_function_used() \
+    qdev_warn_deprecated_function(__func__)
+void qdev_warn_deprecated_function(const char *function);
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 2131c7f951..1134f46631 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -35,6 +35,7 @@
#include "hw/hotplug.h"
#include "hw/irq.h"
#include "hw/qdev-properties.h"
+#include "hw/qdev-deprecated.h"
#include "hw/boards.h"
#include "hw/sysbus.h"
#include "hw/qdev-clock.h"
@@ -838,6 +839,13 @@ void qdev_alias_all_properties(DeviceState *target, Object 
    } while (class != object_class_by_name(TYPE_DEVICE));

+void qdev_warn_deprecated_function(const char *function)
+    warn_report("use of deprecated non-qdev/non-qom code in %s()", function);
static bool device_get_realized(Object *obj, Error **errp)
    DeviceState *dev = DEVICE(obj);

I haven't noticed this series before so sorry that I only comment now. I was first thinking maybe you could use qdev_log_mask with some LOG_* value instead of #ifdef and warn_report so it can be enabled during runtime with some -d option but then I saw that this does not magically detect the sites that should be warned about but has to be added manually everywhere. In that case, it's probably easier to just add a FIXME comment to the places you'd call this function because that's where developers who could convert these will see it and probably more effective than having them to enable an option that they may not even know about and then search for where the warnings are coming from. So likely they'll just see the call in source for which a comment is also suffucient and then you can drop this patch but keep the rest and not throw away the work of identifying all the places that need to be modernised that you've done.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]