unrestrict cssids related patches

The default css 0xfe is currently restricted to virtual subchannel devices. The hope when the decision was made was, that non-virtual subchannel devices will come around when guests can exploit multiple channel subsystems. Since current guests don't do that, the pain of the partitioned (cssid) namespace outweighs the gain.

The default css 0xfe is currently restricted to virtual subchannel devices. The hope when the decision was made was, that non-virtual subchannel devices will come around when guest can exploit multiple channel subsystems. Since the guests generally don't do, the pain of the partitioned (cssid) namespace outweighs the gain.

Let us remove the corresponding restrictions (virtual devices can be put only in 0xfe and non-virtual devices in any css except the 0xfe -- while s390-squash-mcss then remaps everything to cssid 0).

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).

Deprecaton of s390-squash-mcss and indicating the changes via QMP is expected to follow soon (as separate commits).

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 encounter any auto-generated bus ids in production as Libvirt is always explicit about the bus id. Since 8ed179c937 ("s390x/css: catch section mismatch on load", 2017-05-18) the worst that can happen because the same device ended up having a different bus id is a cleanly failed migration. I find it hard to reason about the impact of changed auto-generated bus ids on migration for command line users as I don't know which rules is such an user supposed to follow.

Another pain-point is down- or upgrade of QEMU for command line users. The old way and the new way of doing vfio-ccw are mutually incompatible. Libvirt is only going to support the new way, so for libvirt users, the possible problems at QEMU downgrade are the following. If a domain contains virtual devices placed into a css different than 0xfe the domain will refuse to start with a QEMU not having this patch. Putting devices into a css different that 0xfe however won't make much sense in the near future (guest support). Libvirt will refuse to do vfio-ccw with a QEMU not having this patch. This is business as usual.

Signed-off-by Halil Pasic <[email protected]>

---
Hi!

I've factored out the announcing via QMP interface stuff to ease review. I would not mind the two being squashed together before this hits main, as I would much prefer having the two as one (atomic) change. But the second part turned out so controversial, that splitting is expected to benefit the review process.

v2 -> v3:
* factored out announcing into a separate patch
* reworded commit message
* removed outdated comment about squash

v1 -> v2:
* changed ccw bus id generation too (see commit message)
* moved the property to the machine (see cover letter)
* added a description to the property

---
 hw/s390x/3270-ccw.c        |  2 +-
 hw/s390x/css.c             | 28 ++++------------------------
 hw/s390x/s390-ccw.c        |  2 +-
 hw/s390x/s390-virtio-ccw.c |  1 -
 hw/s390x/virtio-ccw.c      |  2 +-
 include/hw/s390x/css.h     | 12 ++++--------
 6 files changed, 11 insertions(+), 36 deletions(-)

hw/s390x/css.c +0 -4

@@ -2396,19 +2386,8 @@ SubchDev *css_create_sch(CssDevId bus_id, bool is_virtual, bool squash_mcss,
                                            bus_id.devid, &schid, errp)) {
             return NULL;
         }
