Re: [Qemu-block] [Qemu-devel] [PATCH v9 6/6] qemu-iotests: add 184 for throttle filter driver

[Manos Pitsidianakis]

On Tue, Sep 05, 2017 at 04:13:39PM -0500, Eric Blake wrote:
> On 09/05/2017 02:06 PM, Kevin Wolf wrote:
> > Am 05.09.2017 um 18:16 hat Kevin Wolf geschrieben:
> > > Am 25.08.2017 um 15:20 hat Manos Pitsidianakis geschrieben:
> > > > Reviewed-by: Alberto Garcia <address@hidden>
> > > > Signed-off-by: Manos Pitsidianakis <address@hidden>
> > >
> > > Does this test actually (still) pass for you? I can't see that it's
> > > related to any recent change in master, but this is the diff that I get.
> > >
> > > I can update the reference output while applying, but obviously if it's
> > > currently passing for you, it will fail after I "fix" it.
> >
> > For the record, we discussed this on IRC. The test works correctly on
> > master, but on my block branch there is a conflict with "block: pass
> > bdrv_* methods to bs->file by default in block filters".
> >
> > The correct action is to merge this throttle driver series after the
> > conflicting patch because throttle doesn't implement .bdrv_get_info and
> > needs the forwarding that the other patch implements.
> >
> > I updated the test output accordingly and applied the series to my block
> > branch.
>
> Could you also squash this in to 5/6? (as long as we're intentionally
> basing throttle on top of defaults, then we should use the right default
> instead of duplicating things)

Yes, this change makes sense, if it's no trouble with Kevin.


> diff --git i/block/throttle.c w/block/throttle.c
> index 7b33613372..5bca76300f 100644
> --- i/block/throttle.c
> +++ w/block/throttle.c
> @@ -197,19 +197,6 @@ static bool
> throttle_recurse_is_first_non_filter(BlockDriverState *bs,
>     return bdrv_recurse_is_first_non_filter(bs->file->bs, candidate);
> }
>
> -static int64_t coroutine_fn
> throttle_co_get_block_status(BlockDriverState *bs,
> -                                                         int64_t
> sector_num,
> -                                                         int nb_sectors,
> -                                                         int *pnum,
> -
> BlockDriverState **file)
> -{
> -    assert(bs->file && bs->file->bs);
> -    *pnum = nb_sectors;
> -    *file = bs->file->bs;
> -    return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID |
> -           (sector_num << BDRV_SECTOR_BITS);
> -}
> -
> static BlockDriver bdrv_throttle = {
>     .format_name                        =   "throttle",
>     .protocol_name                      =   "throttle",
> @@ -237,7 +224,7 @@ static BlockDriver bdrv_throttle = {
>     .bdrv_reopen_prepare                =   throttle_reopen_prepare,
>     .bdrv_reopen_commit                 =   throttle_reopen_commit,
>     .bdrv_reopen_abort                  =   throttle_reopen_abort,
> -    .bdrv_co_get_block_status           =   throttle_co_get_block_status,
> +    .bdrv_co_get_block_status           =
> bdrv_co_get_block_status_from_file,
>
>     .is_filter                          =   true,
> };
>
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org

signature.asc
Description: PGP signature