Re: [PATCH v5 3/4] blockcommit: track job type in xml

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

 



On 07/29/14 06:20, Eric Blake wrote:
> A future patch is going to wire up qemu active block commit jobs;
> but as they have similar events and are canceled/pivoted in the
> same way as block copy jobs, it is easiest to track all bookkeeping
> for the commit job by reusing the <mirror> element.  This patch
> adds domain XML to track which job was responsible for creating a
> mirroring situation, and adds a job='copy' attribute to all
> existing uses of <mirror>.  Along the way, it also massages the
> qemu monitor backend to read the new field in order to generate
> the correct type of libvirt job (even though it requires a
> future patch to actually cause a qemu event that can be reported
> as an active commit).  It also prepares to update persistent XML
> to match changes made to live XML when a copy completes.
> 
> * docs/schemas/domaincommon.rng: Enhance schema.
> * docs/formatdomain.html.in: Document it.
> * src/conf/domain_conf.h (_virDomainDiskDef): Add a field.
> * src/conf/domain_conf.c (virDomainBlockJobType): String conversion.
> (virDomainDiskDefParseXML): Parse job type.
> (virDomainDiskDefFormat): Output job type.
> * src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Distinguish
> active from regular commit.
> * src/qemu/qemu_driver.c (qemuDomainBlockCopy): Set job type.
> (qemuDomainBlockPivot, qemuDomainBlockJobImpl): Clean up job type
> on completion.
> * tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml:
> Update tests.
> * tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Likewise.
> * tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml: New
> file.
> * tests/qemuxml2xmltest.c (mymain): Drive new test.
> 
> Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
>  docs/formatdomain.html.in                          | 15 +++++----
>  docs/schemas/domaincommon.rng                      | 13 ++++++++
>  src/conf/domain_conf.c                             | 33 ++++++++++++++++++-
>  src/conf/domain_conf.h                             |  1 +
>  src/qemu/qemu_driver.c                             |  2 ++
>  src/qemu/qemu_process.c                            | 37 +++++++++++++++++++++-
>  .../qemuxml2argv-disk-active-commit.xml            | 37 ++++++++++++++++++++++
>  .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml  |  6 ++--
>  .../qemuxml2xmlout-disk-mirror-old.xml             |  4 +--
>  tests/qemuxml2xmltest.c                            |  1 +
>  10 files changed, 136 insertions(+), 13 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml
> 

> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index e770a77..4ea5493 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -1017,6 +1017,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>      const char *path;
>      virDomainDiskDefPtr disk;
>      virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    virDomainDiskDefPtr persistDisk = NULL;
>      bool save = false;
> 
>      virObjectLock(vm);
> @@ -1025,6 +1026,9 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>      if (disk) {
>          /* Have to generate two variants of the event for old vs. new
>           * client callbacks */
> +        if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT &&
> +            disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)
> +            type = disk->mirrorJob;

Hmm, looks like this would be better of in the next patch but one of the
tests probably wouldn't work without this.

>          path = virDomainDiskGetSource(disk);
>          event = virDomainEventBlockJobNewFromObj(vm, path, type, status);
>          event2 = virDomainEventBlockJob2NewFromObj(vm, disk->dst, type,
> @@ -1036,6 +1040,31 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>              bool has_mirror = !!disk->mirror;
> 
>              if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_PIVOT) {
> +                if (vm->newDef) {
> +                    int indx = virDomainDiskIndexByName(vm->newDef, disk->dst,
> +                                                        false);
> +                    virStorageSourcePtr copy = NULL;
> +
> +                    if (indx >= 0) {
> +                        persistDisk = vm->newDef->disks[indx];
> +                        copy = virStorageSourceCopy(disk->mirror, false);
> +                        if (virStorageSourceInitChainElement(copy,
> +                                                             persistDisk->src,
> +                                                             false) < 0) {
> +                            VIR_WARN("Unable to update persistent definition "
> +                                     "on vm %s after block job",
> +                                     vm->def->name);
> +                            virStorageSourceFree(copy);
> +                            copy = NULL;
> +                            persistDisk = NULL;
> +                        }
> +                    }
> +                    if (copy) {
> +                        virStorageSourceFree(persistDisk->src);
> +                        persistDisk->src = copy;
> +                    }
> +                }

Won't this allow us to enable block copy on persistent domains?


> +
>                  /* XXX We want to revoke security labels and disk
>                   * lease, as well as audit that revocation, before
>                   * dropping the original source.  But it gets tricky
> @@ -1061,7 +1090,8 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>          }
> 
>          if (disk->mirror &&
> -            type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY) {
> +            (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COPY ||
> +             type == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)) {
>              if (status == VIR_DOMAIN_BLOCK_JOB_READY) {
>                  disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_READY;
>                  save = true;
> @@ -1069,6 +1099,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>                  virStorageSourceFree(disk->mirror);
>                  disk->mirror = NULL;
>                  disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_DEFAULT;
> +                disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
>                  save = true;
>              }
>          }
> @@ -1078,6 +1109,10 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
>          if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
>              VIR_WARN("Unable to save status on vm %s after block job",
>                       vm->def->name);
> +        if (persistDisk && virDomainSaveConfig(cfg->configDir,
> +                                               vm->newDef) < 0)
> +            VIR_WARN("Unable to update persistent definition on vm %s "
> +                     "after block job", vm->def->name);
>      }
>      virObjectUnlock(vm);
>      virObjectUnref(cfg);


ACK, although this patch an the next one are a bit borderline now that
libvirt is frozen for the next release.

Peter

Attachment: signature.asc
Description: OpenPGP digital 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]