From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:53588)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1XToHS-0004ky-JB
	for qemu-devel@nongnu.org; Tue, 16 Sep 2014 04:38:43 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1XToHI-0005un-Us
	for qemu-devel@nongnu.org; Tue, 16 Sep 2014 04:38:34 -0400
Received: from mx1.redhat.com ([209.132.183.28]:64504)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1XToHI-0005tR-NF
	for qemu-devel@nongnu.org; Tue, 16 Sep 2014 04:38:24 -0400
Message-ID: <5417F698.2060001@redhat.com>
Date: Tue, 16 Sep 2014 10:36:40 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1410792279-2488-1-git-send-email-arei.gonglei@huawei.com>	<54170C7F.8080409@redhat.com>
	<20140915174503.GB13548@redhat.com>
	<87wq94xc7v.fsf@blackfin.pond.sub.org>
In-Reply-To: <87wq94xc7v.fsf@blackfin.pond.sub.org>
Content-Type: text/plain; charset=iso-8859-15
Content-Transfer-Encoding: 8bit
Subject: Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias
	properties
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Markus Armbruster <armbru@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
Cc: weidong.huang@huawei.com, stefanha@redhat.com, qemu-devel@nongnu.org, agraf@suse.de, arei.gonglei@huawei.com, aliguori@amazon.com, peter.huangpeng@huawei.com, lcapitulino@redhat.com

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