From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36284) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eMF6h-0002eh-DK for qemu-devel@nongnu.org; Tue, 05 Dec 2017 10:26:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eMF6c-0000NN-ER for qemu-devel@nongnu.org; Tue, 05 Dec 2017 10:26:03 -0500 References: <20171201143136.62497-1-pasic@linux.vnet.ibm.com> <20171201143136.62497-3-pasic@linux.vnet.ibm.com> <012d3c95-a552-9bfb-38c4-f1bab1bea854@redhat.com> From: Thomas Huth Message-ID: <50f3b271-963e-a8c0-1728-a09950af177d@redhat.com> Date: Tue, 5 Dec 2017 16:25:52 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [qemu-s390x] [PATCH 2/3] s390x/css: advertise unrestricted cssids List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Halil Pasic , Boris Fiuczynski , Cornelia Huck Cc: Shalini Chellathurai Saroja , qemu-devel@nongnu.org, Christian Borntraeger , qemu-s390x@nongnu.org, Dong Jia Shi On 05.12.2017 11:08, Halil Pasic wrote: >=20 >=20 > On 12/05/2017 09:28 AM, Thomas Huth wrote: >> On 01.12.2017 15:31, Halil Pasic wrote: >>> Let us advertise the changes introduced by "s390x/css: unrestrict css= ids" >>> to the management software (so it can tell are cssids unrestricted or >>> restricted). >>> >>> Signed-off-by: Halil Pasic >>> --- >>> >>> Boris says having the property on the virtual-css-bridge is good form >>> Libvirt PoV. @Shalini: could you verify that things work out fine >>> (provided we get at least a preliminary blessing from Connie). >>> >>> Consider squashing into "s390x/css: unrestrict cssids". >>> --- >>> hw/s390x/css-bridge.c | 11 +++++++++++ >>> 1 file changed, 11 insertions(+) >>> >>> diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c >>> index c4a9735d71..c7e8998680 100644 >>> --- a/hw/s390x/css-bridge.c >>> +++ b/hw/s390x/css-bridge.c >>> @@ -123,6 +123,11 @@ static Property virtual_css_bridge_properties[] = =3D { >>> DEFINE_PROP_END_OF_LIST(), >>> }; >>> =20 >>> +static bool prop_get_true(Object *obj, Error **errp) >>> +{ >>> + return true; >>> +} >>> + >>> static void virtual_css_bridge_class_init(ObjectClass *klass, void *= data) >>> { >>> HotplugHandlerClass *hc =3D HOTPLUG_HANDLER_CLASS(klass); >>> @@ -131,6 +136,12 @@ static void virtual_css_bridge_class_init(Object= Class *klass, void *data) >>> hc->unplug =3D ccw_device_unplug; >>> set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); >>> dc->props =3D virtual_css_bridge_properties; >>> + object_class_property_add_bool(klass, "cssid-unrestricted", >>> + prop_get_true, NULL, NULL); >>> + object_class_property_set_description(klass, "cssid-unrestricted= ", >>> + "A css device can use any cssid, regardless whether vir= tual" >>> + " or not (read only, always true)", >> >> I'd maybe remove the "always true" in the description here, since that >> might create wrong assumptions with regards to future versions or lead >> to bad code in the upper layers. If someone reads "always true", they >> simply might omit the check of the value of this property. If we then >> ever want to change it to "false" again, we're in trouble (sure, we >> could simply completely remove the property again, but we have to >> remember to do that instead of setting it to false ... so let's better >> play safe right now already, ok?) >> >> Thomas >> >=20 > Libvirt intends to check for the existence of the property and ignore > it's value. OK, I wasn't aware of the fact that libvirt normally only checks for the presence, but not for the value. Then yes, please keep the "always true" in the comment. Thomas