On Thu, 18 Jan 2018 17:52:29 +0100
Claudio Imbrenda <imbre...@linux.vnet.ibm.com> wrote:
> On Thu, 18 Jan 2018 17:20:34 +0100
> Cornelia Huck <coh...@redhat.com> wrote:
>
> > On Mon, 15 Jan 2018 17:52:02 +0100
> > Claudio Imbrenda <imbre...@linux.vnet.ibm.com> wrote:
> >
> > > Fix storage attribute migration so that it does not fail for guests
> > > with more than a few GB of RAM. Migration itself was successful, but
> > > storage attributes were not migrated completely.
> > >
> > > This patch fixes the migration of all storage attributes, even when
> > > the guest have large amounts of memory.
> > >
> > > Signed-off-by: Claudio Imbrenda <imbre...@linux.vnet.ibm.com>
> > > Fixes: 903fd80b03243476 ("s390x/migration: Storage attributes
> > > device") ---
> > > hw/s390x/s390-stattrib-kvm.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/s390x/s390-stattrib-kvm.c
> > > b/hw/s390x/s390-stattrib-kvm.c index 41770a7..480551c 100644
> > > --- a/hw/s390x/s390-stattrib-kvm.c
> > > +++ b/hw/s390x/s390-stattrib-kvm.c
> > > @@ -116,7 +116,7 @@ static void
> > > kvm_s390_stattrib_synchronize(S390StAttribState *sa) for (cx = 0;
> > > cx + len <= max; cx += len) { clog.start_gfn = cx;
> > > clog.count = len;
> > > - clog.values = (uint64_t)(sas->incoming_buffer + cx *
> > > len);
> >
> > Hm, doesn't that even imply that you reference an area beyond the
> > buffer, as the <= max check does not catch this?
>
> what do you mean?
>
> cx + len <= max catches the cases where you would write beyond the end
> of the buffer. if cx + len == max then we are filling the buffer to the
> last byte. and we will get out at the next iteration.
Yes, but the problem is that your offset is too long, isn't it? (Where
cx + len <= max, but you use an offset of cx * len which may be > max.)
But maybe I'm simply too tired.
>
> > > + clog.values = (uint64_t)(sas->incoming_buffer + cx);
> > > r = kvm_vm_ioctl(kvm_state, KVM_S390_SET_CMMA_BITS,
> > > &clog); if (r) {
> > > error_report("KVM_S390_SET_CMMA_BITS failed: %s",
> > > strerror(-r)); @@ -126,7 +126,7 @@ static void
> > > kvm_s390_stattrib_synchronize(S390StAttribState *sa) if (cx < max) {
> > > clog.start_gfn = cx;
> > > clog.count = max - cx;
> > > - clog.values = (uint64_t)(sas->incoming_buffer + cx *
> > > len);
>
> and here we fill in the last pieces if there are any leftovers, which
> at this point are guaranteed to be smaller than len.
>
> > > + clog.values = (uint64_t)(sas->incoming_buffer + cx);
> > > r = kvm_vm_ioctl(kvm_state, KVM_S390_SET_CMMA_BITS,
> > > &clog); if (r) {
> > > error_report("KVM_S390_SET_CMMA_BITS failed: %s",
> > > strerror(-r));
> >
>
