[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties |
Date: |
Tue, 16 Sep 2014 10:36:40 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.0 |
Il 16/09/2014 09:21, Markus Armbruster ha scritto:
> The rebase onto QOM renamed name to legacy_name, to free name for use as
> QOM type name (commit cafe5bd).
Also, the QOM type name has strict rules:
- either it is a QAPI type (primitive, enum or struct)
- or it is link<qom-type-name>
- or it is child<qom-type-name>
The qdev type names had no rules. We had uint8, hex8, on/off, drive, etc.
> Human users do, however. I'd object to a degradation of -device
> FOO,help. Changing it is fine, but it should remain at least as helpful
> as it is now.
The question is if it is really a degradation.
Example 1:
virtio-blk-pci.physical_block_size=blocksize
virtio-blk-pci.logical_block_size=blocksize
vs.
virtio-blk-pci.physical_block_size=uint16
virtio-blk-pci.logical_block_size=uint16
What is a "blocksize"? It is a power of two between 512 and 32768, but
how does the user know? If the value is too small or invalid, the
error message is particularly helpful for QEMU standards:
qemu-system-x86_64: -device virtio-blk-pci,physical_block_size=128,drive=hd:
Property .physical_block_size doesn't take value 128 (minimum: 512,
maximum:
32768)
qemu-system-x86_64: -device
virtio-blk-pci,physical_block_size=1023,drive=hd:
Property .physical_block_size doesn't take value '1023', it's not a power
of 2
I think uint16 is actually more informative than "blocksize".
Example 2:
virtio-blk-pci.drive=drive
vs.
virtio-blk-pci.drive=str
The fact that it points to a -drive is already guessable (for anyone who
knows the relationship between -drive and -device) from the name of the
property.
$ qemu-system-x86_64 -drive if=none,file=$HOME/test2.img,id=hd -device
virtio-blk-pci
qemu-system-x86_64: -device virtio-blk-pci: drive property not set
qemu-system-x86_64: -device virtio-blk-pci: Device initialization failed.
qemu-system-x86_64: -device virtio-blk-pci: Device 'virtio-blk-pci' could
not be
initialized
$ qemu-system-x86_64 -drive if=none,file=$HOME/test2.img,id=hd
-device virtio-blk-pci,drive=ff
qemu-system-x86_64: -device virtio-blk-pci,drive=ff: Property
'virtio-blk-device.drive' can't find value 'ff'
If we QOMified BlockBackend, BTW, it would show up as
virtio-blk-pci.drive=link<block-backend>
I think both "str" and "link<block-backend>" actually are a small degradation
compared to "drive", and this is why I kept the legacy_name. But overall I
think it's not really worth the layering violation that patches 2 and 3 are;
and it's definitely not stable material.
I'd rather drop the legacy_name at all. Here are the legacy_names currently
in use:
hw/core/qdev-properties-system.c: .legacy_name = "drive",
hw/core/qdev-properties-system.c: .legacy_name = "chr",
hw/core/qdev-properties-system.c: .legacy_name = "netdev",
hw/core/qdev-properties-system.c: .legacy_name = "vlan",
hw/core/qdev-properties.c: .legacy_name = "on/off",
hw/core/qdev-properties.c: .legacy_name = "macaddr",
hw/core/qdev-properties.c: .legacy_name = "bios-chs-trans",
hw/core/qdev-properties.c: .legacy_name = "pci-devfn",
hw/core/qdev-properties.c: .legacy_name = "blocksize",
hw/core/qdev-properties.c: .legacy_name = "pci-host-devaddr",
vlan is just a glorified int, not an id like the others. chr should be
named chardev. blocksize, I already covered above. bios-chs-trans is
an enum (QAPI name BiosAtaTranslation) and is useless. on/off vs.
bool is just bikeshedding. macaddr is obviously a string, whose format
is clear from the property name.
pci-host-devaddr and pci-devfn are the only ones that do not have an
obvious property name (respectively "host" and "addr").
Paolo
- [Qemu-devel] [PATCH 1/3] qom: add error handler for object alias property, (continued)
- [Qemu-devel] [PATCH 1/3] qom: add error handler for object alias property, arei.gonglei, 2014/09/15
- [Qemu-devel] [PATCH 2/3] qom: add target object poniter for alias property in ObjectProperty, arei.gonglei, 2014/09/15
- [Qemu-devel] [PATCH 3/3] qmp: print real legacy_name for alias property, arei.gonglei, 2014/09/15
- Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties, Paolo Bonzini, 2014/09/15
- Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties, Eric Blake, 2014/09/15
- Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties, Michael S. Tsirkin, 2014/09/15
- Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties, Markus Armbruster, 2014/09/16
- Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties, Gonglei (Arei), 2014/09/16
- Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties,
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties, Markus Armbruster, 2014/09/16
- Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties, Paolo Bonzini, 2014/09/16
- Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties, Michael S. Tsirkin, 2014/09/16
- Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties, Paolo Bonzini, 2014/09/16
- Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties, Gonglei (Arei), 2014/09/16
- Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties, Michael S. Tsirkin, 2014/09/16
- Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties, Paolo Bonzini, 2014/09/16
- Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties, Michael S. Tsirkin, 2014/09/16
- Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties, Paolo Bonzini, 2014/09/16
- Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties, Michael S. Tsirkin, 2014/09/16