[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/2] s390/kvm: make TOD setting failures fata
From: |
Christian Borntraeger |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/2] s390/kvm: make TOD setting failures fatal for migration |
Date: |
Wed, 4 Oct 2017 13:51:18 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 |
On 10/04/2017 01:48 PM, Thomas Huth wrote:
> On 04.10.2017 12:57, Christian Borntraeger wrote:
>> From: "Collin L. Walling" <address@hidden>
>>
>> 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 <address@hidden>
>> Reviewed-by: Eric Farman <address@hidden>
>> Reviewed-by: Claudio Imbrenda <address@hidden>
>> Signed-off-by: Christian Borntraeger <address@hidden>
>> [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 <address@hidden>
>
> (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.