[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 05/21] xenpaging: add signal handling
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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |