diff mbox

[for-2.10,05/10] s390x/css: provide introspection for virtual subchannel and device busid

Message ID 20170406111646.12624-6-cornelia.huck@de.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Cornelia Huck April 6, 2017, 11:16 a.m. UTC
From: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>

Expose the busids of the virtual I/O subchannel and the virtual CCW
device to ease debugging. This is needed because:
1. subchannel id are assigned dynamically, and cannot be set from
   outside.
2. device busid could possibly be auto generated.

An example of using HMP to retrieve the property values of a
virtio-balloon-ccw device looks like:

[root@localhost ~]# lscss -d 0.0.0004
Device   Subchan.  DevType CU Type Use  PIM PAM POM  CHPIDs
----------------------------------------------------------------------
0.0.0004 0.0.0003  0000/00 3832/05 yes  80  80  ff   00000000 00000000

(qemu) info qtree
... ...
      dev: virtio-balloon-ccw, id "balloon0"
        devno = "<unset>"
        ioeventfd = true
        max_revision = 2 (0x2)
        dev_id = "fe.0.0004"
        subch_id = "fe.0.0003"
... ...

After migration, if we have the same device that shows up on a
different subchannel, we must re-fill the subch_id of the ccw
device with the new schid, or the subch_id will have an old wrong
schid value. So this also re-fills the subch_id after migration.

While we are at it, also neaten the related error handling a bit.

Signed-off-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 hw/s390x/ccw-device.c | 39 +++++++++++++++++++++++++++++++++++++++
 hw/s390x/ccw-device.h |  7 +++++++
 hw/s390x/virtio-ccw.c | 28 ++++++++++++++++++++++------
 3 files changed, 68 insertions(+), 6 deletions(-)

Comments

Thomas Huth April 6, 2017, 12:19 p.m. UTC | #1
On 06.04.2017 13:16, Cornelia Huck wrote:
> From: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> 
> Expose the busids of the virtual I/O subchannel and the virtual CCW
> device to ease debugging. This is needed because:
> 1. subchannel id are assigned dynamically, and cannot be set from
>    outside.
> 2. device busid could possibly be auto generated.
> 
> An example of using HMP to retrieve the property values of a
> virtio-balloon-ccw device looks like:
> 
> [root@localhost ~]# lscss -d 0.0.0004
> Device   Subchan.  DevType CU Type Use  PIM PAM POM  CHPIDs
> ----------------------------------------------------------------------
> 0.0.0004 0.0.0003  0000/00 3832/05 yes  80  80  ff   00000000 00000000
> 
> (qemu) info qtree
> ... ...
>       dev: virtio-balloon-ccw, id "balloon0"
>         devno = "<unset>"
>         ioeventfd = true
>         max_revision = 2 (0x2)
>         dev_id = "fe.0.0004"
>         subch_id = "fe.0.0003"
> ... ...
> 
> After migration, if we have the same device that shows up on a
> different subchannel, we must re-fill the subch_id of the ccw
> device with the new schid, or the subch_id will have an old wrong
> schid value. So this also re-fills the subch_id after migration.
> 
> While we are at it, also neaten the related error handling a bit.
> 
> Signed-off-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
>  hw/s390x/ccw-device.c | 39 +++++++++++++++++++++++++++++++++++++++
>  hw/s390x/ccw-device.h |  7 +++++++
>  hw/s390x/virtio-ccw.c | 28 ++++++++++++++++++++++------
>  3 files changed, 68 insertions(+), 6 deletions(-)
[...]
>  static inline CcwDevice *to_ccw_dev_fast(DeviceState *d)
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 00b3bde4e9..4e59e34d74 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -680,6 +680,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
>  {
>      VirtIOCCWDeviceClass *k = VIRTIO_CCW_DEVICE_GET_CLASS(dev);
>      CcwDevice *ccw_dev = CCW_DEVICE(dev);
> +    CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev);
>      SubchDev *sch = css_create_virtual_sch(ccw_dev->bus_id, errp);
>      Error *err = NULL;
>  
> @@ -689,8 +690,7 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
>      if (!virtio_ccw_rev_max(dev) && dev->force_revision_1) {
>          error_setg(&err, "Invalid value of property max_rev "
>                     "(is %d expected >= 1)", virtio_ccw_rev_max(dev));
> -        error_propagate(errp, err);
> -        return;
> +        goto out_err;
>      }
>  
>      sch->driver_data = dev;
> @@ -713,13 +713,24 @@ static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
>  
>      if (k->realize) {
>          k->realize(dev, &err);
> +        if (err) {
> +            goto out_err;
> +        }
>      }
> +
> +    ck->realize(ccw_dev, &err);
>      if (err) {
> -        error_propagate(errp, err);
> -        css_subch_assign(sch->cssid, sch->ssid, sch->schid, sch->devno, NULL);
> -        ccw_dev->sch = NULL;
> -        g_free(sch);
> +        goto out_err;
>      }
> +
> +    return;
> +
> +out_err:
> +    error_propagate(errp, err);
> +    css_subch_assign(sch->cssid, sch->ssid, sch->schid, sch->devno, NULL);
> +    ccw_dev->sch = NULL;
> +    g_free(sch);
> +    return;
>  }

Cosmetic nit: Remove the unnecessary "return;" statement right before
the closing curly bracket.

 Thomas
Cornelia Huck April 6, 2017, 1:16 p.m. UTC | #2
On Thu, 6 Apr 2017 14:19:10 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 06.04.2017 13:16, Cornelia Huck wrote:

> > +out_err:
> > +    error_propagate(errp, err);
> > +    css_subch_assign(sch->cssid, sch->ssid, sch->schid, sch->devno, NULL);
> > +    ccw_dev->sch = NULL;
> > +    g_free(sch);
> > +    return;
> >  }
> 
> Cosmetic nit: Remove the unnecessary "return;" statement right before
> the closing curly bracket.

Yup, will merge in.
diff mbox

Patch

diff --git a/hw/s390x/ccw-device.c b/hw/s390x/ccw-device.c
index 28ea20440e..8fe3cf49ef 100644
--- a/hw/s390x/ccw-device.c
+++ b/hw/s390x/ccw-device.c
@@ -11,11 +11,50 @@ 
 #include "qemu/osdep.h"
 #include "ccw-device.h"
 
+static void ccw_device_refill_ids(CcwDevice *dev)
+{
+    SubchDev *sch = dev->sch;
+
+    assert(sch);
+
+    dev->dev_id.cssid = sch->cssid;
+    dev->dev_id.ssid = sch->ssid;
+    dev->dev_id.devid = sch->devno;
+    dev->dev_id.valid = true;
+
+    dev->subch_id.cssid = sch->cssid;
+    dev->subch_id.ssid = sch->ssid;
+    dev->subch_id.devid = sch->schid;
+    dev->subch_id.valid = true;
+}
+
+static void ccw_device_realize(CcwDevice *dev, Error **errp)
+{
+    ccw_device_refill_ids(dev);
+}
+
+static Property ccw_device_properties[] = {
+    DEFINE_PROP_CSS_DEV_ID_RO("dev_id", CcwDevice, dev_id),
+    DEFINE_PROP_CSS_DEV_ID_RO("subch_id", CcwDevice, subch_id),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void ccw_device_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    CCWDeviceClass *k = CCW_DEVICE_CLASS(klass);
+
+    k->realize = ccw_device_realize;
+    k->refill_ids = ccw_device_refill_ids;
+    dc->props = ccw_device_properties;
+}
+
 static const TypeInfo ccw_device_info = {
     .name = TYPE_CCW_DEVICE,
     .parent = TYPE_DEVICE,
     .instance_size = sizeof(CcwDevice),
     .class_size = sizeof(CCWDeviceClass),
+    .class_init = ccw_device_class_init,
     .abstract = true,
 };
 
diff --git a/hw/s390x/ccw-device.h b/hw/s390x/ccw-device.h
index 59ba01b6c5..48700fe23a 100644
--- a/hw/s390x/ccw-device.h
+++ b/hw/s390x/ccw-device.h
@@ -19,12 +19,19 @@  typedef struct CcwDevice {
     DeviceState parent_obj;
     SubchDev *sch;
     /* <cssid>.<ssid>.<device number> */
+    /* The user-set busid of the virtual ccw device. */
     CssDevId bus_id;
+    /* The actual busid of the virtual ccw device. */
+    CssDevId dev_id;
+    /* The actual busid of the virtual subchannel. */
+    CssDevId subch_id;
 } CcwDevice;
 
 typedef struct CCWDeviceClass {
     DeviceClass parent_class;
     void (*unplug)(HotplugHandler *, DeviceState *, Error **);
+    void (*realize)(CcwDevice *, Error **);
+    void (*refill_ids)(CcwDevice *);
 } CCWDeviceClass;
 
 static inline CcwDevice *to_ccw_dev_fast(DeviceState *d)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 00b3bde4e9..4e59e34d74 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -680,6 +680,7 @@  static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
 {
     VirtIOCCWDeviceClass *k = VIRTIO_CCW_DEVICE_GET_CLASS(dev);
     CcwDevice *ccw_dev = CCW_DEVICE(dev);
+    CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev);
     SubchDev *sch = css_create_virtual_sch(ccw_dev->bus_id, errp);
     Error *err = NULL;
 
@@ -689,8 +690,7 @@  static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
     if (!virtio_ccw_rev_max(dev) && dev->force_revision_1) {
         error_setg(&err, "Invalid value of property max_rev "
                    "(is %d expected >= 1)", virtio_ccw_rev_max(dev));
-        error_propagate(errp, err);
-        return;
+        goto out_err;
     }
 
     sch->driver_data = dev;
@@ -713,13 +713,24 @@  static void virtio_ccw_device_realize(VirtioCcwDevice *dev, Error **errp)
 
     if (k->realize) {
         k->realize(dev, &err);
+        if (err) {
+            goto out_err;
+        }
     }
+
+    ck->realize(ccw_dev, &err);
     if (err) {
-        error_propagate(errp, err);
-        css_subch_assign(sch->cssid, sch->ssid, sch->schid, sch->devno, NULL);
-        ccw_dev->sch = NULL;
-        g_free(sch);
+        goto out_err;
     }
+
+    return;
+
+out_err:
+    error_propagate(errp, err);
+    css_subch_assign(sch->cssid, sch->ssid, sch->schid, sch->devno, NULL);
+    ccw_dev->sch = NULL;
+    g_free(sch);
+    return;
 }
 
 static int virtio_ccw_exit(VirtioCcwDevice *dev)
@@ -1261,12 +1272,17 @@  static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f)
 {
     VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d);
     CcwDevice *ccw_dev = CCW_DEVICE(d);
+    CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev);
     SubchDev *s = ccw_dev->sch;
     VirtIODevice *vdev = virtio_ccw_get_vdev(s);
     int len;
 
     s->driver_data = dev;
     subch_device_load(s, f);
+    /* Re-fill subch_id after loading the subchannel states.*/
+    if (ck->refill_ids) {
+        ck->refill_ids(ccw_dev);
+    }
     len = qemu_get_be32(f);
     if (len != 0) {
         dev->indicators = get_indicator(qemu_get_be64(f), len);