Re: [Qemu-devel] [PATCH v2 1/9] virtio-net: use aliases instead of duplicate qdev properties

[Michael S. Tsirkin]

On Mon, Sep 15, 2014 at 09:23:26AM +0000, Gonglei (Arei) wrote:
> 
> 
> 
> 
> 
> 
> 
> > -----Original Message-----
> > From: Gonglei (Arei)
> > Sent: Monday, September 15, 2014 5:06 PM
> > To: 'Michael S. Tsirkin'
> > Cc: address@hidden; address@hidden; address@hidden;
> > Huangweidong (C); address@hidden; Huangpeng (Peter); address@hidden;
> > address@hidden; address@hidden; Luonengjun;
> > address@hidden; address@hidden
> > Subject: RE: [PATCH v2 1/9] virtio-net: use aliases instead of duplicate 
> > qdev
> > properties
> > 
> > > > > >
> > > > > > 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,?
> > 
> > OK. Good method.
> > 
> > > changes must not modify output except for adding
> > > new commands, or fixing bugs (with care).
> > >
> > OK.
> > 
> > > > > 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.
> > >
> > Got it.
> > 
> > > > 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.
> > 
> > No, I checked it again, virtio-blk-pci's properties are 'int' type as
> > the same as virtio-blk device, so it will have no problem on this point.
> > 
> Sorry. There are two properties are different
>  (commit caffdac363801cd2cf2bf01ad013a8c1e1e43800)
> 
> Before:
> 
> virtio-blk-pci.physical_block_size=blocksize
> virtio-blk-pci.logical_block_size=blocksize
> 
> After commit caffdac363801cd2cf2bf01ad013a8c1e1e43800 :
> 
> virtio-blk-pci.physical_block_size=uint16
> virtio-blk-pci.logical_block_size=uint16

So that's a regresion in 2.1.0
Would be nice to fix in stable.

> > I will rework this patch series. Thanks a lot for review! :)
> > 
> > Best regards,
> > -Gonglei