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

[Hailiang Zhang <zhang.zhanghailiang@huawei.com>]

On 2016/10/20 11:53, Jason Wang wrote:
>
>
> 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 <zhang.zhanghailiang@huawei.com>
>>>>
>>>> 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?
>

No, we support it, actually, by using unix socket,
we can reduce the number of ports used in COLO process.

> .
>