This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


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

To: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] [qemu] xen_be_init under stubdom
From: Kamala Narasimhan <kamala.narasimhan@xxxxxxxxx>
Date: Thu, 20 Jan 2011 11:26:49 -0500
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Thu, 20 Jan 2011 08:27:28 -0800
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=fAPGWBR1OTcJ9KepvaETIsvEAAEY19Zefrlt2K5Qkhc=; b=xBim0b3JemO0d3yzW/2SK0I7e+ldsUsrQVbDeABoVWqC2Fp4/1CZ1svxmZeJhTKarX Hf2mFTzQ6r3is2bypVddk6F3HU3rkrVpU2h1ATwnA0mNHxzT8HFjUsyv2I6FT+uVP3UC onBbKIc1bHz7doirSqZ+MpX1IOKOZRkAG9yPs=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=X1jqWeH11jvaMFkhs8EbcvNwpy4bGf+z/o+dqG1nMAhONSl0PwTV7hocNWxKHvQJzn nYJEd+yhXY6qQi2rJrn9sNsOEq/H2eF9cFGTcHH57ntPdOnoptlvH5gdS2XicJHFzFk5 4XkT3Qz0gZutQ9NWT1AtSUd5+RSWcJzFCNaOs=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <19768.23796.309250.916556@xxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <AANLkTikFbHCUmSceFR=byYRV+q5j9TnN8_bab6LKUSpe@xxxxxxxxxxxxxx> <19768.23796.309250.916556@xxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
>> 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.


>> @@ -646,6 +645,10 @@ static void xen_be_evtchn_event(void *opaque)
>>  int xen_be_init(void)
>>  {
>> +    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