[RFC,v2,02/23] hw/core/qdev: Add qdev_warn_deprecated_function_used() helper
diff mbox series

Message ID 20200704153908.12118-3-philmd@redhat.com
State New
Headers show
Series
  • hw/qdev: Warn when using pre-qdev/QOM devices
Related show

Commit Message

Philippe Mathieu-Daudé July 4, 2020, 3:38 p.m. UTC
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 pxa2xx_lcdc_init()
  qemu-system-arm: warning: use of deprecated non-qdev/non-qom code in pxa2xx_i2s_init()
  qemu-system-arm: warning: use of deprecated non-qdev/non-qom code in pxa27x_keypad_init()

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
I'd rather use --enable-qdev-debug suggested here:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg719802.html
---
 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

Comments

BALATON Zoltan July 4, 2020, 4:22 p.m. UTC | #1
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 pxa2xx_lcdc_init()
>  qemu-system-arm: warning: use of deprecated non-qdev/non-qom code in pxa2xx_i2s_init()
>  qemu-system-arm: warning: use of deprecated non-qdev/non-qom code in pxa27x_keypad_init()
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> I'd rather use --enable-qdev-debug suggested here:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg719802.html
> ---
> 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"
> pkgversion=""
> pie=""
> qom_cast_debug="yes"
> +qdev_deprecation_warning="no"
> trace_backends="log"
> trace_file="trace"
> spice=""
> @@ -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 available:
>   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
> fi
> +if test "$qdev_deprecation_warning" = "yes" ; then
> +  echo "CONFIG_QDEV_DEPRECATION_WARNING=y" >> $config_host_mak
> +fi
> 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.
> + */
> +#ifndef HW_QDEV_DEPRECATED_H
> +#define HW_QDEV_DEPRECATED_H
> +
> +/**
> + * 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);
> +
> +#endif
> 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 *source)
>     } while (class != object_class_by_name(TYPE_DEVICE));
> }
>
> +void qdev_warn_deprecated_function(const char *function)
> +{
> +#ifdef CONFIG_QDEV_DEPRECATION_WARNING
> +    warn_report("use of deprecated non-qdev/non-qom code in %s()", function);
> +#endif
> +}
> +
> 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.

Regards,
BALATON Zoltan

Patch
diff mbox series

diff --git a/configure b/configure
index 8a65240d4a..aac3dc0767 100755
--- a/configure
+++ b/configure
@@ -441,6 +441,7 @@  edk2_blobs="no"
 pkgversion=""
 pie=""
 qom_cast_debug="yes"
+qdev_deprecation_warning="no"
 trace_backends="log"
 trace_file="trace"
 spice=""
@@ -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 available:
   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
 fi
+if test "$qdev_deprecation_warning" = "yes" ; then
+  echo "CONFIG_QDEV_DEPRECATION_WARNING=y" >> $config_host_mak
+fi
 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.
+ */
+#ifndef HW_QDEV_DEPRECATED_H
+#define HW_QDEV_DEPRECATED_H
+
+/**
+ * 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);
+
+#endif
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 *source)
     } while (class != object_class_by_name(TYPE_DEVICE));
 }
 
+void qdev_warn_deprecated_function(const char *function)
+{
+#ifdef CONFIG_QDEV_DEPRECATION_WARNING
+    warn_report("use of deprecated non-qdev/non-qom code in %s()", function);
+#endif
+}
+
 static bool device_get_realized(Object *obj, Error **errp)
 {
     DeviceState *dev = DEVICE(obj);