| Message ID | CD6AD65A07E882F403F4BA91@Ximines.local |
|---|---|
| State | New |
| Headers | show |
On 10/15/2012 11:29 AM, Alex Bligh wrote: > This patch allows qemu-img rebase to rebase an image to > have no backing file, as opposed to merely allowing it to > rebase to an existing backing file. You can already do that by rebasing to the empty string. And it is feasible (although unlikely) to have a file named '-', where your patch would make it impossible to use that file directly (although you could still use './-'). NACK. $ qemu-img info bar image: bar file format: qcow2 virtual size: 0 (0 bytes) disk size: 192K cluster_size: 65536 backing file: foo $ qemu-img rebase -u -b '' bar $ qemu-img info bar image: bar file format: qcow2 virtual size: 0 (0 bytes) disk size: 192K cluster_size: 65536
On 10/15/2012 12:07 PM, Eric Blake wrote: > On 10/15/2012 11:29 AM, Alex Bligh wrote: >> This patch allows qemu-img rebase to rebase an image to >> have no backing file, as opposed to merely allowing it to >> rebase to an existing backing file. > > You can already do that by rebasing to the empty string. And it is > feasible (although unlikely) to have a file named '-', where your patch > would make it impossible to use that file directly (although you could > still use './-'). NACK. > > $ qemu-img info bar > image: bar > file format: qcow2 > virtual size: 0 (0 bytes) > disk size: 192K > cluster_size: 65536 > backing file: foo > $ qemu-img rebase -u -b '' bar > $ qemu-img info bar > image: bar > file format: qcow2 > virtual size: 0 (0 bytes) > disk size: 192K > cluster_size: 65536 On the other hand, if you don't use -u, then qemu-img complains: $ qemu-img rebase -b '' bar qemu-img: Could not open new backing file '' So I think a better patch would be to allow rebase-by-pull to work the same as unsafe rebase, by honoring the empty string as a request to pull the entire chain into the destination and leave no backing file.
On Mon, Oct 15, 2012 at 12:07:37PM -0600, Eric Blake wrote: > On 10/15/2012 11:29 AM, Alex Bligh wrote: > > This patch allows qemu-img rebase to rebase an image to > > have no backing file, as opposed to merely allowing it to > > rebase to an existing backing file. > > You can already do that by rebasing to the empty string. And it is > feasible (although unlikely) to have a file named '-', where your patch > would make it impossible to use that file directly (although you could > still use './-'). NACK. Also - is often used to tell tools that expect a file argument to use stdin/out. Something that would be quite useful for some qemu-img subcommands.
diff --git a/qemu-img.c b/qemu-img.c index f17f187..770e221 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1558,13 +1558,14 @@ static int img_rebase(int argc, char **argv) error_report("Could not open old backing file '%s'", backing_name); goto out; } - - bs_new_backing = bdrv_new("new_backing"); - ret = bdrv_open(bs_new_backing, out_baseimg, BDRV_O_FLAGS, + if (strcmp("-", out_baseimg)) { + bs_new_backing = bdrv_new("new_backing"); + ret = bdrv_open(bs_new_backing, out_baseimg, BDRV_O_FLAGS, new_backing_drv); - if (ret) { - error_report("Could not open new backing file '%s'", out_baseimg); - goto out; + if (ret) { + error_report("Could not open new backing file '%s'", out_baseimg); + goto out; + } } } @@ -1580,7 +1581,7 @@ static int img_rebase(int argc, char **argv) if (!unsafe) { uint64_t num_sectors; uint64_t old_backing_num_sectors; - uint64_t new_backing_num_sectors; + uint64_t new_backing_num_sectors=0; uint64_t sector; int n; uint8_t * buf_old; @@ -1592,7 +1593,8 @@ static int img_rebase(int argc, char **argv) bdrv_get_geometry(bs, &num_sectors); bdrv_get_geometry(bs_old_backing, &old_backing_num_sectors); - bdrv_get_geometry(bs_new_backing, &new_backing_num_sectors); + if (bs_new_backing) + bdrv_get_geometry(bs_new_backing, &new_backing_num_sectors); local_progress = (float)100 / (num_sectors / MIN(num_sectors, IO_BUF_SIZE / 512)); @@ -1629,7 +1631,7 @@ static int img_rebase(int argc, char **argv) } } - if (sector >= new_backing_num_sectors) { + if (sector >= new_backing_num_sectors || !bs_new_backing) { memset(buf_new, 0, n * BDRV_SECTOR_SIZE); } else { if (sector + n > new_backing_num_sectors) { @@ -1675,7 +1677,11 @@ static int img_rebase(int argc, char **argv) * backing file are overwritten in the COW file now, so the visible content * doesn't change when we switch the backing file. */ - ret = bdrv_change_backing_file(bs, out_baseimg, out_basefmt); + if (bs_new_backing) + ret = bdrv_change_backing_file(bs, out_baseimg, out_basefmt); + else + ret = bdrv_change_backing_file(bs, NULL, NULL); + if (ret == -ENOSPC) { error_report("Could not change the backing file to '%s': No " "space left in the file header", out_baseimg); diff --git a/qemu-img.texi b/qemu-img.texi index 8b05f2c..7c29f23 100644 --- a/qemu-img.texi +++ b/qemu-img.texi @@ -148,7 +148,9 @@ Changes the backing file of an image. Only the formats @code{qcow2} and The backing file is changed to @var{backing_file} and (if the image format of @var{filename} supports this) the backing file format is changed to -@var{backing_fmt}. +@var{backing_fmt}. If @var{backing_file} is specified as ``-'', then the image +is rebased onto no backing file (i.e. it will exist independently of any +backing file). There are two different modes in which @code{rebase} can operate: @table @option
This patch allows qemu-img rebase to rebase an image to have no backing file, as opposed to merely allowing it to rebase to an existing backing file. Patch below, or pull from git at: https://github.com/abligh/qemu.git Commit visible at: https://github.com/abligh/qemu/commit/4d5b3b431d8dd276f4c564d8a82c6d25cb111381 Signed-off-by: Alex Bligh <alex@alex.org.uk> --- qemu-img.c | 26 ++++++++++++++++---------- qemu-img.texi | 4 +++- 2 files changed, 19 insertions(+), 11 deletions(-)