On Fri, Feb 25, 2011 at 07:04:10PM +0100, Jiri Denemark wrote:
> ---
> src/qemu/qemu_driver.c | 125 +++++++++++++++++++++++++++++------------------
> 1 files changed, 77 insertions(+), 48 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 7fc08e8..ba046d7 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5987,6 +5987,81 @@ static int qemuDomainSnapshotIsAllowed(virDomainObjPtr vm)
> return 1;
> }
>
> +/* The domain is expected to be locked and inactive. */
> +static int
> +qemuDomainSnapshotCreateInactive(virDomainObjPtr vm,
> + virDomainSnapshotObjPtr snap)
> +{
> + const char *qemuimgarg[] = { NULL, "snapshot", "-c", NULL, NULL, NULL };
> + int ret = -1;
> + int i;
> +
> + qemuimgarg[0] = qemuFindQemuImgBinary();
> + if (qemuimgarg[0] == NULL) {
> + /* qemuFindQemuImgBinary set the error */
> + goto cleanup;
> + }
> +
> + qemuimgarg[3] = snap->def->name;
> +
> + for (i = 0; i < vm->def->ndisks; i++) {
> + /* FIXME: we also need to handle LVM here */
> + /* FIXME: if we fail halfway through this loop, we are in an
> + * inconsistent state. I'm not quite sure what to do about that
> + */
> + if (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
> + if (!vm->def->disks[i]->driverType ||
> + STRNEQ(vm->def->disks[i]->driverType, "qcow2")) {
> + qemuReportError(VIR_ERR_OPERATION_INVALID,
> + _("Disk device '%s' does not support"
> + " snapshotting"),
> + vm->def->disks[i]->info.alias);
> + goto cleanup;
> + }
> +
> + qemuimgarg[4] = vm->def->disks[i]->src;
> +
> + if (virRun(qemuimgarg, NULL) < 0) {
> + virReportSystemError(errno,
> + _("Failed to run '%s' to create snapshot"
> + " '%s' from disk '%s'"),
> + qemuimgarg[0], snap->def->name,
> + vm->def->disks[i]->src);
> + goto cleanup;
> + }
> + }
> + }
> +
> + ret = 0;
> +
> +cleanup:
> + VIR_FREE(qemuimgarg[0]);
> + return ret;
> +}
> +
> +/* The domain is expected to be locked and active. */
> +static int
> +qemuDomainSnapshotCreateActive(struct qemud_driver *driver,
> + virDomainObjPtr *vmptr,
> + virDomainSnapshotObjPtr snap)
> +{
> + virDomainObjPtr vm = *vmptr;
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + int ret;
> +
> + if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
> + return -1;
> +
> + qemuDomainObjEnterMonitorWithDriver(driver, vm);
> + ret = qemuMonitorCreateSnapshot(priv->mon, snap->def->name);
> + qemuDomainObjExitMonitorWithDriver(driver, vm);
> +
> + if (qemuDomainObjEndJob(vm) == 0)
> + *vmptr = NULL;
> +
> + return ret;
> +}
> +
> static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
> const char *xmlDesc,
> unsigned int flags)
> @@ -5997,8 +6072,6 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
> virDomainSnapshotPtr snapshot = NULL;
> char uuidstr[VIR_UUID_STRING_BUFLEN];
> virDomainSnapshotDefPtr def;
> - const char *qemuimgarg[] = { NULL, "snapshot", "-c", NULL, NULL, NULL };
> - int i;
>
> virCheckFlags(0, NULL);
>
> @@ -6029,54 +6102,11 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
>
> /* actually do the snapshot */
> if (!virDomainObjIsActive(vm)) {
> - qemuimgarg[0] = qemuFindQemuImgBinary();
> - if (qemuimgarg[0] == NULL)
> - /* qemuFindQemuImgBinary set the error */
> + if (qemuDomainSnapshotCreateInactive(vm, snap) < 0)
> goto cleanup;
> -
> - qemuimgarg[3] = snap->def->name;
> -
> - for (i = 0; i < vm->def->ndisks; i++) {
> - /* FIXME: we also need to handle LVM here */
> - /* FIXME: if we fail halfway through this loop, we are in an
> - * inconsistent state. I'm not quite sure what to do about that
> - */
> - if (vm->def->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK) {
> - if (!vm->def->disks[i]->driverType ||
> - STRNEQ(vm->def->disks[i]->driverType, "qcow2")) {
> - qemuReportError(VIR_ERR_OPERATION_INVALID,
> - _("Disk device '%s' does not support snapshotting"),
> - vm->def->disks[i]->info.alias);
> - goto cleanup;
> - }
> -
> - qemuimgarg[4] = vm->def->disks[i]->src;
> -
> - if (virRun(qemuimgarg, NULL) < 0) {
> - virReportSystemError(errno,
> - _("Failed to run '%s' to create snapshot '%s' from disk '%s'"),
> - qemuimgarg[0], snap->def->name,
> - vm->def->disks[i]->src);
> - goto cleanup;
> - }
> - }
> - }
> }
> else {
> - qemuDomainObjPrivatePtr priv;
> - int ret;
> -
> - if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
> - goto cleanup;
> - priv = vm->privateData;
> - qemuDomainObjEnterMonitorWithDriver(driver, vm);
> - ret = qemuMonitorCreateSnapshot(priv->mon, def->name);
> - qemuDomainObjExitMonitorWithDriver(driver, vm);
> - if (qemuDomainObjEndJob(vm) == 0) {
> - vm = NULL;
> - goto cleanup;
> - }
> - if (ret < 0)
> + if (qemuDomainSnapshotCreateActive(driver, &vm, snap) < 0)
> goto cleanup;
> }
>
> @@ -6106,7 +6136,6 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
> snapshot = virGetDomainSnapshot(domain, snap->def->name);
>
> cleanup:
> - VIR_FREE(qemuimgarg[0]);
> if (vm)
> virDomainObjUnlock(vm);
> qemuDriverUnlock(driver);
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list