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

[Benoît Canet]

Le Tuesday 24 Jul 2012 à 15:29:11 (+0200), Kevin Wolf a écrit :
> Am 24.07.2012 14:55, schrieb Luiz Capitulino:
> > On Tue, 24 Jul 2012 12:10:39 +0200
> > Benoît Canet <address@hidden> wrote:
> > 
> >> Le Monday 23 Jul 2012 à 14:15:01 (-0300), Luiz Capitulino a écrit :
> >>> On Mon, 23 Jul 2012 16:22:58 +0200
> >>> address@hidden wrote:
> >>>
> >>>> From: Benoît Canet <address@hidden>
> >>>>
> >>>> 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 <address@hidden>
> >>>> ---
> >>>>  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.
> 

Just to be sure.

In case of a migration with shared storage the migration stops the streaming
when the switch between vm is done.
So starting a streaming after the begining of a migration is also right.
Is that correct ?

Benoît