All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Kevin Wolf <kwolf@redhat.com>,
	 qemu-devel@nongnu.org,  Hanna Reitz <hreitz@redhat.com>,
	 Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>,
	Michael Tokarev <mjt@tls.msk.ru>
Subject: Re: [PULL 14/15] qdev: Base object creation on QDict rather than QemuOpts
Date: Tue, 05 Jul 2022 11:57:08 +0200	[thread overview]
Message-ID: <87a69nubkb.fsf@pond.sub.org> (raw)
In-Reply-To: <87wnctzdl9.fsf@pond.sub.org> (Markus Armbruster's message of "Mon, 04 Jul 2022 06:49:54 +0200")

Markus Armbruster <armbru@redhat.com> writes:

> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> On Fri, 15 Oct 2021 at 16:01, Kevin Wolf <kwolf@redhat.com> wrote:
>>> QDicts are both what QMP natively uses and what the keyval parser
>>> produces. Going through QemuOpts isn't useful for either one, so switch
>>> the main device creation function to QDicts. By sharing more code with
>>> the -object/object-add code path, we can even reduce the code size a
>>> bit.
>>>
>>> This commit doesn't remove the detour through QemuOpts from any code
>>> path yet, but it allows the following commits to do so.
>>>
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>> Message-Id: <20211008133442.141332-15-kwolf@redhat.com>
>>> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>>> Tested-by: Peter Krempa <pkrempa@redhat.com>
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>
>> Hi; we discovered via a report on IRC this this commit broke
>> handling of "array properties", of which one example is:
>> qemu-system-x86_64 -netdev user,id=a -device rocker,len-ports=1,ports[0]=a
>>
>> This used to work, and now fails with
>>  qemu-system-x86_64: -device rocker,len-ports=1,ports[0]=a: Property
>> 'rocker.ports[0]' not found
>>
>> I think this happens because array properties have the
>> requirement that the len-foo property is set first before
>> any of the foo[n] properties can be set. In the old code
>> I guess we used to set properties from the command line
>> in the order they were specified, whereas in the new code
>> we end up in object_set_properties_from_qdict() which
>> tries to set them in whatever order the qdict hash table
>> provides them, which turns out to be the wrong one :-(
>>
>> Any suggestions for how to address this ?
>
> My initial (knee-jerk) reaction to breaking array properties: Faster,
> Pussycat! Kill! Kill!
>
> Back to serious: replace the implementation of QDict so it iterates in
> order?

I just sent

    [RFC PATCH] qobject: Rewrite implementation of QDict for in-order traversal
    Message-Id: <20220705095421.2455041-1-armbru@redhat.com>

Please test whether this fixes the regressions you observed.



  reply	other threads:[~2022-07-05  9:58 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-15 14:46 [PULL 00/15] qdev: Add JSON -device Kevin Wolf
2021-10-15 14:46 ` [PULL 01/15] net: Introduce NetClientInfo.check_peer_type() Kevin Wolf
2021-10-15 14:46 ` [PULL 02/15] net/vhost-user: Fix device compatibility check Kevin Wolf
2021-10-15 14:46 ` [PULL 03/15] net/vhost-vdpa: " Kevin Wolf
2021-10-15 14:46 ` [PULL 04/15] qom: Reduce use of error_propagate() Kevin Wolf
2021-10-15 14:46 ` [PULL 05/15] iotests/245: Fix type for iothread property Kevin Wolf
2021-10-15 14:46 ` [PULL 06/15] iotests/051: Fix typo Kevin Wolf
2021-10-15 14:46 ` [PULL 07/15] qdev: Avoid using string visitor for properties Kevin Wolf
2021-10-15 14:46 ` [PULL 08/15] qdev: Make DeviceState.id independent of QemuOpts Kevin Wolf
2021-10-15 14:46 ` [PULL 09/15] softmmu/qdev-monitor: add error handling in qdev_set_id Kevin Wolf
2021-10-15 14:46 ` [PULL 10/15] qemu-option: Allow deleting opts during qemu_opts_foreach() Kevin Wolf
2021-10-15 14:46 ` [PULL 11/15] qdev: Add Error parameter to hide_device() callbacks Kevin Wolf
2021-10-15 14:46 ` [PULL 12/15] virtio-net: Store failover primary opts pointer locally Kevin Wolf
2021-10-15 14:46 ` [PULL 13/15] virtio-net: Avoid QemuOpts in failover_find_primary_device() Kevin Wolf
2021-10-15 14:46 ` [PULL 14/15] qdev: Base object creation on QDict rather than QemuOpts Kevin Wolf
2022-07-01 13:37   ` Peter Maydell
2022-07-04  4:49     ` Markus Armbruster
2022-07-05  9:57       ` Markus Armbruster [this message]
2022-07-07 20:24       ` Peter Maydell
2022-07-08 11:40         ` The case for array properties (was: [PULL 14/15] qdev: Base object creation on QDict rather than QemuOpts) Markus Armbruster
2022-07-08 11:50           ` Daniel P. Berrangé
2022-07-08 12:41             ` The case for array properties Markus Armbruster
2022-07-11 10:48           ` The case for array properties (was: [PULL 14/15] qdev: Base object creation on QDict rather than QemuOpts) Peter Maydell
2022-07-27 19:59         ` [PULL 14/15] qdev: Base object creation on QDict rather than QemuOpts Kevin Wolf
2021-10-15 14:46 ` [PULL 15/15] vl: Enable JSON syntax for -device Kevin Wolf
2021-10-15 20:26 ` [PULL 00/15] qdev: Add JSON -device Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a69nubkb.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=mjt@tls.msk.ru \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.