From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58023) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDepG-0001Qj-JK for qemu-devel@nongnu.org; Thu, 07 Mar 2013 12:42:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UDepD-0000Wy-Sd for qemu-devel@nongnu.org; Thu, 07 Mar 2013 12:41:54 -0500 Received: from e23smtp09.au.ibm.com ([202.81.31.142]:60332) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDepD-0000VC-9X for qemu-devel@nongnu.org; Thu, 07 Mar 2013 12:41:51 -0500 Received: from /spool/local by e23smtp09.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 8 Mar 2013 03:34:05 +1000 Received: from d23relay03.au.ibm.com (d23relay03.au.ibm.com [9.190.235.21]) by d23dlp03.au.ibm.com (Postfix) with ESMTP id 2B76A357804E for ; Fri, 8 Mar 2013 04:41:42 +1100 (EST) Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r27HfdBk48169020 for ; Fri, 8 Mar 2013 04:41:39 +1100 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r27HffTF013943 for ; Fri, 8 Mar 2013 04:41:41 +1100 From: Anthony Liguori In-Reply-To: <5138CCEE.7020108@suse.de> References: <1360108037-9211-1-git-send-email-jlarrew@linux.vnet.ibm.com> <1360108037-9211-3-git-send-email-jlarrew@linux.vnet.ibm.com> <513621F7.9030403@suse.de> <51362575.2000908@de.ibm.com> <87621319kc.fsf@codemonkey.ws> <5138C007.6080305@de.ibm.com> <5138C26B.5010408@suse.de> <87txon6ty5.fsf@codemonkey.ws> <5138CCEE.7020108@suse.de> Date: Thu, 07 Mar 2013 11:41:35 -0600 Message-ID: <87boavds4w.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/3] hw/virtio-net.c: set config size using host features List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?utf-8?Q?F=C3=A4rber?= Cc: Alexander Graf , "Michael S. Tsirkin" , Stefan Hajnoczi , Jesse Larrew , qemu-devel@nongnu.org, Christian Borntraeger , Jens Freimann , Cornelia Huck Andreas F=C3=A4rber writes: > Am 07.03.2013 17:44, schrieb Anthony Liguori: >> Andreas F=C3=A4rber writes: >>=20 >>> Am 07.03.2013 17:27, schrieb Christian Borntraeger: >>>>> It's a bug in both virtio-ccw that features=3D0 when get_features is >>>>> called. You can also tell this with: >>>>> >>>>> [10:02 AM] anthony@titi:~/git/qemu/hw/s390x$ grep DEFINE_VIRTIO_NET_F= EATURES * >>>>> virtio-ccw.c: DEFINE_VIRTIO_NET_FEATURES(VirtioCcwDevice, host_fea= tures[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 transpo= rt >>>> 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). >>=20 >> Hrm, I don't understand your statement. >>=20 >> Can you elaborate? > > aliguori_, borntraeger added new qdev static properties as > bug fix > aliguori_, I was saying if errors setting such properties > were not silently ignored, we would notice such bugs earlier > > I.e., no new field was added, no new value set, so apparently something > somewhere is setting some of these properties that are defined via > virtio-net.h:DEFINE_VIRTIO_NET_FEATURES() using DEFINE_PROP_BIT(). No code is explicitly setting the property. Each property has a default value (usually true) and that's how we get the initial non-zero value. So in the absence of the define, we end up with no properties and no value for this field. Regards, Anthony Liguori > > And if code expects these properties to be settable, failing to set them > should be treated as error and an appropriate API for setting individual > bits with Error **errp argument should be provided. > > Unfortunately I don't see any property matching Alex' VIRTIO_NET_F_MAC, > so I can't draft a patch for illustration. > > Regards, > Andreas > > --=20 > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3= =BCrnberg