Rename qemuDomainObjSaveJob and create a wrapper for it which does not require 'driver' to be passed and export it so that other palces can easily save the status XML without having to invoke virDomainSaveStatus which has unpleasing parameters. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 23 ++++++++++++++++------- src/qemu/qemu_domain.h | 2 ++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0f1fda2384..2c0096d2ac 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7198,7 +7198,8 @@ virDomainDefParserConfig virQEMUDriverDomainDefParserConfig = { static void -qemuDomainObjSaveJob(virQEMUDriverPtr driver, virDomainObjPtr obj) +qemuDomainObjSaveStatus(virQEMUDriverPtr driver, + virDomainObjPtr obj) { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); @@ -7210,6 +7211,14 @@ qemuDomainObjSaveJob(virQEMUDriverPtr driver, virDomainObjPtr obj) virObjectUnref(cfg); } + +void +qemuDomainSaveStatus(virDomainObjPtr obj) +{ + qemuDomainObjSaveStatus(QEMU_DOMAIN_PRIVATE(obj)->driver, obj); +} + + void qemuDomainObjSetJobPhase(virQEMUDriverPtr driver, virDomainObjPtr obj, @@ -7233,7 +7242,7 @@ qemuDomainObjSetJobPhase(virQEMUDriverPtr driver, priv->job.phase = phase; priv->job.asyncOwner = me; - qemuDomainObjSaveJob(driver, obj); + qemuDomainObjSaveStatus(driver, obj); } void @@ -7256,7 +7265,7 @@ qemuDomainObjDiscardAsyncJob(virQEMUDriverPtr driver, virDomainObjPtr obj) if (priv->job.active == QEMU_JOB_ASYNC_NESTED) qemuDomainObjResetJob(priv); qemuDomainObjResetAsyncJob(priv); - qemuDomainObjSaveJob(driver, obj); + qemuDomainObjSaveStatus(driver, obj); } void @@ -7439,7 +7448,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, } if (qemuDomainTrackJob(job)) - qemuDomainObjSaveJob(driver, obj); + qemuDomainObjSaveStatus(driver, obj); virObjectUnref(cfg); return 0; @@ -7683,7 +7692,7 @@ qemuDomainObjEndJob(virQEMUDriverPtr driver, virDomainObjPtr obj) qemuDomainObjResetJob(priv); if (qemuDomainTrackJob(job)) - qemuDomainObjSaveJob(driver, obj); + qemuDomainObjSaveStatus(driver, obj); /* We indeed need to wake up ALL threads waiting because * grabbing a job requires checking more variables. */ virCondBroadcast(&priv->job.cond); @@ -7727,7 +7736,7 @@ qemuDomainObjEndJobWithAgent(virQEMUDriverPtr driver, qemuDomainObjResetJob(priv); qemuDomainObjResetAgentJob(priv); if (qemuDomainTrackJob(job)) - qemuDomainObjSaveJob(driver, obj); + qemuDomainObjSaveStatus(driver, obj); /* We indeed need to wake up ALL threads waiting because * grabbing a job requires checking more variables. */ virCondBroadcast(&priv->job.cond); @@ -7745,7 +7754,7 @@ qemuDomainObjEndAsyncJob(virQEMUDriverPtr driver, virDomainObjPtr obj) obj, obj->def->name); qemuDomainObjResetAsyncJob(priv); - qemuDomainObjSaveJob(driver, obj); + qemuDomainObjSaveStatus(driver, obj); virCondBroadcast(&priv->job.asyncCond); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 3eea8b0f96..5853852880 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -206,6 +206,8 @@ typedef void (*qemuDomainCleanupCallback)(virQEMUDriverPtr driver, #define QEMU_DOMAIN_MASTER_KEY_LEN 32 /* 32 bytes for 256 bit random key */ +void qemuDomainSaveStatus(virDomainObjPtr obj); + /* helper data types for async device unplug */ typedef enum { -- 2.21.0

Similarly to qemuDomainSaveStatus add a helper to save the config XML named qemuDomainSaveConfig. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 22 ++++++++++++++++++++++ src/qemu/qemu_domain.h | 1 + 2 files changed, 23 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2c0096d2ac..073c9744d3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7219,6 +7219,28 @@ qemuDomainSaveStatus(virDomainObjPtr obj) } +void +qemuDomainSaveConfig(virDomainObjPtr obj) +{ + virQEMUDriverPtr driver = QEMU_DOMAIN_PRIVATE(obj)->driver; + VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; + virDomainDefPtr def = NULL; + + if (virDomainObjIsActive(obj)) + def = obj->newDef; + else + def = obj->def; + + if (!def) + return; + + cfg = virQEMUDriverGetConfig(driver); + + if (virDomainSaveConfig(cfg->configDir, driver->caps, def) < 0) + VIR_WARN("Failed to save config of vm %s", obj->def->name); +} + + void qemuDomainObjSetJobPhase(virQEMUDriverPtr driver, virDomainObjPtr obj, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 5853852880..12882d5b91 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -207,6 +207,7 @@ typedef void (*qemuDomainCleanupCallback)(virQEMUDriverPtr driver, #define QEMU_DOMAIN_MASTER_KEY_LEN 32 /* 32 bytes for 256 bit random key */ void qemuDomainSaveStatus(virDomainObjPtr obj); +void qemuDomainSaveConfig(virDomainObjPtr obj); /* helper data types for async device unplug */ -- 2.21.0

Simplify error paths. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_blockjob.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index b3bdbeb990..b45103f2f3 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -73,8 +73,7 @@ static qemuBlockJobDataPtr qemuBlockJobDataNew(qemuBlockJobType type, const char *name) { - qemuBlockJobDataPtr job = NULL; - qemuBlockJobDataPtr ret = NULL; + VIR_AUTOUNREF(qemuBlockJobDataPtr) job = NULL; if (qemuBlockJobDataInitialize() < 0) return NULL; @@ -83,17 +82,13 @@ qemuBlockJobDataNew(qemuBlockJobType type, return NULL; if (VIR_STRDUP(job->name, name) < 0) - goto cleanup; + return NULL; job->state = QEMU_BLOCKJOB_STATE_NEW; job->newstate = -1; job->type = type; - VIR_STEAL_PTR(ret, job); - - cleanup: - virObjectUnref(job); - return ret; + VIR_RETURN_PTR(job); } -- 2.21.0

Rename and move qemuBlockJobTerminate to qemuBlockJobUnregister and separate bits from qemuBlockJobDiskNew which register the job with the disk. This creates an unified interface for other APIs to use. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_blockjob.c | 62 ++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index b45103f2f3..c102417e43 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -92,6 +92,37 @@ qemuBlockJobDataNew(qemuBlockJobType type, } +static int +qemuBlockJobRegister(qemuBlockJobDataPtr job, + virDomainDiskDefPtr disk) +{ + if (disk) { + job->disk = disk; + QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob = virObjectRef(job); + } + + return 0; +} + + +static void +qemuBlockJobUnregister(qemuBlockJobDataPtr job) +{ + qemuDomainDiskPrivatePtr diskPriv; + + if (job->disk) { + diskPriv = QEMU_DOMAIN_DISK_PRIVATE(job->disk); + + if (job == diskPriv->blockjob) { + virObjectUnref(diskPriv->blockjob); + diskPriv->blockjob = NULL; + } + + job->disk = NULL; + } +} + + /** * qemuBlockJobDiskNew: * @disk: disk definition @@ -105,16 +136,15 @@ qemuBlockJobDiskNew(virDomainDiskDefPtr disk, qemuBlockJobType type, const char *jobname) { - qemuBlockJobDataPtr job = NULL; + VIR_AUTOUNREF(qemuBlockJobDataPtr) job = NULL; if (!(job = qemuBlockJobDataNew(type, jobname))) return NULL; - job->disk = disk; - if (disk) - QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob = virObjectRef(job); + if (qemuBlockJobRegister(job, disk) < 0) + return NULL; - return job; + VIR_RETURN_PTR(job); } @@ -150,22 +180,6 @@ qemuBlockJobStarted(qemuBlockJobDataPtr job) } -static void -qemuBlockJobTerminate(qemuBlockJobDataPtr job) -{ - qemuDomainDiskPrivatePtr diskPriv; - - if (job->disk) { - diskPriv = QEMU_DOMAIN_DISK_PRIVATE(job->disk); - - if (job == diskPriv->blockjob) { - virObjectUnref(diskPriv->blockjob); - diskPriv->blockjob = NULL; - } - } -} - - /** * qemuBlockJobStartupFinalize: * @job: job being started @@ -181,7 +195,7 @@ qemuBlockJobStartupFinalize(qemuBlockJobDataPtr job) return; if (job->state == QEMU_BLOCKJOB_STATE_NEW) - qemuBlockJobTerminate(job); + qemuBlockJobUnregister(job); virObjectUnref(job); } @@ -300,7 +314,7 @@ qemuBlockJobEventProcessLegacyCompleted(virQEMUDriverPtr driver, virStorageSourceBackingStoreClear(disk->src); ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk, NULL, true)); ignore_value(qemuBlockNodeNamesDetect(driver, vm, asyncJob)); - qemuBlockJobTerminate(job); + qemuBlockJobUnregister(job); } @@ -355,7 +369,7 @@ qemuBlockJobEventProcessLegacy(virQEMUDriverPtr driver, } disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; - qemuBlockJobTerminate(job); + qemuBlockJobUnregister(job); break; case VIR_DOMAIN_BLOCK_JOB_LAST: -- 2.21.0

Block jobs currently belong to disks only so we can look up the block job data for them in the corresponding disks. This won't be the case when using blockdev as certain jobs don't even correspond to a disk and most of them can run on a part of the backing chain. Add a global table of blockjobs which can be used to look up the data for the blockjobs when the job events need to be processed. The table is a hash table organized by job name and has a reference to the job. New and running jobs will later be added to this table. Reference counting will allow to reap job state for synchronous callers. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 7 +++++++ src/qemu/qemu_domain.h | 3 +++ 2 files changed, 10 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 073c9744d3..5af8f3b30c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1982,6 +1982,9 @@ qemuDomainObjPrivateAlloc(void *opaque) if (!(priv->devs = virChrdevAlloc())) goto error; + if (!(priv->blockjobs = virHashCreate(5, virObjectFreeHashData))) + goto error; + priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; priv->driver = opaque; @@ -2051,6 +2054,8 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv) qemuDomainObjResetJob(priv); qemuDomainObjResetAsyncJob(priv); + + virHashRemoveAll(priv->blockjobs); } @@ -2082,6 +2087,8 @@ qemuDomainObjPrivateFree(void *data) qemuDomainSecretInfoFree(&priv->migSecinfo); qemuDomainMasterKeyFree(priv); + virHashFree(priv->blockjobs); + VIR_FREE(priv); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 12882d5b91..b42b205398 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -389,6 +389,9 @@ struct _qemuDomainObjPrivate { /* true if global -mem-prealloc appears on cmd line */ bool memPrealloc; + + /* running block jobs */ + virHashTablePtr blockjobs; }; #define QEMU_DOMAIN_PRIVATE(vm) \ -- 2.21.0

Add the job structure to the table when instantiating a new job and remove it when it terminates/fails. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_blockjob.c | 29 ++++++++++++++++++++++------- src/qemu/qemu_blockjob.h | 6 ++++-- src/qemu/qemu_driver.c | 16 ++++++++-------- src/qemu/qemu_migration.c | 4 ++-- src/qemu/qemu_process.c | 6 +++--- 5 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index c102417e43..8cbfc556b3 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -94,8 +94,16 @@ qemuBlockJobDataNew(qemuBlockJobType type, static int qemuBlockJobRegister(qemuBlockJobDataPtr job, + virDomainObjPtr vm, virDomainDiskDefPtr disk) { + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (virHashAddEntry(priv->blockjobs, job->name, virObjectRef(job)) < 0) { + virObjectUnref(job); + return -1; + } + if (disk) { job->disk = disk; QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob = virObjectRef(job); @@ -106,8 +114,10 @@ qemuBlockJobRegister(qemuBlockJobDataPtr job, static void -qemuBlockJobUnregister(qemuBlockJobDataPtr job) +qemuBlockJobUnregister(qemuBlockJobDataPtr job, + virDomainObjPtr vm) { + qemuDomainObjPrivatePtr priv = vm->privateData; qemuDomainDiskPrivatePtr diskPriv; if (job->disk) { @@ -120,6 +130,9 @@ qemuBlockJobUnregister(qemuBlockJobDataPtr job) job->disk = NULL; } + + /* this may remove the last reference of 'job' */ + virHashRemoveEntry(priv->blockjobs, job->name); } @@ -132,7 +145,8 @@ qemuBlockJobUnregister(qemuBlockJobDataPtr job) * Returns 0 on success and -1 on failure. */ qemuBlockJobDataPtr -qemuBlockJobDiskNew(virDomainDiskDefPtr disk, +qemuBlockJobDiskNew(virDomainObjPtr vm, + virDomainDiskDefPtr disk, qemuBlockJobType type, const char *jobname) { @@ -141,7 +155,7 @@ qemuBlockJobDiskNew(virDomainDiskDefPtr disk, if (!(job = qemuBlockJobDataNew(type, jobname))) return NULL; - if (qemuBlockJobRegister(job, disk) < 0) + if (qemuBlockJobRegister(job, vm, disk) < 0) return NULL; VIR_RETURN_PTR(job); @@ -189,13 +203,14 @@ qemuBlockJobStarted(qemuBlockJobDataPtr job) * to @job if it was started. */ void -qemuBlockJobStartupFinalize(qemuBlockJobDataPtr job) +qemuBlockJobStartupFinalize(virDomainObjPtr vm, + qemuBlockJobDataPtr job) { if (!job) return; if (job->state == QEMU_BLOCKJOB_STATE_NEW) - qemuBlockJobUnregister(job); + qemuBlockJobUnregister(job, vm); virObjectUnref(job); } @@ -314,7 +329,7 @@ qemuBlockJobEventProcessLegacyCompleted(virQEMUDriverPtr driver, virStorageSourceBackingStoreClear(disk->src); ignore_value(qemuDomainDetermineDiskChain(driver, vm, disk, NULL, true)); ignore_value(qemuBlockNodeNamesDetect(driver, vm, asyncJob)); - qemuBlockJobUnregister(job); + qemuBlockJobUnregister(job, vm); } @@ -369,7 +384,7 @@ qemuBlockJobEventProcessLegacy(virQEMUDriverPtr driver, } disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN; - qemuBlockJobUnregister(job); + qemuBlockJobUnregister(job, vm); break; case VIR_DOMAIN_BLOCK_JOB_LAST: diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index da529090ad..b7aaa86f4d 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -80,7 +80,8 @@ struct _qemuBlockJobData { qemuBlockJobDataPtr -qemuBlockJobDiskNew(virDomainDiskDefPtr disk, +qemuBlockJobDiskNew(virDomainObjPtr vm, + virDomainDiskDefPtr disk, qemuBlockJobType type, const char *jobname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); @@ -98,7 +99,8 @@ qemuBlockJobIsRunning(qemuBlockJobDataPtr job) ATTRIBUTE_NONNULL(1); void -qemuBlockJobStartupFinalize(qemuBlockJobDataPtr job); +qemuBlockJobStartupFinalize(virDomainObjPtr vm, + qemuBlockJobDataPtr job); int qemuBlockJobUpdate(virDomainObjPtr vm, qemuBlockJobDataPtr job, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e41a3001ae..879a78858b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4702,7 +4702,7 @@ processBlockJobEvent(virQEMUDriverPtr driver, } if (!(job = qemuBlockJobDiskGetJob(disk))) { - if (!(job = qemuBlockJobDiskNew(disk, type, diskAlias))) + if (!(job = qemuBlockJobDiskNew(vm, disk, type, diskAlias))) goto endjob; qemuBlockJobStarted(job); } @@ -4712,7 +4712,7 @@ processBlockJobEvent(virQEMUDriverPtr driver, qemuBlockJobUpdate(vm, job, QEMU_ASYNC_JOB_NONE); endjob: - qemuBlockJobStartupFinalize(job); + qemuBlockJobStartupFinalize(vm, job); qemuDomainObjEndJob(driver, vm); } @@ -17084,7 +17084,7 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver, speed <<= 20; } - if (!(job = qemuBlockJobDiskNew(disk, QEMU_BLOCKJOB_TYPE_PULL, device))) + if (!(job = qemuBlockJobDiskNew(vm, disk, QEMU_BLOCKJOB_TYPE_PULL, device))) goto endjob; qemuDomainObjEnterMonitor(driver, vm); @@ -17110,7 +17110,7 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver, qemuDomainObjEndJob(driver, vm); cleanup: - qemuBlockJobStartupFinalize(job); + qemuBlockJobStartupFinalize(vm, job); virDomainObjEndAPI(&vm); return ret; } @@ -17655,7 +17655,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, if (qemuDomainStorageSourceChainAccessAllow(driver, vm, mirror) < 0) goto endjob; - if (!(job = qemuBlockJobDiskNew(disk, QEMU_BLOCKJOB_TYPE_COPY, device))) + if (!(job = qemuBlockJobDiskNew(vm, disk, QEMU_BLOCKJOB_TYPE_COPY, device))) goto endjob; disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; @@ -17691,7 +17691,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, VIR_WARN("%s", _("unable to remove just-created copy target")); virStorageFileDeinit(mirror); qemuDomainObjEndJob(driver, vm); - qemuBlockJobStartupFinalize(job); + qemuBlockJobStartupFinalize(vm, job); return ret; } @@ -18042,7 +18042,7 @@ qemuDomainBlockCommit(virDomainPtr dom, qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, false, false) < 0)) goto endjob; - if (!(job = qemuBlockJobDiskNew(disk, jobtype, device))) + if (!(job = qemuBlockJobDiskNew(vm, disk, jobtype, device))) goto endjob; disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE; @@ -18089,7 +18089,7 @@ qemuDomainBlockCommit(virDomainPtr dom, virFreeError(orig_err); } } - qemuBlockJobStartupFinalize(job); + qemuBlockJobStartupFinalize(vm, job); qemuDomainObjEndJob(driver, vm); cleanup: diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 1fb88c11b6..e72553befc 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -921,7 +921,7 @@ qemuMigrationSrcNBDStorageCopyOne(virQEMUDriverPtr driver, if (!(diskAlias = qemuAliasDiskDriveFromDisk(disk))) goto cleanup; - if (!(job = qemuBlockJobDiskNew(disk, QEMU_BLOCKJOB_TYPE_COPY, diskAlias))) + if (!(job = qemuBlockJobDiskNew(vm, disk, QEMU_BLOCKJOB_TYPE_COPY, diskAlias))) goto cleanup; qemuBlockJobSyncBegin(job); @@ -949,7 +949,7 @@ qemuMigrationSrcNBDStorageCopyOne(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuBlockJobStartupFinalize(job); + qemuBlockJobStartupFinalize(vm, job); VIR_FREE(diskAlias); return ret; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index aa09ef175a..29124ae184 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -976,7 +976,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } cleanup: - qemuBlockJobStartupFinalize(job); + qemuBlockJobStartupFinalize(vm, job); qemuProcessEventFree(processEvent); virObjectUnlock(vm); return 0; @@ -7816,7 +7816,7 @@ qemuProcessRefreshLegacyBlockjob(void *payload, disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT) jobtype = disk->mirrorJob; - if (!(job = qemuBlockJobDiskNew(disk, jobtype, jobname))) + if (!(job = qemuBlockJobDiskNew(vm, disk, jobtype, jobname))) return -1; qemuBlockJobStarted(job); @@ -7850,7 +7850,7 @@ qemuProcessRefreshLegacyBlockjob(void *payload, } cleanup: - qemuBlockJobStartupFinalize(job); + qemuBlockJobStartupFinalize(vm, job); return 0; } -- 2.21.0

Hi guys, Em sex, 12 de jul de 2019 às 13:06, Peter Krempa <pkrempa(a)redhat.com&gt; escreveu: I think this patch is missing an attribute shift... Compilation is failing: diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index d07ab75c8b..b0d17963fd 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -106,7 +106,7 @@ qemuBlockJobDiskNew(virDomainObjPtr vm, virDomainDiskDefPtr disk, qemuBlockJobType type, const char *jobname) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4); qemuBlockJobDataPtr qemuBlockJobDiskGetJob(virDomainDiskDefPtr disk) -- Julio Faracco

Later on we'll format these values into the status XML so the from/to string functions will come handy. The implementation also notes that these will be used in the status XML to avoid somebody changing the values. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_blockjob.c | 9 +++++++++ src/qemu/qemu_blockjob.h | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 8cbfc556b3..bba1b9d656 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -43,6 +43,15 @@ VIR_LOG_INIT("qemu.qemu_blockjob"); +/* Note that qemuBlockjobState and qemuBlockjobType values are formatted into + * the status XML */ +VIR_ENUM_IMPL(qemuBlockjobState, + QEMU_BLOCKJOB_STATE_LAST, + "completed", "failed", "cancelled", "ready", "new", "running"); + +VIR_ENUM_IMPL(qemuBlockjob, + QEMU_BLOCKJOB_TYPE_LAST, + "", "pull", "copy", "commit", "active-commit", ""); static virClassPtr qemuBlockJobDataClass; diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index b7aaa86f4d..77298a4bea 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -42,6 +42,8 @@ typedef enum { } qemuBlockjobState; verify((int)QEMU_BLOCKJOB_STATE_NEW == VIR_DOMAIN_BLOCK_JOB_LAST); +VIR_ENUM_DECL(qemuBlockjobState); + /** * This enum has to map all known block job types from enum virDomainBlockJobType * to the same values. All internal blockjobs can be mapped after and don't @@ -60,6 +62,8 @@ typedef enum { } qemuBlockJobType; verify((int)QEMU_BLOCKJOB_TYPE_INTERNAL == VIR_DOMAIN_BLOCK_JOB_TYPE_LAST); +VIR_ENUM_DECL(qemuBlockjob); + typedef struct _qemuBlockJobData qemuBlockJobData; typedef qemuBlockJobData *qemuBlockJobDataPtr; -- 2.21.0

When parsing the status XML we need to register all existing jobs. Export the functions so that they are usable in other modules. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_blockjob.c | 4 ++-- src/qemu/qemu_blockjob.h | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index bba1b9d656..78d4938684 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -78,7 +78,7 @@ qemuBlockJobDataOnceInit(void) VIR_ONCE_GLOBAL_INIT(qemuBlockJobData); -static qemuBlockJobDataPtr +qemuBlockJobDataPtr qemuBlockJobDataNew(qemuBlockJobType type, const char *name) { @@ -101,7 +101,7 @@ qemuBlockJobDataNew(qemuBlockJobType type, } -static int +int qemuBlockJobRegister(qemuBlockJobDataPtr job, virDomainObjPtr vm, virDomainDiskDefPtr disk) diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index 77298a4bea..fe16badbc8 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -82,6 +82,16 @@ struct _qemuBlockJobData { int newstate; /* qemuBlockjobState, subset of events emitted by qemu */ }; +int +qemuBlockJobRegister(qemuBlockJobDataPtr job, + virDomainObjPtr vm, + virDomainDiskDefPtr disk) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +qemuBlockJobDataPtr +qemuBlockJobDataNew(qemuBlockJobType type, + const char *name) + ATTRIBUTE_NONNULL(2); qemuBlockJobDataPtr qemuBlockJobDiskNew(virDomainObjPtr vm, -- 2.21.0

The job data saved in the XML may be partially invalid e.g. if something is missing. To prevent losing a domain with such a job add a flag to the job data so that job APIs can ignore such a job and we can just cancel it. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_blockjob.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index fe16badbc8..5b3af69d89 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -80,6 +80,8 @@ struct _qemuBlockJobData { bool synchronous; /* API call is waiting for this job */ int newstate; /* qemuBlockjobState, subset of events emitted by qemu */ + + bool invalidData; /* the job data (except name) is not valid */ }; int -- 2.21.0

We need to store the block job state in the status XML so that we can properly recover any data when reconnecting after startup and also in the end to be able to do any transition of the backing chain that happened while libvirt was not connected to the monitor. First step is to note the name, type, state and corresponding disk into the status XML. We also need to make sure that a broken blockjob does not make libvirt to lose the VM, thus many of the errors are just mark the job as invalid. Later on we'll cancel all invalid jobs. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 123 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 119 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5af8f3b30c..59225c3ca9 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -32,6 +32,7 @@ #include "qemu_migration_params.h" #include "qemu_security.h" #include "qemu_extdevice.h" +#include "qemu_blockjob.h" #include "viralloc.h" #include "virlog.h" #include "virerror.h" @@ -2312,17 +2313,57 @@ qemuDomainObjPrivateXMLFormatAutomaticPlacement(virBufferPtr buf, } +static int +qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *data) +{ + VIR_AUTOCLEAN(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + VIR_AUTOCLEAN(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; + qemuBlockJobDataPtr job = payload; + virBufferPtr buf = data; + const char *state = qemuBlockjobStateTypeToString(job->state); + const char *newstate = NULL; + + if (job->newstate != -1) + newstate = qemuBlockjobStateTypeToString(job->newstate); + + virBufferSetChildIndent(&childBuf, buf); + + virBufferEscapeString(&attrBuf, " name='%s'", job->name); + virBufferEscapeString(&attrBuf, " type='%s'", qemuBlockjobTypeToString(job->type)); + virBufferEscapeString(&attrBuf, " state='%s'", state); + virBufferEscapeString(&attrBuf, " newstate='%s'", newstate); + virBufferEscapeString(&childBuf, "<errmsg>%s</errmsg>", job->errmsg); + + if (job->disk) + virBufferEscapeString(&childBuf, "<disk dst='%s'/>\n", job->disk->dst); + + return virXMLFormatElement(buf, "blockjob", &attrBuf, &childBuf); +} + + static int qemuDomainObjPrivateXMLFormatBlockjobs(virBufferPtr buf, virDomainObjPtr vm) { - virBuffer attrBuf = VIR_BUFFER_INITIALIZER; + qemuDomainObjPrivatePtr priv = vm->privateData; + VIR_AUTOCLEAN(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + VIR_AUTOCLEAN(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; bool bj = qemuDomainHasBlockjob(vm, false); virBufferAsprintf(&attrBuf, " active='%s'", virTristateBoolTypeToString(virTristateBoolFromBool(bj))); - return virXMLFormatElement(buf, "blockjobs", &attrBuf, NULL); + virBufferSetChildIndent(&childBuf, buf); + + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV) && + virHashForEach(priv->blockjobs, + qemuDomainObjPrivateXMLFormatBlockjobIterator, + &childBuf) < 0) + return -1; + + return virXMLFormatElement(buf, "blockjobs", &attrBuf, &childBuf); } @@ -2662,16 +2703,90 @@ qemuDomainObjPrivateXMLParseAutomaticPlacement(xmlXPathContextPtr ctxt, static int -qemuDomainObjPrivateXMLParseBlockjobs(qemuDomainObjPrivatePtr priv, +qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr vm, + xmlNodePtr node, + xmlXPathContextPtr ctxt) +{ + VIR_XPATH_NODE_AUTORESTORE(ctxt); + virDomainDiskDefPtr disk = NULL; + VIR_AUTOUNREF(qemuBlockJobDataPtr) job = NULL; + VIR_AUTOFREE(char *) name = NULL; + VIR_AUTOFREE(char *) typestr = NULL; + int type; + VIR_AUTOFREE(char *) statestr = NULL; + int state = QEMU_BLOCKJOB_STATE_FAILED; + VIR_AUTOFREE(char *) diskdst = NULL; + VIR_AUTOFREE(char *) newstatestr = NULL; + int newstate = -1; + bool invalidData = false; + + ctxt->node = node; + + if (!(name = virXPathString("string(./@name)", ctxt))) { + VIR_WARN("malformed block job data for vm '%s'", vm->def->name); + return 0; + } + + /* if the job name is known we need to register such a job so that we can + * clean it up */ + if (!(typestr = virXPathString("string(./@type)", ctxt)) || + (type = qemuBlockjobTypeFromString(typestr)) < 0) { + type = QEMU_BLOCKJOB_TYPE_NONE; + invalidData = true; + } + + if (!(job = qemuBlockJobDataNew(type, name))) + return -1; + + if (!(statestr = virXPathString("string(./@state)", ctxt)) || + (state = qemuBlockjobStateTypeFromString(statestr)) < 0) + invalidData = true; + + if ((newstatestr = virXPathString("string(./@newstate)", ctxt)) && + (newstate = qemuBlockjobStateTypeFromString(newstatestr)) < 0) + invalidData = true; + + if ((diskdst = virXPathString("string(./disk/@dst)", ctxt)) && + !(disk = virDomainDiskByName(vm->def, diskdst, false))) + invalidData = true; + + job->state = state; + job->newstate = newstate; + job->errmsg = virXPathString("string(./errmsg)", ctxt); + job->invalidData = invalidData; + + if (qemuBlockJobRegister(job, vm, disk) < 0) + return -1; + + return 0; +} + + +static int +qemuDomainObjPrivateXMLParseBlockjobs(virDomainObjPtr vm, + qemuDomainObjPrivatePtr priv, xmlXPathContextPtr ctxt) { + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; + ssize_t nnodes = 0; VIR_AUTOFREE(char *) active = NULL; int tmp; + size_t i; if ((active = virXPathString("string(./blockjobs/@active)", ctxt)) && (tmp = virTristateBoolTypeFromString(active)) > 0) priv->reconnectBlockjobs = tmp; + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { + if ((nnodes = virXPathNodeSet("./blockjobs/blockjob", ctxt, &nodes)) < 0) + return -1; + + for (i = 0; i < nnodes; i++) { + if (qemuDomainObjPrivateXMLParseBlockjobData(vm, nodes[i], ctxt) < 0) + return -1; + } + } + return 0; } @@ -3029,7 +3144,7 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, qemuDomainObjPrivateXMLParsePR(ctxt, &priv->prDaemonRunning); - if (qemuDomainObjPrivateXMLParseBlockjobs(priv, ctxt) < 0) + if (qemuDomainObjPrivateXMLParseBlockjobs(vm, priv, ctxt) < 0) goto error; qemuDomainStorageIdReset(priv); -- 2.21.0

Now that block job data is stored in the status XML portion we need to make sure that everything which changes the state also saves the status XML. The job registering function is used while parsing the status XML so in that case we need to skip the XML saving. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_blockjob.c | 15 ++++++++++++--- src/qemu/qemu_blockjob.h | 8 +++++--- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 8 ++++---- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_process.c | 4 ++-- 6 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 78d4938684..3211230811 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -104,7 +104,8 @@ qemuBlockJobDataNew(qemuBlockJobType type, int qemuBlockJobRegister(qemuBlockJobDataPtr job, virDomainObjPtr vm, - virDomainDiskDefPtr disk) + virDomainDiskDefPtr disk, + bool savestatus) { qemuDomainObjPrivatePtr priv = vm->privateData; @@ -118,6 +119,9 @@ qemuBlockJobRegister(qemuBlockJobDataPtr job, QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob = virObjectRef(job); } + if (savestatus) + qemuDomainSaveStatus(vm); + return 0; } @@ -142,6 +146,8 @@ qemuBlockJobUnregister(qemuBlockJobDataPtr job, /* this may remove the last reference of 'job' */ virHashRemoveEntry(priv->blockjobs, job->name); + + qemuDomainSaveStatus(vm); } @@ -164,7 +170,7 @@ qemuBlockJobDiskNew(virDomainObjPtr vm, if (!(job = qemuBlockJobDataNew(type, jobname))) return NULL; - if (qemuBlockJobRegister(job, vm, disk) < 0) + if (qemuBlockJobRegister(job, vm, disk, true) < 0) return NULL; VIR_RETURN_PTR(job); @@ -196,10 +202,13 @@ qemuBlockJobDiskGetJob(virDomainDiskDefPtr disk) * Mark @job as started in qemu. */ void -qemuBlockJobStarted(qemuBlockJobDataPtr job) +qemuBlockJobStarted(qemuBlockJobDataPtr job, + virDomainObjPtr vm) { if (job->state == QEMU_BLOCKJOB_STATE_NEW) job->state = QEMU_BLOCKJOB_STATE_RUNNING; + + qemuDomainSaveStatus(vm); } diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index 5b3af69d89..c5fd636340 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -87,7 +87,8 @@ struct _qemuBlockJobData { int qemuBlockJobRegister(qemuBlockJobDataPtr job, virDomainObjPtr vm, - virDomainDiskDefPtr disk) + virDomainDiskDefPtr disk, + bool savestatus) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); qemuBlockJobDataPtr @@ -107,8 +108,9 @@ qemuBlockJobDiskGetJob(virDomainDiskDefPtr disk) ATTRIBUTE_NONNULL(1); void -qemuBlockJobStarted(qemuBlockJobDataPtr job) - ATTRIBUTE_NONNULL(1); +qemuBlockJobStarted(qemuBlockJobDataPtr job, + virDomainObjPtr vm) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); bool qemuBlockJobIsRunning(qemuBlockJobDataPtr job) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 59225c3ca9..7162fca71b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2755,7 +2755,7 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr vm, job->errmsg = virXPathString("string(./errmsg)", ctxt); job->invalidData = invalidData; - if (qemuBlockJobRegister(job, vm, disk) < 0) + if (qemuBlockJobRegister(job, vm, disk, false) < 0) return -1; return 0; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 879a78858b..a0c246a416 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4704,7 +4704,7 @@ processBlockJobEvent(virQEMUDriverPtr driver, if (!(job = qemuBlockJobDiskGetJob(disk))) { if (!(job = qemuBlockJobDiskNew(vm, disk, type, diskAlias))) goto endjob; - qemuBlockJobStarted(job); + job->state = QEMU_BLOCKJOB_STATE_RUNNING; } job->newstate = status; @@ -17100,7 +17100,7 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver, if (ret < 0) goto endjob; - qemuBlockJobStarted(job); + qemuBlockJobStarted(job, vm); if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) VIR_WARN("Unable to save status on vm %s after state change", @@ -17676,11 +17676,11 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, } /* Update vm in place to match changes. */ - qemuBlockJobStarted(job); need_unlink = false; virStorageFileDeinit(mirror); VIR_STEAL_PTR(disk->mirror, mirror); disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY; + qemuBlockJobStarted(job, vm); if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) VIR_WARN("Unable to save status on vm %s after state change", @@ -18066,11 +18066,11 @@ qemuDomainBlockCommit(virDomainPtr dom, goto endjob; } - qemuBlockJobStarted(job); if (mirror) { VIR_STEAL_PTR(disk->mirror, mirror); disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT; } + qemuBlockJobStarted(job, vm); if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) VIR_WARN("Unable to save status on vm %s after block job", diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e72553befc..cbacb886f8 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -944,7 +944,7 @@ qemuMigrationSrcNBDStorageCopyOne(virQEMUDriverPtr driver, goto cleanup; diskPriv->migrating = true; - qemuBlockJobStarted(job); + qemuBlockJobStarted(job, vm); ret = 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 29124ae184..f39143d499 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7819,8 +7819,6 @@ qemuProcessRefreshLegacyBlockjob(void *payload, if (!(job = qemuBlockJobDiskNew(vm, disk, jobtype, jobname))) return -1; - qemuBlockJobStarted(job); - if (disk->mirror) { if (info->ready == 1 || (info->ready == -1 && info->end == info->cur)) { @@ -7849,6 +7847,8 @@ qemuProcessRefreshLegacyBlockjob(void *payload, } } + qemuBlockJobStarted(job, vm); + cleanup: qemuBlockJobStartupFinalize(vm, job); -- 2.21.0

Now that the blockjob handling code deals with the status XML we don't need to save it explicitly when starting blockjobs. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a0c246a416..a1dc2634ca 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17012,7 +17012,6 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver, unsigned int flags) { qemuDomainObjPrivatePtr priv = vm->privateData; - VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); VIR_AUTOFREE(char *) device = NULL; virDomainDiskDefPtr disk; virStorageSourcePtr baseSource = NULL; @@ -17102,10 +17101,6 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver, qemuBlockJobStarted(job, vm); - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) - VIR_WARN("Unable to save status on vm %s after state change", - vm->def->name); - endjob: qemuDomainObjEndJob(driver, vm); @@ -17682,10 +17677,6 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY; qemuBlockJobStarted(job, vm); - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) - VIR_WARN("Unable to save status on vm %s after state change", - vm->def->name); - endjob: if (need_unlink && virStorageFileUnlink(mirror) < 0) VIR_WARN("%s", _("unable to remove just-created copy target")); @@ -17880,7 +17871,6 @@ qemuDomainBlockCommit(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; - VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; qemuDomainObjPrivatePtr priv; virDomainObjPtr vm = NULL; VIR_AUTOFREE(char *) device = NULL; @@ -17909,7 +17899,6 @@ qemuDomainBlockCommit(virDomainPtr dom, if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; priv = vm->privateData; - cfg = virQEMUDriverGetConfig(driver); if (virDomainBlockCommitEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -18072,10 +18061,6 @@ qemuDomainBlockCommit(virDomainPtr dom, } qemuBlockJobStarted(job, vm); - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) - VIR_WARN("Unable to save status on vm %s after block job", - vm->def->name); - endjob: if (ret < 0 && clean_access) { virErrorPtr orig_err = virSaveLastError(); -- 2.21.0

Now that the blockjob handling code deals with the status XML we don't need to save it explicitly when starting blockjobs. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- Note that this depends now on: https://www.redhat.com/archives/libvir-list/2019-July/msg01088.html Or fetch the whole updated series (due to a trivial rebase conflict) at: git fetch https://gitlab.com/pipo.sk/libvirt.git job-tracking-send-2 v2: - fixes qemuBlockJobEventProcessLegacy src/qemu/qemu_blockjob.c | 5 +---- src/qemu/qemu_driver.c | 15 --------------- 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index d9d22d8f61..4e832963e4 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -378,7 +378,6 @@ qemuBlockJobEventProcessLegacy(virQEMUDriverPtr driver, qemuBlockJobDataPtr job, int asyncJob) { - VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); virDomainDiskDefPtr disk = job->disk; VIR_DEBUG("disk=%s, mirrorState=%s, type=%d, state=%d, newstate=%d", @@ -405,6 +404,7 @@ qemuBlockJobEventProcessLegacy(virQEMUDriverPtr driver, case VIR_DOMAIN_BLOCK_JOB_READY: disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY; + qemuDomainSaveStatus(vm); break; case VIR_DOMAIN_BLOCK_JOB_FAILED: @@ -422,9 +422,6 @@ qemuBlockJobEventProcessLegacy(virQEMUDriverPtr driver, case VIR_DOMAIN_BLOCK_JOB_LAST: break; } - - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) - VIR_WARN("Unable to save status on vm %s after block job", vm->def->name); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index af58dacbaa..d8a49d1fea 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17012,7 +17012,6 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver, unsigned int flags) { qemuDomainObjPrivatePtr priv = vm->privateData; - VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver); VIR_AUTOFREE(char *) device = NULL; virDomainDiskDefPtr disk; virStorageSourcePtr baseSource = NULL; @@ -17102,10 +17101,6 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver, qemuBlockJobStarted(job, vm); - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) - VIR_WARN("Unable to save status on vm %s after state change", - vm->def->name); - endjob: qemuDomainObjEndJob(driver, vm); @@ -17682,10 +17677,6 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_COPY; qemuBlockJobStarted(job, vm); - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) - VIR_WARN("Unable to save status on vm %s after state change", - vm->def->name); - endjob: if (need_unlink && virStorageFileUnlink(mirror) < 0) VIR_WARN("%s", _("unable to remove just-created copy target")); @@ -17880,7 +17871,6 @@ qemuDomainBlockCommit(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; - VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = NULL; qemuDomainObjPrivatePtr priv; virDomainObjPtr vm = NULL; VIR_AUTOFREE(char *) device = NULL; @@ -17909,7 +17899,6 @@ qemuDomainBlockCommit(virDomainPtr dom, if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; priv = vm->privateData; - cfg = virQEMUDriverGetConfig(driver); if (virDomainBlockCommitEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -18072,10 +18061,6 @@ qemuDomainBlockCommit(virDomainPtr dom, } qemuBlockJobStarted(job, vm); - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0) - VIR_WARN("Unable to save status on vm %s after block job", - vm->def->name); - endjob: if (ret < 0 && clean_access) { virErrorPtr orig_err = virSaveLastError(); -- 2.21.0

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- .../blockjob-blockdev-in.xml | 366 ++++++++++++++++++ .../blockjob-blockdev-out.xml | 1 + tests/qemuxml2xmltest.c | 2 + 3 files changed, 369 insertions(+) create mode 100644 tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml create mode 120000 tests/qemustatusxml2xmldata/blockjob-blockdev-out.xml diff --git a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml new file mode 100644 index 0000000000..115eaa4887 --- /dev/null +++ b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml @@ -0,0 +1,366 @@ +<domstatus state='running' reason='booted' pid='7690'> + <taint flag='high-privileges'/> + <monitor path='/var/lib/libvirt/qemu/domain-4-copy/monitor.sock' type='unix'/> + <namespaces> + <mount/> + </namespaces> + <vcpus> + <vcpu id='0' pid='7696'/> + <vcpu id='1' pid='7697'/> + </vcpus> + <qemuCaps> + <flag name='kvm'/> + <flag name='no-hpet'/> + <flag name='spice'/> + <flag name='hda-duplex'/> + <flag name='ccid-emulated'/> + <flag name='ccid-passthru'/> + <flag name='virtio-tx-alg'/> + <flag name='virtio-blk-pci.ioeventfd'/> + <flag name='sga'/> + <flag name='virtio-blk-pci.event_idx'/> + <flag name='virtio-net-pci.event_idx'/> + <flag name='piix3-usb-uhci'/> + <flag name='piix4-usb-uhci'/> + <flag name='usb-ehci'/> + <flag name='ich9-usb-ehci1'/> + <flag name='vt82c686b-usb-uhci'/> + <flag name='pci-ohci'/> + <flag name='usb-redir'/> + <flag name='usb-hub'/> + <flag name='ich9-ahci'/> + <flag name='no-acpi'/> + <flag name='virtio-blk-pci.scsi'/> + <flag name='scsi-disk.channel'/> + <flag name='scsi-block'/> + <flag name='transaction'/> + <flag name='block-job-async'/> + <flag name='scsi-cd'/> + <flag name='ide-cd'/> + <flag name='hda-micro'/> + <flag name='dump-guest-memory'/> + <flag name='nec-usb-xhci'/> + <flag name='balloon-event'/> + <flag name='lsi'/> + <flag name='virtio-scsi-pci'/> + <flag name='blockio'/> + <flag name='disable-s3'/> + <flag name='disable-s4'/> + <flag name='usb-redir.filter'/> + <flag name='ide-drive.wwn'/> + <flag name='scsi-disk.wwn'/> + <flag name='seccomp-sandbox'/> + <flag name='reboot-timeout'/> + <flag name='seamless-migration'/> + <flag name='block-commit'/> + <flag name='vnc'/> + <flag name='drive-mirror'/> + <flag name='blockdev-snapshot-sync'/> + <flag name='qxl'/> + <flag name='VGA'/> + <flag name='cirrus-vga'/> + <flag name='vmware-svga'/> + <flag name='device-video-primary'/> + <flag name='usb-serial'/> + <flag name='nbd-server'/> + <flag name='virtio-rng'/> + <flag name='rng-random'/> + <flag name='rng-egd'/> + <flag name='megasas'/> + <flag name='tpm-passthrough'/> + <flag name='tpm-tis'/> + <flag name='pci-bridge'/> + <flag name='vfio-pci'/> + <flag name='mem-merge'/> + <flag name='drive-discard'/> + <flag name='mlock'/> + <flag name='device-del-event'/> + <flag name='dmi-to-pci-bridge'/> + <flag name='i440fx-pci-hole64-size'/> + <flag name='q35-pci-hole64-size'/> + <flag name='usb-storage'/> + <flag name='usb-storage.removable'/> + <flag name='ich9-intel-hda'/> + <flag name='kvm-pit-lost-tick-policy'/> + <flag name='boot-strict'/> + <flag name='pvpanic'/> + <flag name='spice-file-xfer-disable'/> + <flag name='usb-kbd'/> + <flag name='msg-timestamp'/> + <flag name='active-commit'/> + <flag name='change-backing-file'/> + <flag name='memory-backend-ram'/> + <flag name='numa'/> + <flag name='memory-backend-file'/> + <flag name='usb-audio'/> + <flag name='rtc-reset-reinjection'/> + <flag name='splash-timeout'/> + <flag name='iothread'/> + <flag name='migrate-rdma'/> + <flag name='ivshmem'/> + <flag name='drive-iotune-max'/> + <flag name='VGA.vgamem_mb'/> + <flag name='vmware-svga.vgamem_mb'/> + <flag name='qxl.vgamem_mb'/> + <flag name='pc-dimm'/> + <flag name='machine-vmport-opt'/> + <flag name='aes-key-wrap'/> + <flag name='dea-key-wrap'/> + <flag name='pci-serial'/> + <flag name='vhost-user-multiqueue'/> + <flag name='migration-event'/> + <flag name='ioh3420'/> + <flag name='x3130-upstream'/> + <flag name='xio3130-downstream'/> + <flag name='rtl8139'/> + <flag name='e1000'/> + <flag name='virtio-net'/> + <flag name='gic-version'/> + <flag name='incoming-defer'/> + <flag name='virtio-gpu'/> + <flag name='virtio-gpu.virgl'/> + <flag name='virtio-keyboard'/> + <flag name='virtio-mouse'/> + <flag name='virtio-tablet'/> + <flag name='virtio-input-host'/> + <flag name='chardev-file-append'/> + <flag name='ich9-disable-s3'/> + <flag name='ich9-disable-s4'/> + <flag name='vserport-change-event'/> + <flag name='virtio-balloon-pci.deflate-on-oom'/> + <flag name='mptsas1068'/> + <flag name='spice-gl'/> + <flag name='qxl.vram64_size_mb'/> + <flag name='chardev-logfile'/> + <flag name='debug-threads'/> + <flag name='secret'/> + <flag name='pxb'/> + <flag name='pxb-pcie'/> + <flag name='device-tray-moved-event'/> + <flag name='nec-usb-xhci-ports'/> + <flag name='virtio-scsi-pci.iothread'/> + <flag name='name-guest'/> + <flag name='qxl.max_outputs'/> + <flag name='spice-unix'/> + <flag name='drive-detect-zeroes'/> + <flag name='tls-creds-x509'/> + <flag name='intel-iommu'/> + <flag name='smm'/> + <flag name='virtio-pci-disable-legacy'/> + <flag name='query-hotpluggable-cpus'/> + <flag name='virtio-net.rx_queue_size'/> + <flag name='virtio-vga'/> + <flag name='drive-iotune-max-length'/> + <flag name='ivshmem-plain'/> + <flag name='ivshmem-doorbell'/> + <flag name='query-qmp-schema'/> + <flag name='gluster.debug_level'/> + <flag name='vhost-scsi'/> + <flag name='drive-iotune-group'/> + <flag name='query-cpu-model-expansion'/> + <flag name='virtio-net.host_mtu'/> + <flag name='spice-rendernode'/> + <flag name='nvdimm'/> + <flag name='pcie-root-port'/> + <flag name='query-cpu-definitions'/> + <flag name='block-write-threshold'/> + <flag name='query-named-block-nodes'/> + <flag name='cpu-cache'/> + <flag name='qemu-xhci'/> + <flag name='kernel-irqchip'/> + <flag name='kernel-irqchip.split'/> + <flag name='intel-iommu.intremap'/> + <flag name='intel-iommu.caching-mode'/> + <flag name='intel-iommu.eim'/> + <flag name='intel-iommu.device-iotlb'/> + <flag name='virtio.iommu_platform'/> + <flag name='virtio.ats'/> + <flag name='loadparm'/> + <flag name='vnc-multi-servers'/> + <flag name='virtio-net.tx_queue_size'/> + <flag name='chardev-reconnect'/> + <flag name='virtio-gpu.max_outputs'/> + <flag name='vxhs'/> + <flag name='virtio-blk.num-queues'/> + <flag name='vmcoreinfo'/> + <flag name='numa.dist'/> + <flag name='disk-share-rw'/> + <flag name='iscsi.password-secret'/> + <flag name='isa-serial'/> + <flag name='dump-completed'/> + <flag name='qcow2-luks'/> + <flag name='pcie-pci-bridge'/> + <flag name='seccomp-blacklist'/> + <flag name='query-cpus-fast'/> + <flag name='disk-write-cache'/> + <flag name='nbd-tls'/> + <flag name='tpm-crb'/> + <flag name='pr-manager-helper'/> + <flag name='qom-list-properties'/> + <flag name='memory-backend-file.discard-data'/> + <flag name='sdl-gl'/> + <flag name='screendump_device'/> + <flag name='hda-output'/> + <flag name='blockdev-del'/> + <flag name='vmgenid'/> + <flag name='vhost-vsock'/> + <flag name='chardev-fd-pass'/> + <flag name='tpm-emulator'/> + <flag name='mch'/> + <flag name='mch.extended-tseg-mbytes'/> + <flag name='usb-storage.werror'/> + <flag name='egl-headless'/> + <flag name='vfio-pci.display'/> + <flag name='blockdev'/> + <flag name='memory-backend-memfd'/> + <flag name='memory-backend-memfd.hugetlb'/> + <flag name='iothread.poll-max-ns'/> + <flag name='egl-headless.rendernode'/> + </qemuCaps> + <devices> + <device alias='rng0'/> + <device alias='sound0-codec0'/> + <device alias='virtio-disk0'/> + <device alias='virtio-serial0'/> + <device alias='video0'/> + <device alias='serial0'/> + <device alias='sound0'/> + <device alias='channel1'/> + <device alias='channel0'/> + <device alias='usb'/> + </devices> + <numad nodeset='0' cpuset='0-7'/> + <libDir path='/var/lib/libvirt/qemu/domain-4-copy'/> + <channelTargetDir path='/var/lib/libvirt/qemu/channel/target/domain-4-copy'/> + <chardevStdioLogd/> + <allowReboot value='yes'/> + <nodename index='0'/> + <blockjobs active='yes'> + <blockjob name='drive-virtio-disk0' type='copy' state='ready'> + <disk dst='vda'/> + </blockjob> + </blockjobs> + <domain type='kvm' id='4'> + <name>copy</name> + <uuid>0439a4a8-db56-4933-9183-d8681d7b0746</uuid> + <memory unit='KiB'>1024000</memory> + <currentMemory unit='KiB'>1024000</currentMemory> + <vcpu placement='auto' current='2'>8</vcpu> + <numatune> + <memory mode='strict' placement='auto'/> + </numatune> + <resource> + <partition>/machine</partition> + </resource> + <os> + <type arch='x86_64' machine='pc-i440fx-2.9'>hvm</type> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <features> + <acpi/> + <apic/> + <vmport state='off'/> + </features> + <cpu> + <numa> + <cell id='0' cpus='0,2,4,6' memory='512000' unit='KiB'/> + <cell id='1' cpus='1,3,5,7' memory='512000' unit='KiB'/> + </numa> + </cpu> + <clock offset='utc'> + <timer name='rtc' tickpolicy='catchup'/> + <timer name='pit' tickpolicy='delay'/> + <timer name='hpet' present='no'/> + </clock> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <pm> + <suspend-to-mem enabled='no'/> + <suspend-to-disk enabled='no'/> + </pm> + <devices> + <emulator>/home/pipo/git/qemu.git/x86_64-softmmu/qemu-system-x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='/tmp/src'/> + <backingStore/> + <mirror type='file' file='/tmp/copy' format='qcow2' job='copy' ready='yes'> + <format type='qcow2'/> + <source file='/tmp/copy'/> + </mirror> + <target dev='vda' bus='virtio'/> + <alias name='virtio-disk0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> + </disk> + <controller type='usb' index='0' model='piix3-uhci'> + <alias name='usb'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci.0'/> + </controller> + <controller type='virtio-serial' index='0'> + <alias name='virtio-serial0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </controller> + <serial type='pty'> + <source path='/dev/pts/34'/> + <target type='isa-serial' port='0'> + <model name='isa-serial'/> + </target> + <alias name='serial0'/> + </serial> + <console type='pty' tty='/dev/pts/34'> + <source path='/dev/pts/34'/> + <target type='serial' port='0'/> + <alias name='serial0'/> + </console> + <channel type='unix'> + <source mode='bind' path='/var/lib/libvirt/qemu/channel/target/domain-4-copy/org.qemu.guest_agent.0'/> + <target type='virtio' name='org.qemu.guest_agent.0' state='disconnected'/> + <alias name='channel0'/> + <address type='virtio-serial' controller='0' bus='0' port='1'/> + </channel> + <channel type='spicevmc'> + <target type='virtio' name='com.redhat.spice.0' state='disconnected'/> + <alias name='channel1'/> + <address type='virtio-serial' controller='0' bus='0' port='2'/> + </channel> + <input type='mouse' bus='ps2'> + <alias name='input0'/> + </input> + <input type='keyboard' bus='ps2'> + <alias name='input1'/> + </input> + <graphics type='spice' port='5900' autoport='yes' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1' fromConfig='1' autoGenerated='no'/> + <image compression='off'/> + </graphics> + <sound model='ich6'> + <alias name='sound0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </sound> + <video> + <model type='qxl' ram='65536' vram='65536' vgamem='16384' heads='1' primary='yes'/> + <alias name='video0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='none'/> + <rng model='virtio'> + <backend model='random'>/dev/random</backend> + <alias name='rng0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </rng> + </devices> + <seclabel type='dynamic' model='selinux' relabel='yes'> + <label>unconfined_u:unconfined_r:svirt_t:s0:c550,c786</label> + <imagelabel>unconfined_u:object_r:svirt_image_t:s0:c550,c786</imagelabel> + </seclabel> + <seclabel type='dynamic' model='dac' relabel='yes'> + <label>+0:+0</label> + <imagelabel>+0:+0</imagelabel> + </seclabel> + </domain> +</domstatus> diff --git a/tests/qemustatusxml2xmldata/blockjob-blockdev-out.xml b/tests/qemustatusxml2xmldata/blockjob-blockdev-out.xml new file mode 120000 index 0000000000..cdca6e4a8b --- /dev/null +++ b/tests/qemustatusxml2xmldata/blockjob-blockdev-out.xml @@ -0,0 +1 @@ +blockjob-blockdev-in.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 6d808e172f..09c86eda2a 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1284,6 +1284,8 @@ mymain(void) DO_TEST_STATUS("migration-out-nbd-tls"); DO_TEST_STATUS("disk-secinfo-upgrade"); + DO_TEST_STATUS("blockjob-blockdev"); + DO_TEST("vhost-vsock", QEMU_CAPS_DEVICE_VHOST_VSOCK); DO_TEST("vhost-vsock-auto", QEMU_CAPS_DEVICE_VHOST_VSOCK); DO_TEST("vhost-vsock-ccw", QEMU_CAPS_DEVICE_VHOST_VSOCK, -- 2.21.0

This new state is entered when qemu finished the job but libvirt does not know whether it was successful or not. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_blockjob.c | 2 +- src/qemu/qemu_blockjob.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 3211230811..5177a361ec 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -47,7 +47,7 @@ VIR_LOG_INIT("qemu.qemu_blockjob"); * the status XML */ VIR_ENUM_IMPL(qemuBlockjobState, QEMU_BLOCKJOB_STATE_LAST, - "completed", "failed", "cancelled", "ready", "new", "running"); + "completed", "failed", "cancelled", "ready", "new", "running", "concluded"); VIR_ENUM_IMPL(qemuBlockjob, QEMU_BLOCKJOB_TYPE_LAST, diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index c5fd636340..743f47ee89 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -38,6 +38,8 @@ typedef enum { /* Additional enum values local to qemu */ QEMU_BLOCKJOB_STATE_NEW, QEMU_BLOCKJOB_STATE_RUNNING, + QEMU_BLOCKJOB_STATE_CONCLUDED, /* job has finished, but it's unknown + whether it has failed or not */ QEMU_BLOCKJOB_STATE_LAST } qemuBlockjobState; verify((int)QEMU_BLOCKJOB_STATE_NEW == VIR_DOMAIN_BLOCK_JOB_LAST); -- 2.21.0

With blockdev we'll need to use the JOB_STATUS_CHANGE so gate the old events by the blockdev capability. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_process.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f39143d499..4e24201674 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -932,6 +932,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, const char *error, void *opaque) { + qemuDomainObjPrivatePtr priv; virQEMUDriverPtr driver = opaque; struct qemuProcessEvent *processEvent = NULL; virDomainDiskDefPtr disk; @@ -940,6 +941,12 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virObjectLock(vm); + priv = vm->privateData; + + /* with QEMU_CAPS_BLOCKDEV we handle block job events via JOB_STATUS_CHANGE */ + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) + goto cleanup; + VIR_DEBUG("Block job for device %s (domain: %p,%s) type %d status %d", diskAlias, vm, vm->def->name, type, status); -- 2.21.0

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_blockjob.c | 41 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_blockjob.h | 3 +++ 2 files changed, 44 insertions(+) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 5177a361ec..dd6071dae1 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -513,3 +513,44 @@ qemuBlockJobGetByDisk(virDomainDiskDefPtr disk) return virObjectRef(job); } + + +/** + * @monitorstatus: Status of the blockjob from qemu monitor (qemuMonitorJobStatus) + * + * Converts the block job status from the monitor to the one used by + * qemuBlockJobData. If the status is unknown or does not require any handling + * QEMU_BLOCKJOB_TYPE_LAST is returned. + */ +qemuBlockjobState +qemuBlockjobConvertMonitorStatus(int monitorstatus) +{ + qemuBlockjobState ret = QEMU_BLOCKJOB_STATE_LAST; + + switch ((qemuMonitorJobStatus) monitorstatus) { + case QEMU_MONITOR_JOB_STATUS_READY: + ret = QEMU_BLOCKJOB_STATE_READY; + break; + + case QEMU_MONITOR_JOB_STATUS_CONCLUDED: + ret = QEMU_BLOCKJOB_STATE_CONCLUDED; + break; + + case QEMU_MONITOR_JOB_STATUS_UNKNOWN: + case QEMU_MONITOR_JOB_STATUS_CREATED: + case QEMU_MONITOR_JOB_STATUS_RUNNING: + case QEMU_MONITOR_JOB_STATUS_PAUSED: + case QEMU_MONITOR_JOB_STATUS_STANDBY: + case QEMU_MONITOR_JOB_STATUS_WAITING: + case QEMU_MONITOR_JOB_STATUS_PENDING: + case QEMU_MONITOR_JOB_STATUS_ABORTING: + case QEMU_MONITOR_JOB_STATUS_UNDEFINED: + case QEMU_MONITOR_JOB_STATUS_NULL: + case QEMU_MONITOR_JOB_STATUS_LAST: + default: + break; + } + + return ret; + +} diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index 743f47ee89..a558b0a5a2 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -134,3 +134,6 @@ void qemuBlockJobSyncEnd(virDomainObjPtr vm, qemuBlockJobDataPtr qemuBlockJobGetByDisk(virDomainDiskDefPtr disk) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; + +qemuBlockjobState +qemuBlockjobConvertMonitorStatus(int monitorstatus); -- 2.21.0

Add support for handling the event either synchronously or asynchronously using the event thread. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 3 ++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 23 +++++++++++++++ src/qemu/qemu_process.c | 63 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7162fca71b..13a90ab60b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -14480,6 +14480,9 @@ qemuProcessEventFree(struct qemuProcessEvent *event) case QEMU_PROCESS_EVENT_MONITOR_EOF: VIR_FREE(event->data); break; + case QEMU_PROCESS_EVENT_JOB_STATUS_CHANGE: + virObjectUnref(event->data); + break; case QEMU_PROCESS_EVENT_PR_DISCONNECT: case QEMU_PROCESS_EVENT_LAST: break; diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index b42b205398..3ccea3177e 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -514,6 +514,7 @@ typedef enum { QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED, QEMU_PROCESS_EVENT_SERIAL_CHANGED, QEMU_PROCESS_EVENT_BLOCK_JOB, + QEMU_PROCESS_EVENT_JOB_STATUS_CHANGE, QEMU_PROCESS_EVENT_MONITOR_EOF, QEMU_PROCESS_EVENT_PR_DISCONNECT, QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a1dc2634ca..604beca155 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4717,6 +4717,26 @@ processBlockJobEvent(virQEMUDriverPtr driver, } +static void +processJobStatusChangeEvent(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuBlockJobDataPtr job) +{ + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) + return; + + if (!virDomainObjIsActive(vm)) { + VIR_DEBUG("Domain is not running"); + goto endjob; + } + + qemuBlockJobUpdate(vm, job, QEMU_ASYNC_JOB_NONE); + + endjob: + qemuDomainObjEndJob(driver, vm); +} + + static void processMonitorEOFEvent(virQEMUDriverPtr driver, virDomainObjPtr vm) @@ -4855,6 +4875,9 @@ static void qemuProcessEventHandler(void *data, void *opaque) processEvent->action, processEvent->status); break; + case QEMU_PROCESS_EVENT_JOB_STATUS_CHANGE: + processJobStatusChangeEvent(driver, vm, processEvent->data); + break; case QEMU_PROCESS_EVENT_MONITOR_EOF: processMonitorEOFEvent(driver, vm); break; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 4e24201674..416f4f5c9a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -990,6 +990,68 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } +static int +qemuProcessHandleJobStatusChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED, + virDomainObjPtr vm, + const char *jobname, + int status, + void *opaque) +{ + virQEMUDriverPtr driver = opaque; + qemuDomainObjPrivatePtr priv; + struct qemuProcessEvent *processEvent = NULL; + qemuBlockJobDataPtr job = NULL; + int jobnewstate; + + virObjectLock(vm); + priv = vm->privateData; + + VIR_DEBUG("job '%s'(domain: %p,%s) state changed to '%s'(%d)", + jobname, vm, vm->def->name, + qemuMonitorJobStatusTypeToString(status), status); + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { + VIR_DEBUG("job '%s' handled by old blockjob handler", jobname); + goto cleanup; + } + + if ((jobnewstate = qemuBlockjobConvertMonitorStatus(status)) == QEMU_BLOCKJOB_STATE_LAST) + goto cleanup; + + if (!(job = virHashLookup(priv->blockjobs, jobname))) { + VIR_DEBUG("job '%s' not registered", jobname); + goto cleanup; + } + + job->newstate = jobnewstate; + + if (job->synchronous) { + VIR_DEBUG("job '%s' handled synchronously", jobname); + virDomainObjBroadcast(vm); + } else { + VIR_DEBUG("job '%s' handled by event thread", jobname); + if (VIR_ALLOC(processEvent) < 0) + goto cleanup; + + processEvent->eventType = QEMU_PROCESS_EVENT_JOB_STATUS_CHANGE; + processEvent->vm = virObjectRef(vm); + processEvent->data = virObjectRef(job); + + if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) { + ignore_value(virObjectUnref(vm)); + goto cleanup; + } + + processEvent = NULL; + } + + cleanup: + qemuProcessEventFree(processEvent); + virObjectUnlock(vm); + return 0; +} + + static int qemuProcessHandleGraphics(qemuMonitorPtr mon ATTRIBUTE_UNUSED, virDomainObjPtr vm, @@ -1820,6 +1882,7 @@ static qemuMonitorCallbacks monitorCallbacks = { .domainIOError = qemuProcessHandleIOError, .domainGraphics = qemuProcessHandleGraphics, .domainBlockJob = qemuProcessHandleBlockJob, + .jobStatusChange = qemuProcessHandleJobStatusChange, .domainTrayChange = qemuProcessHandleTrayChange, .domainPMWakeup = qemuProcessHandlePMWakeup, .domainPMSuspend = qemuProcessHandlePMSuspend, -- 2.21.0

Add the infrastructure to handle block job events in the -blockdev era. Some complexity is required as qemu does not bother to notify whether the job was concluded successfully or failed. Thus it's necessary to re-query the monitor. To minimize the possibility of stuck jobs save the state into the XML prior to handling everything so that the reconnect code can potentially continue with the cleanup. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_blockjob.c | 168 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 167 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index dd6071dae1..8b142f1aba 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -423,6 +423,169 @@ qemuBlockJobEventProcessLegacy(virQEMUDriverPtr driver, } +static void +qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob ATTRIBUTE_UNUSED) +{ + switch ((qemuBlockjobState) job->newstate) { + case QEMU_BLOCKJOB_STATE_COMPLETED: + switch ((qemuBlockJobType) job->type) { + case QEMU_BLOCKJOB_TYPE_PULL: + case QEMU_BLOCKJOB_TYPE_COMMIT: + case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT: + case QEMU_BLOCKJOB_TYPE_COPY: + case QEMU_BLOCKJOB_TYPE_NONE: + case QEMU_BLOCKJOB_TYPE_INTERNAL: + case QEMU_BLOCKJOB_TYPE_LAST: + default: + break; + } + break; + + case QEMU_BLOCKJOB_STATE_FAILED: + case QEMU_BLOCKJOB_STATE_CANCELLED: + switch ((qemuBlockJobType) job->type) { + case QEMU_BLOCKJOB_TYPE_PULL: + case QEMU_BLOCKJOB_TYPE_COMMIT: + case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT: + case QEMU_BLOCKJOB_TYPE_COPY: + case QEMU_BLOCKJOB_TYPE_NONE: + case QEMU_BLOCKJOB_TYPE_INTERNAL: + case QEMU_BLOCKJOB_TYPE_LAST: + default: + break; + } + break; + + /* states below are impossible in this handler */ + case QEMU_BLOCKJOB_STATE_READY: + case QEMU_BLOCKJOB_STATE_NEW: + case QEMU_BLOCKJOB_STATE_RUNNING: + case QEMU_BLOCKJOB_STATE_CONCLUDED: + case QEMU_BLOCKJOB_STATE_LAST: + default: + break; + } + + qemuBlockJobEmitEvents(driver, vm, job->disk, job->type, job->newstate); + job->state = job->newstate; + job->newstate = -1; +} + + +static void +qemuBlockJobEventProcessConcluded(qemuBlockJobDataPtr job, + virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) +{ + qemuMonitorJobInfoPtr *jobinfo = NULL; + size_t njobinfo = 0; + size_t i; + int rc = 0; + bool dismissed = false; + bool refreshed = false; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + goto cleanup; + + /* we need to fetch the error state as the event does not propagate it */ + if (job->newstate == QEMU_BLOCKJOB_STATE_CONCLUDED && + (rc = qemuMonitorGetJobInfo(qemuDomainGetMonitor(vm), &jobinfo, &njobinfo)) == 0) { + + for (i = 0; i < njobinfo; i++) { + if (STRNEQ_NULLABLE(job->name, jobinfo[i]->id)) + continue; + + if (VIR_STRDUP(job->errmsg, jobinfo[i]->error) < 0) + rc = -1; + + if (job->errmsg) + job->newstate = QEMU_BLOCKJOB_STATE_FAILED; + else + job->newstate = QEMU_BLOCKJOB_STATE_COMPLETED; + + refreshed = true; + + break; + } + + if (i == njobinfo) { + VIR_WARN("failed to refresh job '%s'", job->name); + rc = -1; + } + } + + /* dismiss job in qemu */ + if (rc >= 0) { + if ((rc = qemuMonitorJobDismiss(qemuDomainGetMonitor(vm), job->name)) >= 0) + dismissed = true; + } + + if (job->invalidData) { + VIR_WARN("terminating job '%s' with invalid data", job->name); + goto cleanup; + } + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + goto cleanup; + + if (refreshed) + qemuDomainSaveStatus(vm); + + VIR_DEBUG("handling job '%s' state '%d' newstate '%d'", job->name, job->state, job->newstate); + + qemuBlockJobEventProcessConcludedTransition(job, driver, vm, asyncJob); + + cleanup: + if (dismissed) { + qemuBlockJobUnregister(job, vm); + job = NULL; + qemuDomainSaveConfig(vm); + } + + for (i = 0; i < njobinfo; i++) + qemuMonitorJobInfoFree(jobinfo[i]); + VIR_FREE(jobinfo); +} + + +static void +qemuBlockJobEventProcess(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuBlockJobDataPtr job, + qemuDomainAsyncJob asyncJob) + +{ + switch ((qemuBlockjobState) job->newstate) { + case QEMU_BLOCKJOB_STATE_COMPLETED: + case QEMU_BLOCKJOB_STATE_FAILED: + case QEMU_BLOCKJOB_STATE_CANCELLED: + case QEMU_BLOCKJOB_STATE_CONCLUDED: + qemuBlockJobEventProcessConcluded(job, driver, vm, asyncJob); + break; + + case QEMU_BLOCKJOB_STATE_READY: + if (job->disk && job->disk->mirror) { + job->disk->mirrorState = VIR_DOMAIN_BLOCK_JOB_READY; + qemuBlockJobEmitEvents(driver, vm, job->disk, job->type, job->newstate); + } + job->state = job->newstate; + job->newstate = -1; + qemuDomainSaveStatus(vm); + break; + + case QEMU_BLOCKJOB_STATE_NEW: + case QEMU_BLOCKJOB_STATE_RUNNING: + case QEMU_BLOCKJOB_STATE_LAST: + default: + job->newstate = -1; + } +} + + /** * qemuBlockJobUpdate: * @vm: domain @@ -444,7 +607,10 @@ qemuBlockJobUpdate(virDomainObjPtr vm, if (job->newstate == -1) return -1; - qemuBlockJobEventProcessLegacy(priv->driver, vm, job, asyncJob); + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) + qemuBlockJobEventProcess(priv->driver, vm, job, asyncJob); + else + qemuBlockJobEventProcessLegacy(priv->driver, vm, job, asyncJob); return job->state; } -- 2.21.0

Refresh the state of the jobs and process any events that might have happened while libvirt was not running. The job state processing requires some care to figure out if a job needs to be bumped. For any invalid job try doing our best to cancel it. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_blockjob.c | 109 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_blockjob.h | 5 ++ src/qemu/qemu_process.c | 7 ++- 3 files changed, 120 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 8b142f1aba..360fc40e61 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -242,6 +242,115 @@ qemuBlockJobIsRunning(qemuBlockJobDataPtr job) } +/* returns 1 for a job we didn't reconnect to */ +static int +qemuBlockJobRefreshJobsFindInactive(const void *payload, + const void *name ATTRIBUTE_UNUSED, + const void *data ATTRIBUTE_UNUSED) +{ + const qemuBlockJobData *job = payload; + + return !job->reconnected; +} + + +int +qemuBlockJobRefreshJobs(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + qemuMonitorJobInfoPtr *jobinfo = NULL; + size_t njobinfo = 0; + qemuBlockJobDataPtr job = NULL; + int newstate; + size_t i; + int ret = -1; + int rc; + + qemuDomainObjEnterMonitor(driver, vm); + + rc = qemuMonitorGetJobInfo(priv->mon, &jobinfo, &njobinfo); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + goto cleanup; + + for (i = 0; i < njobinfo; i++) { + if (!(job = virHashLookup(priv->blockjobs, jobinfo[i]->id))) { + VIR_DEBUG("ignoring untracked job '%s'", jobinfo[i]->id); + continue; + } + + /* try cancelling invalid jobs - this works only if the job is not + * concluded. In such case it will fail. We'll leave such job linger + * in qemu and just forget about it in libvirt because there's not much + * we coud do besides killing the VM */ + if (job->invalidData) { + qemuDomainObjEnterMonitor(driver, vm); + + rc = qemuMonitorJobCancel(priv->mon, job->name, true); + if (rc == -1 && jobinfo[i]->status == QEMU_MONITOR_JOB_STATUS_CONCLUDED) + VIR_WARN("can't cancel job '%s' with invalid data", job->name); + + if (qemuDomainObjExitMonitor(driver, vm) < 0) + goto cleanup; + + if (rc < 0) + qemuBlockJobUnregister(job, vm); + job = NULL; + continue; + } + + if ((newstate = qemuBlockjobConvertMonitorStatus(jobinfo[i]->status)) < 0) + continue; + + if (newstate != job->state) { + if ((job->state == QEMU_BLOCKJOB_STATE_FAILED || + job->state == QEMU_BLOCKJOB_STATE_COMPLETED)) { + /* preserve the old state but allow the job to be bumped to + * execute the finishing steps */ + job->newstate = job->state; + } else if (newstate == QEMU_BLOCKJOB_STATE_CONCLUDED) { + if (VIR_STRDUP(job->errmsg, jobinfo[i]->error) < 0) + goto cleanup; + + if (job->errmsg) + job->newstate = QEMU_BLOCKJOB_STATE_FAILED; + else + job->newstate = QEMU_BLOCKJOB_STATE_COMPLETED; + } else if (newstate == QEMU_BLOCKJOB_STATE_READY) { + /* Apply _READY state only if it was not applied before */ + if (job->state == QEMU_BLOCKJOB_STATE_NEW || + job->state == QEMU_BLOCKJOB_STATE_RUNNING) + job->newstate = newstate; + } + /* don't update the job otherwise */ + } + + job->reconnected = true; + + if (job->newstate != -1) + qemuBlockJobUpdate(vm, job, QEMU_ASYNC_JOB_NONE); + job = NULL; /* job may have become invalid here */ + } + + /* remove data for job which qemu didn't report (the algorithm is + * inefficient, but the possibility of such jobs is very low */ + while ((job = virHashSearch(priv->blockjobs, qemuBlockJobRefreshJobsFindInactive, NULL, NULL))) { + qemuBlockJobUnregister(job, vm); + job = NULL; + } + + ret = 0; + + cleanup: + for (i = 0; i < njobinfo; i++) + qemuMonitorJobInfoFree(jobinfo[i]); + VIR_FREE(jobinfo); + + return ret; +} + + /** * qemuBlockJobEmitEvents: * diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index a558b0a5a2..2d8ecdd4c3 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -84,6 +84,7 @@ struct _qemuBlockJobData { int newstate; /* qemuBlockjobState, subset of events emitted by qemu */ bool invalidData; /* the job data (except name) is not valid */ + bool reconnected; /* internal field for tracking whether job is live after reconnect to qemu */ }; int @@ -122,6 +123,10 @@ void qemuBlockJobStartupFinalize(virDomainObjPtr vm, qemuBlockJobDataPtr job); +int +qemuBlockJobRefreshJobs(virQEMUDriverPtr driver, + virDomainObjPtr vm); + int qemuBlockJobUpdate(virDomainObjPtr vm, qemuBlockJobDataPtr job, int asyncJob); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 416f4f5c9a..3afdaafb23 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7953,7 +7953,12 @@ static int qemuProcessRefreshBlockjobs(virQEMUDriverPtr driver, virDomainObjPtr vm) { - return qemuProcessRefreshLegacyBlockjobs(driver, vm); + qemuDomainObjPrivatePtr priv = vm->privateData; + + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) + return qemuBlockJobRefreshJobs(driver, vm); + else + return qemuProcessRefreshLegacyBlockjobs(driver, vm); } -- 2.21.0

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 7 +++++++ src/libvirt_private.syms | 1 + 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3323c9a5b1..50a01d4e03 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9326,7 +9326,7 @@ virDomainStorageSourceParse(xmlNodePtr node, } -static int +int virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, virStorageSourcePtr src, unsigned int flags, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c1b5fc1337..d126e24a3e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3523,6 +3523,13 @@ int virDomainStorageSourceParse(xmlNodePtr node, virDomainXMLOptionPtr xmlopt) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +int +virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, + virStorageSourcePtr src, + unsigned int flags, + virDomainXMLOptionPtr xmlopt) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + int virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def, int maplen, int ncpumaps, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 02d5b7acce..733aa9b570 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -311,6 +311,7 @@ virDomainDeviceInfoIterate; virDomainDeviceSetData; virDomainDeviceTypeToString; virDomainDeviceValidateAliasForHotplug; +virDomainDiskBackingStoreParse; virDomainDiskBusTypeToString; virDomainDiskByAddress; virDomainDiskByName; -- 2.21.0

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 6 ++++++ src/libvirt_private.syms | 1 + 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 50a01d4e03..fc0b07cf5a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -24054,7 +24054,7 @@ virDomainDiskSourceFormat(virBufferPtr buf, } -static int +int virDomainDiskBackingStoreFormat(virBufferPtr buf, virStorageSourcePtr src, virDomainXMLOptionPtr xmlopt, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d126e24a3e..6031cc11ae 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3072,6 +3072,12 @@ int virDomainDiskSourceFormat(virBufferPtr buf, unsigned int flags, virDomainXMLOptionPtr xmlopt); +int +virDomainDiskBackingStoreFormat(virBufferPtr buf, + virStorageSourcePtr src, + virDomainXMLOptionPtr xmlopt, + unsigned int flags); + int virDomainNetDefFormat(virBufferPtr buf, virDomainNetDefPtr def, char *prefix, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 733aa9b570..b01aa8fb85 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -311,6 +311,7 @@ virDomainDeviceInfoIterate; virDomainDeviceSetData; virDomainDeviceTypeToString; virDomainDeviceValidateAliasForHotplug; +virDomainDiskBackingStoreFormat; virDomainDiskBackingStoreParse; virDomainDiskBusTypeToString; virDomainDiskByAddress; -- 2.21.0

When the guest unplugs the disk frontend libvirt is responsible for deleting the backend. Since a blockjob may still have a reference to the backing chain when it is running we'll have to store the metadata for the unplugged disk for future reference. This patch adds 'chain' and 'mirrorChain' fields to 'qemuBlockJobData' to keep them around with the job along with status XML machinery and tests. Later patches will then add code to change the ownership of the chain when unplugging the disk backend. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_blockjob.c | 5 + src/qemu/qemu_blockjob.h | 2 + src/qemu/qemu_domain.c | 124 ++++++++++++++++-- .../blockjob-blockdev-in.xml | 37 ++++++ 4 files changed, 160 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 360fc40e61..70b223b29d 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -61,6 +61,9 @@ qemuBlockJobDataDispose(void *obj) { qemuBlockJobDataPtr job = obj; + virObjectUnref(job->chain); + virObjectUnref(job->mirrorChain); + VIR_FREE(job->name); VIR_FREE(job->errmsg); } @@ -116,6 +119,8 @@ qemuBlockJobRegister(qemuBlockJobDataPtr job, if (disk) { job->disk = disk; + job->chain = virObjectRef(disk->src); + job->mirrorChain = virObjectRef(disk->mirror); QEMU_DOMAIN_DISK_PRIVATE(disk)->blockjob = virObjectRef(job); } diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h index 2d8ecdd4c3..d07ab75c8b 100644 --- a/src/qemu/qemu_blockjob.h +++ b/src/qemu/qemu_blockjob.h @@ -75,6 +75,8 @@ struct _qemuBlockJobData { char *name; virDomainDiskDefPtr disk; /* may be NULL, if blockjob does not correspond to any disk */ + virStorageSourcePtr chain; /* Reference to the chain the job operates on. */ + virStorageSourcePtr mirrorChain; /* reference to 'mirror' part of the job */ int type; /* qemuBlockJobType */ int state; /* qemuBlockjobState */ diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 13a90ab60b..48b99e5511 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2313,22 +2313,59 @@ qemuDomainObjPrivateXMLFormatAutomaticPlacement(virBufferPtr buf, } +typedef struct qemuDomainPrivateBlockJobFormatData { + virDomainXMLOptionPtr xmlopt; + virBufferPtr buf; +} qemuDomainPrivateBlockJobFormatData; + + +static int +qemuDomainObjPrivateXMLFormatBlockjobFormatChain(virBufferPtr buf, + const char *chainname, + virStorageSourcePtr src, + virDomainXMLOptionPtr xmlopt) +{ + VIR_AUTOCLEAN(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + VIR_AUTOCLEAN(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; + unsigned int xmlflags = VIR_DOMAIN_DEF_FORMAT_STATUS; + + virBufferSetChildIndent(&childBuf, buf); + + virBufferAsprintf(&attrBuf, " type='%s' format='%s'", + virStorageTypeToString(src->type), + virStorageFileFormatTypeToString(src->format)); + + if (virDomainDiskSourceFormat(&childBuf, src, "source", 0, true, xmlflags, xmlopt) < 0) + return -1; + + if (virDomainDiskBackingStoreFormat(&childBuf, src, xmlopt, xmlflags) < 0) + return -1; + + if (virXMLFormatElement(buf, chainname, &attrBuf, &childBuf) < 0) + return -1; + + return 0; +} + + static int qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload, const void *name ATTRIBUTE_UNUSED, - void *data) + void *opaque) { VIR_AUTOCLEAN(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; VIR_AUTOCLEAN(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; + VIR_AUTOCLEAN(virBuffer) chainsBuf = VIR_BUFFER_INITIALIZER; qemuBlockJobDataPtr job = payload; - virBufferPtr buf = data; const char *state = qemuBlockjobStateTypeToString(job->state); const char *newstate = NULL; + struct qemuDomainPrivateBlockJobFormatData *data = opaque; if (job->newstate != -1) newstate = qemuBlockjobStateTypeToString(job->newstate); - virBufferSetChildIndent(&childBuf, buf); + virBufferSetChildIndent(&childBuf, data->buf); + virBufferSetChildIndent(&chainsBuf, &childBuf); virBufferEscapeString(&attrBuf, " name='%s'", job->name); virBufferEscapeString(&attrBuf, " type='%s'", qemuBlockjobTypeToString(job->type)); @@ -2336,10 +2373,28 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void *payload, virBufferEscapeString(&attrBuf, " newstate='%s'", newstate); virBufferEscapeString(&childBuf, "<errmsg>%s</errmsg>", job->errmsg); - if (job->disk) + if (job->disk) { virBufferEscapeString(&childBuf, "<disk dst='%s'/>\n", job->disk->dst); + } else { + if (job->chain && + qemuDomainObjPrivateXMLFormatBlockjobFormatChain(&chainsBuf, + "disk", + job->chain, + data->xmlopt) < 0) + return -1; + + if (job->mirrorChain && + qemuDomainObjPrivateXMLFormatBlockjobFormatChain(&chainsBuf, + "mirror", + job->mirrorChain, + data->xmlopt) < 0) + return -1; + + if (virXMLFormatElement(&childBuf, "chains", NULL, &chainsBuf) < 0) + return -1; + } - return virXMLFormatElement(buf, "blockjob", &attrBuf, &childBuf); + return virXMLFormatElement(data->buf, "blockjob", &attrBuf, &childBuf); } @@ -2351,6 +2406,8 @@ qemuDomainObjPrivateXMLFormatBlockjobs(virBufferPtr buf, VIR_AUTOCLEAN(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; VIR_AUTOCLEAN(virBuffer) childBuf = VIR_BUFFER_INITIALIZER; bool bj = qemuDomainHasBlockjob(vm, false); + struct qemuDomainPrivateBlockJobFormatData iterdata = { priv->driver->xmlopt, + &childBuf }; virBufferAsprintf(&attrBuf, " active='%s'", virTristateBoolTypeToString(virTristateBoolFromBool(bj))); @@ -2360,7 +2417,7 @@ qemuDomainObjPrivateXMLFormatBlockjobs(virBufferPtr buf, if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV) && virHashForEach(priv->blockjobs, qemuDomainObjPrivateXMLFormatBlockjobIterator, - &childBuf) < 0) + &iterdata) < 0) return -1; return virXMLFormatElement(buf, "blockjobs", &attrBuf, &childBuf); @@ -2702,10 +2759,49 @@ qemuDomainObjPrivateXMLParseAutomaticPlacement(xmlXPathContextPtr ctxt, } +static virStorageSourcePtr +qemuDomainObjPrivateXMLParseBlockjobChain(xmlNodePtr node, + xmlXPathContextPtr ctxt, + virDomainXMLOptionPtr xmlopt) + +{ + VIR_XPATH_NODE_AUTORESTORE(ctxt); + VIR_AUTOFREE(char *) format = NULL; + VIR_AUTOFREE(char *) type = NULL; + VIR_AUTOFREE(char *) index = NULL; + VIR_AUTOUNREF(virStorageSourcePtr) src = NULL; + xmlNodePtr sourceNode; + unsigned int xmlflags = VIR_DOMAIN_DEF_PARSE_STATUS; + + ctxt->node = node; + + if (!(type = virXMLPropString(ctxt->node, "type")) || + !(format = virXMLPropString(ctxt->node, "format")) || + !(index = virXPathString("string(./source/@index)", ctxt)) || + !(sourceNode = virXPathNode("./source", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing job chain data")); + return NULL; + } + + if (!(src = virDomainStorageSourceParseBase(type, format, index))) + return NULL; + + if (virDomainStorageSourceParse(sourceNode, ctxt, src, xmlflags, xmlopt) < 0) + return NULL; + + if (virDomainDiskBackingStoreParse(ctxt, src, xmlflags, xmlopt) < 0) + return NULL; + + VIR_RETURN_PTR(src); +} + + static int qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr vm, xmlNodePtr node, - xmlXPathContextPtr ctxt) + xmlXPathContextPtr ctxt, + virDomainXMLOptionPtr xmlopt) { VIR_XPATH_NODE_AUTORESTORE(ctxt); virDomainDiskDefPtr disk = NULL; @@ -2719,6 +2815,7 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr vm, VIR_AUTOFREE(char *) newstatestr = NULL; int newstate = -1; bool invalidData = false; + xmlNodePtr tmp; ctxt->node = node; @@ -2750,6 +2847,16 @@ qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr vm, !(disk = virDomainDiskByName(vm->def, diskdst, false))) invalidData = true; + if (!disk && !invalidData) { + if ((tmp = virXPathNode("./chains/disk", ctxt)) && + !(job->chain = qemuDomainObjPrivateXMLParseBlockjobChain(tmp, ctxt, xmlopt))) + invalidData = true; + + if ((tmp = virXPathNode("./chains/mirror", ctxt)) && + !(job->mirrorChain = qemuDomainObjPrivateXMLParseBlockjobChain(tmp, ctxt, xmlopt))) + invalidData = true; + } + job->state = state; job->newstate = newstate; job->errmsg = virXPathString("string(./errmsg)", ctxt); @@ -2782,7 +2889,8 @@ qemuDomainObjPrivateXMLParseBlockjobs(virDomainObjPtr vm, return -1; for (i = 0; i < nnodes; i++) { - if (qemuDomainObjPrivateXMLParseBlockjobData(vm, nodes[i], ctxt) < 0) + if (qemuDomainObjPrivateXMLParseBlockjobData(vm, nodes[i], ctxt, + priv->driver->xmlopt) < 0) return -1; } } diff --git a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml index 115eaa4887..538385000c 100644 --- a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml +++ b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml @@ -239,6 +239,43 @@ <blockjob name='drive-virtio-disk0' type='copy' state='ready'> <disk dst='vda'/> </blockjob> + <blockjob name='test-orphan-job0' type='copy' state='ready'> + <chains> + <disk type='file' format='qcow2'> + <source file='/orphan/source/top.qcow2' index='123'> + <privateData> + <nodenames> + <nodename type='storage' name='orphan-source-top-storage'/> + <nodename type='format' name='orphan-source-top-format'/> + </nodenames> + </privateData> + </source> + <backingStore type='file' index='321'> + <format type='qcow2'/> + <source file='/orphan/source/base.qcow2'> + <privateData> + <nodenames> + <nodename type='storage' name='orphan-source-base-storage'/> + <nodename type='format' name='orphan-source-base-format'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </backingStore> + </disk> + <mirror type='file' format='raw'> + <source file='/orphan/mirror/raw' index='42'> + <privateData> + <nodenames> + <nodename type='storage' name='orphan-mirror-raw-storage'/> + <nodename type='format' name='orphan-mirror-raw-format'/> + </nodenames> + </privateData> + </source> + <backingStore/> + </mirror> + </chains> + </blockjob> </blockjobs> <domain type='kvm' id='4'> <name>copy</name> -- 2.21.0

The PR manager is a property of the format layer in qemu so we need to be able to track it also in the chains of orphaned block jobs. Add a helper for qemu to look also into the blockjob state. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 41 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 2 ++ src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 2 +- 5 files changed, 46 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 48b99e5511..3a5bd2f921 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -14740,3 +14740,44 @@ qemuDomainPausedReasonToSuspendedEvent(virDomainPausedReason reason) return VIR_DOMAIN_EVENT_SUSPENDED_PAUSED; } + + +static int +qemuDomainDefHasManagedPRBlockjobIterator(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) +{ + qemuBlockJobDataPtr job = payload; + bool *hasPR = opaque; + + if (job->disk) + return 0; + + if ((job->chain && virStorageSourceChainHasManagedPR(job->chain)) || + (job->mirrorChain && virStorageSourceChainHasManagedPR(job->mirrorChain))) + *hasPR = true; + + return 0; +} + + +/** + * qemuDomainDefHasManagedPR: + * @vm: domain object + * + * @vm must be an active VM. Returns true if @vm has any storage source with + * managed persistent reservations. + */ +bool +qemuDomainDefHasManagedPR(virDomainObjPtr vm) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + bool jobPR = false; + + if (virDomainDefHasManagedPR(vm->def)) + return true; + + virHashForEach(priv->blockjobs, qemuDomainDefHasManagedPRBlockjobIterator, &jobPR); + + return jobPR; +} diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 3ccea3177e..11dad90c81 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1141,6 +1141,8 @@ qemuDomainDiskCachemodeFlags(int cachemode, char * qemuDomainGetManagedPRSocketPath(qemuDomainObjPrivatePtr priv); +bool qemuDomainDefHasManagedPR(virDomainObjPtr vm); + unsigned int qemuDomainStorageIdNew(qemuDomainObjPrivatePtr priv); void qemuDomainStorageIdReset(qemuDomainObjPrivatePtr priv); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 604beca155..238d59466c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4792,7 +4792,7 @@ processPRDisconnectEvent(virDomainObjPtr vm) return; if (!priv->prDaemonRunning && - virDomainDefHasManagedPR(vm->def)) + qemuDomainDefHasManagedPR(vm)) qemuProcessStartManagedPRDaemon(vm); } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 7e9c1a1649..c50e41144e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -397,7 +397,7 @@ qemuHotplugRemoveManagedPR(virQEMUDriverPtr driver, virErrorPtr orig_err; int ret = -1; - if (virDomainDefHasManagedPR(vm->def)) + if (qemuDomainDefHasManagedPR(vm)) return 0; virErrorPreserveLast(&orig_err); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3afdaafb23..aaff1beeaa 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2309,7 +2309,7 @@ qemuRefreshPRManagerState(virQEMUDriverPtr driver, int ret = -1; if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER) || - !virDomainDefHasManagedPR(vm->def)) + !qemuDomainDefHasManagedPR(vm)) return 0; qemuDomainObjEnterMonitor(driver, vm); -- 2.21.0

In cases when the disk frontend was unplugged while a blockjob was running the blockjob inherits the backing chain. When the blockjob is then terminated we need to unplug the chain as it will not be used any more. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_blockjob.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 70b223b29d..8a1cf6893b 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -537,6 +537,28 @@ qemuBlockJobEventProcessLegacy(virQEMUDriverPtr driver, } +static void +qemuBlockJobEventProcessConcludedRemoveChain(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob, + virStorageSourcePtr chain) +{ + VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; + + if (!(data = qemuBlockStorageSourceChainDetachPrepareBlockdev(chain))) + return; + + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return; + + qemuBlockStorageSourceChainDetach(qemuDomainGetMonitor(vm), data); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + return; + + qemuDomainStorageSourceChainAccessRevoke(driver, vm, chain); +} + + static void qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job, virQEMUDriverPtr driver, @@ -653,6 +675,16 @@ qemuBlockJobEventProcessConcluded(qemuBlockJobDataPtr job, qemuBlockJobEventProcessConcludedTransition(job, driver, vm, asyncJob); + /* unplug the backing chains in case the job inherited them */ + if (!job->disk) { + if (job->chain) + qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, + job->chain); + if (job->mirrorChain) + qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, + job->mirrorChain); + } + cleanup: if (dismissed) { qemuBlockJobUnregister(job, vm); -- 2.21.0

When removing the disk fronted while any block job is still active we need to transfer the ownership of the backing chain to the job itself as the job still holds the reference to the chain members and thus attempts to remove them would fail. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_hotplug.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c50e41144e..7501ae5029 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -4228,8 +4228,15 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, if (VIR_STRDUP(corAlias, diskPriv->nodeCopyOnRead) < 0) goto cleanup; - if (!(diskBackend = qemuBlockStorageSourceChainDetachPrepareBlockdev(disk->src))) - goto cleanup; + if (diskPriv->blockjob) { + /* the block job keeps reference to the disk chain */ + diskPriv->blockjob->disk = NULL; + virObjectUnref(diskPriv->blockjob); + diskPriv->blockjob = NULL; + } else { + if (!(diskBackend = qemuBlockStorageSourceChainDetachPrepareBlockdev(disk->src))) + goto cleanup; + } } else { char *driveAlias; @@ -4252,7 +4259,8 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, if (corAlias) ignore_value(qemuMonitorDelObject(priv->mon, corAlias)); - qemuBlockStorageSourceChainDetach(priv->mon, diskBackend); + if (diskBackend) + qemuBlockStorageSourceChainDetach(priv->mon, diskBackend); if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; @@ -4262,7 +4270,8 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, qemuDomainReleaseDeviceAddress(vm, &disk->info); /* tear down disk security access */ - qemuDomainStorageSourceChainAccessRevoke(driver, vm, disk->src); + if (diskBackend) + qemuDomainStorageSourceChainAccessRevoke(driver, vm, disk->src); dev.type = VIR_DOMAIN_DEVICE_DISK; dev.data.disk = disk; -- 2.21.0