Re: [Qemu-devel] [PATCH 4/5] qdev: add qdev_{create, free} tracepoints

["Andreas Färber" <afaerber@suse.de>]

Am 22.03.2013 09:29, schrieb Kazuya Saito:
> This patch adds tracepoints at creating and removing virtual
> devices. It is useful for investigation of trouble related to virtual
> devices.
> 
> Signed-off-by: Kazuya Saito <saito.kazuya@jp.fujitsu.com>

I would prefer not to do this. I had previously posted a patch to remove
qdev_free() in favor of using the QOM function object_unparent()
directly, which adding stuff to qdev_free() would interfere with. And
you should rather add a tracepoint to object_new() or better to
object_initialize() than into the legacy qdev_create() - which doesn't
cover qdev_try_create() btw. Either way, adding new tracepoints with the
legacy "qdev" in the name is ugly.

Regards,
Andreas

P.S. Your patches arrived in HTML format, please check your workflow.

> ---
>  hw/qdev.c    |    3 +++
>  trace-events |    4 ++++
>  2 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 0b20280..0fda23e 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -30,6 +30,7 @@
>  #include "qapi/error.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qapi/visitor.h"
> +#include "trace.h"
> 
>  int qdev_hotplug = 0;
>  static bool qdev_hot_added = false;
> @@ -124,6 +125,7 @@ DeviceState *qdev_create(BusState *bus, const char *name)
>          }
>      }
> 
> +    trace_qdev_create(dev, dev->parent_bus);
>      return dev;
>  }
> 
> @@ -268,6 +270,7 @@ void qdev_init_nofail(DeviceState *dev)
>  /* Unlink device from bus and free the structure.  */
>  void qdev_free(DeviceState *dev)
>  {
> +    trace_qdev_free(dev, dev->parent_bus);
>      object_unparent(OBJECT(dev));
>  }
> 
> diff --git a/trace-events b/trace-events
> index c691ce4..235b978 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1102,3 +1102,7 @@ kvm_ioctl(int type) "type %d"
>  kvm_vm_ioctl(int type) "type %d"
>  kvm_vcpu_ioctl(int type) "type %d"
>  kvm_run_exit(uint32_t reason) "reason %d"
> +
> +# qdev.c
> +qdev_create(void *dev, void *bus) "dev %p, bus %p"
> +qdev_free(void *dev, void *bus) "dev %p, bus %p"
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg