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.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER, 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 C4B41C433DB for ; Mon, 25 Jan 2021 08:01:47 +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 39B0122C9C for ; Mon, 25 Jan 2021 08:01:47 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 39B0122C9C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:38520 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1l3woj-0006iy-GT for qemu-devel@archiver.kernel.org; Mon, 25 Jan 2021 03:01:45 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:52182) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1l3wm3-0005yo-5H for qemu-devel@nongnu.org; Mon, 25 Jan 2021 02:59:02 -0500 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:50368) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1l3wm0-0004YD-0T for qemu-devel@nongnu.org; Mon, 25 Jan 2021 02:58:58 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1611561534; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=T+7WjiOmDR+iRM8h23QCGyeWUjgzrOVXBEkOoR1Jzpc=; b=e2/Z9AV7GsW90ff0/9Gt5EfPcOPMbxk8+PvdAEntTFFTolUS8EdfJm/Ov5pqGlXiuAugfH V8tl0vuF94+Mc9lfKQw2tras3FGAq1GXrJeN/BPwQ8kQ3FeSx9RyOUR6pTZ4fJLc4F2E9a FZEiFpQBPu2kULKY7NrJTS9kLtrajYQ= Received: from mail-pg1-f198.google.com (mail-pg1-f198.google.com [209.85.215.198]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-157-faHdP4_TPLGiUeNrw78dSA-1; Mon, 25 Jan 2021 02:58:51 -0500 X-MC-Unique: faHdP4_TPLGiUeNrw78dSA-1 Received: by mail-pg1-f198.google.com with SMTP id 18so7508942pgp.22 for ; Sun, 24 Jan 2021 23:58:51 -0800 (PST) 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; bh=T+7WjiOmDR+iRM8h23QCGyeWUjgzrOVXBEkOoR1Jzpc=; b=OzM/7PHigtZnMzJq5wc9MK1g7QojzjPm3PGt240QOQYkgio0bG32bZmcxQC2dLM840 kI0c0IdllzyfvPRPsAGpA8D9CLKlrOl2OWqzNx5JkDYeusdXnslbQGoVIXZRXsg9MQtf 8AEnRrzj5+dyTfX4wffOOKqTTsLgSkKRHybXChCoYRnRfUCw5qat9BBN6PUna4Sc+ysY U7LjQzEtwQT8P4Nn7XvYOG68cwE/W1QBYqh1KNQdUQRClP9ZzOJ+Ksv1+50+nM3nfO71 /GBFQJ0jiUYIljeCkdxj/rxLdReuAubd3OGnA91BTnO7T1tP2CMFf45dEMXkDIhcCrEZ rkOg== X-Gm-Message-State: AOAM531bwNF5HZlfp41VO0xsG66nwLLQCaBuUpV3+EKo1kzlGQT3+vMq KljkLpNtH1QVcrKM6j9iRfaweeAshhAAWX1krxYbKstE48uTLwrY63equPHS3XWVR7GUb3J3g0z e4eK1OOQFf3Qct+bfEQ564QjCvJJRGbM= X-Received: by 2002:a63:cb06:: with SMTP id p6mr33472pgg.146.1611561530330; Sun, 24 Jan 2021 23:58:50 -0800 (PST) X-Google-Smtp-Source: ABdhPJwS87XsqtPxf2gJbrD4hqTwqrP45JJpuDQVQ/0GYM6o12IEj8GnfQKy/V8E3YmrA06bLvwkNi6kDc6ABBjKzDY= X-Received: by 2002:a63:cb06:: with SMTP id p6mr33457pgg.146.1611561530091; Sun, 24 Jan 2021 23:58:50 -0800 (PST) MIME-Version: 1.0 References: <20210123143128.1167797-1-pbonzini@redhat.com> <20210123143128.1167797-30-pbonzini@redhat.com> <878s8hqx7u.fsf@dusky.pond.sub.org> In-Reply-To: <878s8hqx7u.fsf@dusky.pond.sub.org> From: Paolo Bonzini Date: Mon, 25 Jan 2021 08:58:37 +0100 Message-ID: Subject: Re: [PULL 29/31] qemu-option: clean up id vs. list->merge_lists To: Markus Armbruster Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=pbonzini@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/alternative; boundary="00000000000088ff6505b9b4e855" Received-SPF: pass client-ip=216.205.24.124; envelope-from=pbonzini@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -30 X-Spam_score: -3.1 X-Spam_bar: --- X-Spam_report: (-3.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.25, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, 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: Kevin Wolf , qemu-devel Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --00000000000088ff6505b9b4e855 Content-Type: text/plain; charset="UTF-8" Too late but I will point it out in the commit that cleans up the iteration. Paolo Il lun 25 gen 2021, 08:42 Markus Armbruster ha scritto: > Paolo Bonzini writes: > > > Looking at all merge-lists QemuOptsList, here is how they access their > > QemuOpts: > > > > reopen_opts in qemu-io-cmds.c ("qemu-img reopen -o") > > qemu_opts_find(&reopen_opts, NULL) > > > > empty_opts in qemu-io.c ("qemu-io open -o") > > qemu_opts_find(&empty_opts, NULL) > > > > qemu_rtc_opts ("-rtc") > > qemu_find_opts_singleton("rtc") > > > > qemu_machine_opts ("-M") > > qemu_find_opts_singleton("machine") > > > > qemu_action_opts ("-name") > > Pasto: it's "-action". > > > qemu_opts_foreach->process_runstate_actions > > > > qemu_boot_opts ("-boot") > > in hw/nvram/fw_cfg.c and hw/s390x/ipl.c: > > QTAILQ_FIRST(&qemu_find_opts("bootopts")->head) > > in softmmu/vl.c: > > qemu_opts_find(qemu_find_opts("boot-opts"), NULL) > > > > qemu_name_opts ("-name") > > qemu_opts_foreach->parse_name > > parse_name does not use id > > > > qemu_mem_opts ("-m") > > qemu_find_opts_singleton("memory") > > > > qemu_icount_opts ("-icount") > > qemu_opts_foreach->do_configure_icount > > do_configure_icount->icount_configure > > icount_configure does not use id > > > > qemu_smp_opts ("-smp") > > qemu_opts_find(qemu_find_opts("smp-opts"), NULL) > > > > qemu_spice_opts ("-spice") > > QTAILQ_FIRST(&qemu_spice_opts.head) > > > > i.e. they don't need an id. Sometimes its presence is ignored > > (e.g. when using qemu_opts_foreach), sometimes all the options > > with the id are skipped, sometimes only the first option on the > > Let's insert > > (when using qemu_find_opts_singleton() or qemu_opts_find(list, NULL)) > > right after skipped, and > > > command line is considered. -boot does two different things > > (when using QTAILQ_FIRST) > > right after considered. > > > depending on who's looking at the options. > > > > With this patch we just forbid id on merge-lists QemuOptsLists; if the > > command line still works, it has the same semantics as before. > > > > qemu_opts_create's fail_if_exists parameter is now unnecessary: > > > > - it is unused if id is NULL > > > > - opts_parse only passes false if reached from qemu_opts_set_defaults, > > in which case this patch enforces that id must be NULL > > > > - other callers that can pass a non-NULL id always set it to true > > > > Assert that it is true in the only case where "fail_if_exists" matters, > > i.e. "id && !lists->merge_lists". This means that if an id is present, > > duplicates are always forbidden, which was already the status quo. > > > > Discounting the case that aborts as it's not user-controlled (it's > > "just" a matter of inspecting qemu_opts_create callers), the paths > > through qemu_opts_create can be summarized as: > > > > - merge_lists = true: singleton opts with NULL id; non-NULL id fails > > > > - merge_lists = false: always return new opts; non-NULL id fails if dup > > > > Reviewed-by: Kevin Wolf > > Signed-off-by: Paolo Bonzini > > Reviewed-by: Markus Armbruster > > --00000000000088ff6505b9b4e855 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Too late but I will point it out in the= commit that cleans up the iteration.

Paol= o

Il lun 25 gen 2021, 08:42 Markus Armbruster <<= a href=3D"mailto:armbru@redhat.com" target=3D"_blank" rel=3D"noreferrer">ar= mbru@redhat.com> ha scritto:
Paolo Bonzini <pbonzini@redhat.com> writes:

> Looking at all merge-lists QemuOptsList, here is how they access their=
> QemuOpts:
>
> reopen_opts in qemu-io-cmds.c ("qemu-img reopen -o")
>=C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_opts_find(&reopen_opts, NULL)
>
> empty_opts in qemu-io.c ("qemu-io open -o")
>=C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_opts_find(&empty_opts, NULL)
>
> qemu_rtc_opts ("-rtc")
>=C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_find_opts_singleton("rtc") >
> qemu_machine_opts ("-M")
>=C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_find_opts_singleton("machine"= )
>
> qemu_action_opts ("-name")

Pasto: it's "-action".

>=C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_opts_foreach->process_runstate_actio= ns
>
> qemu_boot_opts ("-boot")
>=C2=A0 =C2=A0 =C2=A0 =C2=A0in hw/nvram/fw_cfg.c and hw/s390x/ipl.c:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0QTAILQ_FIRST(&qemu_find_opts(&quo= t;bootopts")->head)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0in softmmu/vl.c:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_opts_find(qemu_find_opts("b= oot-opts"), NULL)
>
> qemu_name_opts ("-name")
>=C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_opts_foreach->parse_name
>=C2=A0 =C2=A0 =C2=A0 =C2=A0parse_name does not use id
>
> qemu_mem_opts ("-m")
>=C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_find_opts_singleton("memory")=
>
> qemu_icount_opts ("-icount")
>=C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_opts_foreach->do_configure_icount >=C2=A0 =C2=A0 =C2=A0 =C2=A0do_configure_icount->icount_configure
>=C2=A0 =C2=A0 =C2=A0 =C2=A0icount_configure does not use id
>
> qemu_smp_opts ("-smp")
>=C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_opts_find(qemu_find_opts("smp-opts= "), NULL)
>
> qemu_spice_opts ("-spice")
>=C2=A0 =C2=A0 =C2=A0 =C2=A0QTAILQ_FIRST(&qemu_spice_opts.head)
>
> i.e. they don't need an id.=C2=A0 Sometimes its presence is ignore= d
> (e.g. when using qemu_opts_foreach), sometimes all the options
> with the id are skipped, sometimes only the first option on the

Let's insert

=C2=A0 =C2=A0 (when using qemu_find_opts_singleton() or qemu_opts_find(list= , NULL))

right after skipped, and

> command line is considered.=C2=A0 -boot does two different things

=C2=A0 =C2=A0 (when using QTAILQ_FIRST)

right after considered.

> depending on who's looking at the options.
>
> With this patch we just forbid id on merge-lists QemuOptsLists; if the=
> command line still works, it has the same semantics as before.
>
> qemu_opts_create's fail_if_exists parameter is now unnecessary: >
> - it is unused if id is NULL
>
> - opts_parse only passes false if reached from qemu_opts_set_defaults,=
> in which case this patch enforces that id must be NULL
>
> - other callers that can pass a non-NULL id always set it to true
>
> Assert that it is true in the only case where "fail_if_exists&quo= t; matters,
> i.e. "id && !lists->merge_lists".=C2=A0 This mean= s that if an id is present,
> duplicates are always forbidden, which was already the status quo.
>
> Discounting the case that aborts as it's not user-controlled (it&#= 39;s
> "just" a matter of inspecting qemu_opts_create callers), the= paths
> through qemu_opts_create can be summarized as:
>
> - merge_lists =3D true: singleton opts with NULL id; non-NULL id fails=
>
> - merge_lists =3D false: always return new opts; non-NULL id fails if = dup
>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com&g= t;

Reviewed-by: Markus Armbruster <armbru@redhat.com>

--00000000000088ff6505b9b4e855--