diff mbox

[3/3] s390x: deprecate s390-squash-mcss machine prop

Message ID 20171201143136.62497-4-pasic@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Halil Pasic Dec. 1, 2017, 2:31 p.m. UTC
With the cssids unrestricted (commit <COMMIT HERE> "s390x/css: unrestrict
cssids") the s390-squash-mcss machine property should not be used.
Actually libvirt never supported this, so the expectation is that
removing it should be pretty painless.  But let's play nice and deprecate
it first.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
---

I would like to reference a commit for "s390x/css: unrestrict cssids"
which is currently in RFC status on the list.
---
 hw/s390x/s390-virtio-ccw.c | 6 +++++-
 qemu-doc.texi              | 8 ++++++++
 qemu-options.hx            | 8 +++++++-
 3 files changed, 20 insertions(+), 2 deletions(-)

Comments

Cornelia Huck Dec. 4, 2017, 11:28 a.m. UTC | #1
On Fri,  1 Dec 2017 15:31:36 +0100
Halil Pasic <pasic@linux.vnet.ibm.com> wrote:

> With the cssids unrestricted (commit <COMMIT HERE> "s390x/css: unrestrict

I won't add the commit id, as I intend to merge this in one go and I
don't want to edit this every time I rebase prior to pull.

> cssids") the s390-squash-mcss machine property should not be used.
> Actually libvirt never supported this, so the expectation is that
> removing it should be pretty painless.  But let's play nice and deprecate
> it first.
> 
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> ---
> 
> I would like to reference a commit for "s390x/css: unrestrict cssids"
> which is currently in RFC status on the list.
> ---
>  hw/s390x/s390-virtio-ccw.c | 6 +++++-
>  qemu-doc.texi              | 8 ++++++++
>  qemu-options.hx            | 8 +++++++-
>  3 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 4d65a50334..3796f666e6 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -553,6 +553,10 @@ static inline void machine_set_squash_mcss(Object *obj, bool value,
>      S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>  
>      ms->s390_squash_mcss = value;
> +    if (ms->s390_squash_mcss) {
> +        warn_report("The machine property 's390-squash-mcss' is deprecated"
> +                    " (obsoleted by lifting the cssid restrictions).");

Too bad that we can't warn when this is explicitly set to false - OTOH
I don't really expect anyone doing that.

> +    }
>  }
>  
>  static inline void s390_machine_initfn(Object *obj)
> @@ -582,7 +586,7 @@ static inline void s390_machine_initfn(Object *obj)
>      object_property_add_bool(obj, "s390-squash-mcss",
>                               machine_get_squash_mcss,
>                               machine_set_squash_mcss, NULL);
> -    object_property_set_description(obj, "s390-squash-mcss",
> +    object_property_set_description(obj, "s390-squash-mcss", "(deprecated) "
>              "enable/disable squashing subchannels into the default css",
>              NULL);
>      object_property_set_bool(obj, false, "s390-squash-mcss", NULL);
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index db2351c746..874432d87c 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -2501,6 +2501,14 @@ enabled via the ``-machine usb=on'' argument.
>  
>  The ``-nodefconfig`` argument is a synonym for ``-no-user-config``.
>  
> +@subsection -machine virtio-ccw,s390-squash-mcss=on|off (since 2.12.0)
> +
> +The ``s390-squash-mcss=on`` property has been obsoleted by allowing the
> +cssid to be chosen freely. Instead of squashing subchannels into the
> +default channel subsystem image for guests that do not support multiple
> +channel subsystems, all devices can be put into the default channel
> +subsystem image.
> +
>  @section qemu-img command line arguments
>  
>  @subsection convert -s (since 2.0.0)
> diff --git a/qemu-options.hx b/qemu-options.hx
> index f11c4ac960..fe0c29271f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -43,7 +43,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>      "                suppress-vmdesc=on|off disables self-describing migration (default=off)\n"
>      "                nvdimm=on|off controls NVDIMM support (default=off)\n"
>      "                enforce-config-section=on|off enforce configuration section migration (default=off)\n"
> -    "                s390-squash-mcss=on|off controls support for squashing into default css (default=off)\n",
> +    "                s390-squash-mcss=on|off (deprecated) controls support for squashing into default css (default=off)\n",
>      QEMU_ARCH_ALL)
>  STEXI
>  @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
> @@ -98,6 +98,12 @@ Enables or disables NVDIMM support. The default is off.
>  @item s390-squash-mcss=on|off
>  Enables or disables squashing subchannels into the default css.
>  The default is off.
> +NOTE: This property is deprecated and will be removed in future releases.
> +The ``s390-squash-mcss=on`` property has been obsoleted by allowing the
> +cssid to be chosen freely. Instead of squashing subchannels into the
> +default channel subsystem image for guests that do not support multiple
> +channel subsystems, all devices can be put into the default channel
> +subsystem image.
>  @item enforce-config-section=on|off
>  If @option{enforce-config-section} is set to @var{on}, force migration
>  code to send configuration section even if the machine-type sets the

Looks sane. We should put a note into the 2.12 changelog as well.
Halil Pasic Dec. 4, 2017, 3:32 p.m. UTC | #2
On 12/04/2017 12:28 PM, Cornelia Huck wrote:
> On Fri,  1 Dec 2017 15:31:36 +0100
> Halil Pasic <pasic@linux.vnet.ibm.com> wrote:
> 
>> With the cssids unrestricted (commit <COMMIT HERE> "s390x/css: unrestrict
> 
> I won't add the commit id, as I intend to merge this in one go and I
> don't want to edit this every time I rebase prior to pull.
> 

I'm fine with dropping the commit id.

>> cssids") the s390-squash-mcss machine property should not be used.
>> Actually libvirt never supported this, so the expectation is that
>> removing it should be pretty painless.  But let's play nice and deprecate
>> it first.
>>
>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
>> ---
>>
>> I would like to reference a commit for "s390x/css: unrestrict cssids"
>> which is currently in RFC status on the list.
>> ---
>>  hw/s390x/s390-virtio-ccw.c | 6 +++++-
>>  qemu-doc.texi              | 8 ++++++++
>>  qemu-options.hx            | 8 +++++++-
>>  3 files changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 4d65a50334..3796f666e6 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -553,6 +553,10 @@ static inline void machine_set_squash_mcss(Object *obj, bool value,
>>      S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>>  
>>      ms->s390_squash_mcss = value;
>> +    if (ms->s390_squash_mcss) {
>> +        warn_report("The machine property 's390-squash-mcss' is deprecated"
>> +                    " (obsoleted by lifting the cssid restrictions).");
> 
> Too bad that we can't warn when this is explicitly set to false - OTOH
> I don't really expect anyone doing that.
> 

I did not really dig deep. It may be possible, but I had other priorities,
and was not sure if it's worth (as I also don't expect it being set to false
explicit in the wild).

>> +    }
>>  }
>>  
>>  static inline void s390_machine_initfn(Object *obj)
>> @@ -582,7 +586,7 @@ static inline void s390_machine_initfn(Object *obj)
>>      object_property_add_bool(obj, "s390-squash-mcss",
>>                               machine_get_squash_mcss,
>>                               machine_set_squash_mcss, NULL);
>> -    object_property_set_description(obj, "s390-squash-mcss",
>> +    object_property_set_description(obj, "s390-squash-mcss", "(deprecated) "
>>              "enable/disable squashing subchannels into the default css",
>>              NULL);
>>      object_property_set_bool(obj, false, "s390-squash-mcss", NULL);
>> diff --git a/qemu-doc.texi b/qemu-doc.texi
>> index db2351c746..874432d87c 100644
>> --- a/qemu-doc.texi
>> +++ b/qemu-doc.texi
>> @@ -2501,6 +2501,14 @@ enabled via the ``-machine usb=on'' argument.
>>  
>>  The ``-nodefconfig`` argument is a synonym for ``-no-user-config``.
>>  
>> +@subsection -machine virtio-ccw,s390-squash-mcss=on|off (since 2.12.0)
>> +
>> +The ``s390-squash-mcss=on`` property has been obsoleted by allowing the
>> +cssid to be chosen freely. Instead of squashing subchannels into the
>> +default channel subsystem image for guests that do not support multiple
>> +channel subsystems, all devices can be put into the default channel
>> +subsystem image.
>> +
>>  @section qemu-img command line arguments
>>  
>>  @subsection convert -s (since 2.0.0)
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index f11c4ac960..fe0c29271f 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -43,7 +43,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
>>      "                suppress-vmdesc=on|off disables self-describing migration (default=off)\n"
>>      "                nvdimm=on|off controls NVDIMM support (default=off)\n"
>>      "                enforce-config-section=on|off enforce configuration section migration (default=off)\n"
>> -    "                s390-squash-mcss=on|off controls support for squashing into default css (default=off)\n",
>> +    "                s390-squash-mcss=on|off (deprecated) controls support for squashing into default css (default=off)\n",
>>      QEMU_ARCH_ALL)
>>  STEXI
>>  @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
>> @@ -98,6 +98,12 @@ Enables or disables NVDIMM support. The default is off.
>>  @item s390-squash-mcss=on|off
>>  Enables or disables squashing subchannels into the default css.
>>  The default is off.
>> +NOTE: This property is deprecated and will be removed in future releases.
>> +The ``s390-squash-mcss=on`` property has been obsoleted by allowing the
>> +cssid to be chosen freely. Instead of squashing subchannels into the
>> +default channel subsystem image for guests that do not support multiple
>> +channel subsystems, all devices can be put into the default channel
>> +subsystem image.
>>  @item enforce-config-section=on|off
>>  If @option{enforce-config-section} is set to @var{on}, force migration
>>  code to send configuration section even if the machine-type sets the
> 
> Looks sane. We should put a note into the 2.12 changelog as well.
> 

I agree. Who would be responsible for updating the changelog. I'm not
familiar with that process yet. To be honest, I wouldn't mind having
the changelog notice in your writing style. Guess would be better for
everyone ;).

There are also other things we identified as TODOs:
* Updating https://wiki.qemu.org/Features/Channel_I/O_Passthrough
(@Dong Jia, could you take this one)
* In tree and/or on wiki documentation which is up-to-date and
more verbose than commit messages are supposed to be (and Dong
Jia's write-up could be incorporated to). I see this one as
lower prio though. Any volunteers?

Many thanks for the guidance!

Halil
Cornelia Huck Dec. 4, 2017, 4:11 p.m. UTC | #3
On Mon, 4 Dec 2017 16:32:16 +0100
Halil Pasic <pasic@linux.vnet.ibm.com> wrote:

> On 12/04/2017 12:28 PM, Cornelia Huck wrote:
> > On Fri,  1 Dec 2017 15:31:36 +0100
> > Halil Pasic <pasic@linux.vnet.ibm.com> wrote:

> >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> >> index 4d65a50334..3796f666e6 100644
> >> --- a/hw/s390x/s390-virtio-ccw.c
> >> +++ b/hw/s390x/s390-virtio-ccw.c
> >> @@ -553,6 +553,10 @@ static inline void machine_set_squash_mcss(Object *obj, bool value,
> >>      S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
> >>  
> >>      ms->s390_squash_mcss = value;
> >> +    if (ms->s390_squash_mcss) {
> >> +        warn_report("The machine property 's390-squash-mcss' is deprecated"
> >> +                    " (obsoleted by lifting the cssid restrictions).");  
> > 
> > Too bad that we can't warn when this is explicitly set to false - OTOH
> > I don't really expect anyone doing that.
> >   
> 
> I did not really dig deep. It may be possible, but I had other priorities,
> and was not sure if it's worth (as I also don't expect it being set to false
> explicit in the wild).

It's not worth spending any more time on this.

> 
> >> +    }
> >>  }
> >>  
> >>  static inline void s390_machine_initfn(Object *obj)

(...)

> > Looks sane. We should put a note into the 2.12 changelog as well.
> >   
> 
> I agree. Who would be responsible for updating the changelog. I'm not
> familiar with that process yet. To be honest, I wouldn't mind having
> the changelog notice in your writing style. Guess would be better for
> everyone ;).

