WARNING - OLD ARCHIVES

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/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH 05/21] xenpaging: add signal handling

To: Olaf Hering <olaf@xxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 05/21] xenpaging: add signal handling
From: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
Date: Thu, 2 Dec 2010 11:48:11 +0000
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Thu, 02 Dec 2010 03:49:05 -0800
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:sender:received :in-reply-to:references:date:x-google-sender-auth:message-id:subject :from:to:cc:content-type; bh=S9ULGNtZhMVSx582th5Ij2TpjwoRiensH0iIlFJ71GA=; b=rF062iTjv2AZ27xx6lhEyO/x0ChRx0JQ8hU7FFXxdYjIVNGs+zfGEA91mQdODin7BX EjRoWk4dK3KiXv91dkWSobgs1RT+Ziu7fAG91Ytvns4Ayd0IHE1AZTVDyypUOYM9ShPm oqUjigKSHQkSfXXMLlCROuVDFq6EFxKXcPkpw=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; b=gsAzBopo+LN51QwDtSaeP2MSoee3pSHZuw1leONvwz5/9j1Z39vZhTybeTzfdVO2YS WBJJjr4kOW7pVefTLtC1B+K8xZs1DOsEII3Ozt0pVR3zRXGSKg1F3w0vh9xkftAwQmCB 9hZ5MvHniGdrZ8/lIBEsg5sPzV6nFy5nsO4wc=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20101202095914.GB29462@xxxxxxxxx>
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: <20101126134901.384130351@xxxxxxxxx> <20101126134903.163026660@xxxxxxxxx> <AANLkTikVqFs7SmNkqJc+d82q1PX4RdpscMeySRpKqvWK@xxxxxxxxxxxxxx> <20101202095914.GB29462@xxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Yeah, they certainly seem like reasonable changes.  They just need to
be pointed out, so that people have an idea the actual impact of the
patch (both now when accepting it, and later when going back trying to
figure out where something broke).

FYI, I normally do a bunch of work (involving a large number of
changes) first, then do "hg diff > working.diff ; hg update -C" and
then go through working.diff to see where the individual changes are
best suited, whether in a new patch, or in a modification to an
existing patch.  (emacs diff-mode is really helpful here.)  That helps
me review my own code, and promotes good patch hygiene. :-)

Peace,
 -George

On Thu, Dec 2, 2010 at 9:59 AM, Olaf Hering <olaf@xxxxxxxxx> wrote:
> On Fri, Nov 26, George Dunlap wrote:
>
>> Olaf, I haven't been looking at these patches as we've been going
>> along, but there seem to be two things happening in this patch not
>> mentioned in the description:
>
> This was a "grown" patch.
>
>> * Making xenpaging_teardown() not skip when a tear-down item fails,
>> but continue to try to tear down the rest
>
> If a domain is shutting down, xc_mem_event_disable will always fail
> because d->is_dying is checked. Thats why I removed the bail_out part.
>
>> * Making return values for the program as a whole (1 for initializing
>> the paging, 2 for a failed file open)
>
> return codes are currently not perfect, sometimes -1 is leaked.
> I think xenpaging should either return 0 or 1.
>
>> These kinds of things should at least be mentioned in the description;
>> and I would personally probably pull them out and put them in a
>> separate patch.
>
> Will do better next time.
> Thanks for the comment.
>
>
> Olaf
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
>

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

<Prev in Thread] Current Thread [Next in Thread>