On Mon, Feb 19, 2024 at 11:32 PM Markus Armbruster <arm...@redhat.com> wrote:
> Yong Huang <yong.hu...@smartx.com> writes: > > > On Tue, Feb 13, 2024 at 6:26 PM Michael S. Tsirkin <m...@redhat.com> > wrote: > > > >> On Fri, Feb 02, 2024 at 10:32:15PM +0800, Hyman Huang wrote: > >> > x-query-virtio-status returns several sets of virtio feature and > >> > status flags. It goes back to v7.2.0. > >> > > >> > In the initial commit 90c066cd682 (qmp: add QMP command > >> > x-query-virtio-status), we returned them as numbers, using virtio's > >> > well-known binary encoding. > >> > > >> > The next commit f3034ad71fc (qmp: decode feature & status bits in > >> > virtio-status) replaced the numbers by objects. The objects represent > >> > bits QEMU knows symbolically, and any unknown bits numerically just > like > >> > before. > >> > > >> > Commit 8a8287981d1 (hmp: add virtio commands) the matching HMP command > >> > "info virtio" (and a few more, which aren't relevant here). > >> > > >> > The symbolic representation uses lists of strings. The string format > is > >> > undocumented. The strings look like "WELL_KNOWN_SYMBOL: human > readable > >> > explanation". > >> > > >> > This symbolic representation is nice for humans. Machines it can save > >> > the trouble of decoding virtio's well-known binary encoding. > >> > > >> > However, we sometimes want to compare features and status bits without > >> > caring for their exact meaning. Say we want to verify the correctness > >> > of the virtio negotiation between guest, QEMU, and OVS-DPDK. We can > use > >> > QMP command x-query-virtio-status to retrieve vhost-user net device > >> > features, and the "ovs-vsctl list interface" command to retrieve > >> > interface features. Without commit f3034ad71fc, we could then simply > >> > compare the numbers. With this commit, we first have to map from the > >> > strings back to the numeric encoding. > >> > > >> > Revert the decoding for QMP, but keep it for HMP. > >> > >> Is there a way to maybe have both decoded and numerical one? > > > > What if the next patch went back to this implementation in the following > > patch? All you need to do is add a matching xxx_bits entry for each > feature > > and status. > > > > > https://patchew.org/QEMU/cover.1700319559.git.yong.hu...@smartx.com/3c7161a47b141af04b1f8272e8e24c5faa46ddb2.1700319559.git.yong.hu...@smartx.com/ > > > > > > E.g. I mostly use QMP even when I read it myself. > >> > > > > Thus, it is recommended that both numerical and decoded types be kept for > > QMP; this approach can be at odds with what was previously discussed. > > What do you think about this, Markus? > > QMP is for machines. > > That said, I won't object to adding development & debugging aids meant > for humans as long as > > 1. they're marked unstable, and > > 2. they either add only a modest amount of output, or the additional > output is off by default. > > What exactly is a "modest amount" depends on how machines use the > command. If they use it all the time, even going from 8KiB to 64KiB > could be too much. If they use it just once per guest, we can afford > more waste. > > In this particular case, we could add an unstable member of type ['str'] > next to the int-valued bits. For each set bit in the int, add a string > to the list. If QEMU knows the bit, use the well-known name. I'd omit > the common prefix, though: use "GUEST_ANNOUNCE" or "guest-announce" > instead of "VIRTIO_NET_F_GUEST_ANNOUNCE". If QEMU doesn't know the bit, > you could use the bit's numeric value. > > Recommend a separate patch, to avoid delaying this series. > > Makes sense? > > I agree, and thanks for the comments. Yong -- Best regards
