[libvirt] [PATCH 04/20] snapshot: merge pre-snapshot checks

Osier Yang jyang at redhat.com
Wed Oct 24 05:18:34 UTC 2012


On 2012年10月23日 23:12, Peter Krempa wrote:
> From: Eric Blake<eblake at redhat.com>
>
> Both system checkpoint snapshots and disk snapshots were iterating
> over all disks, doing a final sanity check before doing any work.
> But since future patches will allow offline snapshots to be either
> external or internal, it makes sense to share the pass over all
> disks, and then relax restrictions in that pass as new modes are
> implemented.  Future patches can then handle external disks when
> the domain is offline, then handle offline --disk-snapshot, and
> finally, combine with migration to file to gain a complete external
> system checkpoint snapshot of an active domain without using 'savevm'.
>
> * src/qemu/qemu_driver.c (qemuDomainSnapshotDiskPrepare)
> (qemuDomainSnapshotIsAllowed): Merge...
> (qemuDomainSnapshotPrepare): ...into one function.
> (qemuDomainSnapshotCreateXML): Update caller.
> ---
>   src/qemu/qemu_driver.c | 92 +++++++++++++++++++-------------------------------
>   1 file changed, 34 insertions(+), 58 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index cbabd62..df02783 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -10516,34 +10516,6 @@ cleanup:
>   }
>
>
> -static bool
> -qemuDomainSnapshotIsAllowed(virDomainObjPtr vm)
> -{
> -    int i;
> -
> -    /* FIXME: we need to figure out what else here might succeed; in
> -     * particular, if it's a raw device but on LVM, we could probably make
> -     * that succeed as well
> -     */
> -    for (i = 0; i<  vm->def->ndisks; i++) {
> -        virDomainDiskDefPtr disk = vm->def->disks[i];
> -        if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK&&
> -            (disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG ||
> -             disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD))
> -            continue;
> -
> -        if ((disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) ||
> -            (disk->device == VIR_DOMAIN_DISK_DEVICE_DISK&&
> -             disk->format>  0&&  disk->format != VIR_STORAGE_FILE_QCOW2)) {
> -            virReportError(VIR_ERR_OPERATION_INVALID,
> -                           _("Disk '%s' does not support snapshotting"),
> -                           disk->src);
> -            return false;
> -        }
> -    }
> -
> -    return true;
> -}
>
>   /* this function expects the driver lock to be held by the caller */
>   static int
> @@ -10697,8 +10669,8 @@ endjob:
>   }
>
>   static int
> -qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def,
> -                              unsigned int *flags)
> +qemuDomainSnapshotPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def,
> +                          unsigned int *flags)
>   {
>       int ret = -1;
>       int i;
> @@ -10710,7 +10682,8 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def,
>       int external = 0;
>       qemuDomainObjPrivatePtr priv = vm->privateData;
>
> -    if (reuse&&  !qemuCapsGet(priv->caps, QEMU_CAPS_TRANSACTION)) {
> +    if (def->state == VIR_DOMAIN_DISK_SNAPSHOT&&
> +        reuse&&  !qemuCapsGet(priv->caps, QEMU_CAPS_TRANSACTION)) {
>           virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                          _("reuse is not supported with this QEMU binary"));
>           goto cleanup;
> @@ -10718,15 +10691,16 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def,
>
>       for (i = 0; i<  def->ndisks; i++) {
>           virDomainSnapshotDiskDefPtr disk =&def->disks[i];
> +        virDomainDiskDefPtr dom_disk = vm->def->disks[i];
>
>           switch (disk->snapshot) {
>           case VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL:
> -            if (active) {
> -                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                               _("active qemu domains require external disk "
> -                                 "snapshots; disk %s requested internal"),
> -                               disk->name);
> -                goto cleanup;
> +            if (def->state != VIR_DOMAIN_DISK_SNAPSHOT&&
> +                dom_disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK&&
> +                (dom_disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG ||
> +                 dom_disk->protocol == VIR_DOMAIN_DISK_PROTOCOL_RBD)) {
> +                found = true;
> +                break;
>               }
>               if (vm->def->disks[i]->format>  0&&
>                   vm->def->disks[i]->format != VIR_STORAGE_FILE_QCOW2) {
> @@ -10738,10 +10712,25 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def,
>                                      vm->def->disks[i]->format));
>                   goto cleanup;
>               }
> +            if (def->state == VIR_DOMAIN_DISK_SNAPSHOT&&  active) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                               _("active qemu domains require external disk "
> +                                 "snapshots; disk %s requested internal"),
> +                               disk->name);
> +                goto cleanup;
> +            }
>               found = true;
>               break;
>
>           case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL:
> +            if (def->state != VIR_DOMAIN_DISK_SNAPSHOT) {
> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                               _("system checkpoint external snapshot for "
> +                                 "disk %s not implemented yet"),
> +                               disk->name);
> +                goto cleanup;
> +            }
> +
>               if (!disk->format) {
>                   disk->format = VIR_STORAGE_FILE_QCOW2;
>               } else if (disk->format != VIR_STORAGE_FILE_QCOW2&&
> @@ -10789,11 +10778,11 @@ qemuDomainSnapshotDiskPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def,
>
>       if (!found) {
>           virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("disk snapshots require at least one disk to be "
> +                       _("snapshots require at least one disk to be "
>                            "selected for snapshot"));
>           goto cleanup;
>       }
> -    if (active) {
> +    if (def->state != VIR_DOMAIN_DISK_SNAPSHOT&&  active) {
>           if (external == 1 ||
>               qemuCapsGet(priv->caps, QEMU_CAPS_TRANSACTION)) {
>               *flags |= VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC;
> @@ -11004,7 +10993,7 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
>       /* For multiple disks, libvirt must pause externally to get all
>        * snapshots to be at the same point in time, unless qemu supports
>        * transactions.  For a single disk, snapshot is atomic without
> -     * requiring a pause.  Thanks to qemuDomainSnapshotDiskPrepare, if
> +     * requiring a pause.  Thanks to qemuDomainSnapshotPrepare, if
>        * we got to this point, the atomic flag now says whether we need
>        * to pause, and a capability bit says whether to use transaction.
>        */
> @@ -11030,7 +11019,7 @@ qemuDomainSnapshotCreateDiskActive(virConnectPtr conn,
>
>       /* No way to roll back if first disk succeeds but later disks
>        * fail, unless we have transaction support.
> -     * Based on earlier qemuDomainSnapshotDiskPrepare, all
> +     * Based on earlier qemuDomainSnapshotPrepare, all
>        * disks in this list are now either SNAPSHOT_NO, or
>        * SNAPSHOT_EXTERNAL with a valid file name and qcow2 format.  */
>       qemuDomainObjEnterMonitorWithDriver(driver, vm);
> @@ -11334,31 +11323,18 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
>               }
>               align_location = VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
>               align_match = false;
> -            if (virDomainSnapshotAlignDisks(def, align_location,
> -                                            align_match)<  0 ||
> -                qemuDomainSnapshotDiskPrepare(vm, def,&flags)<  0)
> -                goto cleanup;
>               def->state = VIR_DOMAIN_DISK_SNAPSHOT;
>               def->memory = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
>           } else {
> -            /* In a perfect world, we would allow qemu to tell us this.
> -             * The problem is that qemu only does this check
> -             * device-by-device; so if you had a domain that booted from a
> -             * large qcow2 device, but had a secondary raw device
> -             * attached, you wouldn't find out that you can't snapshot
> -             * your guest until *after* it had spent the time to snapshot
> -             * the boot device.  This is probably a bug in qemu, but we'll
> -             * work around it here for now.
> -             */
> -            if (!qemuDomainSnapshotIsAllowed(vm) ||
> -                virDomainSnapshotAlignDisks(def, align_location,
> -                                            align_match)<  0)
> -                goto cleanup;
>               def->state = virDomainObjGetState(vm, NULL);
>               def->memory = (def->state == VIR_DOMAIN_SHUTOFF ?
>                              VIR_DOMAIN_SNAPSHOT_LOCATION_NONE :
>                              VIR_DOMAIN_SNAPSHOT_LOCATION_INTERNAL);
>           }
> +        if (virDomainSnapshotAlignDisks(def, align_location,
> +                                        align_match)<  0 ||
> +            qemuDomainSnapshotPrepare(vm, def,&flags)<  0)
> +            goto cleanup;
>       }
>
>       if (snap)

ACK




More information about the libvir-list mailing list