Just a one-liner once we have the 2.12 changelog page.

> 
> There are also other things we identified as TODOs:
> * Updating https://wiki.qemu.org/Features/Channel_I/O_Passthrough
> (@Dong Jia, could you take this one)
> * In tree and/or on wiki documentation which is up-to-date and
> more verbose than commit messages are supposed to be (and Dong
> Jia's write-up could be incorporated to). I see this one as
> lower prio though. Any volunteers?

Long-ish things with links etc. should probably go into the wiki.

Anyway, feel free to go ahead (it's a wiki :) We can always change
things later on.
Dong Jia Shi Dec. 5, 2017, 7:43 a.m. UTC | #4
* Cornelia Huck <cohuck@redhat.com> [2017-12-04 17:11:24 +0100]:

[...]

This one looks good to me too, so:
Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>

> 
> (...)
> 
> > > Looks sane. We should put a note into the 2.12 changelog as well.
> > >   
> > 
> > I agree. Who would be responsible for updating the changelog. I'm not
> > familiar with that process yet. To be honest, I wouldn't mind having
> > the changelog notice in your writing style. Guess would be better for
> > everyone ;).
> 
> Just a one-liner once we have the 2.12 changelog page.
> 
> > 
> > There are also other things we identified as TODOs:
> > * Updating https://wiki.qemu.org/Features/Channel_I/O_Passthrough
> > (@Dong Jia, could you take this one)
Ok. Will do soon.

> > * In tree and/or on wiki documentation which is up-to-date and
> > more verbose than commit messages are supposed to be (and Dong
> > Jia's write-up could be incorporated to). I see this one as
> > lower prio though. Any volunteers?
> 
> Long-ish things with links etc. should probably go into the wiki.
> 
> Anyway, feel free to go ahead (it's a wiki :) We can always change
> things later on.
> 
Let me update the wiki based on my understanding (and style :) firstly,
then you can login and improve it as you wish.
Dong Jia Shi Dec. 5, 2017, 7:48 a.m. UTC | #5
* Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> [2017-12-05 15:43:00 +0800]:

> * Cornelia Huck <cohuck@redhat.com> [2017-12-04 17:11:24 +0100]:
> 
> [...]
> 
> This one looks good to me too, so:
> Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> 
BTW, since we are deprecating s390-squash-mcss, I think any more
improment on it (e.g. more accurate and helpful hint message) is not
worthy. So I will not send such pathes in future, if nobody disagrees.

[...]
Thomas Huth Dec. 5, 2017, 8:41 a.m. UTC | #6
On 01.12.2017 15:31, Halil Pasic wrote:
> With the cssids unrestricted (commit <COMMIT HERE> "s390x/css: unrestrict
> cssids") the s390-squash-mcss machine property should not be used.
> Actually libvirt never supported this, so the expectation is that
> removing it should be pretty painless.  But let's play nice and deprecate
> it first.
> 
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> ---
[...]
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index db2351c746..874432d87c 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -2501,6 +2501,14 @@ enabled via the ``-machine usb=on'' argument.
>  
>  The ``-nodefconfig`` argument is a synonym for ``-no-user-config``.
>  
> +@subsection -machine virtio-ccw,s390-squash-mcss=on|off (since 2.12.0)

The machine is called "s390-ccw-virtio", not "virtio-ccw". Anyway, I
think you could also rather omit the machine name here (since it is the
default) and just write "-machine s390-squash-mcss=on|off".

 Thomas
Halil Pasic Dec. 5, 2017, 12:05 p.m. UTC | #7
On 12/05/2017 09:41 AM, Thomas Huth wrote:
> On 01.12.2017 15:31, Halil Pasic wrote:
>> With the cssids unrestricted (commit <COMMIT HERE> "s390x/css: unrestrict
>> cssids") the s390-squash-mcss machine property should not be used.
>> Actually libvirt never supported this, so the expectation is that
>> removing it should be pretty painless.  But let's play nice and deprecate
>> it first.
>>
>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
>> ---
> [...]
>> diff --git a/qemu-doc.texi b/qemu-doc.texi
>> index db2351c746..874432d87c 100644
>> --- a/qemu-doc.texi
>> +++ b/qemu-doc.texi
>> @@ -2501,6 +2501,14 @@ enabled via the ``-machine usb=on'' argument.
>>  
>>  The ``-nodefconfig`` argument is a synonym for ``-no-user-config``.
>>  
>> +@subsection -machine virtio-ccw,s390-squash-mcss=on|off (since 2.12.0)
> 
> The machine is called "s390-ccw-virtio", not "virtio-ccw". Anyway, I
> think you could also rather omit the machine name here (since it is the
> default) and just write "-machine s390-squash-mcss=on|off".
> 
>  Thomas
> 

Good catch! Regarding omitting the machine name, I was surprised that
it works, but it does. Our documentation is a bit strange on this: we
document the machine name as non-optional:
"-machine [type=]name[,prop=value[,...]]"
but then use -machine prop=value in the deprecation section (that's
with no name).

Since the documentation doesn't make it clear that the property is
existent only for the s390-ccw-virtio family of the machines I think
being your suggestion has a lot of charm. @Connie: do you agree to
removing the machine name (type)?

Thanks!
Halil
Halil Pasic Dec. 5, 2017, 12:13 p.m. UTC | #8
On 12/05/2017 08:48 AM, Dong Jia Shi wrote:
> * Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> [2017-12-05 15:43:00 +0800]:
> 
>> * Cornelia Huck <cohuck@redhat.com> [2017-12-04 17:11:24 +0100]:
>>
>> [...]
>>
>> This one looks good to me too, so:
>> Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
>>
> BTW, since we are deprecating s390-squash-mcss, I think any more
> improment on it (e.g. more accurate and helpful hint message) is not
> worthy. So I will not send such pathes in future, if nobody disagrees.
> 

Makes sense to me.

Halil
Cornelia Huck Dec. 5, 2017, 12:59 p.m. UTC | #9
On Tue, 5 Dec 2017 13:05:11 +0100
Halil Pasic <pasic@linux.vnet.ibm.com> wrote:

> On 12/05/2017 09:41 AM, Thomas Huth wrote:
> > On 01.12.2017 15:31, Halil Pasic wrote:  
> >> With the cssids unrestricted (commit <COMMIT HERE> "s390x/css: unrestrict
> >> cssids") the s390-squash-mcss machine property should not be used.
> >> Actually libvirt never supported this, so the expectation is that
> >> removing it should be pretty painless.  But let's play nice and deprecate
> >> it first.
> >>
> >> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> >> ---  
> > [...]  
> >> diff --git a/qemu-doc.texi b/qemu-doc.texi
> >> index db2351c746..874432d87c 100644
> >> --- a/qemu-doc.texi
> >> +++ b/qemu-doc.texi
> >> @@ -2501,6 +2501,14 @@ enabled via the ``-machine usb=on'' argument.
> >>  
> >>  The ``-nodefconfig`` argument is a synonym for ``-no-user-config``.
> >>  
> >> +@subsection -machine virtio-ccw,s390-squash-mcss=on|off (since 2.12.0)  
> > 
> > The machine is called "s390-ccw-virtio", not "virtio-ccw". Anyway, I
> > think you could also rather omit the machine name here (since it is the
> > default) and just write "-machine s390-squash-mcss=on|off".
> > 
> >  Thomas
> >   
> 
> Good catch! Regarding omitting the machine name, I was surprised that
> it works, but it does. Our documentation is a bit strange on this: we
> document the machine name as non-optional:
> "-machine [type=]name[,prop=value[,...]]"
> but then use -machine prop=value in the deprecation section (that's
> with no name).
> 
> Since the documentation doesn't make it clear that the property is
> existent only for the s390-ccw-virtio family of the machines I think
> being your suggestion has a lot of charm. @Connie: do you agree to
> removing the machine name (type)?

Let's drop the machine name.
Eric Blake Dec. 5, 2017, 2:54 p.m. UTC | #10
On 12/05/2017 06:05 AM, Halil Pasic wrote:

> Good catch! Regarding omitting the machine name, I was surprised that
> it works, but it does. Our documentation is a bit strange on this: we
> document the machine name as non-optional:
> "-machine [type=]name[,prop=value[,...]]"
> but then use -machine prop=value in the deprecation section (that's
> with no name).

QemuOpts at their "finest".  If it sees the first hunk as a value
without an =, then that value is fed to type=name; but you are also free
to make the first hunk be an explicit name=value (whether type=name or
prop=value), at which point it is up to the backend whether type can be
omitted.  So in this case, '-machine prop=value' manages to work just
fine, and it's a question of how you would convey that information in
the --help documentation.

> 
> Since the documentation doesn't make it clear that the property is
> existent only for the s390-ccw-virtio family of the machines I think
> being your suggestion has a lot of charm. @Connie: do you agree to
> removing the machine name (type)?
>
Halil Pasic Dec. 5, 2017, 3:21 p.m. UTC | #11
On 12/05/2017 03:54 PM, Eric Blake wrote:
> On 12/05/2017 06:05 AM, Halil Pasic wrote:
> 
>> Good catch! Regarding omitting the machine name, I was surprised that
>> it works, but it does. Our documentation is a bit strange on this: we
>> document the machine name as non-optional:
>> "-machine [type=]name[,prop=value[,...]]"
>> but then use -machine prop=value in the deprecation section (that's
>> with no name).
> 
> QemuOpts at their "finest".  If it sees the first hunk as a value
> without an =, then that value is fed to type=name; but you are also free
> to make the first hunk be an explicit name=value (whether type=name or
> prop=value), at which point it is up to the backend whether type can be
> omitted.  So in this case, '-machine prop=value' manages to work just
> fine, and it's a question of how you would convey that information in
> the --help documentation.
> 

I would have spontaneously said something like 
-machine [[type=]name | prop=value][,prop=value,[,...]]

It is kind of complicated and it does not precisely reflect what is possible
(one can specify the machine multiple times, and it does not have to be
a prefix of the list for example) but that ain't necessarily a problem.

I'm not saying we should change this. I just wanted to say: wow, this was
new to me.

>>
>> Since the documentation doesn't make it clear that the property is
>> existent only for the s390-ccw-virtio family of the machines I think
>> being your suggestion has a lot of charm. @Connie: do you agree to
>> removing the machine name (type)?
>>
Dong Jia Shi Dec. 12, 2017, 2:05 p.m. UTC | #12
* Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> [2017-12-05 15:43:00 +0800]:

> * Cornelia Huck <cohuck@redhat.com> [2017-12-04 17:11:24 +0100]:
> 
> [...]
> 
> This one looks good to me too, so:
> Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> 
> > 
> > (...)
> > 
> > > > Looks sane. We should put a note into the 2.12 changelog as well.
> > > >   
> > > 
> > > I agree. Who would be responsible for updating the changelog. I'm not
> > > familiar with that process yet. To be honest, I wouldn't mind having
> > > the changelog notice in your writing style. Guess would be better for
> > > everyone ;).
> > 
> > Just a one-liner once we have the 2.12 changelog page.
> > 
> > > 
> > > There are also other things we identified as TODOs:
> > > * Updating https://wiki.qemu.org/Features/Channel_I/O_Passthrough
> > > (@Dong Jia, could you take this one)
> Ok. Will do soon.
> 
> > > * In tree and/or on wiki documentation which is up-to-date and
> > > more verbose than commit messages are supposed to be (and Dong
> > > Jia's write-up could be incorporated to). I see this one as
> > > lower prio though. Any volunteers?
> > 
> > Long-ish things with links etc. should probably go into the wiki.
> > 
> > Anyway, feel free to go ahead (it's a wiki :) We can always change
> > things later on.
> > 
> Let me update the wiki based on my understanding (and style :) firstly,
> then you can login and improve it as you wish.
The following wiki is just updated. Please feel free to make additional
correction:
https://wiki.qemu.org/Features/Channel_I/O_Passthrough

BTW, Conny mentioned before that we should also update this one:
https://wiki.qemu.org/Features/LegacyRemoval

Should it be done now?

@Halil, if you would like to do the update operation, I will leave it to
you. If you want to delegate, I can do it.
Cornelia Huck Dec. 12, 2017, 2:20 p.m. UTC | #13
On Tue, 12 Dec 2017 22:05:24 +0800
Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> wrote:

> * Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> [2017-12-05 15:43:00 +0800]:
> 
> > * Cornelia Huck <cohuck@redhat.com> [2017-12-04 17:11:24 +0100]:
> > 
> > [...]
> > 
> > This one looks good to me too, so:
> > Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> >   
> > > 
> > > (...)
> > >   
> > > > > Looks sane. We should put a note into the 2.12 changelog as well.
> > > > >     
> > > > 
> > > > I agree. Who would be responsible for updating the changelog. I'm not
> > > > familiar with that process yet. To be honest, I wouldn't mind having
> > > > the changelog notice in your writing style. Guess would be better for
> > > > everyone ;).  
> > > 
> > > Just a one-liner once we have the 2.12 changelog page.
> > >   
> > > > 
> > > > There are also other things we identified as TODOs:
> > > > * Updating https://wiki.qemu.org/Features/Channel_I/O_Passthrough
> > > > (@Dong Jia, could you take this one)  
> > Ok. Will do soon.
> >   
> > > > * In tree and/or on wiki documentation which is up-to-date and
> > > > more verbose than commit messages are supposed to be (and Dong
> > > > Jia's write-up could be incorporated to). I see this one as
> > > > lower prio though. Any volunteers?  
> > > 
> > > Long-ish things with links etc. should probably go into the wiki.
> > > 
> > > Anyway, feel free to go ahead (it's a wiki :) We can always change
> > > things later on.
> > >   
> > Let me update the wiki based on my understanding (and style :) firstly,
> > then you can login and improve it as you wish.  
> The following wiki is just updated. Please feel free to make additional
> correction:
> https://wiki.qemu.org/Features/Channel_I/O_Passthrough

Looks good, fixed minor typos.

> 
> BTW, Conny mentioned before that we should also update this one:
> https://wiki.qemu.org/Features/LegacyRemoval
> 
> Should it be done now?

Already done :)
diff mbox

Patch

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 4d65a50334..3796f666e6 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -553,6 +553,10 @@  static inline void machine_set_squash_mcss(Object *obj, bool value,
     S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
 
     ms->s390_squash_mcss = value;
+    if (ms->s390_squash_mcss) {
+        warn_report("The machine property 's390-squash-mcss' is deprecated"
+                    " (obsoleted by lifting the cssid restrictions).");
+    }
 }
 
 static inline void s390_machine_initfn(Object *obj)
@@ -582,7 +586,7 @@  static inline void s390_machine_initfn(Object *obj)
     object_property_add_bool(obj, "s390-squash-mcss",
                              machine_get_squash_mcss,
                              machine_set_squash_mcss, NULL);
-    object_property_set_description(obj, "s390-squash-mcss",
+    object_property_set_description(obj, "s390-squash-mcss", "(deprecated) "
             "enable/disable squashing subchannels into the default css",
             NULL);
     object_property_set_bool(obj, false, "s390-squash-mcss", NULL);
diff --git a/qemu-doc.texi b/qemu-doc.texi
index db2351c746..874432d87c 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2501,6 +2501,14 @@  enabled via the ``-machine usb=on'' argument.
 
 The ``-nodefconfig`` argument is a synonym for ``-no-user-config``.
 
+@subsection -machine virtio-ccw,s390-squash-mcss=on|off (since 2.12.0)
+
+The ``s390-squash-mcss=on`` property has been obsoleted by allowing the
+cssid to be chosen freely. Instead of squashing subchannels into the
+default channel subsystem image for guests that do not support multiple
+channel subsystems, all devices can be put into the default channel
+subsystem image.
+
 @section qemu-img command line arguments
 
 @subsection convert -s (since 2.0.0)
diff --git a/qemu-options.hx b/qemu-options.hx
index f11c4ac960..fe0c29271f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -43,7 +43,7 @@  DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
     "                suppress-vmdesc=on|off disables self-describing migration (default=off)\n"
     "                nvdimm=on|off controls NVDIMM support (default=off)\n"
     "                enforce-config-section=on|off enforce configuration section migration (default=off)\n"
-    "                s390-squash-mcss=on|off controls support for squashing into default css (default=off)\n",
+    "                s390-squash-mcss=on|off (deprecated) controls support for squashing into default css (default=off)\n",
     QEMU_ARCH_ALL)
 STEXI
 @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
@@ -98,6 +98,12 @@  Enables or disables NVDIMM support. The default is off.
 @item s390-squash-mcss=on|off
 Enables or disables squashing subchannels into the default css.
 The default is off.
+NOTE: This property is deprecated and will be removed in future releases.
+The ``s390-squash-mcss=on`` property has been obsoleted by allowing the
+cssid to be chosen freely. Instead of squashing subchannels into the
+default channel subsystem image for guests that do not support multiple
+channel subsystems, all devices can be put into the default channel
+subsystem image.
 @item enforce-config-section=on|off
 If @option{enforce-config-section} is set to @var{on}, force migration
 code to send configuration section even if the machine-type sets the