[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return
From: |
Peter Lieven |
Subject: |
Re: [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return value |
Date: |
Tue, 23 Jul 2013 23:18:18 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120827 Thunderbird/15.0 |
Am 20.07.2013 09:00, schrieb Paolo Bonzini:
> Il 19/07/2013 15:06, Peter Lieven ha scritto:
>>>>> Il 19/07/2013 08:48, Peter Lieven ha scritto:
>>>>>>> - return bdrv_get_block_status(bs, sector_num, nb_sectors, pnum);
>>>>>>> + int64_t ret = bdrv_get_block_status(bs, sector_num, nb_sectors,
>>>>>>> pnum);
>>>>>>> + return
>>>>>>> + (ret & BDRV_BLOCK_DATA) ||
>>>>>>> + ((ret & BDRV_BLOCK_ZERO) && !bdrv_has_zero_init(bs));
>>>>>> i do also not understand the "((ret & BDRV_BLOCK_ZERO) &&
>>>>>> !bdrv_has_zero_init(bs))";
>>>>>> if a block is unallocated and reads as zero, but the device lacks zero
>>>>>> init, it is declared as allocated with this, isn't it?
>> If it is zero and allocated the API should return only BDRV_BLOCK_DATA
>> and if it is zero and unallocated only BDRV_BLOCK_ZERO or not?
>>
>> What I mean is the new API shouldn't change the behaviour of the old
>> bdrv_is_allocated().
>> It would have returned
>>
>> (ret & BDRV_BLOCK_DATA) regardless if BDRV_BLOCK_ZERO or not.
> bdrv_is_allocated must return true for some zero clusters, even
> if BDRV_BLOCK_DATA = 0. See
>
> commit 381b487d54ba18c73df9db8452028a330058c505
> Author: Paolo Bonzini <address@hidden>
> Date: Wed Mar 6 18:02:01 2013 +0100
>
> qcow2: make is_allocated return true for zero clusters
>
> Otherwise, live migration of the top layer will miss zero clusters and
> let the backing file show through. This also matches what is done in qed.
>
> QCOW2_CLUSTER_ZERO clusters are invalid in v2 image files. Check this
> directly in qcow2_get_cluster_offset instead of replicating the test
> everywhere.
>
> Cc: address@hidden
> Signed-off-by: Paolo Bonzini <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
>
> I think the source of the confusion is that SCSI "GET LBA STATUS"
> does not have to deal with backing files, bdrv_is_allocated must.
> If bs->backing_hd != NULL, bdrv_is_allocated is not about allocation
> of blocks in the SCSI sense; it's a query for "does the block come
> from this BDS or rather from its backing file?".
Ok, this makes it clearer. Thanks for pointing that out.
the bdrv_get_block_status() will add the possibility to
check for the allocation status which is a good thing.
Peter
- [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return value, (continued)
- [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return value, Paolo Bonzini, 2013/07/16
- Re: [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return value, Peter Lieven, 2013/07/19
- Re: [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return value, Peter Lieven, 2013/07/19
- Re: [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return value, Paolo Bonzini, 2013/07/19
- Re: [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return value, Paolo Bonzini, 2013/07/19
- Re: [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return value, Peter Lieven, 2013/07/19
- Re: [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return value, Paolo Bonzini, 2013/07/19
- Re: [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return value, Peter Lieven, 2013/07/19
- Re: [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return value, Paolo Bonzini, 2013/07/20
- Re: [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return value,
Peter Lieven <=
Re: [Qemu-devel] [PATCH v2 10/17] block: define get_block_status return value, Eric Blake, 2013/07/19
[Qemu-devel] [PATCH v2 11/17] block: return get_block_status data and flags for formats, Paolo Bonzini, 2013/07/16
[Qemu-devel] [PATCH v2 13/17] block: use bdrv_has_zero_init to return BDRV_BLOCK_ZERO, Paolo Bonzini, 2013/07/16
[Qemu-devel] [PATCH v2 12/17] qemu-img: add a "map" subcommand, Paolo Bonzini, 2013/07/16