Re: [Qemu-devel] [PATCH] Add readonly flag to -drive command

[Kevin Wolf]

Am 12.10.2009 14:47, schrieb Naphtali Sprei:
> In order to safely share an image between guests (as read only drive), add a 
> 'readonly' flag
> to the -drive command (qemu command line and monitor).
> 
> Still missing passing the read only attribute to the guest, where possible. I 
> don't know which device types supports
> read only, and don't know how to pass this information to guests.
> 
> Also not sure what to do when qemu cannot open the file as writeable. 
> Currently it opens it as read only.
> We might change it to give a warning or even an error.
> 
> From 6e297da79a4c015555e3927e6d28744933a31ebe Mon Sep 17 00:00:00 2001
> From: Naphtali Sprei <address@hidden>
> Date: Mon, 12 Oct 2009 14:25:25 +0200
> Subject: [PATCH] Added readonly flag to -drive command.
>  This enables sharing same image between guests, with readonly access.
>  Implementaion mark the drive as read_only and changes the flags when 
> actually opening the file.

Is this enough? Basically none of the block drivers know that their
image could be read-only, so we'll likely trigger some unexpected error
cases there. For a simple write I guess we'll be okay (not sure if we'll
return the right error code, though), but I have no idea what, say,
savevm would do with a read-only image.

What cases have you tested?

> TODO:
> 1. Pass the readonly attribute to the guest (write-protected drive ??)

I agree. To be useful the read-only attribute should be exposed to the
guest. I think most devices support some sort of write protection.

> 2. Re-visit the scheme where qemu open a file (silently) in read only mode 
> when it can't open for write.
>    Now that user can specify read only (and didn't), might give a warning 
> when not writeable, or even
>    give an error.
> 
> Signed-off-by: Naphtali Sprei <address@hidden>
> ---
>  block.c       |   14 ++++++++++++--
>  block.h       |    1 +
>  qemu-config.c |    3 +++
>  vl.c          |    6 ++++++
>  4 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 33f3d65..01fd289 100644
> --- a/block.c
> +++ b/block.c
> @@ -335,7 +335,8 @@ int bdrv_open2(BlockDriverState *bs, const char 
> *filename, int flags,
>      char tmp_filename[PATH_MAX];
>      char backing_filename[PATH_MAX];
>  
> -    bs->read_only = 0;
> +    /* don't mess with it, should have been zeros, anyway */
> +    /* bs->read_only = 0; */

Why leave that comment instead of just removing it if it's not necessary?

Kevin