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

[Manos Pitsidianakis <el13635@mail.ntua.gr>]

[-- Attachment #1: Type: text/plain, Size: 3085 bytes --]

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 <berto@igalia.com>
>>>> Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
>>>
>>> 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
>




[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]