[{"id":1770839,"web_url":"http://patchwork.ozlabs.org/comment/1770839/","msgid":"<20170918211129.GB27153@umbus>","list_archive_url":null,"date":"2017-09-18T21:11:29","subject":"Re: [Qemu-devel] [PATCH 2/4] spapr: introduce helpers to migrate\n\tHPT chunks and the end marker","submitter":{"id":47,"url":"http://patchwork.ozlabs.org/api/people/47/","name":"David Gibson","email":"david@gibson.dropbear.id.au"},"content":"On Fri, Sep 15, 2017 at 03:16:01PM +0200, Greg Kurz wrote:\n> This consolidates some duplicated code in a single helper.\n> \n> Signed-off-by: Greg Kurz <groug@kaod.org>\n> ---\n>  hw/ppc/spapr.c |   38 +++++++++++++++++++++-----------------\n>  1 file changed, 21 insertions(+), 17 deletions(-)\n> \n> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c\n> index f680f28a15ea..841117f6d185 100644\n> --- a/hw/ppc/spapr.c\n> +++ b/hw/ppc/spapr.c\n> @@ -1708,6 +1708,23 @@ static int htab_save_setup(QEMUFile *f, void *opaque)\n>      return 0;\n>  }\n>  \n> +static void htab_save_chunk(QEMUFile *f, sPAPRMachineState *spapr,\n> +                            int chunkstart, int n_valid, int n_invalid)\n> +{\n> +    qemu_put_be32(f, chunkstart);\n> +    qemu_put_be16(f, n_valid);\n> +    qemu_put_be16(f, n_invalid);\n> +    if (spapr) {\n\nI like the basic idea, but passing NULL for spapr is *only* valid for\nthe end marker case.  The general test here is misleading.  I'd prefer\nto either see the writes opencoded for the end marker, or have an\nassert here (spapr can only be NULL if n_valid == n_invalid == 0).\n\n> +        qemu_put_buffer(f, HPTE(spapr->htab, chunkstart),\n> +                        HASH_PTE_SIZE_64 * n_valid);\n> +    }\n> +}\n> +\n> +static void htab_save_end_marker(QEMUFile *f)\n> +{\n> +    htab_save_chunk(f, NULL, 0, 0, 0);\n> +}\n> +\n>  static void htab_save_first_pass(QEMUFile *f, sPAPRMachineState *spapr,\n>                                   int64_t max_ns)\n>  {\n> @@ -1739,11 +1756,7 @@ static void htab_save_first_pass(QEMUFile *f, sPAPRMachineState *spapr,\n>          if (index > chunkstart) {\n>              int n_valid = index - chunkstart;\n>  \n> -            qemu_put_be32(f, chunkstart);\n> -            qemu_put_be16(f, n_valid);\n> -            qemu_put_be16(f, 0);\n> -            qemu_put_buffer(f, HPTE(spapr->htab, chunkstart),\n> -                            HASH_PTE_SIZE_64 * n_valid);\n> +            htab_save_chunk(f, spapr, chunkstart, n_valid, 0);\n>  \n>              if (has_timeout &&\n>                  (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - starttime) > max_ns) {\n> @@ -1805,11 +1818,7 @@ static int htab_save_later_pass(QEMUFile *f, sPAPRMachineState *spapr,\n>              int n_valid = invalidstart - chunkstart;\n>              int n_invalid = index - invalidstart;\n>  \n> -            qemu_put_be32(f, chunkstart);\n> -            qemu_put_be16(f, n_valid);\n> -            qemu_put_be16(f, n_invalid);\n> -            qemu_put_buffer(f, HPTE(spapr->htab, chunkstart),\n> -                            HASH_PTE_SIZE_64 * n_valid);\n> +            htab_save_chunk(f, spapr, chunkstart, n_valid, n_invalid);\n>              sent += index - chunkstart;\n>  \n>              if (!final && (qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - starttime) > max_ns) {\n> @@ -1872,10 +1881,7 @@ static int htab_save_iterate(QEMUFile *f, void *opaque)\n>          rc = htab_save_later_pass(f, spapr, MAX_ITERATION_NS);\n>      }\n>  \n> -    /* End marker */\n> -    qemu_put_be32(f, 0);\n> -    qemu_put_be16(f, 0);\n> -    qemu_put_be16(f, 0);\n> +    htab_save_end_marker(f);\n>  \n>      return rc;\n>  }\n> @@ -1915,9 +1921,7 @@ static int htab_save_complete(QEMUFile *f, void *opaque)\n>      }\n>  \n>      /* End marker */\n> -    qemu_put_be32(f, 0);\n> -    qemu_put_be16(f, 0);\n> -    qemu_put_be16(f, 0);\n> +    htab_save_end_marker(f);\n>  \n>      return 0;\n>  }\n>","headers":{"Return-Path":"<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=<UNKNOWN>)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=gibson.dropbear.id.au\n\theader.i=@gibson.dropbear.id.au header.b=\"KBLJ9OwT\"; \n\tdkim-atps=neutral"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xxKCc0BCYz9sBZ\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 19 Sep 2017 20:40:44 +1000 (AEST)","from localhost ([::1]:41371 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>)\n\tid 1duFxK-0003zz-49\n\tfor incoming@patchwork.ozlabs.org; Tue, 19 Sep 2017 06:40:42 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:51065)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from <dgibson@ozlabs.org>) id 1duFtS-0001B1-Sm\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 06:36:45 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from <dgibson@ozlabs.org>) id 1duFtQ-0002na-Rm\n\tfor qemu-devel@nongnu.org; Tue, 19 Sep 2017 06:36:42 -0400","from ozlabs.org ([2401:3900:2:1::2]:55493)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from <dgibson@ozlabs.org>)\n\tid 1duFtQ-0002jA-DF; Tue, 19 Sep 2017 06:36:40 -0400","by ozlabs.org (Postfix, from userid 1007)\n\tid 3xxK6p3J6Pz9t2m; Tue, 19 Sep 2017 20:36:33 +1000 (AEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=gibson.dropbear.id.au; s=201602; t=1505817394;\n\tbh=qYtwWaPj6oBWYRR/CtSo5fpwr6iX8VQX7A46hl4GJSA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KBLJ9OwTklZvqysB/Jg5abVg7+Wdyp9XebQKAvNxxI3wABtaxuaViXcRKhT8OKe71\n\tWprqiZFzq7kmWHjHILYzDxcYAK1f4o6HtaZiYY8Rmi+wtP5EbEj8iL1T6UAvBkYhTO\n\taKAvxVt6hdvWWCmRkGdCKbLfmnU98xxczs3wI7aM=","Date":"Tue, 19 Sep 2017 07:11:29 +1000","From":"David Gibson <david@gibson.dropbear.id.au>","To":"Greg Kurz <groug@kaod.org>","Message-ID":"<20170918211129.GB27153@umbus>","References":"<150548133297.5945.7401220081077343726.stgit@bahia.lan>\n\t<150548136111.5945.6437882724141726495.stgit@bahia.lan>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"mojUlQ0s9EVzWg2t\"","Content-Disposition":"inline","In-Reply-To":"<150548136111.5945.6437882724141726495.stgit@bahia.lan>","User-Agent":"Mutt/1.8.3 (2017-05-23)","X-detected-operating-system":"by eggs.gnu.org: Genre and OS details not\n\trecognized.","X-Received-From":"2401:3900:2:1::2","Subject":"Re: [Qemu-devel] [PATCH 2/4] spapr: introduce helpers to migrate\n\tHPT chunks and the end marker","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"<qemu-devel.nongnu.org>","List-Unsubscribe":"<https://lists.nongnu.org/mailman/options/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>","List-Archive":"<http://lists.nongnu.org/archive/html/qemu-devel/>","List-Post":"<mailto:qemu-devel@nongnu.org>","List-Help":"<mailto:qemu-devel-request@nongnu.org?subject=help>","List-Subscribe":"<https://lists.nongnu.org/mailman/listinfo/qemu-devel>,\n\t<mailto:qemu-devel-request@nongnu.org?subject=subscribe>","Cc":"Alexey Kardashevskiy <aik@ozlabs.ru>, Thomas Huth <thuth@redhat.com>,\n\tqemu-ppc@nongnu.org, qemu-devel@nongnu.org","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t<qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org>"}}]