Re: [Qemu-devel] [PATCH v3 17/18] trace: pass trace-events to tracetool as a positional param

2016-09-20 Thread Lluís Vilanova 
Daniel P Berrange writes:

> On Mon, Sep 19, 2016 at 08:18:51PM +0200, Lluís Vilanova wrote:
>> Daniel P Berrange writes:
>> 
>> > Instead of reading the contents of 'trace-events' from stdin,
>> > accept the filename as a positional parameter. This also
>> > allows for reading from multiple files, though this facility
>> > is not used at this time.
>> 
>> > Signed-off-by: Daniel P. Berrange 
>> > ---
>> >  Makefile.target  |  6 +++---
>> >  scripts/tracetool.py |  5 -
>> >  trace/Makefile.objs  | 18 +-
>> >  3 files changed, 16 insertions(+), 13 deletions(-)
>> 
>> > diff --git a/scripts/tracetool.py b/scripts/tracetool.py
>> > index 6accbbf..f66e767 100755
>> > --- a/scripts/tracetool.py
>> > +++ b/scripts/tracetool.py
>> > @@ -129,7 +129,10 @@ def main(args):
>> >  if probe_prefix is None:
>> >  probe_prefix = ".".join(["qemu", target_type, target_name])
>> 
>> > -events = tracetool.read_events(sys.stdin)
>> > +if len(args) != 1:
>> > +error_opt("missing trace-events filepath")
>> > +with open(args[0], "r") as fh:
>> > +events = tracetool.read_events(fh)
>> 
>> I'm not sure that's the proper way to check for a positional argument in 
>> getopt
>> (iff it accepts optional args mingled with positional ones).

> 'args' comes from the return value of getopt.getopt(sys.argv)
> which is documented as:

>   "The return value consists of two elements: the first is a
>list of (option, value) pairs; the second is the list of
>program arguments left after the option list was stripped
>(this is a trailing slice of args"

Oh, I'm sorry. I should've looked more closely into the code.


Cheers,
  Lluis

Re: [Qemu-devel] [PATCH v3 17/18] trace: pass trace-events to tracetool as a positional param

2016-09-20 Thread Daniel P. Berrange 
On Mon, Sep 19, 2016 at 08:18:51PM +0200, Lluís Vilanova wrote:
> Daniel P Berrange writes:
> 
> > Instead of reading the contents of 'trace-events' from stdin,
> > accept the filename as a positional parameter. This also
> > allows for reading from multiple files, though this facility
> > is not used at this time.
> 
> > Signed-off-by: Daniel P. Berrange 
> > ---
> >  Makefile.target  |  6 +++---
> >  scripts/tracetool.py |  5 -
> >  trace/Makefile.objs  | 18 +-
> >  3 files changed, 16 insertions(+), 13 deletions(-)
> 
> > diff --git a/scripts/tracetool.py b/scripts/tracetool.py
> > index 6accbbf..f66e767 100755
> > --- a/scripts/tracetool.py
> > +++ b/scripts/tracetool.py
> > @@ -129,7 +129,10 @@ def main(args):
> >  if probe_prefix is None:
> >  probe_prefix = ".".join(["qemu", target_type, target_name])
>  
> > -events = tracetool.read_events(sys.stdin)
> > +if len(args) != 1:
> > +error_opt("missing trace-events filepath")
> > +with open(args[0], "r") as fh:
> > +events = tracetool.read_events(fh)
>  
> I'm not sure that's the proper way to check for a positional argument in 
> getopt
> (iff it accepts optional args mingled with positional ones).

'args' comes from the return value of getopt.getopt(sys.argv)
which is documented as:

  "The return value consists of two elements: the first is a
   list of (option, value) pairs; the second is the list of
   program arguments left after the option list was stripped
   (this is a trailing slice of args"


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH v3 17/18] trace: pass trace-events to tracetool as a positional param

2016-09-19 Thread Lluís Vilanova 
Daniel P Berrange writes:

> Instead of reading the contents of 'trace-events' from stdin,
> accept the filename as a positional parameter. This also
> allows for reading from multiple files, though this facility
> is not used at this time.

> Signed-off-by: Daniel P. Berrange 
> ---
>  Makefile.target  |  6 +++---
>  scripts/tracetool.py |  5 -
>  trace/Makefile.objs  | 18 +-
>  3 files changed, 16 insertions(+), 13 deletions(-)

> diff --git a/Makefile.target b/Makefile.target
> index 8c7a072..f227408 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -55,7 +55,7 @@ $(QEMU_PROG).stp-installed: $(BUILD_DIR)/trace-events-all
>   --binary=$(bindir)/$(QEMU_PROG) \
>   --target-name=$(TARGET_NAME) \
>   --target-type=$(TARGET_TYPE) \
> - < $< > $@,"  GEN   $(TARGET_DIR)$(QEMU_PROG).stp-installed")
> + $< > $@,"  GEN   $(TARGET_DIR)$(QEMU_PROG).stp-installed")
 
