From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1KOcrZ-0005jd-7K for qemu-devel@nongnu.org; Thu, 31 Jul 2008 14:26:57 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1KOcrW-0005jE-QN for qemu-devel@nongnu.org; Thu, 31 Jul 2008 14:26:55 -0400 Received: from [199.232.76.173] (port=60436 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1KOcrW-0005jB-K8 for qemu-devel@nongnu.org; Thu, 31 Jul 2008 14:26:54 -0400 Received: from wr-out-0506.google.com ([64.233.184.239]:12284) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1KOcrW-00049o-56 for qemu-devel@nongnu.org; Thu, 31 Jul 2008 14:26:54 -0400 Received: by wr-out-0506.google.com with SMTP id c46so540574wra.18 for ; Thu, 31 Jul 2008 11:26:51 -0700 (PDT) Message-ID: <489203C9.1040607@codemonkey.ws> Date: Thu, 31 Jul 2008 13:26:17 -0500 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] PATCH: Control over drive open modes for backing file References: <20080731113120.GJ23888@redhat.com> In-Reply-To: <20080731113120.GJ23888@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" , qemu-devel@nongnu.org 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: > >