From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:55500) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1StgMM-00060g-4E for qemu-devel@nongnu.org; Tue, 24 Jul 2012 10:45:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1StgMC-0005tL-BN for qemu-devel@nongnu.org; Tue, 24 Jul 2012 10:45:14 -0400 Received: from mx1.redhat.com ([209.132.183.28]:22062) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1StgMC-0005rR-3u for qemu-devel@nongnu.org; Tue, 24 Jul 2012 10:45:04 -0400 Message-ID: <500EB46D.7080601@redhat.com> Date: Tue, 24 Jul 2012 16:42:53 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1343053380-12133-1-git-send-email-benoit@irqsave.net> <1343053380-12133-2-git-send-email-benoit@irqsave.net> <20120723141501.735465ca@doriath.home> <20120724101039.GA2118@irqsave.net> <20120724095548.2bf348d8@doriath.home> <500EA327.7040107@redhat.com> <20120724143708.GA2371@irqsave.net> In-Reply-To: <20120724143708.GA2371@irqsave.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH V4 1/3] block: Add bdrv_are_busy() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-1?Q?Beno=EEt_Canet?= Cc: Anthony Liguori , benoit.canet@gmail.com, stefanha@linux.vnet.ibm.com, stefanha@gmail.com, qemu-devel@nongnu.org, Luiz Capitulino , pbonzini@redhat.com Am 24.07.2012 16:37, schrieb Beno=EEt Canet: > Le Tuesday 24 Jul 2012 =E0 15:29:11 (+0200), Kevin Wolf a =E9crit : >> Am 24.07.2012 14:55, schrieb Luiz Capitulino: >>> On Tue, 24 Jul 2012 12:10:39 +0200 >>> Beno=EEt Canet wrote: >>> >>>> Le Monday 23 Jul 2012 =E0 14:15:01 (-0300), Luiz Capitulino a =E9cri= t : >>>>> On Mon, 23 Jul 2012 16:22:58 +0200 >>>>> benoit.canet@gmail.com wrote: >>>>> >>>>>> From: Beno=EEt Canet >>>>>> >>>>>> 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 >>>>>> --- >>>>>> 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; >>>>>> } >>>>>> =20 >>>>>> +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 to= o, 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 p= refer >>>> 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 stu= ff, >>> 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=EEt may have misunderstoo= d >> 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 i= n 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 bein= g >> streamed is perfectly fine. So the details don't really matter any mor= e. >> >=20 > Just to be sure. >=20 > In case of a migration with shared storage the migration stops the stre= aming > when the switch between vm is done. > So starting a streaming after the begining of a migration is also right. > Is that correct ? Yes, starting streaming itself shouldn't be a problem. Usually streaming is combined with doing a snapshot first, though, and that could become a problem if the destination didn't already know the snapshot when it was started. I believe it's already blocked today. Kevin