-    } else if (squash_mcss || is_virtual) {
-        bus_id.cssid = channel_subsys.default_cssid;
-
-        if (!css_find_free_subch_and_devno(bus_id.cssid, &bus_id.ssid,

Quoted 3 lines

Signed-off-by Halil Pasic <[email protected]>

Quoted 6 lines · On Fri, 1 Dec 2017 15:31:34 +0100 Halil Pasic <[email protected]> wrote:

The default css 0xfe is currently restricted to virtual subchannel devices. The hope when the decision was made was, that non-virtual subchannel devices will come around when guests can exploit multiple channel subsystems. Since current guests don't do that, the pain of the partitioned (cssid) namespace outweighs the gain.

Quoted 5 lines

The default css 0xfe is currently restricted to virtual subchannel devices. The hope when the decision was made was, that non-virtual subchannel devices will come around when guest can exploit multiple channel subsystems. Since the guests generally don't do, the pain of the partitioned (cssid) namespace outweighs the gain.

Doubled paragraph?

Quoted 9 lines

Let us remove the corresponding restrictions (virtual devices can be put only in 0xfe and non-virtual devices in any css except the 0xfe -- while s390-squash-mcss then remaps everything to cssid 0).

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

s/Whit/With/

Quoted 5 lines

won't end up hidden to guests not supporting multiple channel subsystems, unless this can not be avoided (default css full).

Deprecaton of s390-squash-mcss and indicating the changes via QMP is

Quoted 1 line

expected to follow soon (as separate commits).

Let's drop this paragraph (the qmp interface should be squashed in, and you mention the deprecation right above.)

Quoted 6 lines

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.

Quoted 8 lines

The auto-generated bus ids are affected by both changes. We hope to not encounter any auto-generated bus ids in production as Libvirt is always explicit about the bus id. Since 8ed179c937 ("s390x/css: catch section mismatch on load", 2017-05-18) the worst that can happen because the same device ended up having a different bus id is a cleanly failed migration. I find it hard to reason about the impact of changed auto-generated bus ids on migration for command line users as I don't know which rules is such an user supposed to follow.

Should we document somewhere that guests supposed to be migrated should make sure that they use explicit devnos?

Quoted 10 lines

Another pain-point is down- or upgrade of QEMU for command line users. The old way and the new way of doing vfio-ccw are mutually incompatible. Libvirt is only going to support the new way, so for libvirt users, the possible problems at QEMU downgrade are the following. If a domain contains virtual devices placed into a css different than 0xfe the domain will refuse to start with a QEMU not having this patch. Putting devices into a css different that 0xfe however won't make much sense in the near future (guest support). Libvirt will refuse to do vfio-ccw with a QEMU not having this patch. This is business as usual.

My writing style would be to have this as a shorter, bulleted list - but no need to rewrite this if this is understandable to the others on cc:

Quoted 39 lines

Signed-off-by Halil Pasic <[email protected]>

---
Hi!

I've factored out the announcing via QMP interface stuff to ease review. I would not mind the two being squashed together before this hits main, as I would much prefer having the two as one (atomic) change. But the second part turned out so controversial, that splitting is expected to benefit the review process.

v2 -> v3:
* factored out announcing into a separate patch
* reworded commit message
* removed outdated comment about squash

v1 -> v2:
* changed ccw bus id generation too (see commit message)
* moved the property to the machine (see cover letter)
* added a description to the property

---
 hw/s390x/3270-ccw.c        |  2 +-
 hw/s390x/css.c             | 28 ++++------------------------
 hw/s390x/s390-ccw.c        |  2 +-
 hw/s390x/s390-virtio-ccw.c |  1 -
 hw/s390x/virtio-ccw.c      |  2 +-
 include/hw/s390x/css.h     | 12 ++++--------
 6 files changed, 11 insertions(+), 36 deletions(-)

hw/s390x/css.c +0 -4

@@ -2396,19 +2386,8 @@ SubchDev *css_create_sch(CssDevId bus_id, bool is_virtual, bool squash_mcss,
                                            bus_id.devid, &schid, errp)) {
             return NULL;
         }
-    } else if (squash_mcss || is_virtual) {
-        bus_id.cssid = channel_subsys.default_cssid;
-
-        if (!css_find_free_subch_and_devno(bus_id.cssid, &bus_id.ssid,

hw/s390x/css.c SubchDev *css_create_sch(CssDevId bus_id, bool is_virtual, bool squash_mcss, · line 2390

Quoted 10 lines

hw/s390x/css.c +1 -8

-                                           &bus_id.devid, &schid, errp)) {
-            return NULL;
-        }
     } else {
-        for (bus_id.cssid = 0; bus_id.cssid < MAX_CSSID; ++bus_id.cssid) {
-            if (bus_id.cssid == VIRTUAL_CSSID) {
-                continue;
-            }
-
+        for (bus_id.cssid = channel_subsys.default_cssid;;) {

This looks a bit ugly, but I don't see another compact way to do this.

hw/s390x/css.c SubchDev *css_create_sch(CssDevId bus_id, bool is_virtual, bool squash_mcss, · line 2404

Quoted 13 lines

hw/s390x/css.c +2 -1

             if (!channel_subsys.css[bus_id.cssid]) {
                 css_create_css_image(bus_id.cssid, false);
             }

@@ -2418,7 +2397,8 @@ SubchDev *css_create_sch(CssDevId bus_id, bool is_virtual, bool squash_mcss,
                                                 NULL)) {
                 break;
             }
-            if (bus_id.cssid == MAX_CSSID) {
+            bus_id.cssid = (bus_id.cssid + 1) % MAX_CSSID;
+            if (bus_id.cssid == channel_subsys.default_cssid) {
                 error_setg(errp, "Virtual channel subsystem is full!");
                 return NULL;
             }

The interface exposing this change definitely needs to be squashed into this patch, but else looks good.

Quoted 9 lines · Cornelia Huck

Quoted 6 lines · On Fri, 1 Dec 2017 15:31:34 +0100 Halil Pasic <[email protected]> wrote:

The default css 0xfe is currently restricted to virtual subchannel devices. The hope when the decision was made was, that non-virtual subchannel devices will come around when guests can exploit multiple channel subsystems. Since current guests don't do that, the pain of the partitioned (cssid) namespace outweighs the gain.

Quoted 8 lines

Quoted 5 lines

The default css 0xfe is currently restricted to virtual subchannel devices. The hope when the decision was made was, that non-virtual subchannel devices will come around when guest can exploit multiple channel subsystems. Since the guests generally don't do, the pain of the partitioned (cssid) namespace outweighs the gain.

Doubled paragraph?

Yep. Copy paste mistake.

Quoted 11 lines

Quoted 9 lines

Let us remove the corresponding restrictions (virtual devices can be put only in 0xfe and non-virtual devices in any css except the 0xfe -- while s390-squash-mcss then remaps everything to cssid 0).

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

s/Whit/With/

Nod

Quoted 5 lines

Quoted 5 lines

won't end up hidden to guests not supporting multiple channel subsystems, unless this can not be avoided (default css full).

Deprecaton of s390-squash-mcss and indicating the changes via QMP is

s/Deprecaton/Deprecation/

Quoted 20 lines

Quoted 1 line

expected to follow soon (as separate commits).

Let's drop this paragraph (the qmp interface should be squashed in, and you mention the deprecation right above.)

Quoted 6 lines

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.

Quoted 8 lines

The auto-generated bus ids are affected by both changes. We hope to not encounter any auto-generated bus ids in production as Libvirt is always explicit about the bus id. Since 8ed179c937 ("s390x/css: catch section mismatch on load", 2017-05-18) the worst that can happen because the same device ended up having a different bus id is a cleanly failed migration. I find it hard to reason about the impact of changed auto-generated bus ids on migration for command line users as I don't know which rules is such an user supposed to follow.

Quoted 3 lines

Should we document somewhere that guests supposed to be migrated should make sure that they use explicit devnos?

I think having a document collecting such migration rules and best practices for command line users (and implicitly also for implementers of management software) would be a good idea. Maybe there is such a documentation, but I don't know where. The devnos should be a part of it for sure. But I'm not volunteering for creating this kind of documentation. Natural languages aren't my forte.

Quoted 12 lines

Quoted 11 lines

Another pain-point is down- or upgrade of QEMU for command line users. The old way and the new way of doing vfio-ccw are mutually incompatible. Libvirt is only going to support the new way, so for libvirt users, the possible problems at QEMU downgrade are the following. If a domain contains virtual devices placed into a css different than 0xfe the domain will refuse to start with a QEMU not having this patch. Putting devices into a css different that 0xfe however won't make much sense in the near future (guest support). Libvirt will refuse to do vfio-ccw with a QEMU not having this patch. This is business as usual.

Quoted 4 lines

My writing style would be to have this as a shorter, bulleted list - but no need to rewrite this if this is understandable to the others on cc:

If you want, we can iterate on the description. My primary concern was to agree on how to advertise this change.

Quoted 3 lines

Quoted 3 lines

Signed-off-by Halil Pasic <[email protected]>

---
 hw/s390x/3270-ccw.c        |  2 +-
 hw/s390x/css.c             | 28 ++++------------------------
 hw/s390x/s390-ccw.c        |  2 +-
 hw/s390x/s390-virtio-ccw.c |  1 -
 hw/s390x/virtio-ccw.c      |  2 +-
 include/hw/s390x/css.h     | 12 ++++--------
 6 files changed, 11 insertions(+), 36 deletions(-)

hw/s390x/3270-ccw.c +1 -1

index 081e3ef6f4..3af13ea027 100644
@@ -104,7 +104,7 @@ static void emulated_ccw_3270_realize(DeviceState *ds, Error **errp)
     SubchDev *sch;
     Error *err = NULL;
 
-    sch = css_create_sch(cdev->devno, true, cbus->squash_mcss, errp);
+    sch = css_create_sch(cdev->devno, cbus->squash_mcss, errp);
     if (!sch) {
         return;
     }

hw/s390x/css.c +4 -24

index f6b5c807cd..cd26f32050 100644
@@ -2370,22 +2370,12 @@ const PropertyInfo css_devid_ro_propinfo = {
     .get = get_css_devid,
 };
 
-SubchDev *css_create_sch(CssDevId bus_id, bool is_virtual, bool squash_mcss,
-                         Error **errp)
+SubchDev *css_create_sch(CssDevId bus_id, bool squash_mcss, Error **errp)
 {
     uint16_t schid = 0;
     SubchDev *sch;
 
     if (bus_id.valid) {
-        if (is_virtual != (bus_id.cssid == VIRTUAL_CSSID)) {
-            error_setg(errp, "cssid %hhx not valid for %s devices",
-                       bus_id.cssid,
-                       (is_virtual ? "virtual" : "non-virtual"));
-            return NULL;
-        }
-    }
-
-    if (bus_id.valid) {
         if (squash_mcss) {
             bus_id.cssid = channel_subsys.default_cssid;
         } else if (!channel_subsys.css[bus_id.cssid]) {

@@ -2396,19 +2386,8 @@ SubchDev *css_create_sch(CssDevId bus_id, bool is_virtual, bool squash_mcss,
                                            bus_id.devid, &schid, errp)) {
             return NULL;
         }
-    } else if (squash_mcss || is_virtual) {
-        bus_id.cssid = channel_subsys.default_cssid;
-
-        if (!css_find_free_subch_and_devno(bus_id.cssid, &bus_id.ssid,
-                                           &bus_id.devid, &schid, errp)) {
-            return NULL;
-        }
     } else {
-        for (bus_id.cssid = 0; bus_id.cssid < MAX_CSSID; ++bus_id.cssid) {
-            if (bus_id.cssid == VIRTUAL_CSSID) {
-                continue;
-            }
-
+        for (bus_id.cssid = channel_subsys.default_cssid;;) {
             if (!channel_subsys.css[bus_id.cssid]) {
                 css_create_css_image(bus_id.cssid, false);
             }

@@ -2418,7 +2397,8 @@ SubchDev *css_create_sch(CssDevId bus_id, bool is_virtual, bool squash_mcss,
                                                 NULL)) {
                 break;
             }
-            if (bus_id.cssid == MAX_CSSID) {
+            bus_id.cssid = (bus_id.cssid + 1) % MAX_CSSID;
+            if (bus_id.cssid == channel_subsys.default_cssid) {
                 error_setg(errp, "Virtual channel subsystem is full!");
                 return NULL;
             }

hw/s390x/s390-ccw.c +1 -1

index 0ef232ec27..4a9d4d2534 100644
@@ -77,7 +77,7 @@ static void s390_ccw_realize(S390CCWDevice *cdev, char *sysfsdev, Error **errp)
         goto out_err_propagate;
     }
 
-    sch = css_create_sch(ccw_dev->devno, false, cbus->squash_mcss, &err);
+    sch = css_create_sch(ccw_dev->devno, cbus->squash_mcss, &err);
     if (!sch) {
         goto out_mdevid_free;
     }

hw/s390x/s390-virtio-ccw.c +0 -1

index 6a57f94197..4d65a50334 100644
@@ -302,7 +302,6 @@ static void ccw_init(MachineState *machine)
     /*
      * Non mcss-e enabled guests only see the devices from the default
      * css, which is determined by the value of the squash_mcss property.
-     * Note: we must not squash non virtual devices to css 0xFE.
      */
     if (css_bus->squash_mcss) {
         ret = css_create_css_image(0, true);

hw/s390x/virtio-ccw.c +1 -1

index 184515ce94..3dd902a664 100644
@@ -701,7 +701,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
     SubchDev *sch;
     Error *err = NULL;
 
-    sch = css_create_sch(ccw_dev->devno, true, cbus->squash_mcss, errp);
+    sch = css_create_sch(ccw_dev->devno, cbus->squash_mcss, errp);
     if (!sch) {
         return;
     }

include/hw/s390x/css.h +4 -8

index ab6ebe66b5..53c270a216 100644
@@ -272,12 +272,9 @@ extern const PropertyInfo css_devid_ro_propinfo;
  * default css image for it.
  * If @p bus_id is valid, and @p squash_mcss is false, verify that it is
  * not already in use, and find a free devno for it.
- * If @p bus_id is not valid, and if either @p squash_mcss or @p is_virtual
- * is true, find a free subchannel id and device number across all
- * subchannel sets from the default css image.
- * If @p bus_id is not valid, and if both @p squash_mcss and @p is_virtual
- * are false, find a non-full css image and find a free subchannel id and
- * device number across all subchannel sets from it.
+ * If @p bus_id is not valid find a free subchannel id and device number
+ * across all subchannel sets and all css images starting from the default
+ * css image.
  *
  * If either of the former actions succeed, allocate a subchannel structure,
  * initialise it with the bus id, subchannel id and device number, register

@@ -286,8 +283,7 @@ extern const PropertyInfo css_devid_ro_propinfo;
  * The caller becomes owner of the returned subchannel structure and
  * is responsible for unregistering and freeing it.
  */
-SubchDev *css_create_sch(CssDevId bus_id, bool is_virtual, bool squash_mcss,
-                         Error **errp);
+SubchDev *css_create_sch(CssDevId bus_id, bool squash_mcss, Error **errp);
 
 /** Turn on css migration */
 void css_register_vmstate(void);

Signature · 1 line

2.13.5

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 <[email protected]>

---

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(+)

hw/s390x/css-bridge.c +11 -0

index c4a9735d71..c7e8998680 100644
@@ -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)",
+            NULL);
 }
 
 static const TypeInfo virtual_css_bridge_info = {

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 <[email protected]>

---

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(+)

hw/s390x/css-bridge.c +5 -0

index c4a9735d71..c7e8998680 100644
@@ -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);

Quoted 34 lines · On Fri, 1 Dec 2017 15:31:35 +0100 Halil Pasic <[email protected]> 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 <[email protected]>

---

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(+)

hw/s390x/css-bridge.c +5 -0

index c4a9735d71..c7e8998680 100644
@@ -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;

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 142

Quoted 6 lines

hw/s390x/css-bridge.c +4 -0

     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"

extra space -----------------------------^

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 143

Quoted 1 line

hw/s390x/css-bridge.c +1 -0

+            " or not (read only, always true)",

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 147

Quoted 4 lines

hw/s390x/css-bridge.c +1 -0

+            NULL);
 }
 
 static const TypeInfo virtual_css_bridge_info = {

Looks reasonable. If this works as expected, I'll squash it into the previous patch.

Quoted 37 lines · Cornelia Huck

Quoted 34 lines · On Fri, 1 Dec 2017 15:31:35 +0100 Halil Pasic <[email protected]> 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 <[email protected]>

---

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(+)

hw/s390x/css-bridge.c +5 -0

index c4a9735d71..c7e8998680 100644
@@ -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;

Quoted 9 lines

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 142

Quoted 6 lines

hw/s390x/css-bridge.c +4 -0

     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"  

extra space -----------------------------^

Nod.

Quoted 1 line

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 143

Quoted 1 line

hw/s390x/css-bridge.c +1 -0

+            " or not (read only, always true)",  

Do we need "." here ----------------------------^ ?

Quoted 6 lines

Quoted 5 lines

hw/s390x/css-bridge.c +1 -0

+            NULL);
 }
 
 static const TypeInfo virtual_css_bridge_info = {  

Quoted 3 lines

Looks reasonable. If this works as expected, I'll squash it into the previous patch.

I've just asked Shalini to verify the libvirt perspective.

Supposed we verify this works as expected, I read I don't have to spin a v2 and you are going to fix the issues found yourself. Right?

Quoted 48 lines · On Mon, 4 Dec 2017 16:07:27 +0100 Halil Pasic <[email protected]> wrote:

Quoted 37 lines · Cornelia Huck

Quoted 34 lines · On Fri, 1 Dec 2017 15:31:35 +0100 Halil Pasic <[email protected]> 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 <[email protected]>

---

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(+)

hw/s390x/css-bridge.c +5 -0

index c4a9735d71..c7e8998680 100644
@@ -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;

Quoted 9 lines

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 142

Quoted 6 lines

hw/s390x/css-bridge.c +4 -0

     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"  

extra space -----------------------------^

Quoted 5 lines

Nod.

Quoted 1 line

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 143

Quoted 1 line

hw/s390x/css-bridge.c +1 -0

+            " or not (read only, always true)",  

Do we need "." here ----------------------------^ ?

None of the other descs do that.

Quoted 10 lines

Quoted 9 lines

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 147

Quoted 5 lines

hw/s390x/css-bridge.c +1 -0

+            NULL);
 }
 
 static const TypeInfo virtual_css_bridge_info = {  

Looks reasonable. If this works as expected, I'll squash it into the previous patch.

Quoted 4 lines

I've just asked Shalini to verify the libvirt perspective.

Supposed we verify this works as expected, I read I don't have to spin a v2 and you are going to fix the issues found yourself. Right?

I'd prefer a v2 that I can simply apply. Let's wait for some more acks/r-bs.

Quoted 37 lines · Cornelia Huck

Quoted 34 lines · On Fri, 1 Dec 2017 15:31:35 +0100 Halil Pasic <[email protected]> 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 <[email protected]>

---

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(+)

hw/s390x/css-bridge.c +5 -0

index c4a9735d71..c7e8998680 100644
@@ -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;

Quoted 9 lines

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 142

Quoted 6 lines

hw/s390x/css-bridge.c +4 -0

     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"

extra space -----------------------------^

Nod.

Quoted 1 line

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 143

Quoted 1 line

hw/s390x/css-bridge.c +1 -0

+            " or not (read only, always true)",

Do we need "." here ----------------------------^ ?

Quoted 5 lines

Quoted 4 lines

hw/s390x/css-bridge.c +1 -0

+            NULL);
 }
 
 static const TypeInfo virtual_css_bridge_info = {

Quoted 3 lines

Looks reasonable. If this works as expected, I'll squash it into the previous patch.

I've just asked Shalini to verify the libvirt perspective.

Supposed we verify this works as expected, I read I don't have to spin a v2 and you are going to fix the issues found yourself. Right?

Quoted 36 lines · Cornelia Huck

Quoted 33 lines · On Fri, 1 Dec 2017 15:31:35 +0100 Halil Pasic <[email protected]> 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 <[email protected]>

---

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(+)

hw/s390x/css-bridge.c +5 -0

index c4a9735d71..c7e8998680 100644
@@ -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)

Quoted 9 lines

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 142

Quoted 7 lines

hw/s390x/css-bridge.c +4 -0

      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"

extra space -----------------------------^

Nod.

Quoted 1 line

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 143

Quoted 1 line

hw/s390x/css-bridge.c +1 -0

+            " or not (read only, always true)",

Do we need "." here ----------------------------^ ?

Quoted 7 lines

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 147

Quoted 4 lines

hw/s390x/css-bridge.c +1 -0

+            NULL);
  }
 
  static const TypeInfo virtual_css_bridge_info = {

Looks reasonable. If this works as expected, I'll squash it into the previous patch.

I've just asked Shalini to verify the libvirt perspective.

Supposed we verify this works as expected, I read I don't have to spin a v2 and you are going to fix the issues found yourself. Right?

Quoted 7 lines · Halil Pasic

Quoted 3 lines · Cornelia Huck

Looks reasonable. If this works as expected, I'll squash it into the previous patch.

I've just asked Shalini to verify the libvirt perspective.

Verified. The patch works as expected.

Quoted 54 lines · Halil Pasic

Quoted 35 lines · Cornelia Huck

Quoted 32 lines · On Fri,  1 Dec 2017 15:31:35 +0100 Halil Pasic <[email protected]> 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 <[email protected]>

---

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(+)

hw/s390x/css-bridge.c +4 -0

index c4a9735d71..c7e8998680 100644
@@ -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)

