Re: [qemu-s390x] [Qemu-devel] [RFC PATCH v2 1/1] s390x/css: unrestrict cssids

[Halil Pasic]

On 11/29/2017 06:29 PM, Cornelia Huck wrote:
[..]
>>>> With this change, however, our schema for generating a css bus ids, if
>>>> none is specified does not make much sense. Currently we start at cssid
>>>> 0x0 for non-virtual devices and use the default css (without
>>>> s390-squash-mcss exclusively) for virtual devices.  That means for
>>>> non-virtual device the device would auto-magically end up non-visible for
>>>> guests (which can't use the other channel subsystems).
>>>>
>>>> Thus let us also change the css bus id auto assignment algorithm,
>>>> so that we first fill the default css, and then proceed to the
>>>> next one (modulo MAX_CSSID).  
>>>
>>> Let's reword the previous two paragraphs:
>>>
>>> "At the same time, change our schema for generating css bus ids to put
>>> both virtual and non-virtual devices into the default css (spilling
>>> over into other css images, if needed) so that devices without a
>>> specified id don't end up hidden to guests not supporting multiple
>>> channel subsystems."
>>>   
>>
>> What I don't like about your explanation is, that "so that devices without
>> a specified id don't end up hidden to guests not supporting multiple channel 
>> subsystems" is not necessarily true: if we spill the devices are going
>> to end up hidden.
> 
> Let's make this "don't always end up hidden".

I would prefer to go with this.


At the same time, change our schema for generating css bus ids to put
both virtual and non-virtual devices into the default css (spilling
over into other css images, if needed). The intention is to deprecate
s390-squash-mcss. Whit this change devices without a specified devno won't
end up hidden to guests not supporting multiple channel subsystems, unless
this can not be avoided (default css full).

> 
>>
>>>>
>>>> The adverse effect of getting rid of the restriction on migration should
>>>> not be too severe.  Vfio-ccw devices are not live-migratable yet, and for
>>>> virtual devices using the extra freedom would only make sense with the
>>>> aforementioned guest support in place.
>>>>
>>>> The auto-generated bus ids are affected by both changes. We hope to not


[..]
>>
>> I tired to be quite elaborate, because at some point of the v1
>> discussion, you said if we are planting landmines you want them explained
>> in the commit message. I'm not sure how this document file is supposed
>> to be called, and look like. I think this stuff is relevant for
>> the decision why is this patch a good one, and what are the trade-offs
>> we make. Referencing to a document would be also OK with me.
> 
> I don't think there will be landmines left, no? Or do I misread?
>

The patch basically just got worse with changing the schema. If there
were landmines they are still there. My original point was that it's
bearable in practice, so that's why I had this short and vague ain't
too bad formulation.
 
>>
>> Regarding the deprecation patch. It's already on the list as RFC. You
>> have even commented on it. I intend to make a v2 once we know what are
>> we going to do here.
> 
> This needs to be a patch series with a cover letter. Discussing in
> multiple places is draining.
> 

Can do. No problem for me.

>>
>>>>
>>>> Signed-off-by: Halil Pasic <address@hidden>
>>>>
>>>> ---
>>>> Hi all!
>>>>
>>>> Moving the property to the machine turned out very ugly (IMHO).  Libvirt
>>>> detects machine properties via query-command-line-options.  This is
>>>> however decoupled from the existence of the actual property: one needs to
>>>> touch util/qemu-config.c (see patch) so the property shows up.
>>>> Furthermore this stuff is global. I've also noticed that the infamous
>>>> s390-squash-mcss does not show up.  
>>>
>>> s390x-softmmu/qemu-system-s390x -machine s390-ccw-virtio,help
>>>
>>> shows it, as does qom-list on /machine.  
>>
>> For qom-list you need an instance. Libvirt probes such stuff using
>> (btw probing is done with machine type none, and qom-list /machine
>> should not list squash if we are running machine type none).
>>
>>>
>>> Output of <qemu binary> --help is very haphazard anyway and you should
>>> rely on the interfaces above.
>>>   
>>
>> I disagree. AFAIK management software should probe using the
>> query-command-line-options QMP command. Or am I missing something?
>>
>> I don't speak about the output of <qemu binary> --help here.
> 
> It's the same interface. It both over- and underreports. Querying the
> actual object is the only way to get this reliable. If that is not
> possible today, it needs to be implemented.
>

Sorry I did not look into how <qemu binary> --help is implemented. For
what we are trying to accomplish here it's irrelevant. Wonder why did you
have to bring this up and make the discussion even more complicated.
 
>>
>>>>
>>>> On the other hand to get the property displayed by -machine
>>>> s390-ccw-virtio,help one needs a setter on the property. So I have
>>>> created a fake setter which produces an error each time called.  
>>>
>>> Yes, this is fugly. A css object would be a better place, but way too
>>> much work for now.
>>>   
>>
>> Actually not necessarily. We could simply put this at the virtual-css-bridge.
>> I don't know if Libvirt would accept that though. The problem regarding
>> virtual-css-bridge (and css object) was rw properties: we can't set a value
>> for a property of the virtual-css-bridge on the command line. That was a
>> problem for default-css or whatever but is completely fine for the read
>> only property 'cssid-unrestricted'.
>>
>>>>
>>>> I would strongly prefer putting back the property to the device level!  
>>>
>>> I continue to strongly oppose that. The device is entirely the wrong
>>> level.
>>>   
>>
>> I don't recall you ever explaining why. If it's completely wrong
>> I would have expected Boris and also me being for long enough around
>> to get it at least after the first hint.
> 
> Just once again: This is a property of the whole css/machine, not of
> the individual device.
> 
> No more from me today.
>

Hope there is more form you coming today. :) I would like to know if
I'm on the right track with the machine property. You did not comment
on a couple of my points, among others this question. This libvirt
may not be using the right interface for discovering machine properties,
and the right interface may note even exist in QEMU just got me more
insecure.

Regards,
Halil