[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Olaf Hering <olaf@xxxxxxxxx>
  • 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
  • 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=
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.