* [Qemu-devel] [RESEND] [PATCH] Properly use backing file argument to git-img convert
@ 2012-09-03 7:46 Brad Campbell
2012-09-03 14:23 ` Andreas Färber
0 siblings, 1 reply; 3+ messages in thread
From: Brad Campbell @ 2012-09-03 7:46 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Brad Campbell
Converting to an image with an output backing file would write out the contents
of the source image whether or not it was already contained in the new backing
file. This commit ensures that the source file is checked against the new
backing file and output is only allocated if it obsoletes or extends data
already in the supplied backing file.
Signed-off-by: Brad Campbell <brad@fnarfbargle.com>
---
qemu-img.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++-------------
1 files changed, 57 insertions(+), 17 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index c8a70ff..3fb6cbf 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -629,11 +629,12 @@ static int img_convert(int argc, char **argv)
int progress = 0, flags;
const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename;
BlockDriver *drv, *proto_drv;
- BlockDriverState **bs = NULL, *out_bs = NULL;
+ BlockDriverState **bs = NULL, *out_bs = NULL, *out_bf = NULL;
int64_t total_sectors, nb_sectors, sector_num, bs_offset;
- uint64_t bs_sectors;
+ uint64_t bf_sectors, bs_sectors;
uint8_t * buf = NULL;
const uint8_t *buf1;
+ uint8_t * bf_buf = NULL;
BlockDriverInfo bdi;
QEMUOptionParameter *param = NULL, *create_options = NULL;
QEMUOptionParameter *out_baseimg_param;
@@ -797,6 +798,16 @@ static int img_convert(int argc, char **argv)
out_baseimg_param = get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
if (out_baseimg_param) {
out_baseimg = out_baseimg_param->value.s;
+
+ /* out_baseimg_parm != NULL even if there is no base img specified! */
+ if (out_baseimg) {
+ out_bf = bdrv_new_open(out_baseimg, NULL, BDRV_O_FLAGS);
+ if (!out_bf) {
+ ret = -1;
+ goto out;
+ }
+ bdrv_get_geometry(out_bf, &bf_sectors);
+ }
}
/* Check if compression is supported */
@@ -862,6 +873,9 @@ static int img_convert(int argc, char **argv)
bs_offset = 0;
bdrv_get_geometry(bs[0], &bs_sectors);
buf = qemu_blockalign(out_bs, IO_BUF_SIZE);
+ if (out_baseimg) {
+ bf_buf = qemu_blockalign(out_bs, IO_BUF_SIZE);
+ }
if (compress) {
ret = bdrv_get_info(out_bs, &bdi);
@@ -979,31 +993,52 @@ static int img_convert(int argc, char **argv)
n = bs_offset + bs_sectors - sector_num;
}
+ ret = bdrv_read(bs[bs_i], sector_num - bs_offset, buf, n);
+ if (ret < 0) {
+ error_report("error while reading sector %" PRId64 ": %s",
+ sector_num - bs_offset, strerror(-ret));
+ goto out;
+ }
+
if (has_zero_init) {
/* If the output image is being created as a copy on write image,
- assume that sectors which are unallocated in the input image
- are present in both the output's and input's base images (no
- need to copy them). */
+ check that sectors which are unallocated in the input image
+ are present in both the output's and input's base images (or
+ copy them). */
if (out_baseimg) {
- if (!bdrv_is_allocated(bs[bs_i], sector_num - bs_offset,
- n, &n1)) {
- sector_num += n1;
- continue;
+ if (sector_num < bf_sectors) {
+ /* Check if source sectors match those in the new backing file and
+ skip the copy if true. */
+ n = MIN(n, bf_sectors - sector_num);
+ ret = bdrv_read(out_bf, sector_num - bs_offset, bf_buf, n);
+ if (ret < 0) {
+ error_report("error while reading backing file");
+ goto out;
+ }
+ if (!compare_sectors(buf, bf_buf, n, &n1)) {
+ sector_num += n1;
+ continue;
+ }
+
+ } else {
+ /* Source file(s) is larger than the backing file. Just check for
+ allocated sectors */
+ if (!is_allocated_sectors_min(buf, n, &n1, min_sparse)) {
+ /* if we got zero, how many of the next ones are zero? */
+ is_allocated_sectors(buf, n, &n1);
+ sector_num += n1;
+ continue;
+ }
}
- /* The next 'n1' sectors are allocated in the input image. Copy
- only those as they may be followed by unallocated sectors. */
+ /* The next 'n1' sectors are allocated in the input image or the
+ backing files are different. Copy only those as they may be
+ followed by unallocated sectors. */
n = n1;
}
} else {
n1 = n;
}
- ret = bdrv_read(bs[bs_i], sector_num - bs_offset, buf, n);
- if (ret < 0) {
- error_report("error while reading sector %" PRId64 ": %s",
- sector_num - bs_offset, strerror(-ret));
- goto out;
- }
/* NOTE: at the same time we convert, we do not write zero
sectors to have a chance to compress the image. Ideally, we
should add a specific call to have the info to go faster */
@@ -1037,6 +1072,7 @@ out:
free_option_parameters(create_options);
free_option_parameters(param);
qemu_vfree(buf);
+ qemu_vfree(bf_buf);
if (out_bs) {
bdrv_delete(out_bs);
}
@@ -1048,6 +1084,10 @@ out:
}
g_free(bs);
}
+ if (out_bf) {
+ bdrv_delete(out_bf);
+ }
+
if (ret) {
return 1;
}
--
1.7.2.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [RESEND] [PATCH] Properly use backing file argument to git-img convert
2012-09-03 7:46 [Qemu-devel] [RESEND] [PATCH] Properly use backing file argument to git-img convert Brad Campbell
@ 2012-09-03 14:23 ` Andreas Färber
2012-09-04 2:31 ` [Qemu-devel] [RESEND] [PATCH] Properly use backing file argument to qemu-img convert Brad Campbell
0 siblings, 1 reply; 3+ messages in thread
From: Andreas Färber @ 2012-09-03 14:23 UTC (permalink / raw)
To: Brad Campbell; +Cc: kwolf, qemu-devel
Am 03.09.2012 09:46, schrieb Brad Campbell:
> Converting to an image with an output backing file would write out the contents
> of the source image whether or not it was already contained in the new backing
> file. This commit ensures that the source file is checked against the new
> backing file and output is only allocated if it obsoletes or extends data
> already in the supplied backing file.
>
>
> Signed-off-by: Brad Campbell <brad@fnarfbargle.com>
For a resend it would've been nice to get the typos out, I'm sure you
mean qemu-img. :) And it would be nice to have that marked as the topic
then so that it's easily recognized ("qemu-img: Bla bla").
> ---
> qemu-img.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++-------------
> 1 files changed, 57 insertions(+), 17 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index c8a70ff..3fb6cbf 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -629,11 +629,12 @@ static int img_convert(int argc, char **argv)
> int progress = 0, flags;
> const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename;
> BlockDriver *drv, *proto_drv;
> - BlockDriverState **bs = NULL, *out_bs = NULL;
> + BlockDriverState **bs = NULL, *out_bs = NULL, *out_bf = NULL;
> int64_t total_sectors, nb_sectors, sector_num, bs_offset;
> - uint64_t bs_sectors;
> + uint64_t bf_sectors, bs_sectors;
> uint8_t * buf = NULL;
> const uint8_t *buf1;
> + uint8_t * bf_buf = NULL;
Should be *bf_buf despite the * buf line above (checkpatch.pl sometimes
mixes up multiplication and declaration of pointer variables).
> BlockDriverInfo bdi;
> QEMUOptionParameter *param = NULL, *create_options = NULL;
> QEMUOptionParameter *out_baseimg_param;
[snip]
Note that the merge window for v1.3 is not yet open so you'll have to be
a bit patient before seeing your patch in qemu.git.
http://wiki.qemu.org/Planning/1.2
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [RESEND] [PATCH] Properly use backing file argument to qemu-img convert
2012-09-03 14:23 ` Andreas Färber
@ 2012-09-04 2:31 ` Brad Campbell
0 siblings, 0 replies; 3+ messages in thread
From: Brad Campbell @ 2012-09-04 2:31 UTC (permalink / raw)
To: Andreas Färber; +Cc: kwolf, qemu-devel
On 03/09/12 22:23, Andreas Färber wrote:
> Am 03.09.2012 09:46, schrieb Brad Campbell:
>> Converting to an image with an output backing file would write out the contents
>> of the source image whether or not it was already contained in the new backing
>> file. This commit ensures that the source file is checked against the new
>> backing file and output is only allocated if it obsoletes or extends data
>> already in the supplied backing file.
>>
>>
>> Signed-off-by: Brad Campbell <brad@fnarfbargle.com>
>
> For a resend it would've been nice to get the typos out, I'm sure you
> mean qemu-img. :) And it would be nice to have that marked as the topic
> then so that it's easily recognized ("qemu-img: Bla bla").
To be honest, with a typo that big I would never have picked it up
myself no matter how many times I went over it. I'll take that
suggestion for the topic, thanks.
>> ---
>> qemu-img.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++-------------
>> 1 files changed, 57 insertions(+), 17 deletions(-)
>>
>> diff --git a/qemu-img.c b/qemu-img.c
>> index c8a70ff..3fb6cbf 100644
>> --- a/qemu-img.c
>> +++ b/qemu-img.c
>> @@ -629,11 +629,12 @@ static int img_convert(int argc, char **argv)
>> int progress = 0, flags;
>> const char *fmt, *out_fmt, *cache, *out_baseimg, *out_filename;
>> BlockDriver *drv, *proto_drv;
>> - BlockDriverState **bs = NULL, *out_bs = NULL;
>> + BlockDriverState **bs = NULL, *out_bs = NULL, *out_bf = NULL;
>> int64_t total_sectors, nb_sectors, sector_num, bs_offset;
>> - uint64_t bs_sectors;
>> + uint64_t bf_sectors, bs_sectors;
>> uint8_t * buf = NULL;
>> const uint8_t *buf1;
>> + uint8_t * bf_buf = NULL;
>
> Should be *bf_buf despite the * buf line above (checkpatch.pl sometimes
> mixes up multiplication and declaration of pointer variables).
I'll correct and re-send.
>> BlockDriverInfo bdi;
>> QEMUOptionParameter *param = NULL, *create_options = NULL;
>> QEMUOptionParameter *out_baseimg_param;
> [snip]
>
> Note that the merge window for v1.3 is not yet open so you'll have to be
> a bit patient before seeing your patch in qemu.git.
I'm not in any hurry, it's just that the mail config on the machine I
used git send-email from is so broken that the mail to Kevin bounced, so
I waited a week before I re-sent it just to avoid bombing the list. As
it turns out it's still broken anyway.
Thanks for the feedback.
Regards,
Brad
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-09-04 2:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-03 7:46 [Qemu-devel] [RESEND] [PATCH] Properly use backing file argument to git-img convert Brad Campbell
2012-09-03 14:23 ` Andreas Färber
2012-09-04 2:31 ` [Qemu-devel] [RESEND] [PATCH] Properly use backing file argument to qemu-img convert Brad Campbell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).