Naphtali Sprei wrote:
> Open backing file for read-only
> During commit upgrade to read-write and back at end to read-only
> + if (ro) { /* re-open as RO */
> + bs_ro = bdrv_new("");
> + ret = bdrv_open2(bs_ro, bs->backing_hd->filename,
> bs->backing_hd->open_flags & ~BDRV_O_RDWR, NULL);
> + if (ret < 0) {
> + bdrv_delete(bs_ro);
> + return -EACCES;
> + }
> + bdrv_close(bs->backing_hd);
> + qemu_free(bs->backing_hd);
> + bs->backing_hd = bs_ro;
> + bs->backing_hd->keep_read_only = 0;
> + }
I think the general idea is perfect.
A couple of concerns come to mind.
1. When changing read-write to read-only, if the backing file is a complex
format like qcow2 (or any others), is it possible for this bdrv_open2()
to read metadata such as format indexes, and even data, _before_
all changes maintained by bs->backing_hd have been written to the file?
(If the complex formats were like real filesystems and had a "mounted"
flags as real filesystems tend to, then it would be an issue, but I'm
not aware of any of them doing that.)
Are there any such issues when switching from read-only to read-write
earlier? (It seems unlikely).
2. Secondly, what if the re-open gets a different file (testable with
fstat()). I know, you get what you deserve if you rename files, but
still, do any of the formats which use backing files have a UUID check
or something to confirm they are using the right backing file, which
might be subverted by this?
3. What about the bdrv file/device-locking which was actively looking
like it might get in a couple of months ago. Did it get in, and if
so does it conflict with this upgrade pattern?
Thanks!
-- Jamie
