Many calls of qemuMonitorDelObject don't actually check the return value or report the error from the object deletion itself since they are on cleanup paths. In some cases this can lead to reporting of spurious errors e.g. when qemuMonitorDelObject is used to clean up a possibly pre-existing objects. Add a new argument for qemuMonitorDelObject which controls whether the internals report errors from qemu and fix all callers accordingly. Note that some of the cases on device unplug which check the error code don't in fact propagate the error to the user, but in this case it is important to add the log entry anyways for tracing that the device deletion failed. https://bugzilla.redhat.com/show_bug.cgi?id=1784040 Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx> --- src/qemu/qemu_block.c | 10 +++++----- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_hotplug.c | 29 +++++++++++++++-------------- src/qemu/qemu_monitor.c | 5 +++-- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 5 +++-- src/qemu/qemu_monitor_json.h | 3 ++- 7 files changed, 31 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index f95ebb6fa7..4ed17b6df3 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1732,19 +1732,19 @@ qemuBlockStorageSourceAttachRollback(qemuMonitorPtr mon, ignore_value(qemuMonitorBlockdevDel(mon, data->storageNodeName)); if (data->prmgrAlias) - ignore_value(qemuMonitorDelObject(mon, data->prmgrAlias)); + ignore_value(qemuMonitorDelObject(mon, data->prmgrAlias, false)); if (data->authsecretAlias) - ignore_value(qemuMonitorDelObject(mon, data->authsecretAlias)); + ignore_value(qemuMonitorDelObject(mon, data->authsecretAlias, false)); if (data->encryptsecretAlias) - ignore_value(qemuMonitorDelObject(mon, data->encryptsecretAlias)); + ignore_value(qemuMonitorDelObject(mon, data->encryptsecretAlias, false)); if (data->httpcookiesecretAlias) - ignore_value(qemuMonitorDelObject(mon, data->httpcookiesecretAlias)); + ignore_value(qemuMonitorDelObject(mon, data->httpcookiesecretAlias, false)); if (data->tlsAlias) - ignore_value(qemuMonitorDelObject(mon, data->tlsAlias)); + ignore_value(qemuMonitorDelObject(mon, data->tlsAlias, false)); virErrorRestore(&orig_err); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b62947fb4a..8a2cd35fa7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6047,7 +6047,7 @@ qemuDomainHotplugDelIOThread(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); - rc = qemuMonitorDelObject(priv->mon, alias); + rc = qemuMonitorDelObject(priv->mon, alias, true); exp_niothreads--; if (rc < 0) goto exit_monitor; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1cab12fb2b..bbe721c02c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -390,7 +390,8 @@ qemuHotplugRemoveManagedPR(virQEMUDriverPtr driver, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) goto cleanup; - ignore_value(qemuMonitorDelObject(priv->mon, qemuDomainGetManagedPRAlias())); + ignore_value(qemuMonitorDelObject(priv->mon, qemuDomainGetManagedPRAlias(), + false)); if (qemuDomainObjExitMonitor(driver, vm) < 0) goto cleanup; @@ -471,7 +472,7 @@ qemuDomainDetachDBusVMState(virQEMUDriverPtr driver, qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1; - ret = qemuMonitorDelObject(priv->mon, alias); + ret = qemuMonitorDelObject(priv->mon, alias, true); if (qemuDomainObjExitMonitor(driver, vm) < 0) return -1; @@ -1685,10 +1686,10 @@ qemuDomainDelTLSObjects(virQEMUDriverPtr driver, goto cleanup; if (tlsAlias) - ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias)); + ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias, false)); if (secAlias) - ignore_value(qemuMonitorDelObject(priv->mon, secAlias)); + ignore_value(qemuMonitorDelObject(priv->mon, secAlias, false)); ignore_value(qemuDomainObjExitMonitor(driver, vm)); @@ -1850,9 +1851,9 @@ qemuDomainDelChardevTLSObjects(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); - ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias)); + ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias, false)); if (secAlias) - ignore_value(qemuMonitorDelObject(priv->mon, secAlias)); + ignore_value(qemuMonitorDelObject(priv->mon, secAlias, false)); if (qemuDomainObjExitMonitor(driver, vm) < 0) return -1; @@ -2307,7 +2308,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, exit_monitor: virErrorPreserveLast(&orig_err); if (objAlias) - ignore_value(qemuMonitorDelObject(priv->mon, objAlias)); + ignore_value(qemuMonitorDelObject(priv->mon, objAlias, false)); if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD && chardevAdded) ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias)); if (qemuDomainObjExitMonitor(driver, vm) < 0) @@ -2443,7 +2444,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver, exit_monitor: virErrorPreserveLast(&orig_err); if (objAdded) - ignore_value(qemuMonitorDelObject(priv->mon, objalias)); + ignore_value(qemuMonitorDelObject(priv->mon, objalias, false)); if (qemuDomainObjExitMonitor(driver, vm) < 0) mem = NULL; @@ -2665,7 +2666,7 @@ qemuDomainAttachHostSCSIDevice(virQEMUDriverPtr driver, drvstr, devstr); } if (secobjAlias) - ignore_value(qemuMonitorDelObject(priv->mon, secobjAlias)); + ignore_value(qemuMonitorDelObject(priv->mon, secobjAlias, false)); ignore_value(qemuDomainObjExitMonitor(driver, vm)); virErrorRestore(&orig_err); @@ -3041,7 +3042,7 @@ qemuDomainAttachShmemDevice(virQEMUDriverPtr driver, if (shmem->server.enabled) ignore_value(qemuMonitorDetachCharDev(priv->mon, charAlias)); else - ignore_value(qemuMonitorDelObject(priv->mon, memAlias)); + ignore_value(qemuMonitorDelObject(priv->mon, memAlias, false)); } if (qemuDomainObjExitMonitor(driver, vm) < 0) @@ -4367,7 +4368,7 @@ qemuDomainRemoveMemoryDevice(virQEMUDriverPtr driver, backendAlias = g_strdup_printf("mem%s", mem->info.alias); qemuDomainObjEnterMonitor(driver, vm); - rc = qemuMonitorDelObject(priv->mon, backendAlias); + rc = qemuMonitorDelObject(priv->mon, backendAlias, true); if (qemuDomainObjExitMonitor(driver, vm) < 0) rc = -1; @@ -4483,7 +4484,7 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, /* If it fails, then so be it - it was a best shot */ if (objAlias) - ignore_value(qemuMonitorDelObject(priv->mon, objAlias)); + ignore_value(qemuMonitorDelObject(priv->mon, objAlias, false)); if (qemuDomainObjExitMonitor(driver, vm) < 0) return -1; @@ -4741,7 +4742,7 @@ qemuDomainRemoveRNGDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); if (rc == 0 && - qemuMonitorDelObject(priv->mon, objAlias) < 0) + qemuMonitorDelObject(priv->mon, objAlias, true) < 0) rc = -1; if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD && @@ -4802,7 +4803,7 @@ qemuDomainRemoveShmemDevice(virQEMUDriverPtr driver, if (shmem->server.enabled) rc = qemuMonitorDetachCharDev(priv->mon, charAlias); else - rc = qemuMonitorDelObject(priv->mon, memAlias); + rc = qemuMonitorDelObject(priv->mon, memAlias, true); if (qemuDomainObjExitMonitor(driver, vm) < 0) return -1; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 4a5bd1d4ab..3ac78016e2 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2966,13 +2966,14 @@ qemuMonitorAddObject(qemuMonitorPtr mon, int qemuMonitorDelObject(qemuMonitorPtr mon, - const char *objalias) + const char *objalias, + bool report_error) { VIR_DEBUG("objalias=%s", objalias); QEMU_CHECK_MONITOR(mon); - return qemuMonitorJSONDelObject(mon, objalias); + return qemuMonitorJSONDelObject(mon, objalias, report_error); } diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 2437375327..e2cc12bd0f 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -931,7 +931,8 @@ int qemuMonitorAddObject(qemuMonitorPtr mon, ATTRIBUTE_NONNULL(2); int qemuMonitorDelObject(qemuMonitorPtr mon, - const char *objalias); + const char *objalias, + bool report_error); int qemuMonitorAddDrive(qemuMonitorPtr mon, const char *drivestr); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c18cef5c1a..a18ab477ee 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -4448,7 +4448,8 @@ qemuMonitorJSONAddObject(qemuMonitorPtr mon, int qemuMonitorJSONDelObject(qemuMonitorPtr mon, - const char *objalias) + const char *objalias, + bool report_error) { g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; @@ -4459,7 +4460,7 @@ qemuMonitorJSONDelObject(qemuMonitorPtr mon, if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) return -1; - if (qemuMonitorJSONCheckError(cmd, reply) < 0) + if (qemuMonitorJSONCheckErrorFull(cmd, reply, report_error) < 0) return -1; return 0; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 5b3bb295eb..46dc22f1a4 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -237,7 +237,8 @@ int qemuMonitorJSONAddObject(qemuMonitorPtr mon, virJSONValuePtr props); int qemuMonitorJSONDelObject(qemuMonitorPtr mon, - const char *objalias); + const char *objalias, + bool report_error); int qemuMonitorJSONTransaction(qemuMonitorPtr mon, virJSONValuePtr *actions) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -- 2.24.1
