Re: [PATCH 11/13] backup: qemu: Implement metadata tracking for checkpoint APIs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 18, 2019 at 22:47:52 -0500, Eric Blake wrote:
> A lot of this work heavily copies from the existing snapshot
> APIs.  The interaction with qemu during create/delete still
> needs to be implemented, but this takes care of all the libvirt
> metadata (saving and restoring XML, and tracking the relations
> between multiple checkpoints).
> 
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
>  src/qemu/qemu_block.h  |   3 +
>  src/qemu/qemu_conf.h   |   2 +
>  src/qemu/qemu_domain.h |  15 +
>  src/qemu/qemu_block.c  |  12 +
>  src/qemu/qemu_conf.c   |   5 +
>  src/qemu/qemu_domain.c | 134 ++++++++
>  src/qemu/qemu_driver.c | 730 +++++++++++++++++++++++++++++++++++++++++
>  7 files changed, 901 insertions(+)

See https://www.redhat.com/archives/libvir-list/2019-April/msg01422.html
for feedback. Most of it was not addressed yet.

> @@ -7753,6 +7922,7 @@ qemuDomainUndefineFlags(virDomainPtr dom,
>          if (qemuDomainSnapshotDiscardAllMetadata(driver, vm) < 0)
>              goto endjob;
>      }
> +    /* TODO: Restrict deletion if checkpoints exist? */

Should not be too hard to resolve the todo right away? [1]

> 
>      name = qemuDomainManagedSavePath(driver, vm);
>      if (name == NULL)


[...]

> +static virDomainCheckpointPtr
> +qemuDomainCheckpointCreateXML(virDomainPtr domain,
> +                              const char *xmlDesc,
> +                              unsigned int flags)
> +{
> +    virQEMUDriverPtr driver = domain->conn->privateData;
> +    virDomainObjPtr vm = NULL;
> +    char *xml = NULL;
> +    virDomainMomentObjPtr chk = NULL;
> +    virDomainCheckpointPtr checkpoint = NULL;
> +    virDomainMomentObjPtr current = NULL;
> +    bool update_current = true;
> +    bool redefine = flags & VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE;
> +    unsigned int parse_flags = 0;
> +    virDomainMomentObjPtr other = NULL;
> +    virQEMUDriverConfigPtr cfg = NULL;
> +    virCapsPtr caps = NULL;
> +    VIR_AUTOUNREF(virDomainCheckpointDefPtr) def = NULL;
> +
> +    virCheckFlags(VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE |
> +                  VIR_DOMAIN_CHECKPOINT_CREATE_CURRENT |
> +                  VIR_DOMAIN_CHECKPOINT_CREATE_NO_METADATA, NULL);

As I've said earlier in this series I think we should avoid implementing
_NO_METADATA to encourage use of our tracking.

> +    /* TODO: VIR_DOMAIN_CHECKPOINT_CREATE_QUIESCE */
> +
> +    if (redefine)
> +        parse_flags |= VIR_DOMAIN_CHECKPOINT_PARSE_REDEFINE;
> +    if ((redefine && !(flags & VIR_DOMAIN_CHECKPOINT_CREATE_CURRENT)) ||
> +        (flags & VIR_DOMAIN_CHECKPOINT_CREATE_NO_METADATA))
> +        update_current = false;

[...]


> @@ -16866,6 +17570,16 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn,
>          goto cleanup;
>      }
> 
> +    if (qemuProcessAttach(conn, driver, vm, pid,
> +                          pidfile, monConfig, monJSON) < 0) {
> +        monConfig = NULL;
> +        qemuDomainRemoveInactive(driver, vm);
> +        qemuDomainObjEndJob(driver, vm);
> +        goto cleanup;
> +    }
> +
> +    monConfig = NULL;


Note that this will cause conflict after me and Jano push patches which
get rid of all of this. Also It does not make sense here anyways.

Also it's probably a rebase conflict anyways.

> +
>      if (qemuProcessAttach(conn, driver, vm, pid,
>                            pidfile, monConfig, monJSON) < 0) {
>          qemuDomainRemoveInactive(driver, vm);
> @@ -21712,6 +22426,12 @@ static int qemuDomainRename(virDomainPtr dom,
>          goto endjob;
>      }
> 
> +    if (virDomainListCheckpoints(vm->checkpoints, NULL, dom, NULL, flags) > 0) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("cannot rename domain with checkpoints"));
> +        goto endjob;
> +    }

[2]

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux