Re: [Qemu-devel] [PATCH 1/1] s390x/css: catch section mismatch on load

[Halil Pasic <pasic@linux.vnet.ibm.com>]

On 05/11/2017 01:02 PM, Cornelia Huck wrote:
> On Wed, 10 May 2017 17:12:09 +0200
> Halil Pasic <pasic@linux.vnet.ibm.com> wrote:
> 
>> Prior to the virtio-ccw-2.7 machine (and commit 2a79eb1a), our virtio
>> devices residing under the virtual-css bus do not have qdev_path based
>> migration stream identifiers (because their qdev_path is NULL). The ids
>> are instead generated when the device is registered as a composition of
>> the so called idstr, which takes the vmsd name as its value, and an
>> instance_id, which is which is calculated as a maximal instance_id
>> registered with the same idstr plus one, or zero (if none was registered
>> previously).
>>
[..]
>> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
>> index 15c4f4b..6cff3a3 100644
>> --- a/hw/s390x/css.c
>> +++ b/hw/s390x/css.c
[..]
>> +        /* Only possible if machine < 2.7 (no css_dev_path) */
>> +
>> +        error_setg(&err, "%x != %x", old_devno,  s->devno);
>> +        error_append_hint(&err, "Devno mismatch, tried to load wrong section!"
>> +                          " Likely reason: some sequences of plug and unplug"
>> +                          " can break migration for machine versions prior"
> 
> s/prior/prior to/

Fixed. Thanks! Will wait a couple of days, maybe somebody has something
else to say, and then re-spin.

Regards,
Halil

> 
>> +                          " 2.7 (known design flaw).\n");
>> +        error_report_err(err);
>> +        return -EINVAL;
>> +    }
>>      /* Re-assign subch. */
>>      if (old_schid != s->schid) {
>>          old_s = channel_subsys.css[s->cssid]->sch_set[s->ssid]->sch[old_schid];
>> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
>> index e7167e3..4f7efa2 100644
>> --- a/hw/s390x/virtio-ccw.c
>> +++ b/hw/s390x/virtio-ccw.c
>> @@ -1274,9 +1274,13 @@ static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f)
>>      SubchDev *s = ccw_dev->sch;
>>      VirtIODevice *vdev = virtio_ccw_get_vdev(s);
>>      int len;
>> +    int ret;
>>
>>      s->driver_data = dev;
>> -    subch_device_load(s, f);
>> +    ret = subch_device_load(s, f);
>> +    if (ret) {
>> +        return ret;
>> +    }
>>      /* Re-fill subch_id after loading the subchannel states.*/
>>      if (ck->refill_ids) {
>>          ck->refill_ids(ccw_dev);
> 
> Patch looks sane to me, but I second the question about how to handle
> this when using vmstates.
>