>  $(QEMU_PROG).stp: $(BUILD_DIR)/trace-events-all
>   $(call quiet-command,$(TRACETOOL) \
> @@ -64,14 +64,14 @@ $(QEMU_PROG).stp: $(BUILD_DIR)/trace-events-all
>   --binary=$(realpath .)/$(QEMU_PROG) \
>   --target-name=$(TARGET_NAME) \
>   --target-type=$(TARGET_TYPE) \
> - < $< > $@,"  GEN   $(TARGET_DIR)$(QEMU_PROG).stp")
> + $< > $@,"  GEN   $(TARGET_DIR)$(QEMU_PROG).stp")
 
>  $(QEMU_PROG)-simpletrace.stp: $(BUILD_DIR)/trace-events-all
>   $(call quiet-command,$(TRACETOOL) \
>   --format=simpletrace-stap \
>   --backends=$(TRACE_BACKENDS) \
>   --probe-prefix=qemu.$(TARGET_TYPE).$(TARGET_NAME) \
> - < $< > $@,"  GEN   $(TARGET_DIR)$(QEMU_PROG)-simpletrace.stp")
> + $< > $@,"  GEN   $(TARGET_DIR)$(QEMU_PROG)-simpletrace.stp")
 
>  else
>  stap:
> diff --git a/scripts/tracetool.py b/scripts/tracetool.py
> index 6accbbf..f66e767 100755
> --- a/scripts/tracetool.py
> +++ b/scripts/tracetool.py
> @@ -129,7 +129,10 @@ def main(args):
>  if probe_prefix is None:
>  probe_prefix = ".".join(["qemu", target_type, target_name])
 
> -events = tracetool.read_events(sys.stdin)
> +if len(args) != 1:
> +error_opt("missing trace-events filepath")
> +with open(args[0], "r") as fh:
> +events = tracetool.read_events(fh)
 
I'm not sure that's the proper way to check for a positional argument in getopt
(iff it accepts optional args mingled with positional ones).


>  try:
>  tracetool.generate(events, arg_format, arg_backends,
> diff --git a/trace/Makefile.objs b/trace/Makefile.objs
> index 83f754e..524c4c9 100644
> --- a/trace/Makefile.objs
> +++ b/trace/Makefile.objs
> @@ -22,7 +22,7 @@ $(obj)/generated-ust-provider.h-timestamp: 
> $(BUILD_DIR)/trace-events-all $(trace
>   $(call quiet-command,$(TRACETOOL) \
>   --format=ust-events-h \
>   --backends=$(TRACE_BACKENDS) \
> - < $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
> + $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
 
>  $(obj)/generated-ust.c: $(obj)/generated-ust.c-timestamp 
> $(BUILD_DIR)/config-host.mak
>   @cmp $< $@ >/dev/null 2>&1 || cp $< $@
> @@ -30,7 +30,7 @@ $(obj)/generated-ust.c-timestamp: 
> $(BUILD_DIR)/trace-events-all $(tracetool-y)
>   $(call quiet-command,$(TRACETOOL) \
>   --format=ust-events-c \
>   --backends=$(TRACE_BACKENDS) \
> - < $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
> + $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
 
>  $(obj)/generated-tracers.h: $(obj)/generated-ust-provider.h
>  $(obj)/generated-tracers.c: $(obj)/generated-ust.c
> @@ -50,7 +50,7 @@ $(obj)/generated-tracers.h-timestamp: 
> $(BUILD_DIR)/trace-events-all $(BUILD_DIR)
>   $(call quiet-command,$(TRACETOOL) \
>   --format=h \
>   --backends=$(TRACE_BACKENDS) \
> - < $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
> + $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
 
>  ##
>  # non-DTrace
> @@ -61,7 +61,7 @@ $(obj)/generated-tracers.c-timestamp: 
> $(BUILD_DIR)/trace-events-all $(BUILD_DIR)
>   $(call quiet-command,$(TRACETOOL) \
>   --format=c \
>   --backends=$(TRACE_BACKENDS) \
> - < $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
> + $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
 
>  $(obj)/generated-tracers.o: $(obj)/generated-tracers.c 
> $(obj)/generated-tracers.h
 
> @@ -79,7 +79,7 @@ $(obj)/generated-tracers-dtrace.dtrace-timestamp: 
> $(BUILD_DIR)/trace-events-all
>   $(call quiet-command,$(TRACETOOL) \
>   --format=d \
>   --backends=$(TRACE_BACKENDS) \
> - < $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
> + $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
 
>  $(obj)/generated-tracers-dtrace.h:

[Qemu-devel] [PATCH v3 17/18] trace: pass trace-events to tracetool as a positional param

2016-09-19 Thread Daniel P. Berrange 
Instead of reading the contents of 'trace-events' from stdin,
accept the filename as a positional parameter. This also
allows for reading from multiple files, though this facility
is not used at this time.

Signed-off-by: Daniel P. Berrange 
---
 Makefile.target  |  6 +++---
 scripts/tracetool.py |  5 -
 trace/Makefile.objs  | 18 +-
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 8c7a072..f227408 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -55,7 +55,7 @@ $(QEMU_PROG).stp-installed: $(BUILD_DIR)/trace-events-all
--binary=$(bindir)/$(QEMU_PROG) \
--target-name=$(TARGET_NAME) \
--target-type=$(TARGET_TYPE) \
-   < $< > $@,"  GEN   $(TARGET_DIR)$(QEMU_PROG).stp-installed")
+   $< > $@,"  GEN   $(TARGET_DIR)$(QEMU_PROG).stp-installed")
 
 $(QEMU_PROG).stp: $(BUILD_DIR)/trace-events-all
$(call quiet-command,$(TRACETOOL) \
@@ -64,14 +64,14 @@ $(QEMU_PROG).stp: $(BUILD_DIR)/trace-events-all
--binary=$(realpath .)/$(QEMU_PROG) \
--target-name=$(TARGET_NAME) \
--target-type=$(TARGET_TYPE) \
-   < $< > $@,"  GEN   $(TARGET_DIR)$(QEMU_PROG).stp")
+   $< > $@,"  GEN   $(TARGET_DIR)$(QEMU_PROG).stp")
 
 $(QEMU_PROG)-simpletrace.stp: $(BUILD_DIR)/trace-events-all
$(call quiet-command,$(TRACETOOL) \
--format=simpletrace-stap \
--backends=$(TRACE_BACKENDS) \
--probe-prefix=qemu.$(TARGET_TYPE).$(TARGET_NAME) \
-   < $< > $@,"  GEN   $(TARGET_DIR)$(QEMU_PROG)-simpletrace.stp")
+   $< > $@,"  GEN   $(TARGET_DIR)$(QEMU_PROG)-simpletrace.stp")
 
 else
 stap:
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index 6accbbf..f66e767 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -129,7 +129,10 @@ def main(args):
 if probe_prefix is None:
 probe_prefix = ".".join(["qemu", target_type, target_name])
 
-events = tracetool.read_events(sys.stdin)
+if len(args) != 1:
+error_opt("missing trace-events filepath")
+with open(args[0], "r") as fh:
+events = tracetool.read_events(fh)
 
 try:
 tracetool.generate(events, arg_format, arg_backends,
diff --git a/trace/Makefile.objs b/trace/Makefile.objs
index 83f754e..524c4c9 100644
--- a/trace/Makefile.objs
+++ b/trace/Makefile.objs
@@ -22,7 +22,7 @@ $(obj)/generated-ust-provider.h-timestamp: 
$(BUILD_DIR)/trace-events-all $(trace
$(call quiet-command,$(TRACETOOL) \
--format=ust-events-h \
--backends=$(TRACE_BACKENDS) \
-   < $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
+   $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
 
 $(obj)/generated-ust.c: $(obj)/generated-ust.c-timestamp 
$(BUILD_DIR)/config-host.mak
@cmp $< $@ >/dev/null 2>&1 || cp $< $@
@@ -30,7 +30,7 @@ $(obj)/generated-ust.c-timestamp: 
$(BUILD_DIR)/trace-events-all $(tracetool-y)
$(call quiet-command,$(TRACETOOL) \
--format=ust-events-c \
--backends=$(TRACE_BACKENDS) \
-   < $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
+   $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
 
 $(obj)/generated-tracers.h: $(obj)/generated-ust-provider.h
 $(obj)/generated-tracers.c: $(obj)/generated-ust.c
@@ -50,7 +50,7 @@ $(obj)/generated-tracers.h-timestamp: 
$(BUILD_DIR)/trace-events-all $(BUILD_DIR)
$(call quiet-command,$(TRACETOOL) \
--format=h \
--backends=$(TRACE_BACKENDS) \
-   < $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
+   $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
 
 ##
 # non-DTrace
@@ -61,7 +61,7 @@ $(obj)/generated-tracers.c-timestamp: 
$(BUILD_DIR)/trace-events-all $(BUILD_DIR)
$(call quiet-command,$(TRACETOOL) \
--format=c \
--backends=$(TRACE_BACKENDS) \
-   < $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
+   $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
 
 $(obj)/generated-tracers.o: $(obj)/generated-tracers.c 
$(obj)/generated-tracers.h
 
@@ -79,7 +79,7 @@ $(obj)/generated-tracers-dtrace.dtrace-timestamp: 
$(BUILD_DIR)/trace-events-all
$(call quiet-command,$(TRACETOOL) \
--format=d \
--backends=$(TRACE_BACKENDS) \
-   < $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
+   $< > $@,"  GEN   $(patsubst %-timestamp,%,$@)")
 
 $(obj)/generated-tracers-dtrace.h: $(obj)/generated-tracers-dtrace.dtrace
$(call quiet-command,dtrace -o $@ -h -s $<, "  GEN   $@")
@@ -98,7 +98,7 @@ $(obj)/generated-helpers-wrappers.h-timestamp: 
$(BUILD_DIR)/trace-events-all $(B
$(call quiet-command,$(TRACETOOL) \
--format=tcg-helper-wrapper-h \