From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36568) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bx5N5-00084N-HF for qemu-devel@nongnu.org; Thu, 20 Oct 2016 00:54:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bx5N0-00007g-QJ for qemu-devel@nongnu.org; Thu, 20 Oct 2016 00:54:27 -0400 Received: from szxga01-in.huawei.com ([58.251.152.64]:40989) by eggs.gnu.org with esmtps (TLS1.0:RSA_ARCFOUR_SHA1:16) (Exim 4.71) (envelope-from ) id 1bx5N0-000070-43 for qemu-devel@nongnu.org; Thu, 20 Oct 2016 00:54:22 -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> From: Hailiang Zhang Message-ID: <58084DD6.1030800@huawei.com> Date: Thu, 20 Oct 2016 12:53:42 +0800 MIME-Version: 1.0 In-Reply-To: <221bf531-e488-6412-8c7b-12d54f9ff8be@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit 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: Jason Wang , Zhang Chen , lizhijian@cn.fujitsu.com Cc: qemu-devel@nongnu.org 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 >>>> >>>> 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. > . >