Re: [Qemu-devel] [PATCH] colo-compare: fix find_and_check_chardev()

[Jason Wang]

On 2016年10月10日 11:49, Zhang Chen wrote:
> On 10/10/2016 11:13 AM, Hailiang Zhang wrote:
> > Hi,
> >
> > On 2016/10/10 10:52, Zhang Chen wrote:
> > > On 09/30/2016 12:06 PM, zhanghailiang wrote:
> > > > find_and_check_chardev() uses 'opts' member of CharDriverState to
> > > > check if the chardev is 'socket' chardev or not, which the opts
> > > > will be NULL if We add the chardev by qmp 'chardev-add' command.
> > > >
> > > > All the related info can be found in 'filename' member of 
> > > > CharDriverState,
> > > > For tcp socket device, it will be like 
> > > > 'disconnected:tcp:9.61.1.8:9004,server'
> > > > or 'tcp:9.61.1.8:9001,server <-> 9.61.1.8:50256', we can simply 
> > > > check it to
> > > > identify if it is a tcp socket char device.
> > > >
> > > > Besides, fix this helper function to return -1 while some errors 
> > > > happen.
> > > >
> > > > Signed-off-by: zhanghailiang <address@hidden>
> > >
> > > This patch looks fine to me.
> >
> > Sorry, I found there are still some problems with this modification,
> > For some local connection between filter objects, I think we can use 
> > unix socket
> > instead of tcp socket. (Or even other char device, for example file 
> > or pipe, but
> > Let's make things simple, we limit it to socket now)
> >
> > So the below check is insufficient, It should be
> >
> > +    if (!strstr((*chr)->filename, "tcp:") && 
> > !strstr((*chr)->filename, "unix:")) {
> >          error_setg(errp, "chardev \"%s\" is not a tcp socket, 
> > filename '%s'",
> >                     chr_name, (*chr)->filename);
> >
> > If you and Jason agree with this, i will send V2.
>
> I find part of codes in this patch has same with another patch:
>
> net: don't poke at chardev internal QemuOpts
>
> I think you can fix and rebase your patch,
> then we need jason's comments for this. 

I think we don't support unix domain socket?