Re: [Qemu-devel] [PATCH V4 1/3] block: Add bdrv_are_busy()

[Kevin Wolf <kwolf@redhat.com>]

Am 24.07.2012 14:55, schrieb Luiz Capitulino:
> On Tue, 24 Jul 2012 12:10:39 +0200
> Benoît Canet <benoit.canet@irqsave.net> wrote:
> 
>> Le Monday 23 Jul 2012 à 14:15:01 (-0300), Luiz Capitulino a écrit :
>>> On Mon, 23 Jul 2012 16:22:58 +0200
>>> benoit.canet@gmail.com wrote:
>>>
>>>> From: Benoît Canet <benoit@irqsave.net>
>>>>
>>>> bdrv_are_busy will be used to check if any of the bs are in use
>>>> or if one of them have a running block job.
>>>>
>>>> The first user will be qmp_migrate().
>>>>
>>>> Signed-off-by: Benoit Canet <benoit@irqsave.net>
>>>> ---
>>>>  block.c |   13 +++++++++++++
>>>>  block.h |    2 ++
>>>>  2 files changed, 15 insertions(+)
>>>>
>>>> diff --git a/block.c b/block.c
>>>> index ce7eb8f..bc8f160 100644
>>>> --- a/block.c
>>>> +++ b/block.c
>>>> @@ -4027,6 +4027,19 @@ out:
>>>>      return ret;
>>>>  }
>>>>  
>>>> +int bdrv_are_busy(void)
>>>> +{
>>>> +    BlockDriverState *bs;
>>>> +
>>>> +    QTAILQ_FOREACH(bs, &bdrv_states, list) {
>>>> +        if (bs->job || bdrv_in_use(bs)) {
>>>> +            return -EBUSY;
>>>> +        }
>>>> +    }
>>>
>>> IMO, this should return true/false. The name is a bit misleading too, as it
>>> gives the impression that are existing bdrvs are busy. I'd call it
>>> bdrv_any_busy() or bdrv_any_in_use().
>>
>> Hello Anthony,
>>
>> Stefanha is in favor of returning -EBUSY and Luiz Capitulino would prefer
>> the function to return a boolean.
>> Could you decide which option is the best ?
> 
> Stefan's opnion certainly has precedence over mine on block layer stuff,
> this was just an IMO.
> 
> Stefan, did you consider returning a boolean?

I'm with you in this point, Luiz (as well as with the rename to
bdrv_is_any_busy). And actually I think Benoît may have misunderstood
and Stefan is as well. What he said is:

> I think bdrv_have_block_jobs() is too specific and would use
> bdrv_in_use(bs) here to give basically an EBUSY-type error.

I don't think this was about bool vs. -errno, but more about checking
only block jobs vs. all kinds of things that can have a block device in use.

Anyway, I believe we came to the conclusion that even the intention of
the series is wrong, as in many cases migrating while an image is being
streamed is perfectly fine. So the details don't really matter any more.

Kevin