Quoted 9 lines

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 142

Quoted 7 lines

hw/s390x/css-bridge.c +4 -0

       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"

extra space -----------------------------^

Nod.

Quoted 1 line

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 143

Quoted 1 line

hw/s390x/css-bridge.c +1 -0

+            " or not (read only, always true)",

Do we need "." here ----------------------------^ ?

Quoted 3 lines

Quoted 3 lines

hw/s390x/css-bridge.c +1 -0

+            NULL);
   }
     static const TypeInfo virtual_css_bridge_info = {

Quoted 4 lines

Quoted 3 lines

Looks reasonable. If this works as expected, I'll squash it into the previous patch.

I've just asked Shalini to verify the libvirt perspective.

Verified. The patch works as expected.

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 <[email protected]>

---

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(+)

hw/s390x/css-bridge.c +5 -0

index c4a9735d71..c7e8998680 100644
@@ -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);

Quoted 33 lines · Halil Pasic

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 <[email protected]>

---

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(+)

hw/s390x/css-bridge.c +5 -0

index c4a9735d71..c7e8998680 100644
@@ -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);

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 143

Quoted 8 lines

hw/s390x/css-bridge.c +5 -0

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

Quoted 52 lines · Thomas Huth

Quoted 33 lines · Halil Pasic

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 <[email protected]>

---

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(+)

hw/s390x/css-bridge.c +5 -0

index c4a9735d71..c7e8998680 100644
@@ -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);

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 143

Quoted 8 lines

hw/s390x/css-bridge.c +5 -0

@@ -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?)

Quoted 2 lines

Thomas

Libvirt intends to check for the existence of the property and ignore it's value. I've been told in Libvirt capabilities are usually tied to existence. For inspecting the value one would have to work on an instance. I don't think that would work with Libvirt's capability probing scheme well.

So what you describe is basically like intended. I was not to document always true though, but then Connie had a version with always true and I got convinced it ain't a bad idea. If both Connie and you agree that 'always true' is to be dropped I'm fine with dropping it.

Quoted 52 lines · Thomas Huth

Quoted 33 lines · Halil Pasic

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 <[email protected]>

---

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(+)

hw/s390x/css-bridge.c +5 -0

index c4a9735d71..c7e8998680 100644
@@ -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);

hw/s390x/css-bridge.c static void virtual_css_bridge_class_init(ObjectClass *klass, void *data) · line 143

Quoted 8 lines

hw/s390x/css-bridge.c +5 -0

@@ -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?)

Quoted 2 lines

Thomas

Libvirt intends to check for the existence of the property and ignore it's value.

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 <[email protected]>

---

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(-)

hw/s390x/s390-virtio-ccw.c +0 -0

index 4d65a50334..3796f666e6 100644

hw/s390x/s390-virtio-ccw.c +2 -1

+    }
 }
 
 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);

qemu-doc.texi +8 -0

index db2351c746..874432d87c 100644
@@ -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)

qemu-options.hx +1 -1

index f11c4ac960..fe0c29271f 100644
@@ -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.

Quoted 1 line · On Fri, 1 Dec 2017 15:31:36 +0100 Halil Pasic <[email protected]> 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.

Quoted 23 lines

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 <[email protected]>

---

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(-)

hw/s390x/s390-virtio-ccw.c +0 -0

index 4d65a50334..3796f666e6 100644
@@ -553,6 +553,10 @@ static inline void machine_set_squash_mcss(Object *obj, bool value,
     S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
 

hw/s390x/s390-virtio-ccw.c static inline void machine_set_squash_mcss(Object *obj, bool value, · line 558

Quoted 4 lines

hw/s390x/s390-virtio-ccw.c +3 -0

     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.

Quoted 52 lines

hw/s390x/s390-virtio-ccw.c +2 -1

+    }
 }
 
 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);

qemu-doc.texi +8 -0

index db2351c746..874432d87c 100644
@@ -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)

qemu-options.hx +4 -1

index f11c4ac960..fe0c29271f 100644
@@ -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

qemu-options.hx Enables or disables NVDIMM support. The default is off. · line 109

Quoted 6 lines

qemu-options.hx +3 -0

+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.

Quoted 10 lines · Cornelia Huck

Quoted 7 lines · On Fri, 1 Dec 2017 15:31:36 +0100 Halil Pasic <[email protected]> wrote:

hw/s390x/s390-virtio-ccw.c +0 -0

index 4d65a50334..3796f666e6 100644
@@ -553,6 +553,10 @@ static inline void machine_set_squash_mcss(Object *obj, bool value,
     S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
 

Quoted 8 lines

hw/s390x/s390-virtio-ccw.c static inline void machine_set_squash_mcss(Object *obj, bool value, · line 558

Quoted 4 lines

hw/s390x/s390-virtio-ccw.c +3 -0

     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).

Quoted 5 lines

Quoted 5 lines

hw/s390x/s390-virtio-ccw.c +1 -0

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

Quoted 2 lines

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?

(...)

Quoted 3 lines

Quoted 2 lines

Looks sane. We should put a note into the 2.12 changelog as well.

Quoted 4 lines

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.

Quoted 4 lines

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)

Quoted 4 lines

* 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.

