Re: [libvirt] [PATCHv2 0/5] Basic Enablement of s390

On 25.06.2012 15:54, Daniel Veillard wrote: > On Mon, Jun 25, 2012 at 09:43:41PM +0800, Daniel Veillard wrote: >> On Mon, Jun 25, 2012 at 03:08:09PM +0200, Viktor Mihajlovski wrote: >>> On 06/25/2012 10:41 AM, Daniel Veillard wrote: >>>> On Fri, Jun 22, 2012 at 01:50:09PM +0200, Viktor Mihajlovski wrote: >>>>> Resending as a thread, same content. Please ignore previous submission. >>>>> >>>>> The first series of patches aimed to enable libvirt to manage >>>>> qemu/kvm domains hosted on the System z platform. >>>>> The patches in this series address s390 specifics of node and hypervisor. >>>>> >>>>> Patches 2,3 and 5 are written by Thang Pham<thang.pham(a)us.ibm.com> >>>>> the others are my doing. Since they all should go together, I am >>>>> sending in Thang's as well, hoping to facilitate the review a bit >>>>> this way. >>>>> >>>>> Thang Pham (3): >>>>> S390: CPU support for s390(x) >>>>> S390: Fixed Parser for /proc/cpuinfo needs to be adapted for your >>>>> architecture >>>>> S390: Added sysinfo for host on s390x. >>>>> >>>>> Viktor Mihajlovski (2): >>>>> S390: Override QEMU_CAPS_NO_ACPI for s390x >>>>> S390: Fixed core identification for s390 >>>>> >>>>> src/Makefile.am | 1 + >>>>> src/cpu/cpu.c | 2 + >>>>> src/cpu/cpu_s390x.c | 80 ++++++++++++++++++++++ >>>>> src/cpu/cpu_s390x.h | 31 +++++++++ >>>>> src/nodeinfo.c | 11 +++ >>>>> src/qemu/qemu_capabilities.c | 5 ++ >>>>> src/util/sysinfo.c | 154 ++++++++++++++++++++++++++++++++++++++++++ >>>>> 7 files changed, 284 insertions(+), 0 deletions(-) >>>>> create mode 100644 src/cpu/cpu_s390x.c >>>>> create mode 100644 src/cpu/cpu_s390x.h >>>> >>>> In general I like seeing additional platform support. If done well >>>> this is an opportunity to improve the code and bust out bug not raised >>>> on the main architecture. >>>> I was tempted to get such patch in before the freeze for 0.9.13, but >>>> it's a bit too late. So let's try to clean an improve the patch set(s) and >>>> push starting next week after the freeze. >>>> Ideally we should make sure we do thing uniformly for s390 s390x ppc64 >>>> and try to be ready if/when arm64 shows up ! >>>> >>>> thanks ! >>>> >>>> Daniel >>>> >>> >>> thanks for reviewing the patches, we will send out a cleaned up >>> version shortly. >> >> Well I reviewed only 2 of the set, lemme try to get a bit further :-) > > Actually now that I went though all 5 and since Dan Berrange reviewed > them too, it seems to me this code should not generate portability > troubles for other platforms (which was my main concern at this point). > So I would agree with trying to push that patch set in rc2 if you fix > the few issues which were raised, and send the new set within a couple > of days. > > Thanks ! > > Daniel > Well, I went ahead and cleaned up some nits you guys pointed our or I've spotted myself. Viktor, can you please give it a try and send any follow up patch if you find anything not working? Michal