From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:41072)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1REi8m-0001qS-Qv
	for qemu-devel@nongnu.org; Fri, 14 Oct 2011 09:49:37 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1REi8g-0002yw-VC
	for qemu-devel@nongnu.org; Fri, 14 Oct 2011 09:49:36 -0400
Received: from mx1.redhat.com ([209.132.183.28]:61479)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1REi8g-0002yk-OC
	for qemu-devel@nongnu.org; Fri, 14 Oct 2011 09:49:30 -0400
Message-ID: <4E983E98.6040202@redhat.com>
Date: Fri, 14 Oct 2011 15:52:24 +0200
From: Kevin Wolf <kwolf@redhat.com>
MIME-Version: 1.0
References: <20111011031046.9587.44474.sendpatchset@skannery.in.ibm.com>
	<20111011031159.9587.57559.sendpatchset@skannery.in.ibm.com>
	<4E95AA53.8050901@redhat.com> <4E983C51.8010008@linux.vnet.ibm.com>
In-Reply-To: <4E983C51.8010008@linux.vnet.ibm.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [v7 Patch 5/5]Qemu: New struct 'BDRVReopenState'
 for image files reopen
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: supriyak@linux.vnet.ibm.com
Cc: Stefan Hajnoczi <stefanha@gmail.com>, qemu-devel@nongnu.org, Christoph Hellwig <hch@lst.de>

Am 14.10.2011 15:42, schrieb Supriya Kannery:
>>> +    /* Use driver specific reopen() if available */
>>> +    if (drv->bdrv_reopen_prepare) {
>>> +        ret = drv->bdrv_reopen_prepare(bs,&rs, bdrv_flags);
>>>           if (ret<  0) {
>>> -            /* Reopen failed with orig and modified flags */
>>> -            abort();
>>> +            goto fail;
>>>           }
>>> -    }
>>> +        if (drv->bdrv_reopen_commit) {
>>> +            ret = drv->bdrv_reopen_commit(rs);
>>> +            if (ret<  0) {
>>> +                goto fail;
>>> +            }
>>> +            return 0;
>>> +        }
>>
>> Pull the return 0; out one level. It would be really strange if we
>> turned a successful prepare into reopen_abort just because the driver
>> doesn't need a commit function.
>>
>> (The other consistent way would be to require that if a driver
>> implements one reopen function, it has to implement all three of them.
>> I'm fine either way.)
>>
> 
> Will give flexibility to drivers, not mandating all the three functions.

Do we have a use case where it is actually possible to implement less
functions without introducing bugs?

If yes, let's keep it as it is.

> Got a question..
>     In raw-posix, the three functions are implemented for file_reopen 
> for now. Should this be extended to hdev, cdrom and floppy?

Yes, that would be good. And I think the same implementation can be used
for all of them.

Kevin