[PATCH 0/5] backup: Preserve error of failed backup job

Aid in problem identification by storing the error for users as it wasn't accessible besides from logs. Peter Krempa (5): remote: remoteDispatchDomainGetJobStats: Encode typed parameter strings qemu: Add free and copy function for qemuDomainJobInfo and use it API: Add VIR_DOMAIN_JOB_ERRMSG domain job statistics field qemu: domain: Add 'errmsg' field to qemuDomainJobInfo backup: Store error message for failed backups include/libvirt/libvirt-domain.h | 9 +++++++ src/conf/backup_conf.c | 1 + src/conf/backup_conf.h | 2 ++ src/qemu/qemu_backup.c | 15 ++++++++---- src/qemu/qemu_backup.h | 1 + src/qemu/qemu_blockjob.c | 2 +- src/qemu/qemu_domain.c | 33 +++++++++++++++++++++---- src/qemu/qemu_domain.h | 10 ++++++++ src/qemu/qemu_driver.c | 37 +++++++++++++++-------------- src/qemu/qemu_migration.c | 16 ++++++------- src/qemu/qemu_migration_cookie.c | 10 ++++---- src/remote/remote_daemon_dispatch.c | 2 +- tools/virsh-domain.c | 8 +++++++ 13 files changed, 101 insertions(+), 45 deletions(-) -- 2.26.0

String typed parameter values were introduced in v0.9.7-30-g40624d32fb. virDomainGetJobStats was introduced in v1.0.2-239-g4dd00f4238 so all clients already support typed parameter stings at that time thus we can enable it unconditionally. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/remote/remote_daemon_dispatch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index c5506c2e11..5d1c6971c0 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -5505,7 +5505,7 @@ remoteDispatchDomainGetJobStats(virNetServerPtr server G_GNUC_UNUSED, REMOTE_DOMAIN_JOB_STATS_MAX, (virTypedParameterRemotePtr *) &ret->params.params_val, &ret->params.params_len, - 0) < 0) + VIR_TYPED_PARAM_STRING_OKAY) < 0) goto cleanup; rv = 0; -- 2.26.0

On 4/16/20 4:55 AM, Peter Krempa wrote:
String typed parameter values were introduced in v0.9.7-30-g40624d32fb. virDomainGetJobStats was introduced in v1.0.2-239-g4dd00f4238 so all clients already support typed parameter stings at that time thus we can enable it unconditionally.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/remote/remote_daemon_dispatch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

