[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 3/8] raw: Reflect read-only protocol layer
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [PATCH 3/8] raw: Reflect read-only protocol layer |
Date: |
Tue, 7 Nov 2017 17:30:10 +0100 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
Am 07.11.2017 um 12:00 hat Paolo Bonzini geschrieben:
> On 07/11/2017 04:02, Eric Blake wrote:
> > We forbid operations like a zero-length write zero or a discard
> > at the protocol layer when it is marked read-only, but those
> > same operations were succeeding at the format layer because the
> > raw format was not reflecting the underlying read-only status
> > to the block layer, which then took short circuit paths on
> > zero-length operations.
> >
> > Signed-off-by: Eric Blake <address@hidden>
> > ---
> > block/raw-format.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/block/raw-format.c b/block/raw-format.c
> > index 830243a8e4..717b8eff65 100644
> > --- a/block/raw-format.c
> > +++ b/block/raw-format.c
> > @@ -418,6 +418,12 @@ static int raw_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > bs->file->bs->supported_write_flags;
> > bs->supported_zero_flags = (BDRV_REQ_FUA | BDRV_REQ_MAY_UNMAP) &
> > bs->file->bs->supported_zero_flags;
> > + if (bdrv_is_read_only(bs->file->bs)) {
> > + ret = bdrv_set_read_only(bs, true, errp);
> > + if (ret < 0) {
> > + return ret;
> > + }
> > + }
> >
> > if (bs->probed && !bdrv_is_read_only(bs)) {
> > fprintf(stderr,
> >
>
> Kevin, perhaps this should be done straight in block.c?
No, I just discussed this with Eric on IRC, and it shouldn't be done
anywhere.
Basically, the way qemu works with read-only images is that you need to
be explicit about it and specify read-only=on. Drivers are not supposed
to magically set the read-only flag if the user didn't request it.
So what NBD needs to do is to error out if you try to open a read-write
connection to a read-only NBD server.
There's a second part, related specifically to this patch, that was a
bit surprising to me at first, but it actually makes sense. I can
successfully create a read-write raw node on top of a read-only
file-posix node:
-blockdev driver=file,filename=/tmp/test.qcow2,node-name=proto,read-only=on
-blockdev driver=raw,file=proto,node-name=format
This is because in this state without a user, the raw node doesn't
actually want to write to the file-posix node. However, if I add a disk:
-device ide-hd,drive=format
That will actually request write permissions throughout the whole chain
and cause an error:
qemu-system-x86_64: -device ide-hd,drive=format: Block node is read-only
Somewhat surprising at first, but it does make sense.
Kevin
- [Qemu-block] [PATCH 0/8] various NBD fixes for 2.11, Eric Blake, 2017/11/06
- [Qemu-block] [PATCH 1/8] nbd-client: Fix error message typos, Eric Blake, 2017/11/06
- [Qemu-block] [PATCH 2/8] nbd/client: Nicer trace of structured reply, Eric Blake, 2017/11/06
- [Qemu-block] [PATCH 3/8] raw: Reflect read-only protocol layer, Eric Blake, 2017/11/06
- [Qemu-block] [PATCH 4/8] nbd-client: Honor server read-only advertisement, Eric Blake, 2017/11/06
- [Qemu-block] [PATCH 5/8] nbd: Fix struct name for structured reads, Eric Blake, 2017/11/06
- [Qemu-block] [PATCH 6/8] nbd-client: Short-circuit 0-length operations, Eric Blake, 2017/11/06
- [Qemu-block] [PATCH 7/8] nbd-client: Stricter enforcing of structured reply spec, Eric Blake, 2017/11/06
- [Qemu-block] [PATCH 8/8] nbd/server: Fix structured read of length 0, Eric Blake, 2017/11/06