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=-5.3 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 39FDFC4338F for ; Fri, 23 Jul 2021 11:21:33 +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 CD51A60ED4 for ; Fri, 23 Jul 2021 11:21:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org CD51A60ED4 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:35762 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m6tFE-0007o8-1t for qemu-devel@archiver.kernel.org; Fri, 23 Jul 2021 07:21:32 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:39896) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m6tEb-00075t-MM for qemu-devel@nongnu.org; Fri, 23 Jul 2021 07:20:53 -0400 Received: from smtp-out2.suse.de ([195.135.220.29]:52858) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1m6tEZ-0003Md-VI for qemu-devel@nongnu.org; Fri, 23 Jul 2021 07:20:53 -0400 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 3DB2F1FF91; Fri, 23 Jul 2021 11:20:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1627039250; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=7PlivnqMbsAq6j1vxiHNv+Td28TjnSv5M8xiI53MWfo=; b=Hvk7eqSvVC/5S0LtpEPCVe/aP/lABEDrkLZE8C3IZHkzLBPs9sL+752nh3lukmDdFGfSAL Qfg51WXE1pa8PkKHsy0xAUq3I9VpgBbmJaNRxNPVk7Wjhs+Ws5vz4vCbgtRc52fV5VDwFP WQfjk/ZMwZnm2axGbEIjhs3dfa634w0= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1627039250; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=7PlivnqMbsAq6j1vxiHNv+Td28TjnSv5M8xiI53MWfo=; b=pJQcQ9ZPFFpwIM9FkplcgX4otGNA4wgII5+TRVnhicIW5AIXviV/MsU6PJx47PISF5BkiG OuRzwcFAgesUDDCA== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id F267E13E02; Fri, 23 Jul 2021 11:20:49 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id IQAmORGm+mCTGgAAMHmgww (envelope-from ); Fri, 23 Jul 2021 11:20:49 +0000 Subject: Re: [PATCH v2 1/1] modules: Improve error message when module is not found To: Gerd Hoffmann References: <20210722220952.17444-1-jziviani@suse.de> <20210722220952.17444-2-jziviani@suse.de> <20210723095231.sfbkhdwdg43nedkk@sirius.home.kraxel.org> From: Claudio Fontana Message-ID: <4b5c010f-1365-e746-c269-9b9e48771f7b@suse.de> Date: Fri, 23 Jul 2021 13:20:49 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <20210723095231.sfbkhdwdg43nedkk@sirius.home.kraxel.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Received-SPF: pass client-ip=195.135.220.29; envelope-from=cfontana@suse.de; helo=smtp-out2.suse.de X-Spam_score_int: -45 X-Spam_score: -4.6 X-Spam_bar: ---- X-Spam_report: (-4.6 / 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, NICE_REPLY_A=-0.203, RCVD_IN_DNSWL_MED=-2.3, 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: pbonzini@redhat.com, =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , richard.henderson@linaro.org, qemu-devel@nongnu.org, "Jose R. Ziviani" Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On 7/23/21 11:52 AM, Gerd Hoffmann wrote: >>> - g_assert(ops != NULL); >>> + if (ops == NULL) { >>> + exit(1); >>> + } >>> + >> >> >> Ah, again, why? >> This change looks wrong to me, >> >> the ops code should be present when ops interfaces are initialized: >> it should be a code level assertion, as it has to do with the proper order of initializations in QEMU, >> >> why would we want to do anything else but to assert here? >> >> Am I blind to something obvious? > > Building tcg accel ops modular moves that from coding error to possible > user error (user wants use tcg but has qemu-accel-tcg-$arch.rpm not > installed). Sorry but without more background I don't buy it. If ops is null at the time accel_init_interfaces is called, it means that we are trying to initialize the board (for softmmu) with an accelerator already selected, and without an accelerator actually available. The problem has happened already a long time before we get here. When we check for viable accelerators, in configure_accelerators, we should check that the code is actually there, before choosing it as a viable accelerator. If we march on and start initializing the machine with an accelerator that is not available, of course things will start failing left and right. If things like: bool have_tcg = accel_find("tcg"); return true when the code is actually not there, there seems to be a larger issue to solve. I think we need to think more broadly about this. Thanks, Claudio > > The second part of the patch makes qemu print a message on the failed > module load, so the user would have a chance to figure where the assert > comes from, but replacing the assert with a more friendly message still > makes sense to me. > > take care, > Gerd >