* Cornelia Huck <[email protected]> [2017-12-04 17:11:24 +0100]:

[...]

This one looks good to me too, so:

Reviewed-by Dong Jia Shi <[email protected]>

Quoted 2 lines · Dong Jia Shi

* Cornelia Huck <[email protected]> [2017-12-04 17:11:24 +0100]:

Quoted 5 lines

[...]

This one looks good to me too, so:

Reviewed-by Dong Jia Shi <[email protected]>

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.

* Cornelia Huck <[email protected]> [2017-12-04 17:11:24 +0100]:

[...]

This one looks good to me too, so:

Reviewed-by Dong Jia Shi <[email protected]>

Quoted 6 lines

(...)

Quoted 3 lines

Quoted 2 lines

Looks sane. We should put a note into the 2.12 changelog as well.

Quoted 11 lines

Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 5 lines

Quoted 4 lines

* 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?

Quoted 5 lines

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.

Quoted 33 lines · Dong Jia Shi

* Cornelia Huck <[email protected]> [2017-12-04 17:11:24 +0100]:

[...]

This one looks good to me too, so:

Reviewed-by Dong Jia Shi <[email protected]>

Quoted 6 lines

(...)

Quoted 3 lines

Quoted 2 lines

Looks sane. We should put a note into the 2.12 changelog as well.

