From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50913) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c40MC-0007ai-Qz for qemu-devel@nongnu.org; Tue, 08 Nov 2016 01:58:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c40M7-0002I3-W4 for qemu-devel@nongnu.org; Tue, 08 Nov 2016 01:58:08 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57916) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c40M7-0002Hr-No for qemu-devel@nongnu.org; Tue, 08 Nov 2016 01:58:03 -0500 References: <1478265017-5700-1-git-send-email-thuth@redhat.com> <20161108011404.GB28688@umbus.fritz.box> From: Thomas Huth Message-ID: <24259ea4-df50-2669-74f8-c3247d07b131@redhat.com> Date: Tue, 8 Nov 2016 07:57:52 +0100 MIME-Version: 1.0 In-Reply-To: <20161108011404.GB28688@umbus.fritz.box> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Gx6qvabdA0qLSwc8FDPMUGbhCl3feB0AH" Subject: Re: [Qemu-devel] [PATCH for-2.8] migration: Fix return code of ram_save_iterate() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: Juan Quintela , Amit Shah , qemu-devel@nongnu.org, "Dr. David Alan Gilbert" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Gx6qvabdA0qLSwc8FDPMUGbhCl3feB0AH Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 08.11.2016 02:14, David Gibson wrote: > On Fri, Nov 04, 2016 at 02:10:17PM +0100, Thomas Huth wrote: >> qemu_savevm_state_iterate() expects the iterators to return 1 >> when they are done, and 0 if there is still something left to do. >> However, ram_save_iterate() does not obey this rule and returns >> the number of saved pages instead. This causes a fatal hang with >> ppc64 guests when you run QEMU like this (also works with TCG): >> >> qemu-img create -f qcow2 /tmp/test.qcow2 1M >> qemu-system-ppc64 -nographic -nodefaults -m 256 \ >> -hda /tmp/test.qcow2 -serial mon:stdio >> >> ... then switch to the monitor by pressing CTRL-a c and try to >> save a snapshot with "savevm test1" for example. >> >> After the first iteration, ram_save_iterate() always returns 0 here, >> so that qemu_savevm_state_iterate() hangs in an endless loop and you >> can only "kill -9" the QEMU process. >> Fix it by using proper return values in ram_save_iterate(). >> >> Signed-off-by: Thomas Huth >=20 > Hmm. I think the change is technically correct, but I'm uneasy with > this approach to the solution. The whole reason this wasn't caught > earlier is that almost nothing looks at the return value. Without > changing that I think it's very likely someone will mess this up > again. >=20 > I think it would be preferable to change the return type to void to > make it explicit that this function is not directly returning the > "completion" status, but instead that's calculated from the other > progress variables it updates. Not sure how such a patch should finally look like. Could you propose a patch? Anyway, we're in soft freeze already ... do we still want such a major change of the logic at this point in time? If not, we should maybe go with fixing the return type only for 2.8, and do the major change for 2.9 instead? Thomas --Gx6qvabdA0qLSwc8FDPMUGbhCl3feB0AH 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.22 (GNU/Linux) iQIcBAEBAgAGBQJYIXd1AAoJEC7Z13T+cC21oCMP+wRXJWmPeP5sz5Tr7z1AJHp1 Ip9vhJmVKq2eXU51REOzCxOzUy1EruuSC8KIJtq4o86L17AXUR7decQcYuWrZUVI jPNoxmOMdFLP/oWtr9ZgW6THdiNYXV7ARF3kZ7IYrzesgUq646AZX5WLRkJH4Tui JdCjYRJ5isNLYbIMV57cbuQZ4IGveU5r28VsiJKoKHa96i3cKocAZ3SeVI8z4lwb 7qr0U83XadLiTPKUiPNS1TktbV64/psp+7c2Qgq/otoE+dihUKFvdApR6CPnXu/W 3Rskj9KAdGGIrGa5g43Gbg9x1hHUKe7HgpL/wYNl4iA9jGLKidm3q6kAO0GOo3II 5pmNtt1uyAMfqzhXJWG9OphVpsWe6G8kiWDKaXCEfpm8kIJlGZTYPHQvcPtOFnbe yvC0tOwKFad5GGY7DAo1c7TUDGLGLQ2oS6Mrr1JZRAeZputMLGP1o1FJOL/EnDvX efsaJMniIeK7Wz3ZIpqX4faYpTtCF2eGfKUAobeor8Dnikze+sTTY4/EWTVihD4l GcIv/C7KhaLWN64NIC5LjTVfy1kYXCyJbTIen0ZYfiWeehQcqMFPJrPJ7KpEXeBV F3W7qx27esaFoAlojnuqiUQWnrruX3sayq8snZDehOqEr0TmzJISIKMpiwyTTGEH XffMenxYo1WK7JBr5va0 =G/0X -----END PGP SIGNATURE----- --Gx6qvabdA0qLSwc8FDPMUGbhCl3feB0AH--