From mboxrd@z Thu Jan 1 00:00:00 1970 From: Igor Mammedov Subject: Re: [Qemu-devel] [PATCH for-3.2 v5 07/19] hw: apply accel compat properties without touching globals Date: Thu, 13 Dec 2018 13:06:54 +0100 Message-ID: <20181213130654.1eb72eb1@redhat.com> References: <20181204142023.15982-1-marcandre.lureau@redhat.com> <20181204142023.15982-8-marcandre.lureau@redhat.com> <20181210174522.04b21215@Igors-MacBook-Pro.local> <20181210175441.7d6e0dcd@Igors-MacBook-Pro.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1gXPlb-0003vt-Er for xen-devel@lists.xenproject.org; Thu, 13 Dec 2018 12:06:59 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" To: =?UTF-8?B?TWFyYy1BbmRyw6k=?= Lureau Cc: Stefano Stabellini , Eduardo Habkost , QEMU , xen-devel@lists.xenproject.org, Anthony Perard , Paolo Bonzini , Andreas =?UTF-8?B?RsOkcmJlcg==?= List-Id: xen-devel@lists.xenproject.org T24gV2VkLCAxMiBEZWMgMjAxOCAxNjowMDowNiArMDQwMApNYXJjLUFuZHLDqSBMdXJlYXUgPG1h cmNhbmRyZS5sdXJlYXVAZ21haWwuY29tPiB3cm90ZToKCj4gSGkKPiBPbiBNb24sIERlYyAxMCwg MjAxOCBhdCA4OjU1IFBNIElnb3IgTWFtbWVkb3YgPGltYW1tZWRvQHJlZGhhdC5jb20+IHdyb3Rl Ogo+ID4KPiA+IE9uIE1vbiwgMTAgRGVjIDIwMTggMTc6NDU6MjIgKzAxMDAKPiA+IElnb3IgTWFt bWVkb3YgPGltYW1tZWRvQHJlZGhhdC5jb20+IHdyb3RlOgo+ID4gIAo+ID4gPiBPbiBUdWUsICA0 IERlYyAyMDE4IDE4OjIwOjExICswNDAwCj4gPiA+IE1hcmMtQW5kcsOpIEx1cmVhdSA8bWFyY2Fu ZHJlLmx1cmVhdUByZWRoYXQuY29tPiB3cm90ZToKPiA+ID4gIAo+ID4gPiA+IEluc3RlYWQgb2Yg cmVnaXN0ZXJpbmcgY29tcGF0IHByb3BlcnRpZXMgYXMgZ2xvYmFscywgbGV0J3Mga2VlcCB0aGVt Cj4gPiA+ID4gaW4gdGhlaXIgb3duIGFycmF5LCB0byBhdm9pZCBtaXhpbmcgd2l0aCB1c2VyIGds b2JhbHMuCj4gPiA+ID4KPiA+ID4gPiBJbnRyb2R1Y2Ugb2JqZWN0X2FwcGx5X2dsb2JhbF9wcm9w cygpIGZ1bmN0aW9uLCB0byBhcHBseSBjb21wYXRpYmlsaXR5Cj4gPiA+ID4gcHJvcGVydGllcyBm cm9tIGEgR1B0ckFycmF5Lgo+ID4gPiA+Cj4gPiA+ID4gU2lnbmVkLW9mZi1ieTogTWFyYy1BbmRy w6kgTHVyZWF1IDxtYXJjYW5kcmUubHVyZWF1QHJlZGhhdC5jb20+Cj4gPiA+ID4gLS0tCj4gPiA+ ID4gIGluY2x1ZGUvaHcvcWRldi1jb3JlLmggfCAxMCArKysrKysrKysrCj4gPiA+ID4gIGluY2x1 ZGUvcW9tL29iamVjdC5oICAgfCAgMyArKysKPiA+ID4gPiAgaW5jbHVkZS9zeXNlbXUvYWNjZWwu aCB8ICA0ICstLS0KPiA+ID4gPiAgYWNjZWwvYWNjZWwuYyAgICAgICAgICB8IDEyIC0tLS0tLS0t LS0tLQo+ID4gPiA+ICBody9jb3JlL3FkZXYuYyAgICAgICAgIHwgIDkgKysrKysrKysrCj4gPiA+ ID4gIGh3L3hlbi94ZW4tY29tbW9uLmMgICAgfCAgOSArKysrKystLS0KPiA+ID4gPiAgcW9tL29i amVjdC5jICAgICAgICAgICB8IDI1ICsrKysrKysrKysrKysrKysrKysrKysrKysKPiA+ID4gPiAg dmwuYyAgICAgICAgICAgICAgICAgICB8ICAxIC0KPiA+ID4gPiAgOCBmaWxlcyBjaGFuZ2VkLCA1 NCBpbnNlcnRpb25zKCspLCAxOSBkZWxldGlvbnMoLSkKPiA+ID4gPiAgCj4gPiA+IFsuLi5dICAK PiA+ID4gPiBkaWZmIC0tZ2l0IGEvaHcveGVuL3hlbi1jb21tb24uYyBiL2h3L3hlbi94ZW4tY29t bW9uLmMKPiA+ID4gPiBpbmRleCA2ZWMxNGM3M2NhLi40NTMyYWE4NjMyIDEwMDY0NAo+ID4gPiA+ IC0tLSBhL2h3L3hlbi94ZW4tY29tbW9uLmMKPiA+ID4gPiArKysgYi9ody94ZW4veGVuLWNvbW1v bi5jCj4gPiA+ID4gQEAgLTE3NCwxOCArMTc0LDIxIEBAIHN0YXRpYyBHbG9iYWxQcm9wZXJ0eSB4 ZW5fY29tcGF0X3Byb3BzW10gPSB7Cj4gPiA+ID4gICAgICAgICAgLmRyaXZlciA9ICJtaWdyYXRp b24iLAo+ID4gPiA+ICAgICAgICAgIC5wcm9wZXJ0eSA9ICJzZW5kLXNlY3Rpb24tZm9vdGVyIiwK PiA+ID4gPiAgICAgICAgICAudmFsdWUgPSAib2ZmIiwKPiA+ID4gPiAtICAgIH0sCj4gPiA+ID4g LSAgICB7IC8qIGVuZCBvZiBsaXN0ICovIH0sCj4gPiA+ID4gKyAgICB9Cj4gPiA+ID4gIH07Cj4g PiA+ID4KPiA+ID4gPiAgc3RhdGljIHZvaWQgeGVuX2FjY2VsX2NsYXNzX2luaXQoT2JqZWN0Q2xh c3MgKm9jLCB2b2lkICpkYXRhKQo+ID4gPiA+ICB7Cj4gPiA+ID4gICAgICBBY2NlbENsYXNzICph YyA9IEFDQ0VMX0NMQVNTKG9jKTsKPiA+ID4gPiArCj4gPiA+ID4gICAgICBhYy0+bmFtZSA9ICJY ZW4iOwo+ID4gPiA+ICAgICAgYWMtPmluaXRfbWFjaGluZSA9IHhlbl9pbml0Owo+ID4gPiA+ICAg ICAgYWMtPnNldHVwX3Bvc3QgPSB4ZW5fc2V0dXBfcG9zdDsKPiA+ID4gPiAgICAgIGFjLT5hbGxv d2VkID0gJnhlbl9hbGxvd2VkOwo+ID4gPiA+IC0gICAgYWMtPmdsb2JhbF9wcm9wcyA9IHhlbl9j b21wYXRfcHJvcHM7Cj4gPiA+ID4gKyAgICBhYy0+Y29tcGF0X3Byb3BzID0gZ19wdHJfYXJyYXlf bmV3KCk7ICAKPiA+ID4gd2hlcmUgaXMgbWF0Y2hpbmcgZnJlZSBmb3IgdGhhdD8gIAo+ID4gY2Fu IHdlIGF0IGxlYXN0IGFubm90YXRlIGl0IHNvbWVob3cgc28gdGhhdCB2YWxncmluZCB3b24ndCBj b21wbGFpbiBhYm91dCB0aGlzIGxlYWs/ICAKPiAKPiBJZiB5b3UgY2hlY2sgbXkgY29tbWl0cyBv biBxZW11LCB5b3Ugc2hvdWxkIHNlZSB0aGF0IEkgZG8gY2FyZSAodG9vCj4gbXVjaD8pIGFib3V0 IGxlYWtzIDopCj4gCj4gSW4gdGhpcyBjYXNlIHRob3VnaCwgSSBkb24ndCBzZWUgdmFsZ3JpbmQg b3IgYXNhbiBjb21wbGFpbmluZywgSSBndWVzcwo+IGl0J3Mgc3RpbGwgYSByZWFjaGFibGUgcmVm ZXJlbmNlLgo+IERvIHlvdSB0aGluayBhIEZJWE1FIGNvbW1lbnQgd291bGQgYmUgaGVscGZ1bD8K SSd2ZSBsb29rZWQgYXQgb3RoZXIgY2FzZXMgd2VyZSB3ZSBsZWFrLCBhbmQgd2VsbCB3ZSBsZWFr IGEgbG90IHNvIGl0J3MKcHJvYmFibHkgZnV0aWxlIGV4ZXJjaXNlLiBCdXQgdGhlIGNvbW1lbnQg d29uJ3QgaHVydCBhbmQgd2lsbCB3b3JrIGFzIHJlbWFpbmRlci4KCgo+ICgvbWUgd2lzaCB3ZSBo YWQgYSBwcm9wZXIgb2JqZWN0IHN5c3RlbSwgR09iamVjdCwgYnV0IHRoYXQgc2hpcCBhcwo+IGxv bmcgc2FpbGVkLi4pCj4gCj4gPiAgCj4gPiA+ICAKPiA+ID4gPiArCj4gPiA+ID4gKyAgICBjb21w YXRfcHJvcHNfYWRkKGFjLT5jb21wYXRfcHJvcHMsCj4gPiA+ID4gKyAgICAgICAgICAgICAgICAg ICAgIHhlbl9jb21wYXRfcHJvcHMsIEdfTl9FTEVNRU5UUyh4ZW5fY29tcGF0X3Byb3BzKSk7Cj4g PiA+ID4gIH0KPiA+ID4gPgo+ID4gPiA+ICAjZGVmaW5lIFRZUEVfWEVOX0FDQ0VMIEFDQ0VMX0NM QVNTX05BTUUoInhlbiIpCj4gPiA+ID4gZGlmZiAtLWdpdCBhL3FvbS9vYmplY3QuYyBiL3FvbS9v YmplY3QuYwo+ID4gPiA+IGluZGV4IDE3OTIxYzBhNzEuLmRiZGFiMGFlYWQgMTAwNjQ0Cj4gPiA+ ID4gLS0tIGEvcW9tL29iamVjdC5jCj4gPiA+ID4gKysrIGIvcW9tL29iamVjdC5jCj4gPiA+ID4g QEAgLTM3MCw2ICszNzAsMzEgQEAgc3RhdGljIHZvaWQgb2JqZWN0X3Bvc3RfaW5pdF93aXRoX3R5 cGUoT2JqZWN0ICpvYmosIFR5cGVJbXBsICp0aSkKPiA+ID4gPiAgICAgIH0KPiA+ID4gPiAgfQo+ ID4gPiA+Cj4gPiA+ID4gK3ZvaWQgb2JqZWN0X2FwcGx5X2dsb2JhbF9wcm9wcyhPYmplY3QgKm9i aiwgY29uc3QgR1B0ckFycmF5ICpwcm9wcywgRXJyb3IgKiplcnJwKQo+ID4gPiA+ICt7Cj4gPiA+ ID4gKyAgICBFcnJvciAqZXJyID0gTlVMTDsKPiA+ID4gPiArICAgIGludCBpOwo+ID4gPiA+ICsK PiA+ID4gPiArICAgIGlmICghcHJvcHMpIHsKPiA+ID4gPiArICAgICAgICByZXR1cm47Cj4gPiA+ ID4gKyAgICB9Cj4gPiA+ID4gKwo+ID4gPiA+ICsgICAgZm9yIChpID0gMDsgaSA8IHByb3BzLT5s ZW47IGkrKykgewo+ID4gPiA+ICsgICAgICAgIEdsb2JhbFByb3BlcnR5ICpwID0gZ19wdHJfYXJy YXlfaW5kZXgocHJvcHMsIGkpOwo+ID4gPiA+ICsKPiA+ID4gPiArICAgICAgICBpZiAob2JqZWN0 X2R5bmFtaWNfY2FzdChvYmosIHAtPmRyaXZlcikgPT0gTlVMTCkgewo+ID4gPiA+ICsgICAgICAg ICAgICBjb250aW51ZTsKPiA+ID4gPiArICAgICAgICB9Cj4gPiA+ID4gKyAgICAgICAgcC0+dXNl ZCA9IHRydWU7Cj4gPiA+ID4gKyAgICAgICAgb2JqZWN0X3Byb3BlcnR5X3BhcnNlKG9iaiwgcC0+ dmFsdWUsIHAtPnByb3BlcnR5LCAmZXJyKTsKPiA+ID4gPiArICAgICAgICBpZiAoZXJyICE9IE5V TEwpIHsKPiA+ID4gPiArICAgICAgICAgICAgZXJyb3JfcHJlcGVuZCgmZXJyLCAiY2FuJ3QgYXBw bHkgZ2xvYmFsICVzLiVzPSVzOiAiLAo+ID4gPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAg IHAtPmRyaXZlciwgcC0+cHJvcGVydHksIHAtPnZhbHVlKTsKPiA+ID4gPiArICAgICAgICAgICAg ZXJyb3JfcHJvcGFnYXRlKGVycnAsIGVycik7Cj4gPiA+ID4gKyAgICAgICAgfQo+ID4gPiA+ICsg ICAgfQo+ID4gPiA+ICt9Cj4gPiA+ID4gKwo+ID4gPiA+ICBzdGF0aWMgdm9pZCBvYmplY3RfaW5p dGlhbGl6ZV93aXRoX3R5cGUodm9pZCAqZGF0YSwgc2l6ZV90IHNpemUsIFR5cGVJbXBsICp0eXBl KQo+ID4gPiA+ICB7Cj4gPiA+ID4gICAgICBPYmplY3QgKm9iaiA9IGRhdGE7Cj4gPiA+ID4gZGlm ZiAtLWdpdCBhL3ZsLmMgYi92bC5jCj4gPiA+ID4gaW5kZXggYTVhZTVmMjNkMi4uODhiYTY1ODU3 MiAxMDA2NDQKPiA+ID4gPiAtLS0gYS92bC5jCj4gPiA+ID4gKysrIGIvdmwuYwo+ID4gPiA+IEBA IC0yOTY4LDcgKzI5NjgsNiBAQCBzdGF0aWMgdm9pZCB1c2VyX3JlZ2lzdGVyX2dsb2JhbF9wcm9w cyh2b2lkKQo+ID4gPiA+ICAgKi8KPiA+ID4gPiAgc3RhdGljIHZvaWQgcmVnaXN0ZXJfZ2xvYmFs X3Byb3BlcnRpZXMoTWFjaGluZVN0YXRlICptcykKPiA+ID4gPiAgewo+ID4gPiA+IC0gICAgYWNj ZWxfcmVnaXN0ZXJfY29tcGF0X3Byb3BzKG1zLT5hY2NlbGVyYXRvcik7Cj4gPiA+ID4gICAgICBt YWNoaW5lX3JlZ2lzdGVyX2NvbXBhdF9wcm9wcyhtcyk7Cj4gPiA+ID4gICAgICB1c2VyX3JlZ2lz dGVyX2dsb2JhbF9wcm9wcygpOwo+ID4gPiA+ICB9ICAKPiA+ID4KPiA+ID4gIAo+ID4KPiA+ICAK PiAKPiAKCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpY ZW4tZGV2ZWwgbWFpbGluZyBsaXN0Clhlbi1kZXZlbEBsaXN0cy54ZW5wcm9qZWN0Lm9yZwpodHRw czovL2xpc3RzLnhlbnByb2plY3Qub3JnL21haWxtYW4vbGlzdGluZm8veGVuLWRldmVs From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60825) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gXPld-0000dn-Ge for qemu-devel@nongnu.org; Thu, 13 Dec 2018 07:07:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gXPlb-0007hv-3b for qemu-devel@nongnu.org; Thu, 13 Dec 2018 07:07:01 -0500 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 1gXPla-0007fO-QD for qemu-devel@nongnu.org; Thu, 13 Dec 2018 07:06:59 -0500 Date: Thu, 13 Dec 2018 13:06:54 +0100 From: Igor Mammedov Message-ID: <20181213130654.1eb72eb1@redhat.com> In-Reply-To: References: <20181204142023.15982-1-marcandre.lureau@redhat.com> <20181204142023.15982-8-marcandre.lureau@redhat.com> <20181210174522.04b21215@Igors-MacBook-Pro.local> <20181210175441.7d6e0dcd@Igors-MacBook-Pro.local> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH for-3.2 v5 07/19] hw: apply accel compat properties without touching globals List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?TWFyYy1BbmRyw6k=?= Lureau Cc: Stefano Stabellini , Eduardo Habkost , QEMU , Paolo Bonzini , Anthony Perard , xen-devel@lists.xenproject.org, Andreas =?UTF-8?B?RsOkcmJlcg==?= On Wed, 12 Dec 2018 16:00:06 +0400 Marc-Andr=C3=A9 Lureau wrote: > Hi > On Mon, Dec 10, 2018 at 8:55 PM Igor Mammedov wrote: > > > > On Mon, 10 Dec 2018 17:45:22 +0100 > > Igor Mammedov wrote: > > =20 > > > On Tue, 4 Dec 2018 18:20:11 +0400 > > > Marc-Andr=C3=A9 Lureau wrote: > > > =20 > > > > Instead of registering compat properties as globals, let's keep them > > > > in their own array, to avoid mixing with user globals. > > > > > > > > Introduce object_apply_global_props() function, to apply compatibil= ity > > > > properties from a GPtrArray. > > > > > > > > Signed-off-by: Marc-Andr=C3=A9 Lureau > > > > --- > > > > include/hw/qdev-core.h | 10 ++++++++++ > > > > include/qom/object.h | 3 +++ > > > > include/sysemu/accel.h | 4 +--- > > > > accel/accel.c | 12 ------------ > > > > hw/core/qdev.c | 9 +++++++++ > > > > hw/xen/xen-common.c | 9 ++++++--- > > > > qom/object.c | 25 +++++++++++++++++++++++++ > > > > vl.c | 1 - > > > > 8 files changed, 54 insertions(+), 19 deletions(-) > > > > =20 > > > [...] =20 > > > > diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c > > > > index 6ec14c73ca..4532aa8632 100644 > > > > --- a/hw/xen/xen-common.c > > > > +++ b/hw/xen/xen-common.c > > > > @@ -174,18 +174,21 @@ static GlobalProperty xen_compat_props[] =3D { > > > > .driver =3D "migration", > > > > .property =3D "send-section-footer", > > > > .value =3D "off", > > > > - }, > > > > - { /* end of list */ }, > > > > + } > > > > }; > > > > > > > > static void xen_accel_class_init(ObjectClass *oc, void *data) > > > > { > > > > AccelClass *ac =3D ACCEL_CLASS(oc); > > > > + > > > > ac->name =3D "Xen"; > > > > ac->init_machine =3D xen_init; > > > > ac->setup_post =3D xen_setup_post; > > > > ac->allowed =3D &xen_allowed; > > > > - ac->global_props =3D xen_compat_props; > > > > + ac->compat_props =3D g_ptr_array_new(); =20 > > > where is matching free for that? =20 > > can we at least annotate it somehow so that valgrind won't complain abo= ut this leak? =20 >=20 > If you check my commits on qemu, you should see that I do care (too > much?) about leaks :) >=20 > In this case though, I don't see valgrind or asan complaining, I guess > it's still a reachable reference. > Do you think a FIXME comment would be helpful? I've looked at other cases were we leak, and well we leak a lot so it's probably futile exercise. But the comment won't hurt and will work as remai= nder. > (/me wish we had a proper object system, GObject, but that ship as > long sailed..) >=20 > > =20 > > > =20 > > > > + > > > > + compat_props_add(ac->compat_props, > > > > + xen_compat_props, G_N_ELEMENTS(xen_compat_pro= ps)); > > > > } > > > > > > > > #define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen") > > > > diff --git a/qom/object.c b/qom/object.c > > > > index 17921c0a71..dbdab0aead 100644 > > > > --- a/qom/object.c > > > > +++ b/qom/object.c > > > > @@ -370,6 +370,31 @@ static void object_post_init_with_type(Object = *obj, TypeImpl *ti) > > > > } > > > > } > > > > > > > > +void object_apply_global_props(Object *obj, const GPtrArray *props= , Error **errp) > > > > +{ > > > > + Error *err =3D NULL; > > > > + int i; > > > > + > > > > + if (!props) { > > > > + return; > > > > + } > > > > + > > > > + for (i =3D 0; i < props->len; i++) { > > > > + GlobalProperty *p =3D g_ptr_array_index(props, i); > > > > + > > > > + if (object_dynamic_cast(obj, p->driver) =3D=3D NULL) { > > > > + continue; > > > > + } > > > > + p->used =3D true; > > > > + object_property_parse(obj, p->value, p->property, &err); > > > > + if (err !=3D NULL) { > > > > + error_prepend(&err, "can't apply global %s.%s=3D%s: ", > > > > + p->driver, p->property, p->value); > > > > + error_propagate(errp, err); > > > > + } > > > > + } > > > > +} > > > > + > > > > static void object_initialize_with_type(void *data, size_t size, T= ypeImpl *type) > > > > { > > > > Object *obj =3D data; > > > > diff --git a/vl.c b/vl.c > > > > index a5ae5f23d2..88ba658572 100644 > > > > --- a/vl.c > > > > +++ b/vl.c > > > > @@ -2968,7 +2968,6 @@ static void user_register_global_props(void) > > > > */ > > > > static void register_global_properties(MachineState *ms) > > > > { > > > > - accel_register_compat_props(ms->accelerator); > > > > machine_register_compat_props(ms); > > > > user_register_global_props(); > > > > } =20 > > > > > > =20 > > > > =20 >=20 >=20