[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-stable] [PATCH 13/38] block: expect errors from b
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [Qemu-stable] [PATCH 13/38] block: expect errors from bdrv_co_is_allocated |
Date: |
Thu, 26 Sep 2013 22:51:40 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130805 Thunderbird/17.0.8 |
Il 25/09/2013 23:27, Doug Goldstein ha scritto:
> On Wed, Sep 25, 2013 at 7:57 AM, Michael Roth <address@hidden> wrote:
>> From: Paolo Bonzini <address@hidden>
>>
>> Some bdrv_is_allocated callers do not expect errors, but the fallback
>> in qcow2.c might make other callers trip on assertion failures or
>> infinite loops.
>>
>> Fix the callers to always look for errors.
>>
>> Cc: address@hidden
>> Reviewed-by: Eric Blake <address@hidden>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> Signed-off-by: Stefan Hajnoczi <address@hidden>
>> (cherry picked from commit d663640c04f2aab810915c556390211d75457704)
>>
>> Conflicts:
>>
>> block/cow.c
>>
>> *modified to avoid dependency on upstream's e641c1e8
>>
>> Signed-off-by: Michael Roth <address@hidden>
>> ---
>> block.c | 7 +++++--
>> block/cow.c | 6 +++++-
>> block/qcow2.c | 4 +---
>> block/stream.c | 2 +-
>> qemu-img.c | 16 ++++++++++++++--
>> qemu-io-cmds.c | 4 ++++
>> 6 files changed, 30 insertions(+), 9 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index d5ce8d3..8ce8b91 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -1803,8 +1803,11 @@ int bdrv_commit(BlockDriverState *bs)
>> buf = g_malloc(COMMIT_BUF_SECTORS * BDRV_SECTOR_SIZE);
>>
>> for (sector = 0; sector < total_sectors; sector += n) {
>> - if (bdrv_is_allocated(bs, sector, COMMIT_BUF_SECTORS, &n)) {
>> -
>> + ret = bdrv_is_allocated(bs, sector, COMMIT_BUF_SECTORS, &n);
>> + if (ret < 0) {
>> + goto ro_cleanup;
>> + }
>> + if (ret) {
>> if (bdrv_read(bs, sector, buf, n) != 0) {
>> ret = -EIO;
>> goto ro_cleanup;
>> diff --git a/block/cow.c b/block/cow.c
>> index 1cc2e89..e1b73d6 100644
>> --- a/block/cow.c
>> +++ b/block/cow.c
>> @@ -189,7 +189,11 @@ static int coroutine_fn cow_read(BlockDriverState *bs,
>> int64_t sector_num,
>> int ret, n;
>>
>> while (nb_sectors > 0) {
>> - if (bdrv_co_is_allocated(bs, sector_num, nb_sectors, &n)) {
>> + ret = bdrv_co_is_allocated(bs, sector_num, nb_sectors, &n);
>
> Is suppose to be ret = cow_co_is_allocated() ?
No, it's correct to have it like this in the backport.
>> + if (ret < 0) {
>> + return ret;
>> + }
>> + if (ret) {
>> ret = bdrv_pread(bs->file,
>> s->cow_sectors_offset + sector_num * 512,
>> buf, n * 512);
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index 3376901..7f7282e 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -648,13 +648,11 @@ static int coroutine_fn
>> qcow2_co_is_allocated(BlockDriverState *bs,
>> int ret;
>>
>> *pnum = nb_sectors;
>> - /* FIXME We can get errors here, but the bdrv_co_is_allocated interface
>> - * can't pass them on today */
>> qemu_co_mutex_lock(&s->lock);
>> ret = qcow2_get_cluster_offset(bs, sector_num << 9, pnum,
>> &cluster_offset);
>> qemu_co_mutex_unlock(&s->lock);
>> if (ret < 0) {
>> - *pnum = 0;
>> + return ret;
>> }
>>
>> return (cluster_offset != 0) || (ret == QCOW2_CLUSTER_ZERO);
>> diff --git a/block/stream.c b/block/stream.c
>> index 7fe9e48..4e8d177 100644
>> --- a/block/stream.c
>> +++ b/block/stream.c
>> @@ -120,7 +120,7 @@ wait:
>> if (ret == 1) {
>> /* Allocated in the top, no need to copy. */
>> copy = false;
>> - } else {
>> + } else if (ret >= 0) {
>> /* Copy if allocated in the intermediate images. Limit to the
>> * known-unallocated area [sector_num, sector_num+n). */
>> ret = bdrv_co_is_allocated_above(bs->backing_hd, base,
>> diff --git a/qemu-img.c b/qemu-img.c
>> index b9a848d..b01998b 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -1485,8 +1485,15 @@ static int img_convert(int argc, char **argv)
>> are present in both the output's and input's base images
>> (no
>> need to copy them). */
>> if (out_baseimg) {
>> - if (!bdrv_is_allocated(bs[bs_i], sector_num - bs_offset,
>> - n, &n1)) {
>> + ret = bdrv_is_allocated(bs[bs_i], sector_num -
>> bs_offset,
>> + n, &n1);
>> + if (ret < 0) {
>> + error_report("error while reading metadata for
>> sector "
>> + "%" PRId64 ": %s",
>> + sector_num - bs_offset,
>> strerror(-ret));
>> + goto out;
>> + }
>> + if (!ret) {
>> sector_num += n1;
>> continue;
>> }
>> @@ -2076,6 +2083,11 @@ static int img_rebase(int argc, char **argv)
>>
>> /* If the cluster is allocated, we don't need to take action */
>> ret = bdrv_is_allocated(bs, sector, n, &n);
>> + if (ret < 0) {
>> + error_report("error while reading image metadata: %s",
>> + strerror(-ret));
>> + goto out;
>> + }
>> if (ret) {
>> continue;
>> }
>> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
>> index ffbcf31..ffe48ad 100644
>> --- a/qemu-io-cmds.c
>> +++ b/qemu-io-cmds.c
>> @@ -1829,6 +1829,10 @@ static int alloc_f(BlockDriverState *bs, int argc,
>> char **argv)
>> sector_num = offset >> 9;
>> while (remaining) {
>> ret = bdrv_is_allocated(bs, sector_num, remaining, &num);
>> + if (ret < 0) {
>> + printf("is_allocated failed: %s\n", strerror(-ret));
>> + return 0;
>> + }
>> sector_num += num;
>> remaining -= num;
>> if (ret) {
>> --
>> 1.7.9.5
>>
>>
>
- [Qemu-devel] [PATCH 04/38] rdma: silly ipv6 bugfix, (continued)
- [Qemu-devel] [PATCH 04/38] rdma: silly ipv6 bugfix, Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 07/38] scsi: Fix scsi_bus_legacy_add_drive() scsi-generic with serial, Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 09/38] pseries: Fix stalls on hypervisor virtual console, Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 08/38] pc: fix regression for 64 bit PCI memory, Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 10/38] virtio: virtqueue_get_avail_bytes: fix desc_pa when loop over the indirect descriptor table, Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 11/38] xhci: fix endpoint interval calculation, Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 12/38] Revert "usb-hub: report status changes only once", Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 13/38] block: expect errors from bdrv_co_is_allocated, Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 14/38] target-i386: fix disassembly with PAE=1, PG=0, Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 15/38] adlib: sort offsets in portio registration, Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 16/38] exec: fix writing to MMIO area with non-power-of-two length, Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 18/38] exec: always use MADV_DONTFORK, Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 17/38] virtio_pci: fix level interrupts with irqfd, Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 19/38] xhci: reset port when disabling slot, Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 20/38] usb: parallelize usb3 streams, Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 21/38] w32: Fix access to host devices (regression), Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 23/38] Revert "memory: Return -1 again on reads from unsigned regions", Michael Roth, 2013/09/25
- [Qemu-devel] [PATCH 24/38] exec: check offset_within_address_space for register subpage, Michael Roth, 2013/09/25