From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.5 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A3A13C433ED for ; Mon, 19 Apr 2021 11:58:05 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 0D0736100C for ; Mon, 19 Apr 2021 11:58:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0D0736100C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:47086 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lYSXT-0007HF-Qq for qemu-devel@archiver.kernel.org; Mon, 19 Apr 2021 07:58:03 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:54342) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lYSWT-0006rG-24 for qemu-devel@nongnu.org; Mon, 19 Apr 2021 07:57:01 -0400 Received: from mail-ua1-x92c.google.com ([2607:f8b0:4864:20::92c]:43905) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lYSWO-000766-L8 for qemu-devel@nongnu.org; Mon, 19 Apr 2021 07:57:00 -0400 Received: by mail-ua1-x92c.google.com with SMTP id a8so7743024uan.10 for ; Mon, 19 Apr 2021 04:56:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=P9S+UiOYuHise9kwgQQz/tkgmRPwWQ4dCc9e8kgRWCs=; b=aAkrVGw+6RfUfdYT22NadkZbfE4BoXlTUTZjlapcXIy9LoJbn1pydR3A2Lm7FDsRk5 2IAwjzJnc14LISXWlwH9LyVLkiqFXG5dkzntSr4tsqQeqS9P1dyy/PUsn8BHp2zdTCOc tzmamsXjDkGFDmByJjWDKQr7lbqImrtPA5b29NmSayHRRLnYQ/IwzSxyuHkllpqFZ+dd XW77/FC/FYkzSq3l/JOQNLFf92oOaZociWfd/Q/zEJKEZHNQIyvRaJY7VvMwC5nG3Cn9 FktqeUHVxEyvZh2MCZUzPd0VKDq4dY3CCrIHthP1cicf9vAKh5E9Q9YqiAUERvLtmUBz H/1g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=P9S+UiOYuHise9kwgQQz/tkgmRPwWQ4dCc9e8kgRWCs=; b=Ggto4p3RZZL0CA4Bf2nGbwdlqJxHffTkp3/0DPlHVEHLmrkfsbqut0j2DxFEYqK0AU BeIFdyTu/dBvliER/4/7fGZ4k1vszRV2CGv2CsAgBdIaDkYgdGpJmFrogq6A02EzRAiV Se30hu/oof8Xm6TKXfia1buUhl+DKfzuFu7uPEp3Z4eCBmqZLePbDK8OKr7576aPqhs0 F/7tf7HiknSxdZ88Rox60Z5wTikVTs1vaArNvdqxptAix6aTmdFjBbepuefK2pbW+KEy nYiBIHo+cKLJYzk1vmnybsDUjY4hCfMHN7jjJC8AI1+vAAJokafRTihvjOgpsfG+U3is P/AA== X-Gm-Message-State: AOAM532SPqzNWNZXe0IPEpCuFwrz/MBA3F5HDdXiA/HOe9Spit7VU6+q M8fMADI4N8v/2toxpB5zFX47NjQUKP8uTvP8CLs= X-Google-Smtp-Source: ABdhPJxvitFaBxhtkccGC7eP4Wwwf45o1OjiAn6lRTXLf/B/hLC7NGWnBgG5VisuWH4t+l3fa/4sd6G8Qr7Vm9y4u0U= X-Received: by 2002:ab0:78d:: with SMTP id c13mr4886686uaf.99.1618833414760; Mon, 19 Apr 2021 04:56:54 -0700 (PDT) MIME-Version: 1.0 References: <20210413213459.629963-1-li.zhang@cloud.ionos.com> <875z0m4733.fsf@dusky.pond.sub.org> <87blad2v9x.fsf@dusky.pond.sub.org> In-Reply-To: <87blad2v9x.fsf@dusky.pond.sub.org> From: Li Zhang Date: Mon, 19 Apr 2021 13:56:43 +0200 Message-ID: Subject: Re: [PATCHv2 1/1] Support monitor chardev hotswap with QMP To: Markus Armbruster Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Received-SPF: pass client-ip=2607:f8b0:4864:20::92c; envelope-from=zhlcindy@gmail.com; helo=mail-ua1-x92c.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Li Zhang , =?UTF-8?B?TWFyYy1BbmRyw6kgTHVyZWF1?= , QEMU , Pankaj Gupta Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On Sat, Apr 17, 2021 at 10:02 AM Markus Armbruster wrot= e: > > Marc-Andr=C3=A9 Lureau writes: > > > Hi > > > > On Fri, Apr 16, 2021 at 6:59 PM Marc-Andr=C3=A9 Lureau < > > marcandre.lureau@gmail.com> wrote: > > > >> Hi > >> > >> On Fri, Apr 16, 2021 at 6:51 PM Markus Armbruster > >> wrote: > >> > >>> Marc-Andr=C3=A9, I'd like your opinion for this one, in particular th= e use of > >>> g_source_remove(). > >>> > >> > >> My opinion isn't really worth much, my review would have a bit more va= lue. > >> > >> GSource has indeed some peculiar lifetime management, that I got wrong= in > >> the past. So I would be extra careful. > >> > >> But before spending time on review, I would also clarify the motivatio= n > >> and ask for testing. > >> > >> Markus, hot-adding/removing monitors isn't supported? > >> > >> > > I realize you answered my question below. That's surprising me. Wouldn'= t it > > make more sense to support it rather than having a pre-opened null-base= d > > monitor that can have its chardev swapped? > > Yes, it would. Patches welcome. > > This patch is a somewhat ham-fisted and limited solution to the problem > stated in the commit message. However, it might *also* be a reasonable > improvement to chardev-change on its own. Not for me to judge. > Okay, Thanks. > chardev-change comes with a number of restrictions. Let's have a closer > look. It fails > > 1. when no such character device exists (d'oh) > > 2. for chardev-mux devices > > 3. in record/replay mode > > 4. when a backend is connected that doesn't implement the chr_be_change() > method > > 5. when chr_be_change() fails > > 6. when creating the new chardev fails[*] > > Items 2, 3, 4 are restrictions. I figure 2 and 4 are simply not > implemented, yet. I'm not sure about 3. > For item 3, source code mentions "chardev cannot be changed in record/replay mode". I never tried it yet. I am not quite sure why it couldn't be changed. > Whether we want to accept patches lifting restrictions is up to the > chardev maintainers. > > This patch lifts restriction 4 for QMP monitor backends. Its monitor > part looks acceptable to me, but I dislike its code duplication. Before > we spend time on cleaning that up (or on deciding to clean it up later), > I'd like to hear the chardev mantainers' judgement, because that's about > more serious matters than cleanliness. > > Do I make sense? Yes, make sense. Thanks. > > [...] > > > [*] The code for creating the new chardev in the "no backend connected" > case > > be =3D chr->be; > if (!be) { > /* easy case */ > object_unparent(OBJECT(chr)); > return qmp_chardev_add(id, backend, errp); > } > > is problematic: when qmp_chardev_add() fails, we already destroyed the > old chardev. It should destroy the old chardev only when it can create > its replacement. > You are right. It is a problem. --=20 Best Regards -Li