[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css |
Date: |
Mon, 15 May 2017 19:01:30 +0100 |
User-agent: |
Mutt/1.8.2 (2017-04-18) |
* Halil Pasic (address@hidden) wrote:
>
>
> On 05/08/2017 06:45 PM, Dr. David Alan Gilbert wrote:
> > * Halil Pasic (address@hidden) wrote:
> >> As a preparation for switching to a vmstate based migration let us
> >> introduce vmstate entities (e.g. VMStateDescription) for the css entities
> >> to be migrated. Alongside some comments explaining or indicating the not
> >> migration of certain members are introduced too.
> >>
> >> No changes in behavior, we just added some dead code -- which should
> >> rise to life soon.
> >>
> >> Signed-off-by: Halil Pasic <address@hidden>
> >> ---
> >> hw/s390x/css.c | 276
> >> +++++++++++++++++++++++++++++++++++++++++++++++++
> >> include/hw/s390x/css.h | 10 +-
> >> 2 files changed, 285 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/s390x/css.c b/hw/s390x/css.c
> >> index c03bb20..2bda7d0 100644
> >> --- a/hw/s390x/css.c
> >> +++ b/hw/s390x/css.c
> >> @@ -20,29 +20,231 @@
> >> #include "hw/s390x/css.h"
> >> #include "trace.h"
> >> #include "hw/s390x/s390_flic.h"
> >> +#include "hw/s390x/s390-virtio-ccw.h"
> >>
>
> [..]
>
> >> +static int css_get_ind_addr(QEMUFile *f, void *pv, size_t size,
> >> + VMStateField *field)
> >> +{
> >> + int32_t len;
> >> + IndAddr **ind_addr = pv;
> >> +
> >> + len = qemu_get_be32(f);
> >> + if (len != 0) {
> >> + *ind_addr = get_indicator(qemu_get_be64(f), len);
> >> + } else {
> >> + qemu_get_be64(f);
> >> + *ind_addr = NULL;
> >> + }
> >> + return 0;
> >> +}
> >> +
> >> +static int css_put_ind_addr(QEMUFile *f, void *pv, size_t size,
> >> + VMStateField *field, QJSON *vmdesc)
> >> +{
> >> + IndAddr *ind_addr = *(IndAddr **) pv;
> >> +
> >> + if (ind_addr != NULL) {
> >> + qemu_put_be32(f, ind_addr->len);
> >> + qemu_put_be64(f, ind_addr->addr);
> >> + } else {
> >> + qemu_put_be32(f, 0);
> >> + qemu_put_be64(f, 0UL);
> >> + }
> >> + return 0;
> >> +}
> >> +
> >> +const VMStateInfo vmstate_info_ind_addr = {
> >> + .name = "s390_ind_addr",
> >> + .get = css_get_ind_addr,
> >> + .put = css_put_ind_addr
> >> +};
> >
> > You should be able to avoid this .get/.put by using VMSTATE_WITH_TMP,
> > declare a temporary struct something like:
> > struct tmp_ind_addr {
> > IndAddr *parent;
> > uint32_t len;
> > uint64_t addr;
> > }
> >
> > and then your .get/.put routines turn into pre_save/post_load
> > routines to just setup the len/addr.
> >
>
> I don't think this is going to work -- unfortunately! You can see below,
> how this IndAddr* migration stuff is supposed to be used:
> the client code just uses the VMSTATE_PTR_TO_IND_ADDR macro as a
> field when describing state which needs and IndAddr* migrated.
>
> The problem is, we do not know in what state will this field
> be embedded, the pre_save/post_load called by put_tmp/get_tmp
> is however copying the pointer to this state into the parent.
> So instead of having a pointer to IndAddr* in those functions
> and updating it accordingly, I would have to find the IndAddr*
> in some arbitrary state (in our case VirtioCcwDevice) first,
> and I lack information for that.
>
> If it's hard to follow I can give you the patch I was debugging
> to come to this conclusion. (By the way I ended up with 10
> lines of code more than in this version, and although I think
> it looks nicer, it's simpler only if one knows how WITH_TMP
> works. My plan was to ask you which version do you like more
> and go with that before I realized it ain't gonna work.)
>
Yes, I see - I've got some similar other cases; the challenge
is it's a custom allocator - 'get_indicator' - and it's used
as fields in a few places. Hmm.
Dave
> >> diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
> >> index f1f0d7f..6a451b2 100644
> >> --- a/include/hw/s390x/css.h
>
> [..]
>
> >>
> >> +extern const VMStateInfo vmstate_info_ind_addr;
> >> +
> >> +#define VMSTATE_PTR_TO_IND_ADDR(_f, _s)
> >> \
> >> + VMSTATE_SINGLE(_f, _s, 1 , vmstate_info_ind_addr, IndAddr*)
> >> +
> >> IndAddr *get_indicator(hwaddr ind_addr, int len);
> >> void release_indicator(AdapterInfo *adapter, IndAddr *indicator);
> >> int map_indicator(AdapterInfo *adapter, IndAddr *indicator);
> >> --
> >> 2.10.2
> >>
> > --
> > Dr. David Alan Gilbert / address@hidden / Manchester, UK
> >
>
>
> Cheers,
> Halil
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- [Qemu-devel] [PATCH 00/10] migration: s390x css migration, Halil Pasic, 2017/05/05
- [Qemu-devel] [PATCH 02/10] s390x: add css_migration_enabled to machine class, Halil Pasic, 2017/05/05
- [Qemu-devel] [PATCH 04/10] s390x/css: add vmstate macro for CcwDevice, Halil Pasic, 2017/05/05
- [Qemu-devel] [PATCH 01/10] s390x: add helper get_machine_class, Halil Pasic, 2017/05/05
- [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css, Halil Pasic, 2017/05/05
- Re: [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css, Dr. David Alan Gilbert, 2017/05/08
- Re: [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css, Halil Pasic, 2017/05/09
- Re: [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css,
Dr. David Alan Gilbert <=
- Re: [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css, Halil Pasic, 2017/05/18
- Re: [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css, Dr. David Alan Gilbert, 2017/05/19
- Re: [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css, Halil Pasic, 2017/05/19
- Re: [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css, Halil Pasic, 2017/05/19
- Re: [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css, Dr. David Alan Gilbert, 2017/05/19
- Re: [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css, Halil Pasic, 2017/05/19
- Re: [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css, Dr. David Alan Gilbert, 2017/05/19
- Re: [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css, Halil Pasic, 2017/05/19
- Re: [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css, Halil Pasic, 2017/05/09
[Qemu-devel] [PATCH 05/10] virtio-ccw: add vmstate entities for VirtioCcwDevice, Halil Pasic, 2017/05/05