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

Re: [PATCH v3 5/6] qemu: Introduce to handle transient disk



On Thu, Sep 17, 2020 at 09:30:44 -0400, Masayoshi Mizuma wrote:
> From: Masayoshi Mizuma <m mizuma jp fujitsu com>
> 
> Here is the implementation of transient option for qcow2 and raw format
> disk. This gets available <transient/> directive in domain xml file
> like as:
> 
>     <disk type='file' device='disk'>
>       <driver name='qemu' type='qcow2'/>
>       <source file='/var/lib/libvirt/images/guest.qcow2'/>
>       <target dev='vda' bus='virtio'/>
>       <transient/>
>     </disk>
> 
> When the qemu command line options are built, a new qcow2 image is
> created with backing qcow2 by using blockdev-snapshot command.
> The backing image is the qcow2 file which is set as <source>.
> The filename of the new qcow2 image is original-source-file.TRANSIENT.
> 
> Signed-off-by: Masayoshi Mizuma <m mizuma jp fujitsu com>
> ---
>  src/qemu/qemu_snapshot.c  | 103 +++++++++++++++++++++++++++++++++++---
>  src/qemu/qemu_snapshot.h  |   5 ++
>  src/util/virstoragefile.c |   2 +
>  src/util/virstoragefile.h |   4 ++
>  4 files changed, 107 insertions(+), 7 deletions(-)
> 
> diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
> index 1e8ea80b22..67fdc488e0 100644
> --- a/src/qemu/qemu_snapshot.c
> +++ b/src/qemu/qemu_snapshot.c
> @@ -1158,7 +1158,8 @@ qemuSnapshotCreateDiskActive(virQEMUDriverPtr driver,
>                               virHashTablePtr blockNamedNodeData,
>                               unsigned int flags,
>                               virQEMUDriverConfigPtr cfg,
> -                             qemuDomainAsyncJob asyncJob)
> +                             qemuDomainAsyncJob asyncJob,
> +                             bool domSave)
>  {
>      qemuDomainObjPrivatePtr priv = vm->privateData;
>      g_autoptr(virJSONValue) actions = NULL;
> @@ -1201,17 +1202,26 @@ qemuSnapshotCreateDiskActive(virQEMUDriverPtr driver,
>  
>          virDomainAuditDisk(vm, dd->disk->src, dd->src, "snapshot", rc >= 0);
>  
> -        if (rc == 0)
> +        if (rc == 0) {
>              qemuSnapshotDiskUpdateSource(driver, vm, dd, blockdev);
> +
> +            if (dd->disk->transient) {
> +                /* Mark the transient working is completed to make sure we can */
> +                /* remove the transient disk when the guest is shutdown. */
> +                dd->disk->src->transientEstablished = true;
> +            }
> +        }
>      }
>  
>      if (rc < 0)
>          goto cleanup;
>  
> -    if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0 ||
> -        (vm->newDef && virDomainDefSave(vm->newDef, driver->xmlopt,
> -                                        cfg->configDir) < 0))
> -        goto cleanup;
> +    if (domSave) {
> +        if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0 ||
> +            (vm->newDef && virDomainDefSave(vm->newDef, driver->xmlopt,
> +                                            cfg->configDir) < 0))
> +            goto cleanup;
> +    }
>  
>      ret = 0;
>  
> @@ -1349,7 +1359,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver,
>  
>      if ((ret = qemuSnapshotCreateDiskActive(driver, vm, snap,
>                                              blockNamedNodeData, flags, cfg,
> -                                            QEMU_ASYNC_JOB_SNAPSHOT)) < 0)
> +                                            QEMU_ASYNC_JOB_SNAPSHOT, true)) < 0)
>          goto cleanup;
>  
>      /* the snapshot is complete now */
> @@ -2264,3 +2274,82 @@ qemuSnapshotDelete(virDomainObjPtr vm,
>   cleanup:
>      return ret;
>  }
> +
> +
> +static int
> +qemuSnapshotSetupTransientDisk(virDomainSnapshotDiskDefPtr def,
> +                               virStorageSourcePtr src)
> +{
> +    g_autoptr(virStorageSource) trans = NULL;
> +
> +    if (!(trans = virStorageSourceNew()))
> +        return -1;
> +
> +    trans->path = g_strdup_printf("%s.TRANSIENT", src->path);
> +    if (virFileExists(trans->path)) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("Transient disk '%s' for '%s' exists"),
> +                       trans->path, src->path);
> +        return -1;
> +    }
> +
> +    trans->type = VIR_STORAGE_TYPE_FILE;
> +    trans->format = VIR_STORAGE_FILE_QCOW2;
> +
> +    def->src = g_steal_pointer(&trans);
> +
> +    return 0;
> +}
> +
> +
> +int
> +qemuSnapshotCreateTransientDisk(virQEMUDriverPtr driver,
> +                                virDomainObjPtr vm,
> +                                int asyncJob)
> +{
> +    int rc;
> +    size_t i;
> +    virDomainMomentObjPtr snap = NULL;
> +    g_autoptr(virDomainSnapshotDef) snapdef = NULL;
> +    g_autoptr(virHashTable) blockNamedNodeData = NULL;
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(priv->driver);
> +
> +    if (!(snapdef = virDomainSnapshotDefNew()))
> +        return -1;

This shouldn't be necessary if you factor out stuff out of
qemuSnapshotCreateDiskActive. I'll send a prerequisite patch doing the
refactor as I've requested last time.

> +
> +    snapdef->parent.name = g_strdup_printf("transient");
> +
> +    snapdef->ndisks = vm->def->ndisks;
> +    if (VIR_ALLOC_N(snapdef->disks, snapdef->ndisks) < 0)
> +        return -1;
> +
> +    for (i = 0; i < vm->def->ndisks; i++) {
> +        virDomainDiskDefPtr disk = vm->def->disks[i];
> +        virDomainSnapshotDiskDefPtr snapdisk = &(snapdef->disks[i]);
> +
> +        if (disk->transient) {
> +            if ((rc = qemuSnapshotSetupTransientDisk(snapdisk, disk->src)) < 0)
> +                return -1;
> +
> +        } else {
> +            snapdisk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE;
> +        }
> +    }
> +
> +    if (!(snap = virDomainSnapshotAssignDef(vm->snapshots, snapdef)))
> +        return -1;
> +
> +    if (!(blockNamedNodeData = qemuBlockGetNamedNodeData(vm, asyncJob)))
> +        goto cleanup;
> +
> +    /* The last argument domSave is false here because transient disk config */
> +    /* is volatile so we don't need to save it. */
> +    rc = qemuSnapshotCreateDiskActive(driver, vm, snap, blockNamedNodeData,
> +                                      0, cfg, asyncJob, false);
> +
> + cleanup:
> +    virDomainSnapshotObjListRemove(vm->snapshots, snap);



> +
> +    return rc;
> +}
> diff --git a/src/qemu/qemu_snapshot.h b/src/qemu/qemu_snapshot.h
> index 8b3ebe87b1..d57ef4b8a4 100644
> --- a/src/qemu/qemu_snapshot.h
> +++ b/src/qemu/qemu_snapshot.h
> @@ -53,3 +53,8 @@ int
>  qemuSnapshotDelete(virDomainObjPtr vm,
>                     virDomainSnapshotPtr snapshot,
>                     unsigned int flags);
> +
> +int
> +qemuSnapshotCreateTransientDisk(virQEMUDriverPtr driver,
> +                                virDomainObjPtr vm,
> +                                int asyncJob);
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index 97a346db28..636f27ef09 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -2456,6 +2456,8 @@ virStorageSourceCopy(const virStorageSource *src,
>      def->ssh_host_key_check_disabled = src->ssh_host_key_check_disabled;
>      def->ssh_user = g_strdup(src->ssh_user);
>  
> +    def->transientEstablished = src->transientEstablished;
> +
>      return g_steal_pointer(&def);
>  }
>  
> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
> index 87763cf389..0dc275c11e 100644
> --- a/src/util/virstoragefile.h
> +++ b/src/util/virstoragefile.h
> @@ -384,6 +384,10 @@ struct _virStorageSource {
>      /* these must not be used apart from formatting the output JSON in the qemu driver */
>      char *ssh_user;
>      bool ssh_host_key_check_disabled;
> +
> +    /* set to true if the storage source is setup as a transient disk. */
> +    /* The changes to the disk are dropped after the guest machine is shutdown. */
> +    bool transientEstablished;

This is not written into the status XML, so after libvirtd restart you
lose the value. That means that the transient file will not be removed
after VM shutdown if libvirtd was restarted.

IMO it's not even necessary to store this, I'll see whether I can remove
it.


>  };
>  
>  G_DEFINE_AUTOPTR_CLEANUP_FUNC(virStorageSource, virObjectUnref);
> -- 
> 2.27.0
> 


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