11:27 a.m.
I still don't have qemu capability detection working reliably,
but want to post this series right now so that it can be built
into a scratch build containing Peter's and my changes. (Or
put another way, I was testing what conflict resolutions are
required - patch 2/5 (virsh) and 5/5 (qemu_driver) has some
conflicts with Peter's addition of relative backing name; and
I think the resolutions were fairly simple).
These patches are tested on top of:
https://www.redhat.com/archives/libvir-list/2014-June/msg00492.html
I may still end up posting a v4 and/or pushing my series before
Peter's, once I get capability detection working the way I want.
Eric Blake (5):
virsh: improve blockcopy UI
virsh: expose new active commit controls
blockcommit: update error messages related to block jobs
blockcommit: track job type in xml
blockcommit: turn on active commit
docs/formatdomain.html.in | 20 +++---
docs/schemas/domaincommon.rng | 6 ++
src/conf/domain_conf.c | 26 ++++++-
src/conf/domain_conf.h | 1 +
src/qemu/qemu_driver.c | 56 ++++++++++++---
src/qemu/qemu_hotplug.c | 2 +-
src/qemu/qemu_process.c | 18 +++--
.../qemuxml2argv-disk-active-commit.xml | 37 ++++++++++
.../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 4 +-
.../qemuxml2xmlout-disk-mirror-old.xml | 4 +-
tests/qemuxml2xmltest.c | 1 +
tools/virsh-domain.c | 83 +++++++++++++++++-----
tools/virsh.pod | 53 +++++++++-----
13 files changed, 242 insertions(+), 69 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
--
1.9.3
11:27 a.m.
New subject: [libvirt] [PATCH v3 1/5] virsh: improve blockcopy UI
Peter's review of my addition of active block commit pointed out
some issues that I had copied from block copy. It makes sense to
allow the shorter command-line of 'blockcopy $dom $disk --pivot'
without having to explicitly specify --wait. And my use of
embedded ?: ternaries bordered on unreadable.
* tools/virsh-domain.c (cmdBlockCopy): Make --pivot and --finish
imply --wait. Drop excess ?: operators.
* tools/virsh.pod (blockcopy): Update documentation.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
tools/virsh-domain.c | 25 ++++++++++++++-----------
tools/virsh.pod | 17 +++++++++--------
2 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index e9162db..294b594 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1786,15 +1786,15 @@ static const vshCmdOptDef opts_block_copy[] = {
},
{.name = "timeout",
.type = VSH_OT_INT,
- .help = N_("with --wait, abort if copy exceeds timeout (in seconds)")
+ .help = N_("implies --wait, abort if copy exceeds timeout (in seconds)")
},
{.name = "pivot",
.type = VSH_OT_BOOL,
- .help = N_("with --wait, pivot when mirroring starts")
+ .help = N_("implies --wait, pivot when mirroring starts")
},
{.name = "finish",
.type = VSH_OT_BOOL,
- .help = N_("with --wait, quit when mirroring starts")
+ .help = N_("implies --wait, quit when mirroring starts")
},
{.name = "async",
.type = VSH_OT_BOOL,
@@ -1808,10 +1808,10 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom = NULL;
bool ret = false;
- bool blocking = vshCommandOptBool(cmd, "wait");
bool verbose = vshCommandOptBool(cmd, "verbose");
bool pivot = vshCommandOptBool(cmd, "pivot");
bool finish = vshCommandOptBool(cmd, "finish");
+ bool blocking = vshCommandOptBool(cmd, "wait");
int timeout = 0;
struct sigaction sig_action;
struct sigaction old_sig_action;
@@ -1822,6 +1822,7 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
bool quit = false;
int abort_flags = 0;
+ blocking |= vshCommandOptBool(cmd, "timeout") || pivot || finish;
if (blocking) {
if (pivot && finish) {
vshError(ctl, "%s", _("cannot mix --pivot and
--finish"));
@@ -1844,8 +1845,7 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
sigaction(SIGINT, &sig_action, &old_sig_action);
GETTIMEOFDAY(&start);
- } else if (verbose || vshCommandOptBool(cmd, "timeout") ||
- vshCommandOptBool(cmd, "async") || pivot || finish) {
+ } else if (verbose || vshCommandOptBool(cmd, "async")) {
vshError(ctl, "%s", _("missing --wait option"));
return false;
}
@@ -1911,11 +1911,14 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
vshError(ctl, _("failed to finish job for disk %s"), path);
goto cleanup;
}
- vshPrint(ctl, "\n%s",
- quit ? _("Copy aborted") :
- pivot ? _("Successfully pivoted") :
- finish ? _("Successfully copied") :
- _("Now in mirroring phase"));
+ if (quit)
+ vshPrint(ctl, "\n%s", _("Copy aborted"));
+ else if (pivot)
+ vshPrint(ctl, "\n%s", _("Successfully pivoted"));
+ else if (finish)
+ vshPrint(ctl, "\n%s", _("Successfully copied"));
+ else
+ vshPrint(ctl, "\n%s", _("Now in mirroring phase"));
ret = true;
cleanup:
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 84a60a7..77013f5 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -804,8 +804,8 @@ I<bandwidth> specifies copying bandwidth limit in MiB/s,
although for
qemu, it may be non-zero only for an online domain.
=item B<blockcopy> I<domain> I<path> I<dest> [I<bandwidth>]
[I<--shallow>]
-[I<--reuse-external>] [I<--raw>] [I<--wait> [I<--verbose>]
-[{I<--pivot> | I<--finish>}] [I<--timeout> B<seconds>]
[I<--async>]]
+[I<--reuse-external>] [I<--raw>] [I<--wait> [I<--async>]
[I<--verbose>]]
+[{I<--pivot> | I<--finish>}] [I<--timeout> B<seconds>]
Copy a disk backing image chain to I<dest>. By default, this command
flattens the entire chain; but if I<--shallow> is specified, the copy
@@ -834,12 +834,13 @@ faithful copy of that point in time. However, if I<--wait> is
specified,
then this command will block until the mirroring phase begins, or cancel
the operation if the optional I<timeout> in seconds elapses or SIGINT is
sent (usually with C<Ctrl-C>). Using I<--verbose> along with
I<--wait>
-will produce periodic status updates. Using I<--pivot> or I<--finish>
-along with I<--wait> will additionally end the job cleanly rather than
-leaving things in the mirroring phase. If job cancellation is triggered,
-I<--async> will return control to the user as fast as possible, otherwise
-the command may continue to block a little while longer until the job
-is done cleaning up.
+will produce periodic status updates. Using I<--pivot> (similar to
+B<blockjob> I<--pivot>) or I<--finish> (similar to B<blockjob>
I<--abort>)
+implies I<--wait>, and will additionally end the job cleanly rather than
+leaving things in the mirroring phase. If job cancellation is triggered
+by timeout or by I<--finish>, I<--async> will return control to the user
+as fast as possible, otherwise the command may continue to block a little
+while longer until the job has actually cancelled.
I<path> specifies fully-qualified path of the disk.
I<bandwidth> specifies copying bandwidth limit in MiB/s.
--
1.9.3
3:45 a.m.
New subject: [libvirt] [PATCH v3 1/5] virsh: improve blockcopy UI
On 06/11/14 18:27, Eric Blake wrote:
Peter's review of my addition of active block commit pointed out
some issues that I had copied from block copy. It makes sense to
allow the shorter command-line of 'blockcopy $dom $disk --pivot'
without having to explicitly specify --wait. And my use of
embedded ?: ternaries bordered on unreadable.
* tools/virsh-domain.c (cmdBlockCopy): Make --pivot and --finish
imply --wait. Drop excess ?: operators.
* tools/virsh.pod (blockcopy): Update documentation.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
tools/virsh-domain.c | 25 ++++++++++++++-----------
tools/virsh.pod | 17 +++++++++--------
2 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index e9162db..294b594 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1808,10 +1808,10 @@ cmdBlockCopy(vshControl *ctl, const vshCmd
*cmd)
{
virDomainPtr dom = NULL;
bool ret = false;
- bool blocking = vshCommandOptBool(cmd, "wait");
bool verbose = vshCommandOptBool(cmd, "verbose");
bool pivot = vshCommandOptBool(cmd, "pivot");
bool finish = vshCommandOptBool(cmd, "finish");
+ bool blocking = vshCommandOptBool(cmd, "wait");
Spurious line move?
int timeout = 0;
struct sigaction sig_action;
struct sigaction old_sig_action;
ACK,
Peter
10:44 a.m.
New subject: [libvirt] [PATCH v3 1/5] virsh: improve blockcopy UI
On 06/12/2014 02:45 AM, Peter Krempa wrote:
On 06/11/14 18:27, Eric Blake wrote:
> Peter's review of my addition of active block commit pointed out
> some issues that I had copied from block copy. It makes sense to
> allow the shorter command-line of 'blockcopy $dom $disk --pivot'
> without having to explicitly specify --wait. And my use of
> embedded ?: ternaries bordered on unreadable.
>
> @@ -1808,10 +1808,10 @@ cmdBlockCopy(vshControl *ctl, const
vshCmd *cmd)
> {
> virDomainPtr dom = NULL;
> bool ret = false;
> - bool blocking = vshCommandOptBool(cmd, "wait");
> bool verbose = vshCommandOptBool(cmd, "verbose");
> bool pivot = vshCommandOptBool(cmd, "pivot");
> bool finish = vshCommandOptBool(cmd, "finish");
> + bool blocking = vshCommandOptBool(cmd, "wait");
Spurious line move?
Yeah, fallout from an earlier attempt to do:
bool blocking = vshCommandOptBool(...) || pivot || finish;
before deciding to add timeout to the list of options that auto-implies
blocking. I'll drop this hunk.
> int timeout = 0;
> struct sigaction sig_action;
> struct sigaction old_sig_action;
ACK,
Pushing shortly, as fixed.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:27 a.m.
New subject: [libvirt] [PATCH v3 2/5] virsh: expose new active commit controls
Add knobs to virsh to manage a 2-phase active commit of the top
layer, similar to knobs already present on blockcopy. While this
code will fail until later patches actually implement the new
knobs in the qemu driver, doing it now proves that the API is
usable and also makes it easier for testing the qemu changes as
they are made.
* tools/virsh-domain.c (cmdBlockCommit): Add --active, --pivot,
and --finish options, modeled after blockcopy.
(blockJobImpl): Support --active flag.
* tools/virsh.pod (blockcommit): Document new flags.
(blockjob): Mention 2-phase commit interaction.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
tools/virsh-domain.c | 58 +++++++++++++++++++++++++++++++++++++++++++++-------
tools/virsh.pod | 36 ++++++++++++++++++++++----------
2 files changed, 76 insertions(+), 18 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 294b594..3db0bae 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1498,6 +1498,10 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
flags |= VIR_DOMAIN_BLOCK_COMMIT_DELETE;
if (vshCommandOptBool(cmd, "keep-relative"))
flags |= VIR_DOMAIN_BLOCK_COMMIT_RELATIVE;
+ if (vshCommandOptBool(cmd, "active") ||
+ vshCommandOptBool(cmd, "pivot") ||
+ vshCommandOptBool(cmd, "keep-overlay"))
+ flags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE;
ret = virDomainBlockCommit(dom, path, base, top, bandwidth, flags);
break;
case VSH_CMD_BLOCK_JOB_COPY:
@@ -1598,13 +1602,18 @@ static const vshCmdOptDef opts_block_commit[] = {
.type = VSH_OT_DATA,
.help = N_("path of top file to commit from (default top of chain)")
},
+ {.name = "active",
+ .type = VSH_OT_BOOL,
+ .help = N_("trigger two-stage active commit of top file")
+ },
{.name = "delete",
.type = VSH_OT_BOOL,
.help = N_("delete files that were successfully committed")
},
{.name = "wait",
.type = VSH_OT_BOOL,
- .help = N_("wait for job to complete")
+ .help = N_("wait for job to complete "
+ "(with --active, wait for job to sync)")
},
{.name = "verbose",
.type = VSH_OT_BOOL,
@@ -1612,7 +1621,15 @@ static const vshCmdOptDef opts_block_commit[] = {
},
{.name = "timeout",
.type = VSH_OT_INT,
- .help = N_("with --wait, abort if copy exceeds timeout (in seconds)")
+ .help = N_("implies --wait, abort if copy exceeds timeout (in seconds)")
+ },
+ {.name = "pivot",
+ .type = VSH_OT_BOOL,
+ .help = N_("implies --active --wait, pivot when commit is synced")
+ },
+ {.name = "keep-overlay",
+ .type = VSH_OT_BOOL,
+ .help = N_("implies --active --wait, quit when commit is synced")
},
{.name = "async",
.type = VSH_OT_BOOL,
@@ -1621,7 +1638,7 @@ static const vshCmdOptDef opts_block_commit[] = {
{.name = "keep-relative",
.type = VSH_OT_BOOL,
.help = N_("keep the backing chain relative if it was relatively "
- "referenced if it was before")
+ "referenced before")
},
{.name = NULL}
};
@@ -1631,8 +1648,11 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom = NULL;
bool ret = false;
- bool blocking = vshCommandOptBool(cmd, "wait");
bool verbose = vshCommandOptBool(cmd, "verbose");
+ bool pivot = vshCommandOptBool(cmd, "pivot");
+ bool finish = vshCommandOptBool(cmd, "keep-overlay");
+ bool active = vshCommandOptBool(cmd, "active") || pivot || finish;
+ bool blocking = vshCommandOptBool(cmd, "wait");
int timeout = 0;
struct sigaction sig_action;
struct sigaction old_sig_action;
@@ -1643,7 +1663,12 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
bool quit = false;
int abort_flags = 0;
+ blocking |= vshCommandOptBool(cmd, "timeout") || pivot || finish;
if (blocking) {
+ if (pivot && finish) {
+ vshError(ctl, "%s", _("cannot mix --pivot and
--keep-overlay"));
+ return false;
+ }
if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0)
return false;
if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
@@ -1661,8 +1686,7 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
sigaction(SIGINT, &sig_action, &old_sig_action);
GETTIMEOFDAY(&start);
- } else if (verbose || vshCommandOptBool(cmd, "timeout") ||
- vshCommandOptBool(cmd, "async")) {
+ } else if (verbose || vshCommandOptBool(cmd, "async")) {
vshError(ctl, "%s", _("missing --wait option"));
return false;
}
@@ -1694,6 +1718,8 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
if (verbose)
vshPrintJobProgress(_("Block Commit"),
info.end - info.cur, info.end);
+ if (active && info.cur == info.end)
+ break;
GETTIMEOFDAY(&curr);
if (intCaught || (timeout &&
@@ -1720,7 +1746,25 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
/* printf [100 %] */
vshPrintJobProgress(_("Block Commit"), 0, 1);
}
- vshPrint(ctl, "\n%s", quit ? _("Commit aborted") : _("Commit
complete"));
+ if (!quit && pivot) {
+ abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT;
+ if (virDomainBlockJobAbort(dom, path, abort_flags) < 0) {
+ vshError(ctl, _("failed to pivot job for disk %s"), path);
+ goto cleanup;
+ }
+ } else if (finish && !quit &&
+ virDomainBlockJobAbort(dom, path, abort_flags) < 0) {
+ vshError(ctl, _("failed to finish job for disk %s"), path);
+ goto cleanup;
+ }
+ if (quit)
+ vshPrint(ctl, "\n%s", _("Commit aborted"));
+ else if (pivot)
+ vshPrint(ctl, "\n%s", _("Successfully pivoted"));
+ else if (!finish)
+ vshPrint(ctl, "\n%s", _("Now in synchronized phase"));
+ else
+ vshPrint(ctl, "\n%s", _("Commit complete"));
ret = true;
cleanup:
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 77013f5..21c29ec 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -769,8 +769,9 @@ address of virtual interface (such as I<detach-interface> or
I<domif-setlink>) will accept the MAC address printed by this command.
=item B<blockcommit> I<domain> I<path> [I<bandwidth>]
-{[I<base>] | [I<--shallow>]} [I<top>] [I<--delete>]
[I<--keep-relative>]
-[I<--wait> [I<--verbose>] [I<--timeout> B<seconds>]
[I<--async>]]
+[I<base>] [I<--shallow>] [I<top>] [I<--delete>]
[I<--keep-relative>]
+[I<--wait> [I<--async>] [I<--verbose>]] [I<--timeout>
B<seconds>]
+[I<--active>] [{I<--pivot> | I<--keep-overlay>}]
Reduce the length of a backing image chain, by committing changes at the
top of the chain (snapshot or delta files) into backing images. By
@@ -780,21 +781,34 @@ operation is constrained to committing just that portion of the
chain;
I<--shallow> can be used instead of I<base> to specify the immediate
backing file of the resulting top image to be committed. The files
being committed are rendered invalid, possibly as soon as the operation
-starts; using the I<--delete> flag will remove these files at the successful
-completion of the commit operation. Using the I<--keep-relative> flag
-will try to keep the backing chain names relative (if they were
-relative before).
+starts; using the I<--delete> flag will attempt to remove these invalidated
+files at the successful completion of the commit operation. Using the
+I<--keep-relative> flag will try to keep the backing chain names relative
+(if they were relative before).
+
+When I<top> is omitted or specified as the active image, it is also
+possible to specify I<--active> to trigger a two-phase active commit. In
+the first phase, I<top> is copied into I<base> and the job can only be
+canceled, with top still containing data not yet in base. In the second
+phase, I<top> and I<base> remain identical until a call to B<blockjob>
+with the I<--abort> flag (keeping top as the active image that tracks
+changes from that point in time) or the I<--pivot> flag (making base
+the new active image and invalidating top).
By default, this command returns as soon as possible, and data for
the entire disk is committed in the background; the progress of the
operation can be checked with B<blockjob>. However, if I<--wait> is
-specified, then this command will block until the operation completes,
-or cancel the operation if the optional I<timeout> in seconds elapses
+specified, then this command will block until the operation completes
+(or for I<--active>, enters the second phase), or until the operation
+is canceled because the optional I<timeout> in seconds elapses
or SIGINT is sent (usually with C<Ctrl-C>). Using I<--verbose> along
with I<--wait> will produce periodic status updates. If job cancellation
is triggered, I<--async> will return control to the user as fast as
possible, otherwise the command may continue to block a little while
-longer until the job is done cleaning up.
+longer until the job is done cleaning up. Using I<--pivot> is shorthand
+for combining I<--active> I<--wait> with an automatic B<blockjob>
+I<--pivot>; and using I<--keep-overlay> is shorthand for combining
+I<--active> I<--wait> with an automatic B<blockjob> I<--abort>.
I<path> specifies fully-qualified path of the disk; it corresponds
to a unique target name (<target dev='name'/>) or source file (<source
@@ -925,8 +939,8 @@ also B<domblklist> for listing these names).
If I<--abort> is specified, the active job on the specified disk will
be aborted. If I<--async> is also specified, this command will return
immediately, rather than waiting for the cancellation to complete. If
-I<--pivot> is specified, this requests that an active copy job
-be pivoted over to the new copy.
+I<--pivot> is specified, this requests that an active copy or active
+commit job be pivoted over to the new image.
If I<--info> is specified, the active job information on the specified
disk will be printed.
I<bandwidth> can be used to set bandwidth limit for the active job.
--
1.9.3
4:18 a.m.
New subject: [libvirt] [PATCH v3 2/5] virsh: expose new active commit controls
On 06/11/14 18:27, Eric Blake wrote:
Add knobs to virsh to manage a 2-phase active commit of the top
layer, similar to knobs already present on blockcopy. While this
code will fail until later patches actually implement the new
knobs in the qemu driver, doing it now proves that the API is
usable and also makes it easier for testing the qemu changes as
they are made.
* tools/virsh-domain.c (cmdBlockCommit): Add --active, --pivot,
and --finish options, modeled after blockcopy.
s/--finish/--keep-overlay/
(blockJobImpl): Support --active flag.
* tools/virsh.pod (blockcommit): Document new flags.
(blockjob): Mention 2-phase commit interaction.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
tools/virsh-domain.c | 58 +++++++++++++++++++++++++++++++++++++++++++++-------
tools/virsh.pod | 36 ++++++++++++++++++++++----------
2 files changed, 76 insertions(+), 18 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 294b594..3db0bae 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1621,7 +1638,7 @@ static const vshCmdOptDef opts_block_commit[] =
{
{.name = "keep-relative",
.type = VSH_OT_BOOL,
.help = N_("keep the backing chain relative if it was relatively "
- "referenced if it was before")
+ "referenced before")
},
This should go into my patch. As we discussed, this stuff has better
chances going in earlier as the relative naming patches as it already is
supported by qemu, so you should rebase this to master instead to my series.
{.name = NULL}
};
@@ -1631,8 +1648,11 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom = NULL;
bool ret = false;
- bool blocking = vshCommandOptBool(cmd, "wait");
Spurious move again?
bool verbose = vshCommandOptBool(cmd, "verbose");
+ bool pivot = vshCommandOptBool(cmd, "pivot");
+ bool finish = vshCommandOptBool(cmd, "keep-overlay");
+ bool active = vshCommandOptBool(cmd, "active") || pivot || finish;
+ bool blocking = vshCommandOptBool(cmd, "wait");
int timeout = 0;
struct sigaction sig_action;
struct sigaction old_sig_action;
@@ -1643,7 +1663,12 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
bool quit = false;
int abort_flags = 0;
+ blocking |= vshCommandOptBool(cmd, "timeout") || pivot || finish;
if (blocking) {
+ if (pivot && finish) {
+ vshError(ctl, "%s", _("cannot mix --pivot and
--keep-overlay"));
+ return false;
+ }
You can use VSH_EXCLUSIVE_OPTIONS("pivot", "keep-overlay") here. Also
you can bove it out of the if (blocking) block as they can't be ever
used together.
(Yes it will add a function call to get the value, but on the other hand
it will unify this piece with a majority of others).
if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) <
0)
return false;
if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
@@ -1694,6 +1718,8 @@ cmdBlockCommit(vshControl *ctl, const vshCmd
*cmd)
if (verbose)
vshPrintJobProgress(_("Block Commit"),
info.end - info.cur, info.end);
Empty line here will separate those two independent blocks visually.
+ if (active && info.cur == info.end)
+ break;
GETTIMEOFDAY(&curr);
if (intCaught || (timeout &&
@@ -1720,7 +1746,25 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
/* printf [100 %] */
vshPrintJobProgress(_("Block Commit"), 0, 1);
}
In this hunk too. Please separate the blocks.
- vshPrint(ctl, "\n%s", quit ? _("Commit
aborted") : _("Commit complete"));
+ if (!quit && pivot) {
+ abort_flags |= VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT;
+ if (virDomainBlockJobAbort(dom, path, abort_flags) < 0) {
+ vshError(ctl, _("failed to pivot job for disk %s"), path);
+ goto cleanup;
+ }
+ } else if (finish && !quit &&
+ virDomainBlockJobAbort(dom, path, abort_flags) < 0) {
+ vshError(ctl, _("failed to finish job for disk %s"), path);
+ goto cleanup;
+ }
+ if (quit)
+ vshPrint(ctl, "\n%s", _("Commit aborted"));
+ else if (pivot)
+ vshPrint(ctl, "\n%s", _("Successfully pivoted"));
+ else if (!finish)
+ vshPrint(ctl, "\n%s", _("Now in synchronized phase"));
+ else
+ vshPrint(ctl, "\n%s", _("Commit complete"));
ret = true;
cleanup:
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 77013f5..21c29ec 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -769,8 +769,9 @@ address of virtual interface (such as I<detach-interface> or
I<domif-setlink>) will accept the MAC address printed by this command.
=item B<blockcommit> I<domain> I<path> [I<bandwidth>]
-{[I<base>] | [I<--shallow>]} [I<top>] [I<--delete>]
[I<--keep-relative>]
-[I<--wait> [I<--verbose>] [I<--timeout> B<seconds>]
[I<--async>]]
+[I<base>] [I<--shallow>] [I<top>] [I<--delete>]
[I<--keep-relative>]
+[I<--wait> [I<--async>] [I<--verbose>]] [I<--timeout>
B<seconds>]
+[I<--active>] [{I<--pivot> | I<--keep-overlay>}]
Again, rebase this to master please if you want to go first.
Reduce the length of a backing image chain, by committing changes at
the
top of the chain (snapshot or delta files) into backing images. By
Otherwise looks good. If you want to get this into upstream prior to my
series then please rebase and repost this patch.
If you are about to wait until the relative-backing stuff goes in, then
ACK with the nits fixed.
Peter
11:27 a.m.
New subject: [libvirt] [PATCH v3 3/5] blockcommit: update error messages related to block jobs
A future patch will add two-phase block commit jobs; as the
mechanism for managing them is similar to managing a block copy
job, existing errors should be made generic enough to occur
for either job type.
* src/conf/domain_conf.c (virDomainHasDiskMirror): Update
comment.
* src/qemu/qemu_driver.c (qemuDomainDefineXML)
(qemuDomainSnapshotCreateXML, qemuDomainRevertToSnapshot)
(qemuDomainBlockJobImpl, qemuDomainBlockCopy): Update error
message.
* src/qemu/qemu_hotplug.c (qemuDomainDetachDiskDevice): Likewise.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/conf/domain_conf.c | 2 +-
src/qemu/qemu_driver.c | 10 +++++-----
src/qemu/qemu_hotplug.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ff2d447..be81dbe 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10379,7 +10379,7 @@ virDomainDiskRemoveByName(virDomainDefPtr def, const char *name)
}
/* Return true if VM has at least one disk involved in a current block
- * copy job (that is, with a <mirror> element in the disk xml). */
+ * copy/commit job (that is, with a <mirror> element in the disk xml). */
bool
virDomainHasDiskMirror(virDomainObjPtr vm)
{
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3f42af9..3399b2a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6260,7 +6260,7 @@ static virDomainPtr qemuDomainDefineXML(virConnectPtr conn, const
char *xml)
def = NULL;
if (virDomainHasDiskMirror(vm)) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, "%s",
- _("domain has active block copy job"));
+ _("domain has active block job"));
virDomainObjAssignDef(vm, NULL, false, NULL);
goto cleanup;
}
@@ -13464,7 +13464,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
}
if (virDomainHasDiskMirror(vm)) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, "%s",
- _("domain has active block copy job"));
+ _("domain has active block job"));
goto cleanup;
}
@@ -14074,7 +14074,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr
snapshot,
if (virDomainHasDiskMirror(vm)) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, "%s",
- _("domain has active block copy job"));
+ _("domain has active block job"));
goto cleanup;
}
@@ -15083,7 +15083,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
if (mode == BLOCK_JOB_PULL && disk->mirror) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
- _("disk '%s' already in active block copy
job"),
+ _("disk '%s' already in active block job"),
disk->dst);
goto endjob;
}
@@ -15318,7 +15318,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
disk = vm->def->disks[idx];
if (disk->mirror) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
- _("disk '%s' already in active block copy
job"),
+ _("disk '%s' already in active block job"),
disk->dst);
goto endjob;
}
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index cd43dd5..7289055 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2986,7 +2986,7 @@ qemuDomainDetachDiskDevice(virQEMUDriverPtr driver,
if (detach->mirror) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
- _("disk '%s' is in an active block copy job"),
+ _("disk '%s' is in an active block job"),
detach->dst);
goto cleanup;
}
--
1.9.3
4:21 a.m.
New subject: [libvirt] [PATCH v3 3/5] blockcommit: update error messages related to block jobs
On 06/11/14 18:27, Eric Blake wrote:
A future patch will add two-phase block commit jobs; as the
mechanism for managing them is similar to managing a block copy
job, existing errors should be made generic enough to occur
for either job type.
* src/conf/domain_conf.c (virDomainHasDiskMirror): Update
comment.
* src/qemu/qemu_driver.c (qemuDomainDefineXML)
(qemuDomainSnapshotCreateXML, qemuDomainRevertToSnapshot)
(qemuDomainBlockJobImpl, qemuDomainBlockCopy): Update error
message.
* src/qemu/qemu_hotplug.c (qemuDomainDetachDiskDevice): Likewise.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/conf/domain_conf.c | 2 +-
src/qemu/qemu_driver.c | 10 +++++-----
src/qemu/qemu_hotplug.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
ACK still stands.
Peter
9:54 p.m.
New subject: [libvirt] [PATCH v3 3/5] blockcommit: update error messages related to block jobs
On 06/12/2014 03:21 AM, Peter Krempa wrote:
On 06/11/14 18:27, Eric Blake wrote:
> A future patch will add two-phase block commit jobs; as the
> mechanism for managing them is similar to managing a block copy
> job, existing errors should be made generic enough to occur
> for either job type.
>
> * src/conf/domain_conf.c (virDomainHasDiskMirror): Update
> comment.
> * src/qemu/qemu_driver.c (qemuDomainDefineXML)
> (qemuDomainSnapshotCreateXML, qemuDomainRevertToSnapshot)
> (qemuDomainBlockJobImpl, qemuDomainBlockCopy): Update error
> message.
> * src/qemu/qemu_hotplug.c (qemuDomainDetachDiskDevice): Likewise.
>
> Signed-off-by: Eric Blake <eblake(a)redhat.com>
> ---
> src/conf/domain_conf.c | 2 +-
> src/qemu/qemu_driver.c | 10 +++++-----
> src/qemu/qemu_hotplug.c | 2 +-
> 3 files changed, 7 insertions(+), 7 deletions(-)
>
ACK still stands.
I pushed this one.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:27 a.m.
New subject: [libvirt] [PATCH v3 4/5] blockcommit: track job type in xml
A future patch is going to wire up qemu active block commit jobs;
but as they have similar events and are canceled/pivoted in the
same way as block copy jobs, it is easiest to track all bookkeeping
for the commit job by reusing the <mirror> element. This patch
adds domain XML to track which job was responsible for creating a
mirroring situation, and adds a job='copy' attribute to all
existing uses of <mirror>. Along the way, it also massages the
qemu monitor backend to read the new field in order to generate
the correct type of libvirt job (even though it requires a
future patch to actually cause a qemu event that can be reported
as an active commit).
* docs/schemas/domaincommon.rng: Enhance schema.
* docs/formatdomain.html.in: Document it.
* src/conf/domain_conf.h (_virDomainDiskDef): Add a field.
* src/conf/domain_conf.c (virDomainBlockJobType): String conversion.
(virDomainDiskDefParseXML): Parse job type.
(virDomainDiskDefFormat): Output job type.
* src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Distinguish
active from regular commit.
* src/qemu/qemu_driver.c (qemuDomainBlockCopy): Set job type.
(qemuDomainBlockPivot, qemuDomainBlockJobImpl): Clean up job type
on completion.
* tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml:
Update tests.
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Likewise.
* tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml: New
file.
* tests/qemuxml2xmltest.c (mymain): Drive new test.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
docs/formatdomain.html.in | 20 ++++++------
docs/schemas/domaincommon.rng | 6 ++++
src/conf/domain_conf.c | 24 ++++++++++++++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_driver.c | 3 ++
src/qemu/qemu_process.c | 18 +++++++----
.../qemuxml2argv-disk-active-commit.xml | 37 ++++++++++++++++++++++
.../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 4 +--
.../qemuxml2xmlout-disk-mirror-old.xml | 4 +--
tests/qemuxml2xmltest.c | 1 +
10 files changed, 98 insertions(+), 20 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 1b6ced8..512132d 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1878,16 +1878,18 @@
</dd>
<dt><code>mirror</code></dt>
<dd>
- This element is present if the hypervisor has started a block
- copy operation (via the <code>virDomainBlockRebase</code>
- API), where the mirror location in the <code>source</code>
- sub-element will eventually have the same contents as the
- source, and with the file format in the
+ This element is present if the hypervisor has started a
+ long-running block job operation, where the mirror location in
+ the <code>source</code> sub-element will eventually have the
+ same contents as the source, and with the file format in the
sub-element <code>format</code> (which might differ from the
- format of the source). The details of the <code>source</code>
- sub-element are determined by the <code>type</code> attribute
- of the mirror, similar to what is done for the
- overall <code>disk</code> device element. If
+ format of the source). The <code>job</code> attribute
+ mentions which API started the operation ("copy" for
+ the <code>virDomainBlockRebase</code> API, or "commit" for
+ the <code>virDomainBlockCommit</code> API). The details of
+ the <code>source</code> sub-element are determined by
+ the <code>type</code> attribute of the mirror, similar to what
+ is done for the overall <code>disk</code> device element. If
attribute <code>ready</code> is present, then it is known the
disk is ready to pivot; otherwise, the disk is probably still
copying. For now, this element only valid in output; it is
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 6cc922c..3badb6d 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4187,6 +4187,12 @@
</optional>
</group>
<group>
+ <attribute name='job'>
+ <choice>
+ <value>copy</value>
+ <value>active-commit</value>
+ </choice>
+ </attribute>
<interleave>
<ref name="diskSource"/>
<optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index be81dbe..fbe9fa4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -760,6 +760,12 @@ VIR_ENUM_IMPL(virDomainDiskDiscard, VIR_DOMAIN_DISK_DISCARD_LAST,
"unmap",
"ignore")
+/* Internal mapping: subset of block job types that can be present in
+ * <mirror> XML (remaining types are not two-phase). */
+VIR_ENUM_DECL(virDomainBlockJob)
+VIR_ENUM_IMPL(virDomainBlockJob, VIR_DOMAIN_BLOCK_JOB_TYPE_LAST,
+ "", "", "copy", "",
"active-commit")
+
#define VIR_DOMAIN_XML_WRITE_FLAGS VIR_DOMAIN_XML_SECURE
#define VIR_DOMAIN_XML_READ_FLAGS VIR_DOMAIN_XML_INACTIVE
@@ -5377,10 +5383,26 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
xmlStrEqual(cur->name, BAD_CAST "mirror") &&
!(flags & VIR_DOMAIN_XML_INACTIVE)) {
char *ready;
+ char *blockJob;
if (VIR_ALLOC(def->mirror) < 0)
goto error;
+ blockJob = virXMLPropString(cur, "job");
+ if (blockJob) {
+ def->mirrorJob = virDomainBlockJobTypeFromString(blockJob);
+ if (def->mirrorJob <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown mirror job type
'%s'"),
+ blockJob);
+ VIR_FREE(blockJob);
+ goto error;
+ }
+ VIR_FREE(blockJob);
+ } else {
+ def->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY;
+ }
+
mirrorType = virXMLPropString(cur, "type");
if (mirrorType) {
def->mirror->type = virStorageTypeFromString(mirrorType);
@@ -15193,6 +15215,8 @@ virDomainDiskDefFormat(virBufferPtr buf,
if (def->mirror && !(flags & VIR_DOMAIN_XML_INACTIVE)) {
virBufferAsprintf(buf, "<mirror type='%s'",
virStorageTypeToString(def->mirror->type));
+ virBufferEscapeString(buf, " job='%s'",
+ virDomainBlockJobTypeToString(def->mirrorJob));
if (def->mirroring)
virBufferAddLit(buf, " ready='yes'");
virBufferAddLit(buf, ">\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a6ac95a..27e6118 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -608,6 +608,7 @@ struct _virDomainDiskDef {
virStorageSourcePtr mirror;
bool mirroring;
+ int mirrorJob; /* virDomainBlockJobType */
struct {
unsigned int cylinders;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3399b2a..728aa85 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14994,6 +14994,7 @@ qemuDomainBlockPivot(virConnectPtr conn,
virStorageSourceFree(disk->mirror);
disk->mirror = NULL;
disk->mirroring = false;
+ disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
cleanup:
if (resume && virDomainObjIsActive(vm) &&
@@ -15155,6 +15156,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
virStorageSourceFree(disk->mirror);
disk->mirror = NULL;
disk->mirroring = false;
+ disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
}
/* With synchronous block cancel, we must synthesize an event, and
@@ -15429,6 +15431,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
need_unlink = false;
disk->mirror = mirror;
mirror = NULL;
+ disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY;
endjob:
if (need_unlink && unlink(dest))
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e4845ba..ed2991a 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1020,26 +1020,30 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias);
if (disk) {
+ if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT &&
+ disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)
+ type = disk->mirrorJob;
path = virDomainDiskGetSource(disk);
event = virDomainEventBlockJobNewFromObj(vm, path, type, status);
/* XXX If we completed a block pull or commit, then recompute
* the cached backing chain to match. Better would be storing
- * the chain ourselves rather than reprobing, but this
- * requires modifying domain_conf and our XML to fully track
- * the chain across libvirtd restarts. For that matter, if
- * qemu gains support for committing the active layer, we have
- * to update disk->src. */
+ * the chain ourselves rather than reprobing, but we haven't
+ * quite completed that conversion to use our XML tracking. */
if ((type == VIR_DOMAIN_BLOCK_JOB_TYPE_PULL ||
- type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT) &&
+ type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT ||
+ type == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT) &&
status == VIR_DOMAIN_BLOCK_JOB_COMPLETED)
qemuDomainDetermineDiskChain(driver, vm, disk, true);
- if (disk->mirror && type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
+ if (disk->mirror &&
+ (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY ||
+ type == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)) {
if (status == VIR_DOMAIN_BLOCK_JOB_READY) {
disk->mirroring = true;
} else if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) {
virStorageSourceFree(disk->mirror);
disk->mirror = NULL;
disk->mirroring = false;
+ disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
}
}
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
new file mode 100644
index 0000000..6ba5724
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
@@ -0,0 +1,37 @@
+<domain type='qemu' id='1'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='file' device='disk'>
+ <driver name='qemu' type='qcow2'/>
+ <source file='/tmp/HostVG/QEMUGuest1-snap'/>
+ <backingStore type='block' index='1'>
+ <format type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <backingStore/>
+ </backingStore>
+ <mirror type='block' job='active-commit'>
+ <format type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ </mirror>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
index a937d0a..6c28687 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml
@@ -17,7 +17,7 @@
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
<backingStore/>
- <mirror type='file' ready='yes'>
+ <mirror type='file' job='copy' ready='yes'>
<source file='/dev/HostVG/QEMUGuest1Copy'/>
</mirror>
<target dev='hda' bus='ide'/>
@@ -33,7 +33,7 @@
<disk type='file' device='disk'>
<source file='/tmp/data.img'/>
<backingStore/>
- <mirror type='file'>
+ <mirror type='file' job='copy'>
<format type='qcow2'/>
<source file='/tmp/copy.img'/>
</mirror>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
index a937d0a..6c28687 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml
@@ -17,7 +17,7 @@
<disk type='block' device='disk'>
<source dev='/dev/HostVG/QEMUGuest1'/>
<backingStore/>
- <mirror type='file' ready='yes'>
+ <mirror type='file' job='copy' ready='yes'>
<source file='/dev/HostVG/QEMUGuest1Copy'/>
</mirror>
<target dev='hda' bus='ide'/>
@@ -33,7 +33,7 @@
<disk type='file' device='disk'>
<source file='/tmp/data.img'/>
<backingStore/>
- <mirror type='file'>
+ <mirror type='file' job='copy'>
<format type='qcow2'/>
<source file='/tmp/copy.img'/>
</mirror>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 200d50f..98b2c15 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -227,6 +227,7 @@ mymain(void)
DO_TEST_DIFFERENT("disk-mirror-old");
DO_TEST_FULL("disk-mirror", false, WHEN_ACTIVE);
DO_TEST_FULL("disk-mirror", true, WHEN_INACTIVE);
+ DO_TEST_FULL("disk-active-commit", false, WHEN_ACTIVE);
DO_TEST("graphics-listen-network");
DO_TEST("graphics-vnc");
DO_TEST("graphics-vnc-websocket");
--
1.9.3
6:51 a.m.
New subject: [libvirt] [PATCH v3 4/5] blockcommit: track job type in xml
On 06/11/14 18:27, Eric Blake wrote:
A future patch is going to wire up qemu active block commit jobs;
but as they have similar events and are canceled/pivoted in the
same way as block copy jobs, it is easiest to track all bookkeeping
for the commit job by reusing the <mirror> element. This patch
adds domain XML to track which job was responsible for creating a
mirroring situation, and adds a job='copy' attribute to all
existing uses of <mirror>. Along the way, it also massages the
qemu monitor backend to read the new field in order to generate
the correct type of libvirt job (even though it requires a
future patch to actually cause a qemu event that can be reported
as an active commit).
* docs/schemas/domaincommon.rng: Enhance schema.
* docs/formatdomain.html.in: Document it.
* src/conf/domain_conf.h (_virDomainDiskDef): Add a field.
* src/conf/domain_conf.c (virDomainBlockJobType): String conversion.
(virDomainDiskDefParseXML): Parse job type.
(virDomainDiskDefFormat): Output job type.
* src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Distinguish
active from regular commit.
* src/qemu/qemu_driver.c (qemuDomainBlockCopy): Set job type.
(qemuDomainBlockPivot, qemuDomainBlockJobImpl): Clean up job type
on completion.
* tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml:
Update tests.
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Likewise.
* tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml: New
file.
* tests/qemuxml2xmltest.c (mymain): Drive new test.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
docs/formatdomain.html.in | 20 ++++++------
docs/schemas/domaincommon.rng | 6 ++++
src/conf/domain_conf.c | 24 ++++++++++++++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_driver.c | 3 ++
src/qemu/qemu_process.c | 18 +++++++----
.../qemuxml2argv-disk-active-commit.xml | 37 ++++++++++++++++++++++
.../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 4 +--
.../qemuxml2xmlout-disk-mirror-old.xml | 4 +--
tests/qemuxml2xmltest.c | 1 +
10 files changed, 98 insertions(+), 20 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
ACK, although this patch pointed me to a problem. If you do a block-copy
of a network disk to a local file (which seems from the code that it's
allowed) and the mirror is being pivoted to the new disk we don't update
the disk type in the code and thus would end up with a very damaged VM.
Fortunately for block-copy this isn't a major problem as the VM has to
be transient, but will be fatal for the active-commit code.
Peter
10:02 p.m.
New subject: [libvirt] [PATCH v3 4/5] blockcommit: track job type in xml
On 06/12/2014 05:51 AM, Peter Krempa wrote:
On 06/11/14 18:27, Eric Blake wrote:
> A future patch is going to wire up qemu active block commit jobs;
> but as they have similar events and are canceled/pivoted in the
> same way as block copy jobs, it is easiest to track all bookkeeping
> for the commit job by reusing the <mirror> element. This patch
> adds domain XML to track which job was responsible for creating a
> mirroring situation, and adds a job='copy' attribute to all
> existing uses of <mirror>. Along the way, it also massages the
> qemu monitor backend to read the new field in order to generate
> the correct type of libvirt job (even though it requires a
> future patch to actually cause a qemu event that can be reported
> as an active commit).
>
>
ACK, although this patch pointed me to a problem. If you do a block-copy
of a network disk to a local file (which seems from the code that it's
allowed) and the mirror is being pivoted to the new disk we don't update
the disk type in the code and thus would end up with a very damaged VM.
Fortunately for block-copy this isn't a major problem as the VM has to
be transient, but will be fatal for the active-commit code.
Oh good point. I'm hoping your series for copying a virStorageSource
will help out here (including however we end up resolving the issue at
tracking the security manager stuff).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:40 a.m.
New subject: [libvirt] [PATCH v3 4/5] blockcommit: track job type in xml
On 06/11/2014 10:27 AM, Eric Blake wrote:
A future patch is going to wire up qemu active block commit jobs;
but as they have similar events and are canceled/pivoted in the
same way as block copy jobs, it is easiest to track all bookkeeping
for the commit job by reusing the <mirror> element. This patch
adds domain XML to track which job was responsible for creating a
mirroring situation, and adds a job='copy' attribute to all
existing uses of <mirror>. Along the way, it also massages the
qemu monitor backend to read the new field in order to generate
the correct type of libvirt job (even though it requires a
future patch to actually cause a qemu event that can be reported
as an active commit).
On IRC, Adam Litke pointed out that we have an issue on the event we
generate:
+++ b/src/qemu/qemu_process.c
@@ -1020,26 +1020,30 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
disk = qemuProcessFindDomainDiskByAlias(vm, diskAlias);
if (disk) {
+ if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT &&
+ disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)
+ type = disk->mirrorJob;
path = virDomainDiskGetSource(disk);
event = virDomainEventBlockJobNewFromObj(vm, path, type, status);
for an original chain 'base <- snap', we are generating the 'ready'
event with a disk name of 'snap', but the 'complete' event with a disk
name of 'base'. Ideally, we WANT to generate both events tied to the
same disk name (even better would be generating the events tied to the
name 'vda' which is unchanging, rather than the disk source name, but I
have to think about whether there are backward-compatibility concerns
before making such a change).
I'll investigate what it will take to get that naming improvement into
the events that we generate.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:27 a.m.
New subject: [libvirt] [PATCH v3 5/5] blockcommit: turn on active commit
With this in place, I can (finally!) now do:
virsh blockcommit $dom vda --shallow --wait --verbose --pivot
and watch qemu shorten the backing chain by one, followed by
libvirt automatically updating the dumpxml output, effectively
undoing the work of virsh snapshot-commit --no-metadata --disk-only.
Commit is SOOOO much faster than blockpull, when I'm still fairly
close in time to when the temporary qcow2 wrapper file was created
via a snapshot operation!
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Implement live
commit to regular files.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/qemu/qemu_driver.c | 43 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 728aa85..60dc9c6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15529,9 +15529,11 @@ qemuDomainBlockCommit(virDomainPtr dom,
char *topPath = NULL;
char *basePath = NULL;
char *backingPath = NULL;
+ virStorageSourcePtr mirror = NULL;
- /* XXX Add support for COMMIT_ACTIVE, COMMIT_DELETE */
+ /* XXX Add support for COMMIT_DELETE */
virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
+ VIR_DOMAIN_BLOCK_COMMIT_ACTIVE |
VIR_DOMAIN_BLOCK_COMMIT_RELATIVE, -1);
if (!(vm = qemuDomObjFromDomain(dom)))
@@ -15578,13 +15580,14 @@ qemuDomainBlockCommit(virDomainPtr dom,
&top_parent)))
goto endjob;
- /* FIXME: qemu 2.0 supports active commit, but as a two-stage
- * process; qemu 2.1 is further improving active commit. We need
- * to start supporting it in libvirt. */
if (topSource == disk->src) {
/* We assume that no one will backport qemu 2.0 active commit
* to an earlier qemu without also backporting the block job
- * ready event; but this makes sure of that fact */
+ * ready event; but this makes sure of that fact.
+ *
+ * XXX Also need to check other capability bit(s): qemu 1.7
+ * supports async blockjob but not active commit; and qemu 2.0
+ * active commit misbehaves on 0-byte file. */
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("active commit not supported with this QEMU
binary"));
@@ -15597,6 +15600,14 @@ qemuDomainBlockCommit(virDomainPtr dom,
disk->dst);
goto endjob;
}
+ if (disk->mirror) {
+ virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
+ _("disk '%s' already in active block job"),
+ disk->dst);
+ goto endjob;
+ }
+ if (VIR_ALLOC(mirror) < 0)
+ goto endjob;
} else if (flags & VIR_DOMAIN_BLOCK_COMMIT_ACTIVE) {
virReportError(VIR_ERR_INVALID_ARG,
_("active commit requested but '%s' is not
active"),
@@ -15667,6 +15678,21 @@ qemuDomainBlockCommit(virDomainPtr dom,
}
}
+ /* For an active commit, clone enough of the base to act as the mirror */
+ if (mirror) {
+ /* XXX Support network commits */
+ if (baseSource->type != VIR_STORAGE_TYPE_FILE &&
+ baseSource->type != VIR_STORAGE_TYPE_BLOCK) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("active commit to non-file not supported yet"));
+ goto endjob;
+ }
+ mirror->type = baseSource->type;
+ if (VIR_STRDUP(mirror->path, baseSource->path) < 0)
+ goto endjob;
+ mirror->format = baseSource->format;
+ }
+
/* Start the commit operation. Pass the user's original spelling,
* if any, through to qemu, since qemu may behave differently
* depending on whether the input was specified as relative or
@@ -15678,6 +15704,12 @@ qemuDomainBlockCommit(virDomainPtr dom,
bandwidth);
qemuDomainObjExitMonitor(driver, vm);
+ if (ret == 0 && mirror) {
+ disk->mirror = mirror;
+ mirror = NULL;
+ disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT;
+ }
+
endjob:
if (ret < 0 && clean_access) {
/* Revert access to read-only, if possible. */
@@ -15688,6 +15720,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
top_parent,
VIR_DISK_CHAIN_READ_ONLY);
}
+ virStorageSourceFree(mirror);
if (!qemuDomainObjEndJob(driver, vm))
vm = NULL;
--
1.9.3
7:01 a.m.
New subject: [libvirt] [PATCH v3 5/5] blockcommit: turn on active commit
On 06/11/14 18:27, Eric Blake wrote:
With this in place, I can (finally!) now do:
virsh blockcommit $dom vda --shallow --wait --verbose --pivot
and watch qemu shorten the backing chain by one, followed by
libvirt automatically updating the dumpxml output, effectively
undoing the work of virsh snapshot-commit --no-metadata --disk-only.
Commit is SOOOO much faster than blockpull, when I'm still fairly
close in time to when the temporary qcow2 wrapper file was created
via a snapshot operation!
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Implement live
commit to regular files.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/qemu/qemu_driver.c | 43 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 728aa85..60dc9c6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15529,9 +15529,11 @@ qemuDomainBlockCommit(virDomainPtr dom,
char *topPath = NULL;
char *basePath = NULL;
char *backingPath = NULL;
+ virStorageSourcePtr mirror = NULL;
- /* XXX Add support for COMMIT_ACTIVE, COMMIT_DELETE */
+ /* XXX Add support for COMMIT_DELETE */
virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
+ VIR_DOMAIN_BLOCK_COMMIT_ACTIVE |
Rebase needed if this should go before my series.
VIR_DOMAIN_BLOCK_COMMIT_RELATIVE, -1);
if (!(vm = qemuDomObjFromDomain(dom)))
@@ -15578,13 +15580,14 @@ qemuDomainBlockCommit(virDomainPtr dom,
&top_parent)))
goto endjob;
- /* FIXME: qemu 2.0 supports active commit, but as a two-stage
- * process; qemu 2.1 is further improving active commit. We need
- * to start supporting it in libvirt. */
if (topSource == disk->src) {
/* We assume that no one will backport qemu 2.0 active commit
* to an earlier qemu without also backporting the block job
- * ready event; but this makes sure of that fact */
+ * ready event; but this makes sure of that fact.
+ *
+ * XXX Also need to check other capability bit(s): qemu 1.7
+ * supports async blockjob but not active commit; and qemu 2.0
+ * active commit misbehaves on 0-byte file. */
How about tying the support to qemu 2.1 if it makes things easier? I
don't see any problem with supporting stuff from later-than-introduced
versions. The only question that remains is whether there's anything
relevant to trigger the capability of.
if (!virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_BLOCKJOB_ASYNC)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("active commit not supported with this QEMU
binary"));
@@ -15667,6 +15678,21 @@ qemuDomainBlockCommit(virDomainPtr dom,
}
}
+ /* For an active commit, clone enough of the base to act as the mirror */
+ if (mirror) {
+ /* XXX Support network commits */
+ if (baseSource->type != VIR_STORAGE_TYPE_FILE &&
+ baseSource->type != VIR_STORAGE_TYPE_BLOCK) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("active commit to non-file not supported yet"));
+ goto endjob;
+ }
+ mirror->type = baseSource->type;
+ if (VIR_STRDUP(mirror->path, baseSource->path) < 0)
+ goto endjob;
+ mirror->format = baseSource->format;
+ }
+
/* Start the commit operation. Pass the user's original spelling,
* if any, through to qemu, since qemu may behave differently
* depending on whether the input was specified as relative or
Hmmm, this commit is invalid here as we are not passing user's spelling
to qemu. It also doesn't make sense to do that. I should revert that
patch explicitly apparently as a part of my series.
Still needs the capability bit figured out. Otherwise looks good.
Peter
10:11 p.m.
New subject: [libvirt] [PATCH v3 5/5] blockcommit: turn on active commit
On 06/12/2014 06:01 AM, Peter Krempa wrote:
On 06/11/14 18:27, Eric Blake wrote:
> With this in place, I can (finally!) now do:
>
> virsh blockcommit $dom vda --shallow --wait --verbose --pivot
>
> - /* FIXME: qemu 2.0 supports active commit, but as a
two-stage
> - * process; qemu 2.1 is further improving active commit. We need
> - * to start supporting it in libvirt. */
> if (topSource == disk->src) {
> /* We assume that no one will backport qemu 2.0 active commit
> * to an earlier qemu without also backporting the block job
> - * ready event; but this makes sure of that fact */
> + * ready event; but this makes sure of that fact.
> + *
> + * XXX Also need to check other capability bit(s): qemu 1.7
> + * supports async blockjob but not active commit; and qemu 2.0
> + * active commit misbehaves on 0-byte file. */
How about tying the support to qemu 2.1 if it makes things easier? I
don't see any problem with supporting stuff from later-than-introduced
versions. The only question that remains is whether there's anything
relevant to trigger the capability of.
Testing qemu 2.1 is easier - merely check whether 'block-commit' with an
omitted top argument is accepted or rejected.
I just realized - rather than trying to go for 2.0 to begin with, it may
be easier to just require 2.1 for now; we can always relax things later
if people complain that they are still stuck on 2.0, but right now,
there's enough other things going on at once (such as your relative
backing naming stuff) that also depends on 2.1 that it just feels easier
to stick to a single version as our baseline, rather than delaying this
further.
But if I require qemu 2.1, then I'm in the same boat as you as not
having anything to push until I have an actual commit in qemu.git to
point to.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
11:34 a.m.
On 06/11/2014 10:27 AM, Eric Blake wrote:
I still don't have qemu capability detection working reliably,
but want to post this series right now so that it can be built
into a scratch build containing Peter's and my changes. (Or
put another way, I was testing what conflict resolutions are
required - patch 2/5 (virsh) and 5/5 (qemu_driver) has some
conflicts with Peter's addition of relative backing name; and
I think the resolutions were fairly simple).
These patches are tested on top of:
https://www.redhat.com/archives/libvir-list/2014-June/msg00492.html
I may still end up posting a v4 and/or pushing my series before
Peter's, once I get capability detection working the way I want.
Phooey, I forgot to send my v3 changelog.
Eric Blake (5):
virsh: improve blockcopy UI
new in v3
virsh: expose new active commit controls
resolves Peter's comments in v2 about ?:, rename --finish to
--keep-overlay, and copy UI enhancements from 1/5. Conflict resolution
with --keep-relative.
blockcommit: update error messages related to block jobs
unchanged from v2
blockcommit: track job type in xml
Reflow html source, tweak comment about why only 2 of the 5 job types
are mapped to string
blockcommit: turn on active commit
Conflict resolution with --keep-relative; still needs proper qemu
capability detection
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:53 p.m.
On 11/06/14 10:27 -0600, Eric Blake wrote:
I still don't have qemu capability detection working reliably,
but want to post this series right now so that it can be built
into a scratch build containing Peter's and my changes. (Or
put another way, I was testing what conflict resolutions are
required - patch 2/5 (virsh) and 5/5 (qemu_driver) has some
conflicts with Peter's addition of relative backing name; and
I think the resolutions were fairly simple).
These patches are tested on top of:
https://www.redhat.com/archives/libvir-list/2014-June/msg00492.html
I may still end up posting a v4 and/or pushing my series before
Peter's, once I get capability detection working the way I want.
Eric Blake (5):
virsh: improve blockcopy UI
virsh: expose new active commit controls
blockcommit: update error messages related to block jobs
blockcommit: track job type in xml
blockcommit: turn on active commit
I've tested this with the following script and it seems that
specifying base and top explicitly does not work. Using --shallow
does work for me though. Running the included script yields the
following output:
$ sudo bash /home/alitke/test/test.sh
Formatting 'base.img', fmt=raw size=1073741824
Formatting 'snap1.img', fmt=qcow2 size=1073741824
backing_file='base.img' backing_fmt='raw' encryption=off
cluster_size=65536 lazy_refcounts=off
Formatting 'snap2.img', fmt=qcow2 size=1073741824
backing_file='snap1.img' backing_fmt='qcow2' encryption=off
cluster_size=65536 lazy_refcounts=off
image: /tmp/snap2.img
file format: qcow2
virtual size: 1.0G (1073741824 bytes)
disk size: 196K
cluster_size: 65536
backing file: snap1.img (actual path: /tmp/snap1.img)
backing file format: qcow2
Format specific information:
compat: 1.1
lazy refcounts: false
Domain testvm1 created from /dev/stdin
error: invalid argument: could not find image '/tmp/snap2.img' in
chain for '/tmp/snap2.img'
Domain testvm1 destroyed
--
#!/bin/sh
rm -f base.img snap1.img snap2.img
# base.img <- snap1.img <- snap2.img
qemu-img create -f raw base.img 1G
qemu-img create -f qcow2 -b base.img -o backing_fmt=raw snap1.img
qemu-img create -f qcow2 -b snap1.img -o backing_fmt=qcow2 snap2.img
chcon -t svirt_image_t base.img snap1.img snap2.img
chmod 666 base.img snap1.img snap2.img
qemu-img info $PWD/snap2.img
virsh create /dev/stdin <<EOF
<domain type='kvm'>
<name>testvm1</name>
<memory unit='MiB'>256</memory>
<vcpu>1</vcpu>
<os>
<type arch='x86_64'>hvm</type>
</os>
<devices>
<disk type='file' device='disk'>
<driver name='qemu' type='qcow2'/>
<source file='$PWD/snap2.img'/>
<target dev='vda' bus='virtio'/>
</disk>
<graphics type='vnc'/>
</devices>
</domain>
EOF
base=$PWD/snap1.img
top=$PWD/snap2.img
virsh blockcommit testvm1 vda 0 $base $top --wait --verbose --pivot
virsh destroy testvm1
--
Adam Litke
5:12 p.m.
On 06/11/2014 01:53 PM, Adam Litke wrote:
I've tested this with the following script and it seems that
specifying base and top explicitly does not work. Using --shallow
does work for me though. Running the included script yields the
following output:
$ sudo bash /home/alitke/test/test.sh Formatting 'base.img', fmt=raw
error: invalid argument: could not find image '/tmp/snap2.img' in
chain for '/tmp/snap2.img'
base=$PWD/snap1.img
top=$PWD/snap2.img
virsh blockcommit testvm1 vda 0 $base $top --wait --verbose --pivot
Okay, I see the flaw - when looking up an explicit top, we aren't
starting from the active layer. Try omitting the --top argument as a
workaround. Meanwhile, this patch should fix it:
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 60dc9c6..aded8e0 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -15575,7 +15575,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
topSource = disk->src;
else if (virStorageFileParseChainIndex(disk->dst, top, &topIndex) <
0 ||
!(topSource = virStorageFileChainLookup(disk->src,
-
disk->src->backingStore,
+ disk->src,
top, topIndex,
&top_parent)))
goto endjob;
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3815
days inactive
3817
days old
18 comments
3 participants
participants (3)
-
Adam Litke -
Eric Blake -
Peter Krempa