Patchwork [4/5] qdev: add qdev_{create,free} tracepoints

login
register
mail settings
Submitter Kazuya Saito
Date March 22, 2013, 8:29 a.m.
Message ID <514C1656.8020503@jp.fujitsu.com>
Download mbox | patch
Permalink /patch/229909/
State New
Headers show

Comments

Kazuya Saito - March 22, 2013, 8:29 a.m.
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>
---
 hw/qdev.c    |    3 +++
 trace-events |    4 ++++
 2 files changed, 7 insertions(+), 0 deletions(-)
Andreas Färber - March 27, 2013, 11:03 a.m.
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"
>
Kazuya Saito - March 28, 2013, 5:56 a.m.
(2013/03/27 20:03), Andreas Färber wrote:> 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.

Just as I replied to Paolo, I won't add these tracepoints. Thank you for
your good information.

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

I checked and modified the setting. Did this mail arrive in plain-text?

Kazuya

Patch

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"