Re: [Qemu-devel] [PATCH 2/5] qdev: Export qdev_hot_removed

[Markus Armbruster]

Cc: Paolo for additional qdev expertise.

Juan Quintela <address@hidden> writes:

> I need to move qdev_unplug to qdev-monitor in the following patch, and
> it needs access to this variable.
>
> Signed-off-by: Juan Quintela <address@hidden>
> ---
>  hw/core/qdev.c         | 2 +-
>  include/hw/qdev-core.h | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 6fa46b5..c26cf84 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -41,7 +41,7 @@
>  
>  bool qdev_hotplug = false;
>  static bool qdev_hot_added = false;
> -static bool qdev_hot_removed = false;
> +bool qdev_hot_removed = false;

Makes qdev_hot_added and qdev_hot_removed differently static, which is
weird.

The reason for this asymmetry is the asymmetry in how they get set:

* qdev_hot_added gets set .instance_init() method in device_initfn()
  when it runs after qdev_machine_creation_done().  It's called by
  qdev_device_add() via object_new()... then.

* qdev_hot_removed gets set directly in qdev_unplug(), not in the
  .instance_finalize() method device_finalize().

  Note that for some devices, qdev_unplug() only requests unplug.
  Actual unplug happens later, or even not at all.  device_finalize()
  runs on actual unplug.

Questions:

* Is setting qdev_hot_removed on unplug *requests* correct?  Or should
  it be set only on *actual* unplug?

* Can we avoid the asymmetry?  It's a bit ugly.

>  const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
>  {
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index a96a913..f09b6b7 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -387,6 +387,7 @@ Object *qdev_get_machine(void);
>  void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
>  
>  extern bool qdev_hotplug;
> +extern bool qdev_hot_removed;
>  
>  char *qdev_get_dev_path(DeviceState *dev);