[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/7] block: Make bdrv_{pread, pwrite}() return 0 on success
From: |
Alberto Faria |
Subject: |
Re: [PATCH 3/7] block: Make bdrv_{pread, pwrite}() return 0 on success |
Date: |
Fri, 13 May 2022 15:56:33 +0100 |
On Fri, May 13, 2022 at 9:22 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> The callers only check the return code of vhdx_log_peek_hdr,
> vhdx_log_read_sectors, vhdx_log_write_sectors with ret < 0. But looking
> at the callers:
>
> - vhdx_log_read_desc propagates ret directly from a successful
> vhdx_log_read_sectors, but its callers don't care about which
> nonnegative result is returned
>
> - vhdx_validate_log_entry might occasionally return ret directly from a
> successful vhdx_log_read_desc or vhdx_log_read_sectors, but
> vhdx_log_search, the caller of vhdx_validate_log_entry, also doesn't
> care about which nonnegative result is returned
>
> - vhdx_log_flush only returns a successful return value from bdrv_flush
>
> - vhdx_log_write propagates ret directly from a successful
> vhdx_log_write_sectors, but vhdx_log_write_and_flush only returns a
> successful return value from vhdx_log_flush
>
> So (perhaps as a separate patch?) you can remove the three assignments
> of ret.
Thanks, I'll fix this. I think I'll just fold it in, but let me know
if it really should be split into a separate patch.
> As an aside, while reviewing I noticed this in qcow2_mark_dirty:
>
> ret = bdrv_pwrite(bs->file, offsetof(QCowHeader,
> incompatible_features),
> &val, sizeof(val));
> if (ret < 0) {
> return ret;
> }
> ret = bdrv_flush(bs->file->bs);
> if (ret < 0) {
> return ret;
> }
>
> I'm not sure if there are other occurrencies, perhaps you can try using
> Coccinelle to find them and change them to a bdrv_pwrite_sync.
Looks like this is the only occurrence. I'll add a patch to convert it
to bdrv_pwrite_sync().
Alberto
- [PATCH 0/7] Implement bdrv_{pread, pwrite, pwrite_sync, pwrite_zeroes}() using generated_co_wrapper, Alberto Faria, 2022/05/12
- [PATCH 3/7] block: Make bdrv_{pread,pwrite}() return 0 on success, Alberto Faria, 2022/05/12
- [PATCH 2/7] block: Change bdrv_{pread, pwrite, pwrite_sync}() param order, Alberto Faria, 2022/05/12
- [PATCH 1/7] block: Add a 'flags' param to bdrv_{pread, pwrite, pwrite_sync}(), Alberto Faria, 2022/05/12
- [PATCH 5/7] block: Make 'bytes' param of bdrv_co_{pread, pwrite, preadv, pwritev}() an int64_t, Alberto Faria, 2022/05/12
- [PATCH 4/7] block: Make bdrv_co_pwrite() take a const buffer, Alberto Faria, 2022/05/12
- [PATCH 6/7] block: Implement bdrv_{pread, pwrite, pwrite_zeroes}() using generated_co_wrapper, Alberto Faria, 2022/05/12