From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH] remove KVM_CAP_USER_MEMORY reference from qemu-kvm.c Date: Wed, 10 Sep 2008 21:59:25 +0200 Message-ID: <48C8271D.5020703@web.de> References: <1220902221-7536-1-git-send-email-gcosta@redhat.com> <1220902221-7536-2-git-send-email-gcosta@redhat.com> <1220902221-7536-3-git-send-email-gcosta@redhat.com> <1220902221-7536-4-git-send-email-gcosta@redhat.com> <1220902221-7536-5-git-send-email-gcosta@redhat.com> <48C81EA6.10307@web.de> <20080910193709.GD17969@poweredge.glommer> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig61CD5AA966399E28FD3832A9" Cc: Glauber Costa , kvm@vger.kernel.org, avi@qumranet.com, aliguori@us.ibm.com To: Glauber Costa Return-path: Received: from fmmailgate03.web.de ([217.72.192.234]:36649 "EHLO fmmailgate03.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752008AbYIJT7d (ORCPT ); Wed, 10 Sep 2008 15:59:33 -0400 In-Reply-To: <20080910193709.GD17969@poweredge.glommer> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig61CD5AA966399E28FD3832A9 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Glauber Costa wrote: > On Wed, Sep 10, 2008 at 09:23:18PM +0200, Jan Kiszka wrote: >> Glauber Costa wrote: >>> From: Glauber Costa >>> >>> kvm_cpu_register_physical_memory() is its only user. Remove it. >>> >>> Signed-off-by: Glauber Costa >>> --- >>> qemu/qemu-kvm.c | 52 +++++++++++++++++++++------------------------= ------- >>> 1 files changed, 21 insertions(+), 31 deletions(-) >>> >>> diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c >>> index 8d366e5..f0ef21e 100644 >>> --- a/qemu/qemu-kvm.c >>> +++ b/qemu/qemu-kvm.c >>> @@ -775,42 +775,32 @@ void kvm_cpu_register_physical_memory(target_ph= ys_addr_t start_addr, >>> unsigned long size, >>> unsigned long phys_offset) >>> { >>> -#ifdef KVM_CAP_USER_MEMORY >>> int r =3D 0; >>> - >>> - r =3D kvm_check_extension(kvm_context, KVM_CAP_USER_MEMORY); >>> - if (r) { >>> - if (!(phys_offset & ~TARGET_PAGE_MASK)) { >>> - r =3D kvm_is_allocated_mem(kvm_context, start_addr, = size); >>> - if (r) >>> - return; >>> - r =3D kvm_is_intersecting_mem(kvm_context, start_addr); >>> - if (r) >>> - kvm_create_mem_hole(kvm_context, start_addr, size); >>> - r =3D kvm_register_userspace_phys_mem(kvm_context, start= _addr, >>> - phys_ram_base + phys= _offset, >>> - size, 0); >>> - } >>> - if (phys_offset & IO_MEM_ROM) { >>> - phys_offset &=3D ~IO_MEM_ROM; >>> - r =3D kvm_is_intersecting_mem(kvm_context, start_addr); >>> - if (r) >>> - kvm_create_mem_hole(kvm_context, start_addr, size); >>> - r =3D kvm_register_userspace_phys_mem(kvm_context, start= _addr, >>> - phys_ram_base + phys= _offset, >>> - size, 0); >>> - } >>> - if (r < 0) { >>> - printf("kvm_cpu_register_physical_memory: failed\n"); >>> - exit(1); >>> - } >>> - return; >>> + if (!(phys_offset & ~TARGET_PAGE_MASK)) { >>> + r =3D kvm_is_allocated_mem(kvm_context, start_addr, size); >>> + if (r) >>> + return; >>> + r =3D kvm_is_intersecting_mem(kvm_context, start_addr); >>> + if (r) >>> + kvm_create_mem_hole(kvm_context, start_addr, size); >>> + r =3D kvm_register_userspace_phys_mem(kvm_context, start_add= r, >>> + phys_ram_base + phys_off= set, >>> + size, 0); >>> } >>> -#endif >>> if (phys_offset & IO_MEM_ROM) { >> At this chance: Shouldn't this become 'else if'? >=20 > fyi: just sent out a new series, this time with numbers and no tabs in = qemu ;-) Yeah, much better. :) >=20 > As a coding style practice, yes. But they are mutually exclusive anyway= , so it shouldn't > matter. We could definitely try to make it better, but they way to go i= s to have only one case > instead of two: look at how close they are to each other! I have a late= r series that do that, > so I'm just trying to set up the ground here. Well, they are close partly because the IO_MEM_ROM part is incomplete (it ignores the 'read-only' property). However, I just stumbled over it while trying to understand how this function works. Without the 'else', I had to check twice that the cases are exclusive in practice... Jan --------------enig61CD5AA966399E28FD3832A9 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAkjIJyMACgkQniDOoMHTA+mr3wCfU2Le9+O1R6vhTAZpx5C68+/9 5o8An0iafNOsTzG0um2Jin2VB2PEucdz =kstk -----END PGP SIGNATURE----- --------------enig61CD5AA966399E28FD3832A9--