Re: [Xen-devel] [PATCH] [qemu] xen_be_init under stubdom

[Kamala Narasimhan <kamala.narasimhan@xxxxxxxxx>]

>> Do nothing in xen_be_init under stubdom plus a minor inconsequential cleanup.
> ...
>> -       goto cleanup;
>> +        return;
> ...
>> -cleanup:
>>      qemu_free(vec);
>>  }
>
> I don't think this is a helpful change.

I did call it inconsequential and wouldn't have sent it if it didn't
happen to be with a one liner that had functional impact that I wanted
changed :)

> There is nothing wrong with
> calling qemu_free(0) and IMO in general functions that "goto cleanup"
> are to be preferred to ones that "return".
>

You would rather check for null pointer and then go to cleanup code
whose sole purpose in this case is to free that pointer and all this
for free to realize that it has nothing to free and then return back!
This as opposed to simply return when it is null!  I understand how
going to cleanup makes a lot of sense when there is at least some
amount of potential cleanup to do.  In this case, clean up does
nothing more than free that pointer!  Anyway, I am not particularly
worried if this doesn't make it.

> Furthermore, even if this patch were good, it's not a bugfix so is not
> acceptable at this stage of the release.
>
No, it does get rid of an issue I encounter.  I was running into data
corruption as this code path was taken with stubdom in my
configuration and it was a pain to debug as with these kinds of
corruption issues the problem showed up elsewhere.

Kamala

>> @@ -646,6 +645,10 @@ static void xen_be_evtchn_event(void *opaque)
>>
>>  int xen_be_init(void)
>>  {
>> +#ifdef CONFIG_STUBDOM
>> +    return 0;
>> +#endif
>
> I don't understand this at all.  Why should stubdom not be able to
> make pv backends if it wants to ?  I agree that it probably doesn't
> want to but if something iswrongly causing it to then the right fix is
> to make it not do so.
>
> Ian.
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel