From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:51944)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1Ufrip-0006RQ-Cq
	for qemu-devel@nongnu.org; Fri, 24 May 2013 09:07:58 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1Ufrii-00070l-Be
	for qemu-devel@nongnu.org; Fri, 24 May 2013 09:07:51 -0400
Received: from mx1.redhat.com ([209.132.183.28]:24291)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1Ufrii-00070C-1n
	for qemu-devel@nongnu.org; Fri, 24 May 2013 09:07:44 -0400
Date: Fri, 24 May 2013 15:06:22 +0200
From: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20130524130622.GA3426@dhcp-200-207.str.redhat.com>
References: <1369331087-22345-1-git-send-email-coreyb@linux.vnet.ibm.com>
	<1369331087-22345-2-git-send-email-coreyb@linux.vnet.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1369331087-22345-2-git-send-email-coreyb@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH 1/7] vnvram: VNVRAM bdrv support
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<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>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Corey Bryant <coreyb@linux.vnet.ibm.com>
Cc: aliguori@us.ibm.com, stefanb@linux.vnet.ibm.com, mdroth@linux.vnet.ibm.com, qemu-devel@nongnu.org, jschopp@linux.vnet.ibm.com, stefanha@redhat.com, lcapitulino@redhat.com

Am 23.05.2013 um 19:44 hat Corey Bryant geschrieben:
> Provides low-level VNVRAM functionality that reads and writes data,
> such as an entry's binary blob, to a drive image using the block
> driver.
> 
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>

> +/*
> + * Increase the drive size if it's too small to fit the VNVRAM data
> + */
> +static int vnvram_drv_adjust_size(VNVRAM *vnvram)
> +{
> +    int rc = 0;
> +    int64_t needed_size;
> +
> +    needed_size = 0;
> +
> +    if (bdrv_getlength(vnvram->bds) < needed_size) {
> +        rc = bdrv_truncate(vnvram->bds, needed_size);
> +        if (rc != 0) {
> +            DPRINTF("%s: VNVRAM drive too small\n", __func__);
> +        }
> +    }
> +
> +    return rc;
> +}

This function doesn't make a whole lot of sense. It truncates the file
to size 0 if and only if bdrv_getlength() returns an error.

> +
> +/*
> + * Write a header to the drive with entry count of zero
> + */
> +static int vnvram_drv_hdr_create_empty(VNVRAM *vnvram)
> +{
> +    VNVRAMDrvHdr hdr;
> +
> +    hdr.version = VNVRAM_CURRENT_VERSION;
> +    hdr.magic = VNVRAM_MAGIC;
> +    hdr.num_entries = 0;
> +
> +    vnvram_drv_hdr_cpu_to_be((&hdr));
> +
> +    if (bdrv_pwrite(vnvram->bds, 0, (&hdr), sizeof(hdr)) != sizeof(hdr)) {
> +        DPRINTF("%s: Write of header to drive failed\n", __func__);
> +        return -EIO;
> +    }
> +
> +    vnvram->end_offset = sizeof(VNVRAMDrvHdr);
> +
> +    return 0;
> +}
> +
> +/*
> + * Read the header from the drive
> + */
> +static int vnvram_drv_hdr_read(VNVRAM *vnvram, VNVRAMDrvHdr *hdr)
> +{
> +    if (bdrv_pread(vnvram->bds, 0, hdr, sizeof(*hdr)) != sizeof(*hdr)) {
> +        DPRINTF("%s: Read of header from drive failed\n", __func__);
> +        return -EIO;
> +    }

Why do you turn all errors into -EIO instead of returning the real error
code? (More instances of the same thing follow)

> +
> +    vnvram_drv_hdr_be_to_cpu(hdr);
> +
> +    return 0;
> +}
> +}

Kevin