[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 0/4] Modifications to the drives' readonly at
From: |
Christoph Hellwig |
Subject: |
Re: [Qemu-devel] [PATCH v2 0/4] Modifications to the drives' readonly attribute |
Date: |
Thu, 21 Jan 2010 14:37:30 +0100 |
User-agent: |
Mutt/1.3.28i |
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.
Re: [Qemu-devel] [PATCH v2 1/4] Make CDROM a read-only drive, Jamie Lokier, 2010/01/19
Re: [Qemu-devel] [PATCH v2 1/4] Make CDROM a read-only drive, Anthony Liguori, 2010/01/20
Re: [Qemu-devel] [PATCH v2 0/4] Modifications to the drives' readonly attribute, Christoph Hellwig, 2010/01/20