Message ID | 20171004105751.24655-3-borntraeger@de.ibm.com |
---|---|
State | New |
Headers | show |
Series | s390/z14: extended TOD-clock support | expand |
On 04.10.2017 12:57, Christian Borntraeger wrote: > From: "Collin L. Walling" <walling@linux.vnet.ibm.com> > > If we fail to set a proper TOD clock on the target system, this can > already result in some problematic cases. We print several warn messages > on source and target in that case. > > If kvm fails to set a nonzero epoch index, then we must ultimately fail > the migration as this will result in a giant time leap backwards. This > patch lets the migration fail if we can not set the guest time on the > target. > > On failure the guest will resume normally on the original host machine. > > Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> > Reviewed-by: Eric Farman <farman@linux.vnet.ibm.com> > Reviewed-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > [split failure change from epoch index change, minor fixups] > --- > hw/s390x/s390-virtio-ccw.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index fafbc6d..b7adb93 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -213,13 +213,10 @@ static int gtod_load(QEMUFile *f, void *opaque, int version_id) > > r = s390_set_clock(&tod_high, &tod_low); > if (r) { > - warn_report("Unable to set guest clock for migration: %s", > - strerror(-r)); > - error_printf("Guest clock will not be restored " > - "which could cause the guest to hang."); > + error_report("Unable to set KVM guest TOD clock: %s", strerror(-r)); > } > > - return 0; > + return r; > } Reviewed-by: Thomas Huth <thuth@redhat.com> (and I wonder whether we should fail the S390_TOD_CLOCK_VALUE_MISSING case with returning an error instead of 0, too?)
On 10/04/2017 01:48 PM, Thomas Huth wrote: > On 04.10.2017 12:57, Christian Borntraeger wrote: >> From: "Collin L. Walling" <walling@linux.vnet.ibm.com> >> >> If we fail to set a proper TOD clock on the target system, this can >> already result in some problematic cases. We print several warn messages >> on source and target in that case. >> >> If kvm fails to set a nonzero epoch index, then we must ultimately fail >> the migration as this will result in a giant time leap backwards. This >> patch lets the migration fail if we can not set the guest time on the >> target. >> >> On failure the guest will resume normally on the original host machine. >> >> Signed-off-by: Collin L. Walling <walling@linux.vnet.ibm.com> >> Reviewed-by: Eric Farman <farman@linux.vnet.ibm.com> >> Reviewed-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com> >> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> >> [split failure change from epoch index change, minor fixups] >> --- >> hw/s390x/s390-virtio-ccw.c | 7 ++----- >> 1 file changed, 2 insertions(+), 5 deletions(-) >> >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >> index fafbc6d..b7adb93 100644 >> --- a/hw/s390x/s390-virtio-ccw.c >> +++ b/hw/s390x/s390-virtio-ccw.c >> @@ -213,13 +213,10 @@ static int gtod_load(QEMUFile *f, void *opaque, int version_id) >> >> r = s390_set_clock(&tod_high, &tod_low); >> if (r) { >> - warn_report("Unable to set guest clock for migration: %s", >> - strerror(-r)); >> - error_printf("Guest clock will not be restored " >> - "which could cause the guest to hang."); >> + error_report("Unable to set KVM guest TOD clock: %s", strerror(-r)); >> } >> >> - return 0; >> + return r; >> } > > Reviewed-by: Thomas Huth <thuth@redhat.com> > > (and I wonder whether we should fail the S390_TOD_CLOCK_VALUE_MISSING > case with returning an error instead of 0, too?) Yes. This patch does the minimal thing in "making things fail on error" now that we have a case where things are guaranteed to be out of sync. One can argue that we actually should fail migration on the source instead of sending S390_TOD_CLOCK_VALUE_MISSING. If you want to send a patch that make migration fail for more cases I am inclined to ack such a thing.
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index fafbc6d..b7adb93 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -213,13 +213,10 @@ static int gtod_load(QEMUFile *f, void *opaque, int version_id) r = s390_set_clock(&tod_high, &tod_low); if (r) { - warn_report("Unable to set guest clock for migration: %s", - strerror(-r)); - error_printf("Guest clock will not be restored " - "which could cause the guest to hang."); + error_report("Unable to set KVM guest TOD clock: %s", strerror(-r)); } - return 0; + return r; } static SaveVMHandlers savevm_gtod = {