| Submitter | Alex Bligh |
|---|---|
| Date | Oct. 15, 2012, 6:42 p.m. |
| Message ID | <7C9D50488E80F46BDF7A16DA@Ximines.local> |
| Download | mbox | patch |
| Permalink | /patch/191628/ |
| State | New |
| Headers | show |
Comments
On 10/15/2012 12:42 PM, Alex Bligh wrote: > Eric, > > --On 15 October 2012 12:11:02 -0600 Eric Blake <eblake@redhat.com> wrote: > >> 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. > > Yes, that's exactly the case we're missing. How about the attached (as > a single patch) > > Also available at: > https://github.com/abligh/qemu > as commits > https://github.com/abligh/qemu/commit/4cce9c961fa52a71bd6520a9c499f4dc4b174b97 > > and > https://github.com/abligh/qemu/commit/4d5b3b431d8dd276f4c564d8a82c6d25cb111381 > > > -- > Alex Bligh Missing a Signed-off-by; as such, it cannot be taken as is. Please repost, and provide an updated subject line and a dedicated commit message (information such as where to download the commit for testing belongs after the --- line). I'm not maintainer for this portion of code, so I can only review it. > @@ -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; As long as you are touching this line, fix the indentation to be consistent (or is this unusual spacing an artifact of how you pasted your commit after your '-- ' signature rather than submitting it via 'git send-email' as a proper patch?). Also, most of qemu uses spaces on both sides of '='. But the overall idea looks nice.
Eric, --On 15 October 2012 13:28:11 -0600 Eric Blake <eblake@redhat.com> wrote: > Missing a Signed-off-by; as such, it cannot be taken as is. New patch sent under separate cover with Signed-Off-By: line. >> @@ -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; > > As long as you are touching this line, fix the indentation to be > consistent (or is this unusual spacing an artifact of how you pasted > your commit after your '-- ' signature rather than submitting it via > 'git send-email' as a proper patch?). I'm totally confused by the spacing artifact. The new patch was generated with git-format-patch and git-send-email, and never touched a mailer (just in case), and also shows under the same circumstances. It doesn't show the bizarre indent in Mulberry, but does in Apple Mail. The patch applies cleanly with git-am and doesn't moan about whitespace, and as far as I can tell the resultant whitespace in the file is correct (i.e. matches the preceding and subsequent lines). > Also, most of qemu uses spaces on both sides of '='. Fixed. > But the overall idea looks nice. Thanks
Patch
diff --git a/qemu-img.c b/qemu-img.c index f17f187..7a4e73f 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 (out_baseimg[0]) { + 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..42ec392 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 ``'' (the empty +string), 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