On 05/31/2017 08:13 PM, Juan Quintela wrote:
> Halil Pasic <pa...@linux.vnet.ibm.com> wrote:
>> Let's vmstatify virtio_ccw_save_config and virtio_ccw_load_config for
>> flexibility (extending using subsections) and for fun.
>>
>> To achieve this we need to hack the config_vector, which is VirtIODevice
>> (that is common virtio) state, in the middle of the VirtioCcwDevice state
>> representation. This somewhat ugly, but we have no choice because the
>> stream format needs to be preserved.
>>
>> Almost no changes in behavior. Exception is everything that comes with
>> vmstate like extra bookkeeping about what's in the stream, and maybe some
>> extra checks and better error reporting.
>>
>> Signed-off-by: Halil Pasic <pa...@linux.vnet.ibm.com>
>
> Reviewed-by: Juan Quintela <quint...@redhat.com>
>
>> +static void subch_dev_pre_save(void *opaque)
>> +{
>> + SubchDev *s = opaque;
>> +
>> + /* Prepare remote_schid for save */
>> + s->migrated_schid = s->schid;
>> +}
>> +
>> +static int subch_dev_post_load(void *opaque, int version_id)
>> +{
>> +
>> + SubchDev *s = opaque;
>> +
>> + /* Re-assign the subchannel to remote_schid if necessary */
>> + if (s->migrated_schid != s->schid) {
>> + if (css_find_subch(true, s->cssid, s->ssid, s->schid) == s) {
>
> I am assuming this is somehow similar to
> old_s = channel_subsys.css[s->cssid]->sch_set[s->ssid]->sch[old_schid];
>
That's right. A quick glance at the return statement(s) of css_find_subch
makes it very obvious. But css_find_subch does some null checks and may
differently for cssid == 0 (which does not matter here).
>> - qemu_put_be32(f, s->curr_status.pmcw.intparm);
>> - qemu_put_be16(f, s->curr_status.pmcw.flags);
>> - qemu_put_be16(f, s->curr_status.pmcw.devno);
>> - qemu_put_byte(f, s->curr_status.pmcw.lpm);
>> - qemu_put_byte(f, s->curr_status.pmcw.pnom);
>> - qemu_put_byte(f, s->curr_status.pmcw.lpum);
>> - qemu_put_byte(f, s->curr_status.pmcw.pim);
>> - qemu_put_be16(f, s->curr_status.pmcw.mbi);
>> - qemu_put_byte(f, s->curr_status.pmcw.pom);
>> - qemu_put_byte(f, s->curr_status.pmcw.pam);
>
> I hope it somehow makes sense, I am having trouble following that you
> have fields named: pim, pam, pom, pnom, lpm, lpum, mda, mba ..... looks
> like hell for reviewing O:-)
>
> And I thought that x86 was weird because it used all three letters
> acronyms
>
> O:-)
nod
>
> Later, Juan.
>
Many thanks for the review and the r-b!
Regards,
Halil