Quoted 11 lines

Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 7 lines

Quoted 4 lines

* 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.

Quoted 5 lines

Quoted 3 lines

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?

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 <[email protected]>

---

Quoted 8 lines · Halil Pasic

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 <[email protected]>

---

[...]

Quoted 8 lines

qemu-doc.texi +0 -0

index db2351c746..874432d87c 100644
@@ -2501,6 +2501,14 @@ enabled via the ``-machine usb=on'' argument.
 
 The ``-nodefconfig`` argument is a synonym for ``-no-user-config``.
 

qemu-doc.texi enabled via the ``-machine usb=on'' argument. · line 2504

Quoted 1 line

qemu-doc.texi +1 -0

+@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

Quoted 18 lines · Thomas Huth

Quoted 8 lines · Halil Pasic

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 <[email protected]>

---

[...]

Quoted 8 lines

qemu-doc.texi +0 -0

index db2351c746..874432d87c 100644
@@ -2501,6 +2501,14 @@ enabled via the ``-machine usb=on'' argument.
 
 The ``-nodefconfig`` argument is a synonym for ``-no-user-config``.
 

Quoted 8 lines

qemu-doc.texi enabled via the ``-machine usb=on'' argument. · line 2504

Quoted 1 line

qemu-doc.texi +1 -0

+@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)?

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)?

Quoted 6 lines · Halil Pasic

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.

Quoted 6 lines

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)?