From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NiaiH-0005jf-Bp for qemu-devel@nongnu.org; Fri, 19 Feb 2010 16:48:41 -0500 Received: from [199.232.76.173] (port=43102 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NiaiG-0005j1-Pk for qemu-devel@nongnu.org; Fri, 19 Feb 2010 16:48:40 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1NiaiE-0007Rf-Lj for qemu-devel@nongnu.org; Fri, 19 Feb 2010 16:48:40 -0500 Received: from mail-yw0-f197.google.com ([209.85.211.197]:52212) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NiaiE-0007RV-B3 for qemu-devel@nongnu.org; Fri, 19 Feb 2010 16:48:38 -0500 Received: by ywh35 with SMTP id 35so510982ywh.4 for ; Fri, 19 Feb 2010 13:48:37 -0800 (PST) Message-ID: <4B7F0733.1060304@codemonkey.ws> Date: Fri, 19 Feb 2010 15:48:35 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH v4 rebased] block: more read-only changes, related to backing files References: <4B77E0E6.1090701@redhat.com> In-Reply-To: <4B77E0E6.1090701@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Naphtali Sprei Cc: Kevin Wolf , qemu-devel@nongnu.org, Sheng Yang On 02/14/2010 05:39 AM, Naphtali Sprei wrote: > Open backing file read-only where possible > Upgrade backing file to read-write during commit, back to read-only after commit > If upgrade fail, back to read-only. If also fail, "disconnect" the drive. > > Signed-off-by: Naphtali Sprei > Applied. Thanks. Regards, Anthony Liguori > --- > block.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++-------- > block_int.h | 2 + > 2 files changed, 73 insertions(+), 12 deletions(-) > > diff --git a/block.c b/block.c > index af56ea7..31d1ba4 100644 > --- a/block.c > +++ b/block.c > @@ -363,6 +363,7 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags, > bs->is_temporary = 0; > bs->encrypted = 0; > bs->valid_key = 0; > + bs->open_flags = flags; > /* buffer_alignment defaulted to 512, drivers can change this value */ > bs->buffer_alignment = 512; > > @@ -450,8 +451,6 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags, > if (flags& (BDRV_O_CACHE_WB|BDRV_O_NOCACHE)) > bs->enable_write_cache = 1; > > - bs->read_only = (flags& BDRV_O_RDWR) == 0; > - > /* > * Clear flags that are internal to the block layer before opening the > * image. > @@ -472,6 +471,7 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags, > goto free_and_fail; > } > > + bs->keep_read_only = bs->read_only = !(open_flags& BDRV_O_RDWR); > if (drv->bdrv_getlength) { > bs->total_sectors = bdrv_getlength(bs)>> BDRV_SECTOR_BITS; > } > @@ -488,13 +488,22 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags, > filename, bs->backing_file); > if (bs->backing_format[0] != '\0') > back_drv = bdrv_find_format(bs->backing_format); > + > + /* backing files always opened read-only */ > + open_flags&= ~BDRV_O_RDWR; > + > ret = bdrv_open2(bs->backing_hd, backing_filename, open_flags, > back_drv); > - bs->backing_hd->read_only = (open_flags& BDRV_O_RDWR) == 0; > if (ret< 0) { > bdrv_close(bs); > return ret; > } > + if (bs->is_temporary) { > + bs->backing_hd->keep_read_only = !(flags& BDRV_O_RDWR); > + } else { > + /* base image inherits from "parent" */ > + bs->backing_hd->keep_read_only = bs->keep_read_only; > + } > } > > if (!bdrv_key_required(bs)) { > @@ -570,19 +579,48 @@ int bdrv_commit(BlockDriverState *bs) > { > BlockDriver *drv = bs->drv; > int64_t i, total_sectors; > - int n, j; > - int ret = 0; > + int n, j, ro, open_flags; > + int ret = 0, rw_ret = 0; > unsigned char sector[512]; > + char filename[1024]; > + BlockDriverState *bs_rw, *bs_ro; > > if (!drv) > return -ENOMEDIUM; > - > - if (bs->read_only) { > - return -EACCES; > + > + if (!bs->backing_hd) { > + return -ENOTSUP; > } > > - if (!bs->backing_hd) { > - return -ENOTSUP; > + if (bs->backing_hd->keep_read_only) { > + return -EACCES; > + } > + > + ro = bs->backing_hd->read_only; > + strncpy(filename, bs->backing_hd->filename, sizeof(filename)); > + open_flags = bs->backing_hd->open_flags; > + > + if (ro) { > + /* re-open as RW */ > + bdrv_delete(bs->backing_hd); > + bs->backing_hd = NULL; > + bs_rw = bdrv_new(""); > + rw_ret = bdrv_open2(bs_rw, filename, open_flags | BDRV_O_RDWR, NULL); > + if (rw_ret< 0) { > + bdrv_delete(bs_rw); > + /* try to re-open read-only */ > + bs_ro = bdrv_new(""); > + ret = bdrv_open2(bs_ro, filename, open_flags& ~BDRV_O_RDWR, NULL); > + if (ret< 0) { > + bdrv_delete(bs_ro); > + /* drive not functional anymore */ > + bs->drv = NULL; > + return ret; > + } > + bs->backing_hd = bs_ro; > + return rw_ret; > + } > + bs->backing_hd = bs_rw; > } > > total_sectors = bdrv_getlength(bs)>> BDRV_SECTOR_BITS; > @@ -590,11 +628,13 @@ int bdrv_commit(BlockDriverState *bs) > if (drv->bdrv_is_allocated(bs, i, 65536,&n)) { > for(j = 0; j< n; j++) { > if (bdrv_read(bs, i, sector, 1) != 0) { > - return -EIO; > + ret = -EIO; > + goto ro_cleanup; > } > > if (bdrv_write(bs->backing_hd, i, sector, 1) != 0) { > - return -EIO; > + ret = -EIO; > + goto ro_cleanup; > } > i++; > } > @@ -614,6 +654,25 @@ int bdrv_commit(BlockDriverState *bs) > */ > if (bs->backing_hd) > bdrv_flush(bs->backing_hd); > + > +ro_cleanup: > + > + if (ro) { > + /* re-open as RO */ > + bdrv_delete(bs->backing_hd); > + bs->backing_hd = NULL; > + bs_ro = bdrv_new(""); > + ret = bdrv_open2(bs_ro, filename, open_flags& ~BDRV_O_RDWR, NULL); > + if (ret< 0) { > + bdrv_delete(bs_ro); > + /* drive not functional anymore */ > + bs->drv = NULL; > + return ret; > + } > + bs->backing_hd = bs_ro; > + bs->backing_hd->keep_read_only = 0; > + } > + > return ret; > } > > diff --git a/block_int.h b/block_int.h > index 930a5a4..50e1a0b 100644 > --- a/block_int.h > +++ b/block_int.h > @@ -130,6 +130,8 @@ struct BlockDriverState { > int64_t total_sectors; /* if we are reading a disk image, give its > size in sectors */ > int read_only; /* if true, the media is read only */ > + int keep_read_only; /* if true, the media was requested to stay read only */ > + int open_flags; /* flags used to open the file, re-used for re-open */ > int removable; /* if true, the media can be removed */ > int locked; /* if true, the media cannot temporarily be ejected */ > int encrypted; /* if true, the media is encrypted */ >