Re: [Qemu-devel] [PATCH] Initial support for Ilumos build and Illumos-kvm

[Andreas Färber]

Am 16.03.2012 10:23, schrieb Lee Essen:
> This fixes a number of issues with the build process (namely ensuring
> the use of bash), adds specific support for the Illumos port of KVM
> and fixes a few general Solaris compatibility issues.
>
> There are still some things outstanding:
>
> - there's a duplicate smb_wmb() definition in qemu-barrier.h and the
> illumos kvm_x86.h which generates some warnings.
> - there's a repeated call to page_size() that should probably be fixed.
> - dtrace support needs to be fixed (-m64/32 option, reserved words and
> linking issues)
> - vnics need to be added
> - the original illumos code added another timer source (multiticks)
> - the issue with Linux needs to be resolved
>
> Other than that, this gets it to the point where it will build and run
> with illumos kvm, and works fine for Windows.
>
> It's my first patch to qemu, and most of the real kvm stuff has come
> from the original illumos-kvm-cmd tree, so be gentle with me!
>
>
> Signed-off-by: Lee Essen <l
> <mailto:address@hidden>address@hidden
> <mailto:address@hidden>>

Your patch is HTML-formatted. Please use git-send-email to avoid that.

It is also doing way too many things at once. Properly using existing
$(SHELL) everywhere in Makefiles could be one patch, for instance,
adding $shell in configure another, same for functional
CONFIG_SOLARIS/__sun__ changes, KVM stuff in yet another. Making the
patches smaller and confined to subsystems or aspects will make it more
reviewable, especially since different maintainers are involved in the
files you touch.

"LEE - todo" doesn't sound too assuring. Either write it as a proper
"TODO This and that needs to be done." so that someone can address it or
send it as an [RFC] rather than a [PATCH].
If this patch introduces an issue for Linux (you don't say which?) while
adding support for illumos, it won't be acceptable in the first place. A
[PATCH] is expected to be of regression-free quality for qemu.git.

Is there any SystemTap on illumos? If not, we don't need the .stp file
at all.

Please resubmit with those issues addressed. I just cc'ed you on the C99
fix mentioned yesterday and am rebasing my queue on OpenIndiana (oi_151a).

Regards,
Andreas