Re: [PATCH 7/7] perf inject: Handle output file via perf_data_file object

[Jiri Olsa <jolsa@redhat.com>]

On Thu, Nov 28, 2013 at 12:14:35PM -0200, Arnaldo Carvalho de Melo wrote:
> Em Thu, Nov 28, 2013 at 11:30:19AM +0100, Jiri Olsa escreveu:
> > Using the perf_data_file object to handle output
> > file processing.
> > 
> > No functional change intended.
> 
> There is one, before we were using:
> 
> -		inject.output = open(output_name, O_CREAT | O_WRONLY | O_TRUNC,
> -						  S_IRUSR | S_IWUSR);
> 
> And 
> 
> +	if (perf_data_file__open(&inject.output)) {
> 
> Does:
> 
>           return open(file->path, O_CREAT|O_RDWR|O_TRUNC, S_IRUSR|S_IWUSR);
> 
> Why do we need to do O_RDWR there? Can we live with plain O_WRONLY as
> before?

I think the reason is the record's buildid's hunt
during the exit.. we read the file we just stored
without reopening it for reading

hum, how about the output file mmapping David is working on,
that might need file to be readable as well..

> 
> Also, while reviewing this I got something that I missed in previous
> patches, I think we could reuse:
> 
>   O_WRONLY to replace the long constant PERF_DATA_MODE_WRITE
>   O_RDONLY ditto -> PERF_DATA_MODE_READ
> 
> Shorter, and matches what we need here: a open flag value to specify
> which access mode the perf.data file is operating, no?

I guess that would work.. but I planned for future more
perf specific flags like PERF_DATA_MODE_DIR or something
like that.. I haven't this fully thought through yet..

jirka