[prev in list] [next in list] [prev in thread] [next in thread] 

List:       qemu-devel
Subject:    Re: [Qemu-devel] PATCH: Control over drive open modes for backing file
From:       Anthony Liguori <anthony () codemonkey ! ws>
Date:       2008-07-31 18:26:17
Message-ID: 489203C9.1040607 () codemonkey ! ws
[Download RAW message or body]

Daniel P. Berrange wrote:
> The current block driver code will attempt to open a file backing a drive
> for read/write with O_RDWR first, and if that fails, fallback to opening
> it readonly with O_RDONLY. So if you set file permissions to readonly on
> the underlying drive backing store, QEMU will fallback to opening it read
> only, and discard any writes.
>   

I'm not sure I agree that this patch is really that useful to an actual 
user.  I think we'll eventually need a read-only flag as paravirtual 
devices do support read-only block devices.  Let's consider a scenario:

A user has multiple block devices including a secondary device that is 
read-only to the guest.  With qcow2 and today's behavior, savevm will 
just work.  With your patch, it will not work.

This is a scenario where just because the block device cannot be written 
to, we still would want to write to the metadata of the image.

So while I think it's valid to have a "read-only disk" exposed to the 
guest, I don't think the user should have anything to do with how we 
open the file.

Is there some specific circumstance you are trying to support?

Regards,

Anthony Liguori

> Xen has a concept of a read-only disks in its configuration format, and
> thus it would be desirable to have an explicit option to request that a
> drive operate read-only, regardless of whether underlying file permissions
> allow write access or not. We'd like to support this in libvirt too for
> QEMU/KVM guests. Finally, in some cases it is desirable to see the failure
> if the disk can't be opened read-write, rather than falling back to read
> only mode - many guests will be more or less inoperable if their root 
> filesystem is read-only so there's little point booting.
>
> The current block.h file did already have flags defined
>
>   #define BDRV_O_RDONLY      0x0000
>   #define BDRV_O_RDWR        0x0002
>   #define BDRV_O_ACCESS      0x0003
>
> However the bdrv_open2() method was treating a 'flags' value of 0, as being
> effectively  RDWR, and nearly all callers pass in 0 even when they expect
> to get a writable file, so the O_RDONLY flag was useless as is.
>
> So this patch does a couple of things:
>
>   



[prev in list] [next in list] [prev in thread] [next in thread]