From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42017) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bzni1-0002VX-Ks for qemu-devel@nongnu.org; Thu, 27 Oct 2016 12:39:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bznhy-0005AW-G4 for qemu-devel@nongnu.org; Thu, 27 Oct 2016 12:39:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38866) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bznhy-0005AN-8N for qemu-devel@nongnu.org; Thu, 27 Oct 2016 12:39:14 -0400 Date: Thu, 27 Oct 2016 14:39:11 -0200 From: Eduardo Habkost Message-ID: <20161027163911.GN5057@thinpad.lan.raisama.net> References: <1477509718-6969-1-git-send-email-ehabkost@redhat.com> <1477509718-6969-2-git-send-email-ehabkost@redhat.com> <20161027112527.GE5057@thinpad.lan.raisama.net> <20161027171334.29db5536@nial.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20161027171334.29db5536@nial.brq.redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/4] e1000e: Use regular DEFINE_PROP_ macros for properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: Dmitry Fleytman , Jason Wang , qemu-devel@nongnu.org On Thu, Oct 27, 2016 at 05:13:34PM +0200, Igor Mammedov wrote: > On Thu, 27 Oct 2016 09:25:27 -0200 > Eduardo Habkost wrote: >=20 > > On Thu, Oct 27, 2016 at 09:38:28AM +0300, Dmitry Fleytman wrote: > > > =20 > > > > On 27 Oct 2016, at 06:12 AM, Jason Wang wro= te: > > > >=20 > > > >=20 > > > >=20 > > > > On 2016=E5=B9=B410=E6=9C=8827=E6=97=A5 03:21, Eduardo Habkost wro= te: =20 > > > >> Instead of hacking custom PropertyInfo structs, use the regular > > > >> DEFINE_PROP_ macros for the e1000e properties. > > > >>=20 > > > >> This also fixes a bug in the disable_vnet_hdr property > > > >> definition, that was using qdev_prop_uint8 for a bool field. > > > >>=20 > > > >> Signed-off-by: Eduardo Habkost > > > >> --- > > > >> hw/net/e1000e.c | 25 +++---------------------- > > > >> 1 file changed, 3 insertions(+), 22 deletions(-) > > > >>=20 > > > >> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c > > > >> index 4994e1c..df24e55 100644 > > > >> --- a/hw/net/e1000e.c > > > >> +++ b/hw/net/e1000e.c > > > >> @@ -638,19 +638,11 @@ static const VMStateDescription e1000e_vms= tate =3D { > > > >> } > > > >> }; > > > >> -static PropertyInfo e1000e_prop_disable_vnet, > > > >> - e1000e_prop_subsys_ven, > > > >> - e1000e_prop_subsys; > > > >> - > > > >> static Property e1000e_properties[] =3D { > > > >> DEFINE_NIC_PROPERTIES(E1000EState, conf), > > > >> - DEFINE_PROP_DEFAULT("disable_vnet_hdr", E1000EState, disabl= e_vnet, false, > > > >> - e1000e_prop_disable_vnet, bool), > > > >> - DEFINE_PROP_DEFAULT("subsys_ven", E1000EState, subsys_ven, > > > >> - PCI_VENDOR_ID_INTEL, > > > >> - e1000e_prop_subsys_ven, uint16_t), > > > >> - DEFINE_PROP_DEFAULT("subsys", E1000EState, subsys, 0, > > > >> - e1000e_prop_subsys, uint16_t), > > > >> + DEFINE_PROP_BOOL("disable_vnet_hdr", E1000EState, disable_v= net, false), > > > >> + DEFINE_PROP_UINT16("subsys_ven", E1000EState, subsys_ven, P= CI_VENDOR_ID_INTEL), > > > >> + DEFINE_PROP_UINT16("subsys", E1000EState, subsys, 0), > > > >> DEFINE_PROP_END_OF_LIST(), > > > >> }; > > > >> @@ -673,17 +665,6 @@ static void e1000e_class_init(ObjectClass = *class, void *data) > > > >> dc->vmsd =3D &e1000e_vmstate; > > > >> dc->props =3D e1000e_properties; > > > >> - e1000e_prop_disable_vnet =3D qdev_prop_uint8; > > > >> - e1000e_prop_disable_vnet.description =3D "Do not use virtio= headers, " > > > >> - "perform SW offloads= emulation " > > > >> - "instead"; > > > >> - > > > >> - e1000e_prop_subsys_ven =3D qdev_prop_uint16; > > > >> - e1000e_prop_subsys_ven.description =3D "PCI device Subsyste= m Vendor ID"; > > > >> - > > > >> - e1000e_prop_subsys =3D qdev_prop_uint16; > > > >> - e1000e_prop_subsys.description =3D "PCI device Subsystem ID= "; > > > >> - > > > >> set_bit(DEVICE_CATEGORY_NETWORK, dc->categories); > > > >> } > > > >> =20 > > > >=20 > > > > Is there a way to keep the description here? At least for "vnet" = I believe. =20 > > >=20 > > > Actually, ability to add description was the only reason for having= custom PropertyInfo structs. > > > Does DEFINE_PROP_=E2=80=A6 macros set worth an improvement? =20 > >=20 > > I think it would be reasonable to add a descrition field to > > struct Property. > >=20 > > Alternatively, you can call object_property_set_description() on > > instance_init. > that would look sort of convoluted in case of class properties. > description should be set at the same time the property is added. In the case of class properties, we have object_class_property_set_description(). It's easier to redo this series on top of the qdev-class-properties series, to avoid conflicts. I will do that. --=20 Eduardo