Re: [Qemu-devel] [PATCH v2 1/1] s390x: vmstatify config migration for virtio-ccw

[Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>]

* Halil Pasic <pasic@linux.vnet.ibm.com> [2017-06-02 16:05:31 +0200]:

Hi Halil,

Sorry for the late show up. I just found some nits, which could be
ignored for me.

> 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 is somewhat ugly, but we have no choice because the
                 ^^
Nit:-------------++

> 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.
> 
[...]

> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
> index 711c11454f..7e7546a576 100644
> --- a/hw/intc/s390_flic.c
> +++ b/hw/intc/s390_flic.c
> @@ -18,6 +18,7 @@
>  #include "trace.h"
>  #include "hw/qdev.h"
>  #include "qapi/error.h"
> +#include "hw/s390x/s390-virtio-ccw.h"
> 
>  S390FLICState *s390_get_flic(void)
>  {
> @@ -137,3 +138,30 @@ static void qemu_s390_flic_register_types(void)
>  }
> 
>  type_init(qemu_s390_flic_register_types)
> +
> +const VMStateDescription vmstate_adapter_info = {
> +    .name = "s390_adapter_info",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT64(ind_offset, AdapterInfo),
> +        /*
> +         * We do not have to migrate neither the id nor the addresses.
> +         * The id is set by css_register_io_adapter and the addresses
> +         * are set based on the IndAddr objects after those get mapped.
> +         */
> +        VMSTATE_END_OF_LIST()
> +    },
> +};
> +
> +const VMStateDescription vmstate_adapter_routes = {
> +
> +    .name = "s390_adapter_routes",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_STRUCT(adapter, AdapterRoutes, 1, vmstate_adapter_info, \
                                                                          ^^
Nit:----------------------------------------------------------------------++

> +                       AdapterInfo),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
[...]

> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> index 1e2f26b65a..348129e1b2 100644
> --- a/hw/s390x/css.c
> +++ b/hw/s390x/css.c
[...]

> +const VMStateDescription vmstate_virtio_ccw_dev = {
> +    .name = "s390_virtio_ccw_dev",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .post_load = virtio_ccw_dev_post_load,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_CCW_DEVICE(parent_obj, VirtioCcwDevice),
> +        VMSTATE_PTR_TO_IND_ADDR(indicators, VirtioCcwDevice),
> +        VMSTATE_PTR_TO_IND_ADDR(indicators2, VirtioCcwDevice),
> +        VMSTATE_PTR_TO_IND_ADDR(summary_indicator, VirtioCcwDevice),
> +        /*
> +         * Ugly hack because VirtIODevice does not migrate itself.
> +         * This also makes legacy via vmstate_save_state possible.
> +         */
> +        VMSTATE_WITH_TMP(VirtioCcwDevice, VirtioCcwDeviceTmp,
> +                         vmstate_virtio_ccw_dev_tmp),
> +        VMSTATE_STRUCT(routes, VirtioCcwDevice, 1, vmstate_adapter_routes, \
                                                                             ^^
Nit:-------------------------------------------------------------------------++

> +                       AdapterRoutes),
> +        VMSTATE_UINT8(thinint_isc, VirtioCcwDevice),
> +        VMSTATE_INT32(revision, VirtioCcwDevice),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static void virtio_ccw_bus_new(VirtioBusState *bus, size_t bus_size,
>                                 VirtioCcwDevice *dev);
> 
[...]

-- 
Dong Jia Shi