[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/7] s390x: fix realize inheritance for kvm-flic
From: |
Christian Borntraeger |
Subject: |
Re: [Qemu-devel] [PATCH 4/7] s390x: fix realize inheritance for kvm-flic |
Date: |
Wed, 5 Jul 2017 12:20:42 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
On 07/04/2017 04:51 PM, Halil Pasic wrote:
>
>
> On 07/04/2017 04:37 PM, Cornelia Huck wrote:
>> On Tue, 4 Jul 2017 16:07:56 +0200
>> Christian Borntraeger <address@hidden> wrote:
>>
>>> From: Halil Pasic <address@hidden>
>>>
>>> Commit f6f4ce4211 ("s390x: add property adapter_routes_max_batch",
>>> 2016-12-09) introduces a common realize (intended to be common for all
>>> the subclasses) for flic, but fails to make sure the kvm-flic which had
>>> it's own is actually calling this common realize.
>>
>> s/it's/its/
>>
>
> Valid. Sorry.
>
>>>
>>> This omission fortunately does not result in a grave problem. The common
>>> realize was only supposed to catch a possible programming mistake by
>>> validating a value of a property set via the compat machine macros. Since
>>> there was no programming mistake we don't need this fixed for stable.
>>>
>>> Let's fix this problem by making sure kvm flic honors the realize of its
>>> parent class.
>>>
>>> Let us also improve on the error message we would hypothetically emit
>>> when the validation fails.
>>>
>>> Signed-off-by: Halil Pasic <address@hidden>
>>> Fixes: f6f4ce4211 ("s390x: add property adapter_routes_max_batch")
>>> Reviewed-by: Dong Jia Shi <address@hidden>
>>> Reviewed-by: Yi Min Zhao <address@hidden>
>>> Signed-off-by: Christian Borntraeger <address@hidden>
>>> ---
>>> hw/intc/s390_flic.c | 4 ++--
>>> hw/intc/s390_flic_kvm.c | 17 +++++++++++++++++
>>> 2 files changed, 19 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
>>> index a99a350..837158b 100644
>>> --- a/hw/intc/s390_flic.c
>>> +++ b/hw/intc/s390_flic.c
>>> @@ -101,8 +101,8 @@ static void s390_flic_common_realize(DeviceState *dev,
>>> Error **errp)
>>> uint32_t max_batch = S390_FLIC_COMMON(dev)->adapter_routes_max_batch;
>>>
>>> if (max_batch > ADAPTER_ROUTES_MAX_GSI) {
>>> - error_setg(errp, "flic adapter_routes_max_batch too big"
>>> - "%d (%d allowed)", max_batch, ADAPTER_ROUTES_MAX_GSI);
>>> + error_setg(errp, "flic property adapter_routes_max_batch too big"
>>> + " (%d > %d)", max_batch, ADAPTER_ROUTES_MAX_GSI);
>>
>> Unrelated message change?
>>
>
> I've mentioned it in the commit message. It was also introduced by the
> patch I'm fixing. But yes strictly it's two different problems.
I will only fix the patch description ( s/it's/its/) and keep the other things
unchanged. Is that fine with you?
>
>
>>> }
>>> }
>>>
>>> diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
>>> index bea3997..535d99d 100644
>>> --- a/hw/intc/s390_flic_kvm.c
>>> +++ b/hw/intc/s390_flic_kvm.c
>>> @@ -392,6 +392,17 @@ static const VMStateDescription kvm_s390_flic_vmstate
>>> = {
>>> }
>>> };
>>>
>>> +typedef struct KVMS390FLICStateClass {
>>> + S390FLICStateClass parent_class;
>>> + DeviceRealize parent_realize;
>>> +} KVMS390FLICStateClass;
>>> +
>>> +#define KVM_S390_FLIC_GET_CLASS(obj) \
>>> + OBJECT_GET_CLASS(KVMS390FLICStateClass, (obj), TYPE_KVM_S390_FLIC)
>>> +
>>> +#define KVM_S390_FLIC_CLASS(klass) \
>>> + OBJECT_CLASS_CHECK(KVMS390FLICStateClass, (klass), TYPE_KVM_S390_FLIC)
>>> +
>>> static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
>>> {
>>> KVMS390FLICState *flic_state = KVM_S390_FLIC(dev);
>>> @@ -400,6 +411,10 @@ static void kvm_s390_flic_realize(DeviceState *dev,
>>> Error **errp)
>>> int ret;
>>> Error *errp_local = NULL;
>>>
>>> + KVM_S390_FLIC_GET_CLASS(dev)->parent_realize(dev, &errp_local);
>>
>> I usually prefer to introduce a local variable for that, but don't care
>> too much.
>>
>>> + if (errp_local) {
>>> + goto fail;
>>> + }
>>> flic_state->fd = -1;
>>> if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {
>>> error_setg_errno(&errp_local, errno, "KVM is missing capability"
>>> @@ -454,6 +469,7 @@ static void kvm_s390_flic_class_init(ObjectClass *oc,
>>> void *data)
>>> DeviceClass *dc = DEVICE_CLASS(oc);
>>> S390FLICStateClass *fsc = S390_FLIC_COMMON_CLASS(oc);
>>>
>>> + KVM_S390_FLIC_CLASS(oc)->parent_realize = dc->realize;
>>
>> dito
>>
>>> dc->realize = kvm_s390_flic_realize;
>>> dc->vmsd = &kvm_s390_flic_vmstate;
>>> dc->reset = kvm_s390_flic_reset;
>>> @@ -468,6 +484,7 @@ static const TypeInfo kvm_s390_flic_info = {
>>> .name = TYPE_KVM_S390_FLIC,
>>> .parent = TYPE_S390_FLIC_COMMON,
>>> .instance_size = sizeof(KVMS390FLICState),
>>> + .class_size = sizeof(KVMS390FLICStateClass),
>>> .class_init = kvm_s390_flic_class_init,
>>> };
>>>
>>
>>
[Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize, Christian Borntraeger, 2017/07/04
- Re: [Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize, Cornelia Huck, 2017/07/04
- Re: [Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize, Halil Pasic, 2017/07/04
- Re: [Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize, Halil Pasic, 2017/07/04
- Re: [Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize, Cornelia Huck, 2017/07/04
- Re: [Qemu-devel] [PATCH 3/7] s390x: fix error propagation in kvm-flic's realize, Christian Borntraeger, 2017/07/04
- [Qemu-devel] [PATCH 1/1] s390x: fixup for "fix error propagation in kvm-flic...", Halil Pasic, 2017/07/05