From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NXxEE-00010V-DO for qemu-devel@nongnu.org; Thu, 21 Jan 2010 08:37:42 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NXxE9-0000xN-MW for qemu-devel@nongnu.org; Thu, 21 Jan 2010 08:37:41 -0500 Received: from [199.232.76.173] (port=42116 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NXxE9-0000xF-G0 for qemu-devel@nongnu.org; Thu, 21 Jan 2010 08:37:37 -0500 Received: from verein.lst.de ([213.95.11.210]:57274) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_3DES_EDE_CBC_SHA1:24) (Exim 4.60) (envelope-from ) id 1NXxE8-0005H0-SI for qemu-devel@nongnu.org; Thu, 21 Jan 2010 08:37:37 -0500 Date: Thu, 21 Jan 2010 14:37:30 +0100 From: Christoph Hellwig Subject: Re: [Qemu-devel] [PATCH v2 0/4] Modifications to the drives' readonly attribute Message-ID: <20100121133730.GA12072@lst.de> References: <1263739695-13043-1-git-send-email-nsprei@redhat.com> <20100120170510.GA8444@lst.de> <4B585460.4070901@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4B585460.4070901@redhat.com> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Naphtali Sprei Cc: Christoph Hellwig , qemu-devel@nongnu.org On Thu, Jan 21, 2010 at 03:19:28PM +0200, Naphtali Sprei wrote: > > > > > - we now normally set the read_only flag from bdrv_open2 when we do > > not have the O_RDWR flag set > > - but the block drivers also mess with it: > > o raw-posix superflously sets it when BDRV_O_RDWR is not in the > > open flags > > Not sure where exactly is the issue. Can you please point the line ? It's really just a now superflous place in the image driver that sets the read_only flag. Currently it's not clear who is supposed to set the flag, we do it both from block.c and the image driver. > > o bochs, cloop, dmg and parallels set it unconditionally given > > that they do not support writing at all. But they do not > > bother to reject opens without BDRV_O_RDWR > > I just changed bochs and parallels not to ask for read-write. > Should all of them test the flags for RDWR and returns failure ? That would be most logical, but might cause regressions for existing setups that did not bother to specify the read-only option on the command line. Another options might be to allow the driver to return EROFS and the retry a read-only open for the block layer for these. > > o vvfat as usual is a complete mess setting and clearing it in > > various places > > Fixed one occurance. More places ? I mean the ->read_only flag setting and clearing. As you've pulled up the main place for setting it to the block layer the drivers shouldn't mess with it anymore. > > - in addition to that bdrv_open2 also sets it after calling itself for > > the backing hd which seems superflous > > Is this a problem ? I thought it's safer to mark it read-only, in case a write operation requested somehow. It's superflous, bdrv_open2 always does it based on the argument, so no need to do it a second time for the snapshot.