Re: [Qemu-devel] [PATCH v3 7/7] block: add throttle block filter driver interface tests

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

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

On Thu, Aug 03, 2017 at 03:32:58PM +0200, Kevin Wolf wrote:
>Am 03.08.2017 um 15:24 hat Manos Pitsidianakis geschrieben:
>> On Thu, Aug 03, 2017 at 10:07:50AM +0200, Kevin Wolf wrote:
>> > Am 31.07.2017 um 11:54 hat Manos Pitsidianakis geschrieben:
>> > > Signed-off-by: Manos Pitsidianakis <el13635@mail.ntua.gr>
>> >
>> > I would add at least two more cases:
>> >
>> > * Both limits and throttle-group are given in blockdev-add
>> This exists in the "property changes in ThrottleGroup" section,
>
>You're right, I missed this. The test result shows that this command
>succeeds. Do we really want to allow other nodes to be affected with a
>blockdev-add? Wouldn't it be cleaner to just forbid the combination of
>limits and throtte-group?

So basically only anonymous, immutable groups can be created through the 
driver then. All other shared group configurations must be explicitly 
created with an -object / object-add syntax. I think this is a neat 
separation and compromise if we allow anonymous groups. If not, we can 
ignore limits on the throttle driver.


>> > * limits and throttle-group are both missing
>> this creates an anonymous group with no limits. Should we fail at this case?
>
>I'm not sure, you could argue either way. But there should be a test to
>check that the semantics won't change.
>
>If we're going with Stefan's suggestion that anonymous groups shouldn't
>exist, then the question is moot anyway.
>
>> > It would also be nice to test that query-block reflects the new throttle
>> > group limits correctly when they are changed after the fact.
>>
>> This belongs to the remove legacy patch, since query-block displays the
>> legacy limits.
>
>Ok, fair enough.
>
>Kevin



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