On 16.04.2014 23:48, Max Reitz wrote:
On 16.04.2014 17:00, Kevin Wolf wrote:
Am 12.04.2014 um 20:57 hat Max Reitz geschrieben:
Implement progress output for the commit command by querying the
progress of the block job.
Signed-off-by: Max Reitz <mre...@redhat.com>
---
qemu-img-cmds.hx | 4 ++--
qemu-img.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
qemu-img.texi | 2 +-
3 files changed, 45 insertions(+), 5 deletions(-)
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index d029609..8bc55cd 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -22,9 +22,9 @@ STEXI
ETEXI
DEF("commit", img_commit,
- "commit [-q] [-f fmt] [-t cache] filename")
+ "commit [-q] [-f fmt] [-t cache] [-p] filename")
STEXI
-@item commit [-q] [-f @var{fmt}] [-t @var{cache}] @var{filename}
+@item commit [-q] [-f @var{fmt}] [-t @var{cache}] [-p] @var{filename}
ETEXI
DEF("compare", img_compare,
diff --git a/qemu-img.c b/qemu-img.c
index 9fe6384..50d7519 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -686,6 +686,9 @@ fail:
struct CommonBlockJobCBInfo {
Error **errp;
bool done;
+ /* If the blockjob works on several sectors at once, this field
has to
+ * contain that granularity in bytes */
+ uint64_t granularity;
};
static void common_block_job_cb(void *opaque, int ret)
@@ -702,12 +705,34 @@ static void common_block_job_cb(void *opaque,
int ret)
static void run_block_job(BlockJob *job, struct
CommonBlockJobCBInfo *cbi)
{
BlockJobInfo *info;
+ uint64_t mod_offset = 0;
do {
qemu_aio_wait();
info = block_job_query(job);
+ if (info->offset) {
+ if (!mod_offset) {
+ /* Some block jobs (at least "commit") will only
work on a
+ * subset of the image file and therefore basically
skip many
+ * sectors at the start (processing them apparently
+ * instantaneously). These sectors should be
ignored when
+ * calculating the progress.
+ * If the blockjob processes several sectors at
once, the first
+ * progress report is likely to come after one such
sector group
+ * (whose size is cbi->granularity), therefore
subtract it from
+ * the current offset in order to obtain a more
probable
+ * starting offset. */
+ mod_offset = info->offset > cbi->granularity
+ ? info->offset - cbi->granularity
+ : 0;
granularity != buf_size. And you still can't distinguish whether the
first chunk was skipped or copied.
In the v2 review I suggested changing mirror_run() to update
s->common.len. There you wouldn't have to make any assumptions, but
would know exactly where the bitmap initialisation is done. And it would
improve traditional block job progress output, too.
Am I missing anything that makes this approach harder than it seems?
I just don't like it, somehow, that's all. But I'll see what I can do.
Okay, now I have a better reason than "Meh, I don't like it" (which is
always a very bad reason, of course), being the following: As
mirror_run() is actually made for mirroring from an active block device,
some sectors may be marked dirty during the block job which the
initialization loop did not consider dirty. This will not occur in the
case of qemu-img commit, obviously, as the block device is not really
used. However, mirror_run() isn't made for use by qemu-img only. If new
sectors are marked dirty during the block job's operation, we'd have to
increase the length of the block job live which seems very crude to me.
Of course, I'd love to be proven wrong, as I do see that the above
solution (taking the granularity into account) is pretty crude as well.
Max