On Mon, Sep 15, 2014 at 07:12:59AM +0000, Gonglei (Arei) wrote: > > From: Michael S. Tsirkin [mailto:m...@redhat.com] > > Sent: Monday, September 15, 2014 2:41 PM > > Subject: Re: [PATCH v2 1/9] virtio-net: use aliases instead of duplicate > > qdev > > properties > > > > On Mon, Sep 15, 2014 at 05:49:42AM +0000, Gonglei (Arei) wrote: > > > > From: Michael S. Tsirkin [mailto:m...@redhat.com] > > > > Sent: Monday, September 15, 2014 1:36 PM > > > > Subject: Re: [PATCH v2 1/9] virtio-net: use aliases instead of > > > > duplicate qdev > > > > properties > > > > > > > > On Mon, Sep 15, 2014 at 01:03:24AM +0000, Gonglei (Arei) wrote: > > > > > > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > > > > > > > index ddb5da1..6722156 100644 > > > > > > > --- a/hw/virtio/virtio-pci.c > > > > > > > +++ b/hw/virtio/virtio-pci.c > > > > > > > @@ -1414,8 +1414,6 @@ static Property virtio_net_properties[] = { > > > > > > > VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, > > > > false), > > > > > > > DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3), > > > > > > > DEFINE_VIRTIO_NET_FEATURES(VirtIOPCIProxy, > > host_features), > > > > > > > - DEFINE_NIC_PROPERTIES(VirtIONetPCI, vdev.nic_conf), > > > > > > > - DEFINE_VIRTIO_NET_PROPERTIES(VirtIONetPCI, > > vdev.net_conf), > > > > > > > DEFINE_PROP_END_OF_LIST(), > > > > > > > }; > > > > > > > > > > > > > > @@ -1456,6 +1454,7 @@ static void > > virtio_net_pci_instance_init(Object > > > > > > *obj) > > > > > > > VirtIONetPCI *dev = VIRTIO_NET_PCI(obj); > > > > > > > object_initialize(&dev->vdev, sizeof(dev->vdev), > > > > TYPE_VIRTIO_NET); > > > > > > > object_property_add_child(obj, "virtio-backend", > > > > OBJECT(&dev->vdev), > > > > > > NULL); > > > > > > > + qdev_alias_all_properties(DEVICE(&dev->vdev), obj); > > > > > > > } > > > > > > > > > > > > > > static const TypeInfo virtio_net_pci_info = { > > > > > > > > > > > > Does -device virtio-net-pci,? still list e.g. the mac property with > > > > > > this > > > > > > patch? > > > > > > > > > > > Yes. Please see the properties of virtio-net-pci with this patch: > > > > > > > > > > #./qemu-system-x86_64 -enable-kvm -m 4096 -smp 4 -name redhat6.2 > > -drive > > > > file=/home/redhat6.2, \ > > > > > if=none,id=drive-ide0-0-0 -device > > > > ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ > > > > > -drive > > > > > > file=/mnt/sdb/gonglei/iso/rhel-server-7.0-x86_64-dvd.iso,if=none,id=drive-ide0- > > > > 0-1 \ > > > > > -device > > ide-cd,bus=ide.0,unit=1,drive=drive-ide0-0-1,id=ide0-0-1,bootindex=4 > > > > -vnc 0.0.0.0:10 \ > > > > > -netdev type=user,id=net0 -device > > > > virtio-net-pci,netdev=net0,bootindex=3,id=nic1 -monitor stdio > > > > > QEMU 2.1.50 monitor - type 'help' for more information > > > > > (qemu) qom-list nic1 > > > > > virtio-net-pci.rom[0] (child<qemu:memory-region>) > > > > > virtio-pci[0] (child<qemu:memory-region>) > > > > > msix-pba[0] (child<qemu:memory-region>) > > > > > msix-table[0] (child<qemu:memory-region>) > > > > > virtio-net-pci-msix[0] (child<qemu:memory-region>) > > > > > virtio-bus (child<virtio-pci-bus>) > > > > > bus master[0] (child<qemu:memory-region>) > > > > > bootindex (int) > > > > > tx (str) > > > > > x-txburst (int32) > > > > > x-txtimer (uint32) > > > > > netdev (str) > > > > > vlan (int32) > > > > > mac (str) > > > > > > > > This is a regression though. > > > > Before: > > > > > > > > -virtio-net-pci.netdev=netdev > > > > -virtio-net-pci.vlan=vlan > > > > -virtio-net-pci.mac=macaddr > > > > > > > > After > > > > > > > > +virtio-net-pci.netdev=str > > > > +virtio-net-pci.vlan=int32 > > > > +virtio-net-pci.mac=str > > > > > > > > Compare to e1000: > > > > e1000.netdev=netdev > > > > e1000.vlan=vlan > > > > e1000.mac=macaddr > > > > > > > > -- > > > > MST > > > > > > Sorry, I don't understand your meaning. :( > > > > I applied your patch, and looked at the property types. > > > Could you tell me your method looked at properties type?
You can use an HMP command, or you can use -device virtio-net-pci,? changes must not modify output except for adding new commands, or fixing bugs (with care). > > They should be netdev/vlan/macaddr. > > Your patch makes them str/int32/str > > > Does those changes have any side-effects? Thanks! Yes. If the types are just "int" and "str" then you have no way to know they should have a specific format, such as a name of a netdev, valid mac address, or a vlan. In fact, vlan=int is a bug since vlan names aren't numbers. > The similar changes for virtio-blk had applied in qemu master. > > Best regards, > -Gonglei Which commit? Did output change? If yes it's a bug.
