Re: [libvirt] [PATCHv2 10/15] blockjob: support pivot operation on cancel
Il 06/04/2012 17:28, Eric Blake ha scritto: > I'm talking about the guest agent. It may make a difference, but cannot > be the default, because you cannot trust the guest agent to be present. > I'm thinking this will be like the --quiesce operation of > snapshot-create, as another situation where the guest agent quiesce can > usefully surround a 'transaction' command to improve the results, but > must not be default. Ah, got it now. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 10/15] blockjob: support pivot operation on cancel
On 04/06/2012 09:19 AM, Paolo Bonzini wrote: > Il 06/04/2012 06:36, Eric Blake ha scritto: >> if >> 'block_job_cancel' were made part of 'transaction', you could >> copy multiple disks at the same point in time without pausing >> the domain. > > This is doable. > > The transactioned command would do a qemu_aio_flush() in the prepare > phase, and a normal block_job_cancel in the commit phase. > >> This also implies we may want to add a --quiesce >> flag to the pivot operation, so that when breaking a mirror, the >> side of the mirror that we are abandoning is at least in a stable >> state with regards to guest I/O. > > I don't think this is needed. Either it makes a difference so it should > be the default, or it doesn't and then it is not a useful knob. I'm talking about the guest agent. It may make a difference, but cannot be the default, because you cannot trust the guest agent to be present. I'm thinking this will be like the --quiesce operation of snapshot-create, as another situation where the guest agent quiesce can usefully surround a 'transaction' command to improve the results, but must not be default. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 10/15] blockjob: support pivot operation on cancel
Il 06/04/2012 06:36, Eric Blake ha scritto: > if > 'block_job_cancel' were made part of 'transaction', you could > copy multiple disks at the same point in time without pausing > the domain. This is doable. The transactioned command would do a qemu_aio_flush() in the prepare phase, and a normal block_job_cancel in the commit phase. > This also implies we may want to add a --quiesce > flag to the pivot operation, so that when breaking a mirror, the > side of the mirror that we are abandoning is at least in a stable > state with regards to guest I/O. I don't think this is needed. Either it makes a difference so it should be the default, or it doesn't and then it is not a useful knob. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 10/15] blockjob: support pivot operation on cancel
On 04/06/2012 01:08 AM, Paolo Bonzini wrote: > Il 06/04/2012 06:36, Eric Blake ha scritto: >> If only qemu could get 'drive-reopen' inside 'transaction'... > > Just a quick answer to this: if qemu could get 'drive-reopen' inside > 'transaction', the standalone command could be made safe just as easily. > In fact, in QEMU 1.1 the blockdev-snapshot-sync command got a similar > treatment: it became transactionable, _and_ the standalone command > became a simple wrapper around a 1-command transaction. Good point. So unless we have a situation where we want to do 'drive-reopen' on multiple disks at once, we don't actually have to use a transaction command to reopen a single disk. Meanwhile, for similarity with blockdev-snapshot-sync, it looks like I will someday be adding a VIR_DOMAIN_BLOCK_JOB_ABORT_ATOMIC flag, which fails if qemu lacks support for reopen within a transaction, and assuming that qemu exposes a way to see which commands are transactionable. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 10/15] blockjob: support pivot operation on cancel
Il 06/04/2012 06:36, Eric Blake ha scritto: > If only qemu could get 'drive-reopen' inside 'transaction'... Just a quick answer to this: if qemu could get 'drive-reopen' inside 'transaction', the standalone command could be made safe just as easily. In fact, in QEMU 1.1 the blockdev-snapshot-sync command got a similar treatment: it became transactionable, _and_ the standalone command became a simple wrapper around a 1-command transaction. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 10/15] blockjob: support pivot operation on cancel
This is the bare minimum to end a copy job (of course, until a
later patch adds the ability to start a copy job, this patch
doesn't do much in isolation; I've just split the patches to
ease the review).
This patch intentionally avoids SELinux, lock manager, and audit
actions, saving that for a later patch that affects the overall
lifecycle of a disk copy. In particular, I'm still fuzzy on the
exact qemu error semantics, and whether I need to make more of
an effort after a 'drive-reopen' fails.
When a mirror job is started, cancelling the job safely reverts back
to the source disk, regardless of whether the destination is in
phase 1 (streaming, in which case the destination is worthless) or
phase 2 (mirroring, in which case the destination is sync'd up to
the source at the time of the cancel). Our existing code does just
fine in either phase, other than some bookkeeping cleanup.
Pivoting the job requires the use of the new 'drive-reopen' command.
Here, failure of the command is potentially catastrophic to the
domain, since it rips out the old disk before attempting to open
the new one; if our recovery path of retrying the reopen on the
original source disk also fails, the domain is hosed. If only qemu
could get 'drive-reopen' inside 'transaction'...
Interesting side note: while snapshot-create --disk-only creates a
copy of the disk at a point in time by moving the domain on to a
new file (the copy is the file now in the just-extended backing
chain), blockjob --abort of a copy job creates a copy of the disk
while keeping the domain on the original file. There may be
potential improvements to the snapshot code to exploit block copy
over multiple disks all at one point in time. And, if
'block_job_cancel' were made part of 'transaction', you could
copy multiple disks at the same point in time without pausing
the domain. This also implies we may want to add a --quiesce
flag to the pivot operation, so that when breaking a mirror, the
side of the mirror that we are abandoning is at least in a stable
state with regards to guest I/O.
* src/qemu/qemu_driver.c (qemuDomainBlockJobAbort): Accept new flag.
(qemuDomainBlockPivot): New helper function.
(qemuDomainBlockJobImpl): Implement it.
---
src/qemu/qemu_driver.c | 118 +---
1 files changed, 112 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c6033f8..22829ba 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11621,6 +11621,86 @@ cleanup:
return ret;
}
+/* Called while holding the VM job lock, to implement a block job
+ * abort with pivot; this updates the VM definition as appropriate, on
+ * either success or failure (although there are some forms of
+ * catastrophic failure that will leave the VM unusable). */
+static int
+qemuDomainBlockPivot(struct qemud_driver *driver, virDomainObjPtr vm,
+ const char *device, virDomainDiskDefPtr disk)
+{
+int ret = -1;
+qemuDomainObjPrivatePtr priv = vm->privateData;
+virDomainBlockJobInfo info;
+
+/* Probe the status, if needed. */
+if (!disk->mirroring) {
+qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ret = qemuMonitorBlockJob(priv->mon, device, NULL, 0, &info,
+ BLOCK_JOB_INFO);
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+if (ret < 0)
+goto cleanup;
+if (!virDomainObjIsActive(vm)) {
+qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
+_("domain is not running"));
+goto cleanup;
+}
+if (ret == 1 && info.cur == info.end &&
+info.type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY)
+disk->mirroring = true;
+}
+
+if (!disk->mirroring) {
+qemuReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
+_("disk '%s' not ready for pivot yet"),
+disk->dst);
+goto cleanup;
+}
+
+/* Attempt the pivot. */
+qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ret = qemuMonitorDriveReopen(priv->mon, device, disk->mirror,
+ disk->mirrorFormat);
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+/* XXX Until qemu adds support for 'drive-reopen' inside
+ * 'transaction', we have the remote risk of a catastrophic
+ * failure, where the drive-reopen fails but can't recover by
+ * reopening the source. Not much we can do about it. */
+
+if (ret == 0) {
+/* XXX We want to revoke security labels and disk lease, as
+ * well as audit that revocation, before dropping the original
+ * source. But it gets tricky if both source and mirror share
+ * common backing files (we want to only revoke the non-shared
+ * portion of the chain, and is made more difficult by the
+ * fact that we aren't tracking the full chain ourselves; so
+ * for
