[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: |
Fri, 19 May 2017 18:47:54 +0100 |
User-agent: |
Mutt/1.8.2 (2017-04-18) |
* Halil Pasic (address@hidden) wrote:
>
>
> On 05/19/2017 04:55 PM, Dr. David Alan Gilbert wrote:
> >> We could also consider making WITH_TMP act as a normal field.
> >> Working on the whole state looks like a bit like a corner case:
> >> we have some stuff adjacent in the migration stream, and we have
> >> to map it on multiple fields (and vice-versa). Getting the whole
> >> state with a pointer to a certain field could work via container_of.
> > You do need to know which field you're working on to be able to safely
> > use container_of, so I'm not sure how it would work for you in this
> > case.
>
>
> Well, if you have to write to just one field you are good because you
> already have a pointer to that field (.offset was added).
>
> If you need to write to multiple fields in post_load then you just pick
> one of the fields you are going to write to (probably the first) and use
> container_of to gain access to the whole state. The logic is specific to
> the bunch of the fields you are going to touch anyway.
>
> In fact any member of the state struct will do it's only important that
> you use the same when creating the VMStateField and when trying to get a
> pointer to the parent in pre_save and post_load.
>
> I haven't tried, so I'm not 100% sure, but if you like I can try, and send
> you a patch if it's viable.
>
> I think the key to a good solution is really what is intended and typical
> usage, and what is corner case. My patch in the other reply shows that we
> can do without changing the ways of VMSTATE_WITH_TMP. I think we can make
> what I'm trying to do here a bit prettier at the expense of making what
> you are doing in virtio-net a bit uglier, but whether it's a good idea to
> do so, I cant tell.
Lets go with what you put in the other patch (I replied to it); I hadn't
realised that was possible (hence my comment below).
Once we have a bunch of different uses of VMSTATE_WITH_TMP in the code
base, I'll step back and see how to tidy them up.
Dave
> >
> > The other thought I'd had was that perhaps we could change the temporary
> > structure in VMSTATE_WITH_TMP to:
> >
> > struct foo {
> > struct whatever **parent;
> >
> > so now you could write to *parent in cases like these.
> >
>
> Sorry, I do not get your idea. If you have some WIP patch in this
> direction I would be happy to provide some feedback.
>
>
> >> Btw, I would rather call it get_indicator a factory method or even a
> >> constructor than an allocator, but I think we understand each-other
> >> anyway.
> > Yes; I'm not too worried about the actual name as long as it's short
> > and obvious.
> >
> > I'd thought of 'allocator' since in most cases it's used where the
> > load-time code allocates memory for the object being loaded.
> > A constructor is normally something I think of as happening after
> > allocation; and a factory, hmm maybe. However, if it does the right
> > thing I wouldn't object to any of those names.
> >
>
> I think we are on the same page.
>
> Cheers,
> Halil
>
> > Dave
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- [Qemu-devel] [PATCH 03/10] s390x/css: add vmstate entities for css, (continued)
- [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, 2017/05/15
- 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 <=
- 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
[Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration, Halil Pasic, 2017/05/05
Re: [Qemu-devel] [PATCH 06/10] virtio-ccw: use vmstate way for config migration, Dr. David Alan Gilbert, 2017/05/08