Re: [PATCH V2 4/5] trace-cmd: Add virt-server mode for a virtualizationenvironment

[Yoshihiro YUNOMAE]

(2013/10/18 11:32), Steven Rostedt wrote:
> On Fri, 13 Sep 2013 11:06:37 +0900
> Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@xxxxxxxxxxx> wrote:
>
> >   static int *create_all_readers(int cpus, const char *node, const char *port,
> > -			       int pagesize, int fd)
> > +			       const char *domain, int virtpid, int pagesize, int fd)
> >   {
> >   	char buf[BUFSIZ];
> > -	int *port_array;
> > +	int *port_array = NULL;
> >   	int *pid_array;
> >   	int start_port;
> >   	int udp_port;
> >   	int cpu;
> >   	int pid;
> >
> > -	port_array = malloc_or_die(sizeof(int) * cpus);
> > +	if (node) {
> > +		port_array = malloc_or_die(sizeof(int) * cpus);
> > +		start_port = START_PORT_SEARCH;
> > +	}
> >   	pid_array = malloc_or_die(sizeof(int) * cpus);
> >   	memset(pid_array, 0, sizeof(int) * cpus);
> >
> > -	start_port = START_PORT_SEARCH;
> > -
> > -	/* Now create a UDP port for each CPU */
> > +	/* Now create a reader for each CPU */
> >   	for (cpu = 0; cpu < cpus; cpu++) {
> > -		udp_port = open_udp(node, port, &pid, cpu,
> > -				    pagesize, start_port);
> > -		if (udp_port < 0)
> > -			goto out_free;
> > -		port_array[cpu] = udp_port;
> > +		if (node) {
> > +			udp_port = open_udp(node, port, &pid, cpu,
> > +					    pagesize, start_port);
> > +			if (udp_port < 0)
> > +				goto out_free;
> > +			port_array[cpu] = udp_port;
> > +			/*
> > +			 * due to some bugging finding ports,
>
> s/due/Due/

Thanks.

> > +			 * force search after last port
> > +			 */
> > +			start_port = udp_port + 1;
> > +		} else {
> > +			if (open_virtio_serial_pipe(&pid, cpu, pagesize,
> > +						    domain, virtpid) < 0)
> > +				goto out_free;
> > +		}
> >   		pid_array[cpu] = pid;
> >   		/*
> >   		 * Due to some bugging finding ports,
>
> Hmm, it seems that you added the start_port = udp_port + 1 above, but
> shouldn't you remove the one here?

Oh, you're right.
I'll delete it here.

> > @@ -482,7 +595,7 @@ static int *create_all_readers(int cpus, const char *node, const char *port,
> >   	return pid_array;
> >
> >    out_free:
> > -	destroy_all_readers(cpus, pid_array, node, port);
> > +	destroy_all_readers(cpus, pid_array, node, port, domain, virtpid);
> >   	return NULL;
> >   }
> >
> > @@ -524,7 +637,7 @@ static void stop_all_readers(int cpus, int *pid_array)
> >   }
> >
> >   static void put_together_file(int cpus, int ofd, const char *node,
> > -			      const char *port)
> > +			      const char *port, const char *domain, int virtpid)
> >   {
> >   	char **temp_files;
> >   	int cpu;
> > @@ -533,25 +646,31 @@ static void put_together_file(int cpus, int ofd, const char *node,
> >   	temp_files = malloc_or_die(sizeof(*temp_files) * cpus);
> >
> >   	for (cpu = 0; cpu < cpus; cpu++)
> > -		temp_files[cpu] = get_temp_file(node, port, cpu);
> > +		temp_files[cpu] = get_temp_file(node, port, domain,
> > +						virtpid, cpu);
> >
> >   	tracecmd_attach_cpu_data_fd(ofd, cpus, temp_files);
> >   	free(temp_files);
> >   }
> >
> > -static void process_client(const char *node, const char *port, int fd)
> > +static void process_client(const char *node, const char *port,
> > +			   const char *domain, int virtpid, int fd)
> >   {
> >   	int *pid_array;
> >   	int pagesize;
> >   	int cpus;
> >   	int ofd;
> >
> > -	if (communicate_with_client(fd, &cpus, &pagesize) < 0)
> > -		return;
> > -
> > -	ofd = create_client_file(node, port);
> > +	if (node) {
> > +		if (communicate_with_client_nw(fd, &cpus, &pagesize) < 0)
>
> I take it _nw is for "network". If so, please use "*_net" instead. "nw"
> is pretty meaningless.
>
> This applies for all functions.

OK, I'll rename all functions using _nw.

Thanks,
Yoshihiro YUNOMAE

--
Yoshihiro YUNOMAE
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: yoshihiro.yunomae.ez@xxxxxxxxxxx


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/