From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41347) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bxPKX-0003qi-Vq for qemu-devel@nongnu.org; Thu, 20 Oct 2016 22:13:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bxPKU-0000tV-0I for qemu-devel@nongnu.org; Thu, 20 Oct 2016 22:13:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46046) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bxPKT-0000t8-Qp for qemu-devel@nongnu.org; Thu, 20 Oct 2016 22:13:05 -0400 References: <1475208369-13756-1-git-send-email-zhang.zhanghailiang@huawei.com> <57FB0759.2080401@huawei.com> <9bab6585-3de0-6f6b-ae4e-1e4191d40e47@cn.fujitsu.com> <221bf531-e488-6412-8c7b-12d54f9ff8be@redhat.com> <58084DD6.1030800@huawei.com> From: Jason Wang Message-ID: <08934068-c205-4406-e8a0-42598d1f8e0d@redhat.com> Date: Fri, 21 Oct 2016 10:13:00 +0800 MIME-Version: 1.0 In-Reply-To: <58084DD6.1030800@huawei.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] colo-compare: fix find_and_check_chardev() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hailiang Zhang , Zhang Chen , lizhijian@cn.fujitsu.com Cc: qemu-devel@nongnu.org On 2016=E5=B9=B410=E6=9C=8820=E6=97=A5 12:53, Hailiang Zhang wrote: > On 2016/10/20 11:53, Jason Wang wrote: >> >> >> On 2016=E5=B9=B410=E6=9C=8810=E6=97=A5 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 >>>>> >>>>> 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. Ok, I see. Thanks