Re: [qemu-s390x] [PATCH 2/3] s390x/css: advertise unrestricted cssids

[Thomas Huth]

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 <address@hidden>
> ---
> 
> 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