[v5] block: add missed block_acct_setup with new block device init procedure

[PATCH v5 0/2] block: add missed block_acct_setup with new block device init procedure

Posted by Denis V. Lunev 1 year, 8 months ago

Commit 5f76a7aac156ca75680dad5df4a385fd0b58f6b1 is looking harmless from
the first glance, but it has changed things a lot. 'libvirt' uses it to
detect that it should follow new initialization way and this changes
things considerably. With this procedure followed, blockdev_init() is
not called anymore and thus block_acct_setup() helper is not called.

This means in particular that defaults for block accounting statistics
are changed and account_invalid/account_failed are actually initialized
as false instead of true originally.

This commit changes things to match original world. There are the following
constraints:
* new default value in block_acct_init() is set to true
* block_acct_setup() inside blockdev_init() is called before
  blkconf_apply_backend_options()
* thus newly created option in block device properties has precedence if
  specified

Changes from v4:
* removed hunk to QAPI which was used to test old initialization path
* added R-b: Vladimir

Changes from v3:
* fixed accidentally wrong submission. Contains changes which should be
  sent as v3

Changes from v2:
* called bool_from_onoffauto(account_..., true) in the first patch to
  preserve original semantics before patch 2

Changes from v1:
* set account_invalid/account_failed to true by default
* pass OnOffAuto to block_acct_init() to handle double initialization (patch 1)
* changed properties on BLK device to OnOffAuto

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Peter Krempa <pkrempa@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: John Snow <jsnow@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Re: [PATCH v5 0/2] block: add missed block_acct_setup with new block device init procedure

Posted by Kevin Wolf 1 year, 7 months ago

Am 24.08.2022 um 11:50 hat Denis V. Lunev geschrieben:
> Commit 5f76a7aac156ca75680dad5df4a385fd0b58f6b1 is looking harmless from
> the first glance, but it has changed things a lot. 'libvirt' uses it to
> detect that it should follow new initialization way and this changes
> things considerably. With this procedure followed, blockdev_init() is
> not called anymore and thus block_acct_setup() helper is not called.
> 
> This means in particular that defaults for block accounting statistics
> are changed and account_invalid/account_failed are actually initialized
> as false instead of true originally.
> 
> This commit changes things to match original world. There are the following
> constraints:
> * new default value in block_acct_init() is set to true
> * block_acct_setup() inside blockdev_init() is called before
>   blkconf_apply_backend_options()
> * thus newly created option in block device properties has precedence if
>   specified

Thanks, applied to the block branch.

Kevin

Re: [PATCH v5 0/2] block: add missed block_acct_setup with new block device init procedure

Posted by Denis V. Lunev 1 year, 7 months ago

On 8/24/22 11:50, Denis V. Lunev wrote:
> Commit 5f76a7aac156ca75680dad5df4a385fd0b58f6b1 is looking harmless from
> the first glance, but it has changed things a lot. 'libvirt' uses it to
> detect that it should follow new initialization way and this changes
> things considerably. With this procedure followed, blockdev_init() is
> not called anymore and thus block_acct_setup() helper is not called.
>
> This means in particular that defaults for block accounting statistics
> are changed and account_invalid/account_failed are actually initialized
> as false instead of true originally.
>
> This commit changes things to match original world. There are the following
> constraints:
> * new default value in block_acct_init() is set to true
> * block_acct_setup() inside blockdev_init() is called before
>    blkconf_apply_backend_options()
> * thus newly created option in block device properties has precedence if
>    specified
>
> Changes from v4:
> * removed hunk to QAPI which was used to test old initialization path
> * added R-b: Vladimir
>
> Changes from v3:
> * fixed accidentally wrong submission. Contains changes which should be
>    sent as v3
>
> Changes from v2:
> * called bool_from_onoffauto(account_..., true) in the first patch to
>    preserve original semantics before patch 2
>
> Changes from v1:
> * set account_invalid/account_failed to true by default
> * pass OnOffAuto to block_acct_init() to handle double initialization (patch 1)
> * changed properties on BLK device to OnOffAuto
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Peter Krempa <pkrempa@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: John Snow <jsnow@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Hanna Reitz <hreitz@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>
>
ping

Re: [PATCH v5 0/2] block: add missed block_acct_setup with new block device init procedure

Posted by Kevin Wolf 1 year, 7 months ago

Am 07.09.2022 um 19:25 hat Denis V. Lunev geschrieben:
> ping

I'll get to it (and quite a few other small series on the list that
should be quick to review), but probably only after KVM Forum. So I'll
have to ask you to be patient for a little longer.

Kevin

Re: [PATCH v5 0/2] block: add missed block_acct_setup with new block device init procedure

Posted by Markus Armbruster 1 year, 7 months ago

"Denis V. Lunev" <den@virtuozzo.com> writes:

> On 8/24/22 11:50, Denis V. Lunev wrote:
>> Commit 5f76a7aac156ca75680dad5df4a385fd0b58f6b1 is looking harmless from
>> the first glance, but it has changed things a lot. 'libvirt' uses it to
>> detect that it should follow new initialization way and this changes
>> things considerably. With this procedure followed, blockdev_init() is
>> not called anymore and thus block_acct_setup() helper is not called.
>>
>> This means in particular that defaults for block accounting statistics
>> are changed and account_invalid/account_failed are actually initialized
>> as false instead of true originally.
>>
>> This commit changes things to match original world. There are the following
>> constraints:
>> * new default value in block_acct_init() is set to true
>> * block_acct_setup() inside blockdev_init() is called before
>>    blkconf_apply_backend_options()
>> * thus newly created option in block device properties has precedence if
>>    specified
>>
>> Changes from v4:
>> * removed hunk to QAPI which was used to test old initialization path
>> * added R-b: Vladimir
>>
>> Changes from v3:
>> * fixed accidentally wrong submission. Contains changes which should be
>>    sent as v3
>>
>> Changes from v2:
>> * called bool_from_onoffauto(account_..., true) in the first patch to
>>    preserve original semantics before patch 2
>>
>> Changes from v1:
>> * set account_invalid/account_failed to true by default
>> * pass OnOffAuto to block_acct_init() to handle double initialization (patch 1)
>> * changed properties on BLK device to OnOffAuto
>>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Peter Krempa <pkrempa@redhat.com>
>> CC: Markus Armbruster <armbru@redhat.com>
>> CC: John Snow <jsnow@redhat.com>
>> CC: Kevin Wolf <kwolf@redhat.com>
>> CC: Hanna Reitz <hreitz@redhat.com>
>> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>>
>>
> ping

Can't find this series anymore.  Care to resend?