In order to add a string to qemuDomainJobInfo we must ensure that it's freed and copied properly. Add helpers to copy and free the structure and adjust the code to use them properly for the new semantics. Additionally also allocation is changed to g_new0 as it includes the type and thus it's very easy to grep for all the allocations of a given type. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_backup.c | 5 ++--- src/qemu/qemu_domain.c | 26 +++++++++++++++++----- src/qemu/qemu_domain.h | 8 +++++++ src/qemu/qemu_driver.c | 37 ++++++++++++++++---------------- src/qemu/qemu_migration.c | 16 ++++++-------- src/qemu/qemu_migration_cookie.c | 10 ++++----- 6 files changed, 61 insertions(+), 41 deletions(-) diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index 5d18720f53..03d34c9378 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -641,9 +641,8 @@ qemuBackupJobTerminate(virDomainObjPtr vm, qemuDomainJobInfoUpdateTime(priv->job.current); - g_free(priv->job.completed); - priv->job.completed = g_new0(qemuDomainJobInfo, 1); - *priv->job.completed = *priv->job.current; + g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree); + priv->job.completed = qemuDomainJobInfoCopy(priv->job.current); priv->job.completed->stats.backup.total = priv->backup->push_total; priv->job.completed->stats.backup.transferred = priv->backup->push_transferred; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 91e234d644..13b1c4e402 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -305,6 +305,23 @@ qemuDomainDisableNamespace(virDomainObjPtr vm, } +void +qemuDomainJobInfoFree(qemuDomainJobInfoPtr info) +{ + g_free(info); +} + + +qemuDomainJobInfoPtr +qemuDomainJobInfoCopy(qemuDomainJobInfoPtr info) +{ + qemuDomainJobInfoPtr ret = g_new0(qemuDomainJobInfo, 1); + + memcpy(ret, info, sizeof(*info)); + + return ret; +} + void qemuDomainEventEmitJobCompleted(virQEMUDriverPtr driver, virDomainObjPtr vm) @@ -385,7 +402,7 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv) job->spiceMigrated = false; job->dumpCompleted = false; VIR_FREE(job->error); - VIR_FREE(job->current); + g_clear_pointer(&job->current, qemuDomainJobInfoFree); qemuMigrationParamsFree(job->migParams); job->migParams = NULL; job->apiFlags = 0; @@ -415,8 +432,8 @@ qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv) { qemuDomainObjResetJob(priv); qemuDomainObjResetAsyncJob(priv); - VIR_FREE(priv->job.current); - VIR_FREE(priv->job.completed); + g_clear_pointer(&priv->job.current, qemuDomainJobInfoFree); + g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree); virCondDestroy(&priv->job.cond); virCondDestroy(&priv->job.asyncCond); } @@ -6291,8 +6308,7 @@ qemuDomainObjBeginJobInternal(virQEMUDriverPtr driver, qemuDomainAsyncJobTypeToString(asyncJob), obj, obj->def->name); qemuDomainObjResetAsyncJob(priv); - if (VIR_ALLOC(priv->job.current) < 0) - goto cleanup; + priv->job.current = g_new0(qemuDomainJobInfo, 1); priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_ACTIVE; priv->job.asyncJob = asyncJob; priv->job.asyncOwner = virThreadSelfID(); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index cf19f4d101..c7f28b04c2 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -177,6 +177,14 @@ struct _qemuDomainJobInfo { qemuDomainMirrorStats mirrorStats; }; +void +qemuDomainJobInfoFree(qemuDomainJobInfoPtr info); + +G_DEFINE_AUTOPTR_CLEANUP_FUNC(qemuDomainJobInfo, qemuDomainJobInfoFree); + +qemuDomainJobInfoPtr +qemuDomainJobInfoCopy(qemuDomainJobInfoPtr info); + typedef struct _qemuDomainJobObj qemuDomainJobObj; typedef qemuDomainJobObj *qemuDomainJobObjPtr; struct _qemuDomainJobObj { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 31f199fdef..63e92b84c6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3746,7 +3746,7 @@ qemuDumpToFd(virQEMUDriverPtr driver, if (detach) priv->job.current->statsType = QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP; else - VIR_FREE(priv->job.current); + g_clear_pointer(&priv->job.current, qemuDomainJobInfoFree); if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1; @@ -13598,16 +13598,16 @@ static int qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, virDomainObjPtr vm, bool completed, - qemuDomainJobInfoPtr jobInfo) + qemuDomainJobInfoPtr *jobInfo) { qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; + *jobInfo = NULL; + if (completed) { if (priv->job.completed && !priv->job.current) - *jobInfo = *priv->job.completed; - else - jobInfo->status = QEMU_DOMAIN_JOB_STATUS_NONE; + *jobInfo = qemuDomainJobInfoCopy(priv->job.completed); return 0; } @@ -13626,26 +13626,25 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver, goto cleanup; if (!priv->job.current) { - jobInfo->status = QEMU_DOMAIN_JOB_STATUS_NONE; ret = 0; goto cleanup; } - *jobInfo = *priv->job.current; + *jobInfo = qemuDomainJobInfoCopy(priv->job.current); - switch (jobInfo->statsType) { + switch ((*jobInfo)->statsType) { case QEMU_DOMAIN_JOB_STATS_TYPE_MIGRATION: case QEMU_DOMAIN_JOB_STATS_TYPE_SAVEDUMP: - if (qemuDomainGetJobInfoMigrationStats(driver, vm, jobInfo) < 0) + if (qemuDomainGetJobInfoMigrationStats(driver, vm, *jobInfo) < 0) goto cleanup; break; case QEMU_DOMAIN_JOB_STATS_TYPE_MEMDUMP: - if (qemuDomainGetJobInfoDumpStats(driver, vm, jobInfo) < 0) + if (qemuDomainGetJobInfoDumpStats(driver, vm, *jobInfo) < 0) goto cleanup; break; case QEMU_DOMAIN_JOB_STATS_TYPE_BACKUP: - if (qemuBackupGetJobInfoStats(driver, vm, jobInfo) < 0) + if (qemuBackupGetJobInfoStats(driver, vm, *jobInfo) < 0) goto cleanup; break; @@ -13666,7 +13665,7 @@ qemuDomainGetJobInfo(virDomainPtr dom, virDomainJobInfoPtr info) { virQEMUDriverPtr driver = dom->conn->privateData; - qemuDomainJobInfo jobInfo; + g_autoptr(qemuDomainJobInfo) jobInfo = NULL; virDomainObjPtr vm; int ret = -1; @@ -13681,12 +13680,13 @@ qemuDomainGetJobInfo(virDomainPtr dom, if (qemuDomainGetJobStatsInternal(driver, vm, false, &jobInfo) < 0) goto cleanup; - if (jobInfo.status == QEMU_DOMAIN_JOB_STATUS_NONE) { + if (!jobInfo || + jobInfo->status == QEMU_DOMAIN_JOB_STATUS_NONE) { ret = 0; goto cleanup; } - ret = qemuDomainJobInfoToInfo(&jobInfo, info); + ret = qemuDomainJobInfoToInfo(jobInfo, info); cleanup: virDomainObjEndAPI(&vm); @@ -13704,7 +13704,7 @@ qemuDomainGetJobStats(virDomainPtr dom, virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; qemuDomainObjPrivatePtr priv; - qemuDomainJobInfo jobInfo; + g_autoptr(qemuDomainJobInfo) jobInfo = NULL; bool completed = !!(flags & VIR_DOMAIN_JOB_STATS_COMPLETED); int ret = -1; @@ -13721,7 +13721,8 @@ qemuDomainGetJobStats(virDomainPtr dom, if (qemuDomainGetJobStatsInternal(driver, vm, completed, &jobInfo) < 0) goto cleanup; - if (jobInfo.status == QEMU_DOMAIN_JOB_STATUS_NONE) { + if (!jobInfo || + jobInfo->status == QEMU_DOMAIN_JOB_STATUS_NONE) { *type = VIR_DOMAIN_JOB_NONE; *params = NULL; *nparams = 0; @@ -13729,10 +13730,10 @@ qemuDomainGetJobStats(virDomainPtr dom, goto cleanup; } - ret = qemuDomainJobInfoToParams(&jobInfo, type, params, nparams); + ret = qemuDomainJobInfoToParams(jobInfo, type, params, nparams); if (completed && ret == 0 && !(flags & VIR_DOMAIN_JOB_STATS_KEEP_COMPLETED)) - VIR_FREE(priv->job.completed); + g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree); cleanup: virDomainObjEndAPI(&vm); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index bc280e856a..02e8271e42 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1724,11 +1724,9 @@ qemuMigrationSrcWaitForCompletion(virQEMUDriverPtr driver, qemuDomainJobInfoUpdateTime(jobInfo); qemuDomainJobInfoUpdateDowntime(jobInfo); - VIR_FREE(priv->job.completed); - if (VIR_ALLOC(priv->job.completed) == 0) { - *priv->job.completed = *jobInfo; - priv->job.completed->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED; - } + g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree); + priv->job.completed = qemuDomainJobInfoCopy(jobInfo); + priv->job.completed->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED; if (asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT && jobInfo->status == QEMU_DOMAIN_JOB_STATUS_QEMU_COMPLETED) @@ -3017,7 +3015,7 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver, if (retcode == 0) jobInfo = priv->job.completed; else - VIR_FREE(priv->job.completed); + g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree); /* Update times with the values sent by the destination daemon */ if (mig->jobInfo && jobInfo) { @@ -5036,7 +5034,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver, : QEMU_MIGRATION_PHASE_FINISH2); qemuDomainCleanupRemove(vm, qemuMigrationDstPrepareCleanup); - VIR_FREE(priv->job.completed); + g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree); cookie_flags = QEMU_MIGRATION_COOKIE_NETWORK | QEMU_MIGRATION_COOKIE_STATS | @@ -5257,7 +5255,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver, * is obsolete anyway. */ if (inPostCopy) - VIR_FREE(priv->job.completed); + g_clear_pointer(&priv->job.completed, qemuDomainJobInfoFree); } qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN, @@ -5268,7 +5266,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver, qemuDomainRemoveInactiveJob(driver, vm); cleanup: - VIR_FREE(jobInfo); + g_clear_pointer(&jobInfo, qemuDomainJobInfoFree); virPortAllocatorRelease(port); if (priv->mon) qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL); diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index 1d88ac1d22..fb8b5bcd92 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -123,7 +123,7 @@ qemuMigrationCookieFree(qemuMigrationCookiePtr mig) VIR_FREE(mig->name); VIR_FREE(mig->lockState); VIR_FREE(mig->lockDriver); - VIR_FREE(mig->jobInfo); + g_clear_pointer(&mig->jobInfo, qemuDomainJobInfoFree); virCPUDefFree(mig->cpu); qemuMigrationCookieCapsFree(mig->caps); VIR_FREE(mig); @@ -513,10 +513,9 @@ qemuMigrationCookieAddStatistics(qemuMigrationCookiePtr mig, if (!priv->job.completed) return 0; - if (!mig->jobInfo && VIR_ALLOC(mig->jobInfo) < 0) - return -1; + g_clear_pointer(&mig->jobInfo, qemuDomainJobInfoFree); + mig->jobInfo = qemuDomainJobInfoCopy(priv->job.completed); - *mig->jobInfo = *priv->job.completed; mig->flags |= QEMU_MIGRATION_COOKIE_STATS; return 0; @@ -1051,8 +1050,7 @@ qemuMigrationCookieStatisticsXMLParse(xmlXPathContextPtr ctxt) if (!(ctxt->node = virXPathNode("./statistics", ctxt))) return NULL; - if (VIR_ALLOC(jobInfo) < 0) - return NULL; + jobInfo = g_new0(qemuDomainJobInfo, 1); stats = &jobInfo->stats.mig; jobInfo->status = QEMU_DOMAIN_JOB_STATUS_COMPLETED; -- 2.26.0

On 4/16/20 4:55 AM, Peter Krempa wrote:
In order to add a string to qemuDomainJobInfo we must ensure that it's freed and copied properly. Add helpers to copy and free the structure and adjust the code to use them properly for the new semantics.
Additionally also allocation is changed to g_new0 as it includes the type and thus it's very easy to grep for all the allocations of a given type.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> ---
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

In some cases it's useful to report the error which caused the domain job to fail. Add an optional field for holding the error message so that it can be later retrieved from statistics of a completed job. Add the field name macro and code for extracting it in virsh. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- include/libvirt/libvirt-domain.h | 9 +++++++++ tools/virsh-domain.c | 8 ++++++++ 2 files changed, 17 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index b440818ec2..f129e6a1af 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -3612,6 +3612,15 @@ typedef enum { */ # define VIR_DOMAIN_JOB_SUCCESS "success" +/** + * VIR_DOMAIN_JOB_ERRMSG: + * + * virDomainGetJobStats field: Present only in statistics for a completed job. + * Optional error message for a failed job. + */ +# define VIR_DOMAIN_JOB_ERRMSG "errmsg" + + /** * VIR_DOMAIN_JOB_DISK_TEMP_USED: * virDomainGetJobStats field: current usage of temporary disk space for the diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index d52eb7bc2f..f2dc41cb31 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6228,6 +6228,7 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd) unsigned long long value; unsigned int flags = 0; int ivalue; + const char *svalue; int op; int rc; size_t i; @@ -6508,6 +6509,13 @@ cmdDomjobinfo(vshControl *ctl, const vshCmd *cmd) vshPrint(ctl, "%-17s %-.3lf %s\n", _("Temporary disk space total:"), val, unit); } + if ((rc = virTypedParamsGetString(params, nparams, VIR_DOMAIN_JOB_ERRMSG, + &svalue)) < 0) { + goto save_error; + } else if (rc == 1) { + vshPrint(ctl, "%-17s %s\n", _("Error message:"), svalue); + } + ret = true; cleanup: -- 2.26.0

On 4/16/20 4:55 AM, Peter Krempa wrote:
In some cases it's useful to report the error which caused the domain job to fail. Add an optional field for holding the error message so that it can be later retrieved from statistics of a completed job.
Add the field name macro and code for extracting it in virsh.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- include/libvirt/libvirt-domain.h | 9 +++++++++ tools/virsh-domain.c | 8 ++++++++ 2 files changed, 17 insertions(+)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

The field can be used by jobs to add an optional error message to a completed (failed) job. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 3 +++ src/qemu/qemu_domain.h | 2 ++ 2 files changed, 5 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 13b1c4e402..dba222dde5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -308,6 +308,7 @@ qemuDomainDisableNamespace(virDomainObjPtr vm, void qemuDomainJobInfoFree(qemuDomainJobInfoPtr info) { + g_free(info->errmsg); g_free(info); } @@ -319,6 +320,8 @@ qemuDomainJobInfoCopy(qemuDomainJobInfoPtr info) memcpy(ret, info, sizeof(*info)); + ret->errmsg = g_strdup(info->errmsg); + return ret; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index c7f28b04c2..639d27d8a5 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -175,6 +175,8 @@ struct _qemuDomainJobInfo { qemuDomainBackupStats backup; } stats; qemuDomainMirrorStats mirrorStats; + + char *errmsg; /* optional error message for failed completed jobs */ }; void -- 2.26.0

On 4/16/20 4:55 AM, Peter Krempa wrote:
The field can be used by jobs to add an optional error message to a completed (failed) job.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 3 +++ src/qemu/qemu_domain.h | 2 ++ 2 files changed, 5 insertions(+)
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

If a backup job fails midway it's hard to figure out what happened as it's running asynchronous. Use the VIR_DOMAIN_JOB_ERRMSG job statistics field to pass through the error from the first failed backup-blockjob so that both the consumer of the virDomainGetJobStats and the corresponding event can see the error. event 'job-completed' for domain backup-test: operation: 9 time_elapsed: 46 disk_total: 104857600 disk_processed: 10158080 disk_remaining: 94699520 success: 0 errmsg: No space left on device virsh domjobinfo backup-test --completed --anystats Job type: Failed Operation: Backup Time elapsed: 46 ms File processed: 9.688 MiB File remaining: 90.312 MiB File total: 100.000 MiB Error message: No space left on device https://bugzilla.redhat.com/show_bug.cgi?id=1812827 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/backup_conf.c | 1 + src/conf/backup_conf.h | 2 ++ src/qemu/qemu_backup.c | 10 ++++++++-- src/qemu/qemu_backup.h | 1 + src/qemu/qemu_blockjob.c | 2 +- src/qemu/qemu_domain.c | 4 ++++ 6 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index 64c8f6cc09..c5677cdb05 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -65,6 +65,7 @@ virDomainBackupDefFree(virDomainBackupDefPtr def) return; g_free(def->incremental); + g_free(def->errmsg); virStorageNetHostDefFree(1, def->server); for (i = 0; i < def->ndisks; i++) { diff --git a/src/conf/backup_conf.h b/src/conf/backup_conf.h index 672fd52ee7..b5685317c5 100644 --- a/src/conf/backup_conf.h +++ b/src/conf/backup_conf.h @@ -79,6 +79,8 @@ struct _virDomainBackupDef { unsigned long long push_total; unsigned long long pull_tmp_used; unsigned long long pull_tmp_total; + + char *errmsg; /* error message of failed sub-blockjob */ }; typedef enum { diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index 03d34c9378..80fc5d77f8 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -650,6 +650,7 @@ qemuBackupJobTerminate(virDomainObjPtr vm, priv->job.completed->stats.backup.tmp_total = priv->backup->pull_tmp_total; priv->job.completed->status = jobstatus; + priv->job.completed->errmsg = g_strdup(priv->backup->errmsg); qemuDomainEventEmitJobCompleted(priv->driver, vm); @@ -951,6 +952,7 @@ void qemuBackupNotifyBlockjobEnd(virDomainObjPtr vm, virDomainDiskDefPtr disk, qemuBlockjobState state, + const char *errmsg, unsigned long long cur, unsigned long long end, int asyncJob) @@ -964,8 +966,8 @@ qemuBackupNotifyBlockjobEnd(virDomainObjPtr vm, virDomainBackupDefPtr backup = priv->backup; size_t i; - VIR_DEBUG("vm: '%s', disk:'%s', state:'%d'", - vm->def->name, disk->dst, state); + VIR_DEBUG("vm: '%s', disk:'%s', state:'%d' errmsg:'%s'", + vm->def->name, disk->dst, state, NULLSTR(errmsg)); if (!backup) return; @@ -985,6 +987,10 @@ qemuBackupNotifyBlockjobEnd(virDomainObjPtr vm, backup->push_total += end; } + /* record first error message */ + if (!backup->errmsg) + backup->errmsg = g_strdup(errmsg); + for (i = 0; i < backup->ndisks; i++) { virDomainBackupDiskDefPtr backupdisk = backup->disks + i; diff --git a/src/qemu/qemu_backup.h b/src/qemu/qemu_backup.h index 3321ba0b6f..b19c3bf1c9 100644 --- a/src/qemu/qemu_backup.h +++ b/src/qemu/qemu_backup.h @@ -38,6 +38,7 @@ void qemuBackupNotifyBlockjobEnd(virDomainObjPtr vm, virDomainDiskDefPtr disk, qemuBlockjobState state, + const char *errmsg, unsigned long long cur, unsigned long long end, int asyncJob); diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c index 2032c0c1c5..b9eecd3f98 100644 --- a/src/qemu/qemu_blockjob.c +++ b/src/qemu/qemu_blockjob.c @@ -1435,7 +1435,7 @@ qemuBlockJobProcessEventConcludedBackup(virQEMUDriverPtr driver, g_autoptr(qemuBlockStorageSourceAttachData) backend = NULL; g_autoptr(virJSONValue) actions = NULL; - qemuBackupNotifyBlockjobEnd(vm, job->disk, newstate, + qemuBackupNotifyBlockjobEnd(vm, job->disk, newstate, job->errmsg, progressCurrent, progressTotal, asyncJob); if (job->data.backup.store && diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index dba222dde5..22bb808177 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -836,6 +836,10 @@ qemuDomainBackupJobInfoToParams(qemuDomainJobInfoPtr jobInfo, VIR_DOMAIN_JOB_SUCCESS) < 0) return -1; + if (jobInfo->errmsg && + virTypedParamListAddString(par, jobInfo->errmsg, VIR_DOMAIN_JOB_ERRMSG) < 0) + return -1; + *nparams = virTypedParamListStealParams(par, params); *type = qemuDomainJobStatusToType(jobInfo->status); return 0; -- 2.26.0

On 4/16/20 4:55 AM, Peter Krempa wrote:
If a backup job fails midway it's hard to figure out what happened as it's running asynchronous. Use the VIR_DOMAIN_JOB_ERRMSG job statistics field to pass through the error from the first failed backup-blockjob so that both the consumer of the virDomainGetJobStats and the corresponding event can see the error.
event 'job-completed' for domain backup-test: operation: 9 time_elapsed: 46 disk_total: 104857600 disk_processed: 10158080 disk_remaining: 94699520 success: 0 errmsg: No space left on device
Nice.
virsh domjobinfo backup-test --completed --anystats Job type: Failed Operation: Backup Time elapsed: 46 ms File processed: 9.688 MiB File remaining: 90.312 MiB File total: 100.000 MiB Error message: No space left on device
https://bugzilla.redhat.com/show_bug.cgi?id=1812827
Signed-off-by: Peter Krempa <pkrempa@redhat.com> ---
Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
participants (2)
-
Eric Blake
-
Peter Krempa