Re: [Qemu-devel] [PATCH 2/3] hw/virtio-net.c: set config size using host features

[Michael S. Tsirkin]

On Thu, Mar 07, 2013 at 10:44:18AM -0600, Anthony Liguori wrote:
> Andreas Färber <address@hidden> writes:
> 
> > Am 07.03.2013 17:27, schrieb Christian Borntraeger:
> >>> It's a bug in both virtio-ccw that features=0 when get_features is
> >>> called.  You can also tell this with:
> >>>
> >>> [10:02 AM] address@hidden:~/git/qemu/hw/s390x$ grep 
> >>> DEFINE_VIRTIO_NET_FEATURES *
> >>> virtio-ccw.c:    DEFINE_VIRTIO_NET_FEATURES(VirtioCcwDevice, 
> >>> host_features[0]),
> >>>
> >>> So virtio-s390 is doing it wrong, but virtio-ccw looks like its doing it
> >>> right.
> >> 
> >> At least, this patch seems to work. (That also implies, that a transport
> >> must not hide virtio feature bits).
> >
> > To me it indicates that the use of the old qdev property setters is
> > hiding errors resulting from trying to set not-existing properties.
> > If we would set the properties in a way that gets us an Error* on
> > failure like the object_property_set_*() do, we would notice on machine
> > creation (or device_add).
> 
> Hrm, I don't understand your statement.
> 
> Can you elaborate?
> 
> Regards,
> 
> Anthony Liguori


Well to me this indicates that s390 virtio is buggy.
It's not supposed to crash whatever set of features
we specify.

> >
> > Andreas
> >
> >> 
> >> 
> >> From: Christian Borntraeger <address@hidden>
> >> Date: Thu, 7 Mar 2013 17:21:41 +0100
> >> Subject: [PATCH] Allow virtio-net features for legacy s390 virtio bus
> >> 
> >> Enable all virtio-net features for the legacy s390 virtio bus.
> >> This also fixes
> >> kernel BUG at 
> >> /usr/src/packages/BUILD/kernel-default-3.0.58/linux-3.0/drivers/s390/kvm/kvm_virtio.c:121!
> >> 
> >> Signed-off-by: Christian Borntraeger <address@hidden>
> >> ---
> >>  hw/s390x/s390-virtio-bus.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >> 
> >> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> >> index 1200691..a8a8e19 100644
> >> --- a/hw/s390x/s390-virtio-bus.c
> >> +++ b/hw/s390x/s390-virtio-bus.c
> >> @@ -399,6 +399,7 @@ static const VirtIOBindings virtio_s390_bindings = {
> >>  
> >>  static Property s390_virtio_net_properties[] = {
> >>      DEFINE_NIC_PROPERTIES(VirtIOS390Device, nic),
> >> +    DEFINE_VIRTIO_NET_FEATURES(VirtIOS390Device, host_features),
> >>      DEFINE_PROP_UINT32("x-txtimer", VirtIOS390Device,
> >>                         net.txtimer, TX_TIMER_INTERVAL),
> >>      DEFINE_PROP_INT32("x-txburst", VirtIOS390Device,
> >> 
> >
> >
> > -- 
> > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg