On 05.12.2017 11:08, Halil Pasic wrote:
>
>
> 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 cssids"
>>> to the management software (so it can tell are cssids unrestricted or
>>> restricted).
>>>
>>> Signed-off-by: Halil Pasic <pa...@linux.vnet.ibm.com>
>>> ---
>>>
>>> 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[] = {
>>> DEFINE_PROP_END_OF_LIST(),
>>> };
>>>
>>> +static bool prop_get_true(Object *obj, Error **errp)
>>> +{
>>> + return true;
>>> +}
>>> +
>>> static void virtual_css_bridge_class_init(ObjectClass *klass, void *data)
>>> {
>>> HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
>>> @@ -131,6 +136,12 @@ static void virtual_css_bridge_class_init(ObjectClass
>>> *klass, void *data)
>>> hc->unplug = ccw_device_unplug;
>>> set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>>> dc->props = 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 virtual"
>>> + " 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
>>
>
> 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
