On Mon, Apr 09, 2012 at 21:52:21 -0600, Eric Blake wrote:
> Minimal patch to wire up all the pieces in the previous patches
> to actually enable a block copy job. By minimal, I mean that
> qemu creates the file (that is, no REUSE_EXT flag support yet),
> SELinux must be disabled, a lock manager is not informed, and
> the audit logs aren't updated. But those will be added as
> improvements in future patches.
>
> * src/qemu/qemu_driver.c (qemuDomainBlockCopy): New function.
> (qemuDomainBlockRebase): Call it when appropriate.
> ---
> src/qemu/qemu_driver.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 113 insertions(+), 1 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 2207184..6d5a5da 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -11877,10 +11877,121 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom, const char *path,
> }
>
> static int
> +qemuDomainBlockCopy(virDomainPtr dom, const char *path,
> + const char *dest, const char *format,
> + unsigned long bandwidth, unsigned int flags)
> +{
> + struct qemud_driver *driver = dom->conn->privateData;
> + virDomainObjPtr vm = NULL;
> + qemuDomainObjPrivatePtr priv;
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
> + char *device = NULL;
> + virDomainDiskDefPtr disk;
> + int ret = -1;
> + int idx;
> +
> + /* Preliminaries: find the disk we are editing, sanity checks */
> + virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW, -1);
> +
> + qemuDriverLock(driver);
> + virUUIDFormat(dom->uuid, uuidstr);
> + vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> + if (!vm) {
> + qemuReportError(VIR_ERR_NO_DOMAIN,
> + _("no domain with matching uuid '%s'"), uuidstr);
> + goto cleanup;
> + }
> +
> + device = qemuDiskPathToAlias(vm, path, &idx);
> + if (!device) {
> + goto cleanup;
> + }
> + disk = vm->def->disks[idx];
> + if (disk->mirror) {
> + qemuReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
> + _("disk '%s' already in active block copy job"),
> + disk->dst);
> + goto cleanup;
> + }
> +
> + priv = vm->privateData;
> + if (!(qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_MIRROR) &&
> + qemuCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE_REOPEN))) {
> + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
We usually use VIR_ERR_CONFIG_UNSUPPORTED in such cases.
> + _("block copy is not supported with this QEMU binary"));
> + goto cleanup;
> + }
> + if (vm->persistent) {
> + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("domain is not transient"));
> + goto cleanup;
> + }
I guess I wasn't paying enough attention somewhere but why do we forbid block
copy for persistent domains? I understand why we want to forbid certain
operations when block copy is active but I don't see a reason for penalizing
persistent domains.
> +
> + if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) < 0)
> + goto cleanup;
> +
> + if (!virDomainObjIsActive(vm)) {
> + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> + _("domain is not running"));
> + goto endjob;
> + }
> +
> + /* Prepare the destination file. */
> + /* XXX We also need to add security labeling, lock manager lease,
> + * and auditing of those events, as well as to support reuse of
> + * existing images, including probing the existing format of an
> + * existing image. */
> + if (!format)
> + format = disk->driverType;
> + if ((format && !(disk->mirrorFormat = strdup(format))) ||
> + !(disk->mirror = strdup(dest))) {
> + virReportOOMError();
> + goto endjob;
> + }
> +
> + /* Actually start the mirroring */
> + qemuDomainObjEnterMonitorWithDriver(driver, vm);
> + ret = qemuMonitorDriveMirror(priv->mon, NULL, device, dest, format, flags);
> + if (ret == 0 && bandwidth != 0)
> + ret = qemuMonitorBlockJob(priv->mon, device, NULL, bandwidth, NULL,
> + BLOCK_JOB_SPEED_INTERNAL);
> + qemuDomainObjExitMonitorWithDriver(driver, vm);
Can BLOCK_JOB_SPEED_INTERNAL fail while block copy remains running? If so, we
should try to abort the job in case we fail to set the speed, otherwise we
will report an error and forget about the job while qemu will keep copying
data.
> +
> +endjob:
> + if (ret < 0) {
> + VIR_FREE(disk->mirror);
> + VIR_FREE(disk->mirrorFormat);
> + }
> + if (qemuDomainObjEndJob(driver, vm) == 0) {
> + vm = NULL;
> + goto cleanup;
> + }
> +
> +cleanup:
> + VIR_FREE(device);
> + if (vm)
> + virDomainObjUnlock(vm);
> + qemuDriverUnlock(driver);
> + return ret;
> +}
> +
> +static int
> qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
> unsigned long bandwidth, unsigned int flags)
> {
> - virCheckFlags(0, -1);
> + virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
> + VIR_DOMAIN_BLOCK_REBASE_COPY |
> + VIR_DOMAIN_BLOCK_REBASE_COPY_RAW, -1);
> +
> + if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY) {
> + const char *format = NULL;
> + if (flags & VIR_DOMAIN_BLOCK_REBASE_COPY_RAW)
> + format = "raw";
> + flags &= ~(VIR_DOMAIN_BLOCK_REBASE_COPY |
> + VIR_DOMAIN_BLOCK_REBASE_COPY_RAW);
> + return qemuDomainBlockCopy(dom, path, base, format, bandwidth, flags);
> + }
> +
> return qemuDomainBlockJobImpl(dom, path, base, bandwidth, NULL,
> BLOCK_JOB_PULL, flags);
> }
> @@ -11889,6 +12000,7 @@ static int
> qemuDomainBlockPull(virDomainPtr dom, const char *path, unsigned long bandwidth,
> unsigned int flags)
> {
> + virCheckFlags(0, -1);
> return qemuDomainBlockRebase(dom, path, NULL, bandwidth, flags);
> }
Hmm, personally, I would make qemuDomainBlockPull call qemuDomainBlockJobImpl
directly instead of going through qemuDomainBlockRebase while adding the flags
check...
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list