From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51796) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDeXa-0007Gs-09 for qemu-devel@nongnu.org; Thu, 07 Mar 2013 12:23:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UDeXY-0001ir-31 for qemu-devel@nongnu.org; Thu, 07 Mar 2013 12:23:37 -0500 Received: from cantor2.suse.de ([195.135.220.15]:46022 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDeXX-0001Zd-Q2 for qemu-devel@nongnu.org; Thu, 07 Mar 2013 12:23:36 -0500 Message-ID: <5138CCEE.7020108@suse.de> Date: Thu, 07 Mar 2013 18:22:54 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 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> In-Reply-To: <87txon6ty5.fsf@codemonkey.ws> 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: Anthony Liguori Cc: Alexander Graf , "Michael S. Tsirkin" , Stefan Hajnoczi , Jesse Larrew , qemu-devel@nongnu.org, Christian Borntraeger , Jens Freimann , Cornelia Huck 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_= FEATURES * >>>> virtio-ccw.c: DEFINE_VIRTIO_NET_FEATURES(VirtioCcwDevice, host_fe= atures[0]), >>>> >>>> So virtio-s390 is doing it wrong, but virtio-ccw looks like its doin= g it >>>> right. >>> >>> At least, this patch seems to work. (That also implies, that a transp= ort >>> 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 machin= e >> 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(). 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=BC= rnberg