[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 04/24] qcow2: Only return status from qcow2_g
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v6 04/24] qcow2: Only return status from qcow2_get_refcount |
Date: |
Wed, 11 Feb 2015 08:47:00 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
On 02/10/2015 01:28 PM, Max Reitz wrote:
> Refcounts can theoretically be of type uint64_t; in order to be able to
> represent the full range, qcow2_get_refcount() cannot use a single
> variable to represent both all refcount values and also keep some values
> reserved for errors.
>
> One solution would be to add an Error pointer parameter to
> qcow2_get_refcount(); however, no caller could (currently) pass that
> error message, so it would have to be emitted immediately and be
> passed to the next caller by returning -EIO or something similar.
> Therefore, an Error parameter does not offer any advantages here.
>
> The solution applied by this patch is simpler to use. Because no caller
> would be able to pass the error message, they would have to print it and
> free it, whereas with this patch the caller only needs to pass the
> returned integer (which is often a no-op from the code perspective,
> because that integer will be stored in a variable "ret" which will be
> returned by the fail path of many callers).
>
> Signed-off-by: Max Reitz <address@hidden>
> ---
> block/qcow2-cluster.c | 8 ++---
> block/qcow2-refcount.c | 86
> +++++++++++++++++++++++++++-----------------------
> block/qcow2.h | 3 +-
> 3 files changed, 53 insertions(+), 44 deletions(-)
>
> @@ -744,10 +749,9 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs,
> uint64_t offset,
> /* Check how many clusters there are free */
> cluster_index = offset >> s->cluster_bits;
> for(i = 0; i < nb_clusters; i++) {
> - refcount = qcow2_get_refcount(bs, cluster_index++);
Worth fixing the spacing after 'for' while touching here? (might even
already happen later in the series). But not important enough for a
respin, and I won't even care if a maintainer doesn't fold it in.
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v6 00/24] qcow2: Support refcount orders != 4, Max Reitz, 2015/02/10
- [Qemu-devel] [PATCH v6 01/24] qcow2: Add two new fields to BDRVQcowState, Max Reitz, 2015/02/10
- [Qemu-devel] [PATCH v6 03/24] qcow2: Do not return new value after refcount update, Max Reitz, 2015/02/10
- [Qemu-devel] [PATCH v6 02/24] qcow2: Add refcount_bits to format-specific info, Max Reitz, 2015/02/10
- [Qemu-devel] [PATCH v6 04/24] qcow2: Only return status from qcow2_get_refcount, Max Reitz, 2015/02/10
- [Qemu-devel] [PATCH v6 05/24] qcow2: Use unsigned addend for update_refcount(), Max Reitz, 2015/02/10
- [Qemu-devel] [PATCH v6 06/24] qcow2: Use 64 bits for refcount values, Max Reitz, 2015/02/10
- [Qemu-devel] [PATCH v6 10/24] qcow2: Open images with refcount order != 4, Max Reitz, 2015/02/10
- [Qemu-devel] [PATCH v6 08/24] qcow2: Helper function for refcount modification, Max Reitz, 2015/02/10
- [Qemu-devel] [PATCH v6 12/24] qcow2: Use symbolic macros in qcow2_amend_options, Max Reitz, 2015/02/10