From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52888) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1axFvP-00018q-Tt for qemu-devel@nongnu.org; Mon, 02 May 2016 11:39:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1axFsm-0001RI-8j for qemu-devel@nongnu.org; Mon, 02 May 2016 11:38:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46288) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1axFsl-0001Ny-W4 for qemu-devel@nongnu.org; Mon, 02 May 2016 11:35:36 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DE5DC2193 for ; Mon, 2 May 2016 15:35:24 +0000 (UTC) Date: Mon, 2 May 2016 17:35:22 +0200 From: Kevin Wolf Message-ID: <20160502153522.GD4882@noname.redhat.com> References: <1461960516-4717-1-git-send-email-eblake@redhat.com> <1461960516-4717-7-git-send-email-eblake@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1461960516-4717-7-git-send-email-eblake@redhat.com> Subject: Re: [Qemu-devel] [PATCH v4 06/14] sd: Switch to byte-based block access List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org Am 29.04.2016 um 22:08 hat Eric Blake geschrieben: > Sector-based blk_write() should die; switch to byte-based > blk_pwrite() instead. Likewise for blk_read(). > > Greatly simplifies the code, now that we let the block layer > take care of alignment and read-modify-write on our behalf :) > > Signed-off-by: Eric Blake > --- > hw/sd/sd.c | 46 +++------------------------------------------- > 1 file changed, 3 insertions(+), 43 deletions(-) > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index b66e5d2..3c2f2f1 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -1577,57 +1577,17 @@ send_response: > > static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len) > { > - uint64_t end = addr + len; > - > DPRINTF("sd_blk_read: addr = 0x%08llx, len = %d\n", > (unsigned long long) addr, len); > - if (!sd->blk || blk_read(sd->blk, addr >> 9, sd->buf, 1) < 0) { > + if (!sd->blk || blk_pread(sd->blk, addr, sd->data, len) < 0) { > fprintf(stderr, "sd_blk_read: read error on host side\n"); > - return; > } > - > - if (end > (addr & ~511) + 512) { > - memcpy(sd->data, sd->buf + (addr & 511), 512 - (addr & 511)); > - > - if (blk_read(sd->blk, end >> 9, sd->buf, 1) < 0) { > - fprintf(stderr, "sd_blk_read: read error on host side\n"); > - return; > - } > - memcpy(sd->data + 512 - (addr & 511), sd->buf, end & 511); > - } else > - memcpy(sd->data, sd->buf + (addr & 511), len); > } We can remove sd->buf from SDState, it should be unused now. I'm not sure why a temporary buffer was ever included in the VMState, but I guess we can make it a VMSTATE_UNUSED_BUFFER(). > static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len) > { > - uint64_t end = addr + len; > - > - if ((addr & 511) || len < 512) > - if (!sd->blk || blk_read(sd->blk, addr >> 9, sd->buf, 1) < 0) { > - fprintf(stderr, "sd_blk_write: read error on host side\n"); > - return; > - } > - > - if (end > (addr & ~511) + 512) { > - memcpy(sd->buf + (addr & 511), sd->data, 512 - (addr & 511)); > - if (blk_write(sd->blk, addr >> 9, sd->buf, 1) < 0) { > - fprintf(stderr, "sd_blk_write: write error on host side\n"); > - return; > - } > - > - if (blk_read(sd->blk, end >> 9, sd->buf, 1) < 0) { > - fprintf(stderr, "sd_blk_write: read error on host side\n"); > - return; > - } > - memcpy(sd->buf, sd->data + 512 - (addr & 511), end & 511); > - if (blk_write(sd->blk, end >> 9, sd->buf, 1) < 0) { > - fprintf(stderr, "sd_blk_write: write error on host side\n"); > - } > - } else { > - memcpy(sd->buf + (addr & 511), sd->data, len); > - if (!sd->blk || blk_write(sd->blk, addr >> 9, sd->buf, 1) < 0) { > - fprintf(stderr, "sd_blk_write: write error on host side\n"); > - } > + if (!sd->blk || blk_pwrite(sd->blk, addr, sd->buf, len, 0) < 0) { I said "should" instead of "is" above because this line is buggy. sd->data should probably be used instead. Kevin