Re: [Qemu-devel] [PATCH v2 1/5] qcow2: Add corrupt bit

[Kevin Wolf]

Am 29.08.2013 um 10:27 hat Max Reitz geschrieben:
> Am 29.08.2013 10:23, schrieb Kevin Wolf:
> >Am 28.08.2013 um 16:55 hat Max Reitz geschrieben:
> >>This adds an incompatible bit indicating corruption to qcow2. Any image
> >>with this bit set may not be written to unless for repairing (and
> >>subsequently clearing the bit if the repair has been successful).
> >>
> >>Signed-off-by: Max Reitz <address@hidden>
> >>---
> >>  block/qcow2.c              | 45 
> >> +++++++++++++++++++++++++++++++++++++++++++++
> >>  block/qcow2.h              |  7 ++++++-
> >>  docs/specs/qcow2.txt       |  7 ++++++-
> >>  tests/qemu-iotests/031.out | 12 ++++++------
> >>  tests/qemu-iotests/036.out |  2 +-
> >>  5 files changed, 64 insertions(+), 9 deletions(-)
> >>@@ -402,6 +433,15 @@ static int qcow2_open(BlockDriverState *bs, QDict 
> >>*options, int flags)
> >>          goto fail;
> >>      }
> >>+    if (s->incompatible_features & QCOW2_INCOMPAT_CORRUPT) {
> >>+        /* Corrupt images may not be written to unless they are being 
> >>repaired
> >>+         */
> >>+        if ((flags & BDRV_O_RDWR) && !(flags & BDRV_O_CHECK)) {
> >>+            ret = -EACCES;
> >Perhaps a (q)error_report() call would be appropriate so that the user
> >isn't confused with only the "Permission denied" message
> Seems reasonable.
> >(should it be
> >EPERM rather than EACCES, too? Or maybe EROFS?)
> I chose the value based on the following:
> $ touch foo
> $ chmod -w foo
> $ echo 'bar' > foo
> zsh: permission denied: foo
> (which is EACCES)

Well, the point is that when I get EACCES (i.e. a "Permission denied"
message), the first thing I do is to check the file permissions of the
image file, because that's really what EACCES says. So it's kind of
misleading.

> EROFS sounds nice, but I wouldn't go for it since it's the image
> that's read-only and not the underlying FS (which I think EROFS is
> for…?)

Unfortunately errno values are hardly ever a perfect match... I think
I'm less likely to be misled by EROFS because I _know_ that my /home (or
/var, for that matter) isn't read-only.

In the end, it's probably not all that important as long as we print a
clear (q)error_report() message in addition.

Kevin