8:31 a.m.
On 2/21/22 14:27, Ján Tomko wrote:
On a Monday in 2022, Michal Privoznik wrote:
> Using the following spatch, I've identified two places which
> could be switched from explicit virDomainObjIsActive() +
> virReportError() to virDomainObjCheckActive():
>
> @@
> expression dom;
> @@
> if (
> - !virDomainObjIsActive(dom)
> + virDomainObjCheckActive(dom) < 0
> ) {
> - virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain
is
> not running"));
> ...
> }
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
>
> BTW. if I'm more aggressive and let virReportError() have just any args
> then even more places fit the rule (approx. two dozen more).
>
Had you sent that as a patch, we would have been able to see what kind
of error messages are used in those places without having to run the
spatch.
Here you go. Problem is, we'd be changing the error code which may break
some apps (e.g. those who check for it).
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 093b719b2c..1a20f11227 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3956,9 +3956,7 @@ virDomainObjWait(virDomainObj *vm)
return -1;
}
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("domain is not running"));
+ if (virDomainObjCheckActive(vm) < 0) {
return -1;
}
@@ -6361,9 +6359,7 @@ virDomainDefLifecycleActionAllowed(virDomainLifecycle type,
int
virDomainObjCheckActive(virDomainObj *dom)
{
- if (!virDomainObjIsActive(dom)) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("domain is not running"));
+ if (virDomainObjCheckActive(dom) < 0) {
return -1;
}
return 0;
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 6944c77eed..4f40dd97e5 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -1266,11 +1266,8 @@ libxlDomainMigrationDstFinish(virConnectPtr dconn,
goto cleanup;
/* Check if domain is alive */
- if (!virDomainObjIsActive(vm)) {
+ if (virDomainObjCheckActive(vm) < 0) {
/* Migration failed if domain is inactive */
- virReportError(VIR_ERR_OPERATION_FAILED,
- "%s", _("Migration failed. Domain is not running
"
- "on destination host"));
goto cleanup;
}
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index 2471242e60..3b44ff8e7f 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -801,9 +801,7 @@ qemuBackupBegin(virDomainObj *vm,
qemuDomainJobSetStatsType(priv->job.current,
QEMU_DOMAIN_JOB_STATS_TYPE_BACKUP);
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("cannot perform disk backup for inactive domain"));
+ if (virDomainObjCheckActive(vm) < 0) {
goto endjob;
}
diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c
index 69f287399b..8701bd8083 100644
--- a/src/qemu/qemu_checkpoint.c
+++ b/src/qemu/qemu_checkpoint.c
@@ -588,9 +588,7 @@ qemuCheckpointCreateXML(virDomainPtr domain,
}
if (!redefine) {
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("cannot create checkpoint for inactive domain"));
+ if (virDomainObjCheckActive(vm) < 0) {
return NULL;
}
@@ -856,9 +854,7 @@ qemuCheckpointDelete(virDomainObj *vm,
return -1;
if (!metadata_only) {
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("cannot delete checkpoint for inactive domain"));
+ if (virDomainObjCheckActive(vm) < 0) {
goto endjob;
}
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6a70b72760..cdf523b304 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5866,9 +5866,7 @@ qemuDomainObjEnterMonitorInternal(virQEMUDriver *driver,
int ret;
if ((ret = qemuDomainObjBeginNestedJob(driver, obj, asyncJob)) < 0)
return ret;
- if (!virDomainObjIsActive(obj)) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("domain is no longer running"));
+ if (virDomainObjCheckActive(obj) < 0) {
qemuDomainObjEndJob(driver, obj);
return -1;
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e417d358cd..3c38ff71cd 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2652,9 +2652,7 @@ qemuDomainSaveInternal(virQEMUDriver *driver,
VIR_DOMAIN_JOB_OPERATION_SAVE, flags) < 0)
goto cleanup;
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest unexpectedly quit"));
+ if (virDomainObjCheckActive(vm) < 0) {
goto endjob;
}
@@ -2668,9 +2666,7 @@ qemuDomainSaveInternal(virQEMUDriver *driver,
QEMU_ASYNC_JOB_SAVE) < 0)
goto endjob;
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest unexpectedly quit"));
+ if (virDomainObjCheckActive(vm) < 0) {
goto endjob;
}
}
@@ -3178,9 +3174,7 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom,
goto endjob;
paused = true;
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest unexpectedly quit"));
+ if (virDomainObjCheckActive(vm) < 0) {
goto endjob;
}
}
@@ -4752,9 +4746,7 @@ qemuDomainGetVcpus(virDomainPtr dom,
if (virDomainGetVcpusEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot retrieve vcpu information for inactive
domain"));
+ if (virDomainObjCheckActive(vm) < 0) {
goto cleanup;
}
@@ -4796,10 +4788,7 @@ qemuDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags)
if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0)
goto cleanup;
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("vCPU count provided by the guest agent can only be
"
- "requested for live domains"));
+ if (virDomainObjCheckActive(vm) < 0) {
goto endjob;
}
@@ -4881,9 +4870,7 @@ qemuDomainGetIOThreadsLive(virQEMUDriver *driver,
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
goto cleanup;
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("cannot list IOThreads for an inactive domain"));
+ if (virDomainObjCheckActive(vm) < 0) {
goto endjob;
}
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c70bc361fd..fbf90773ca 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -971,9 +971,7 @@ qemuDomainFindOrCreateSCSIDiskController(virQEMUDriver *driver,
return NULL;
}
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest unexpectedly quit"));
+ if (virDomainObjCheckActive(vm) < 0) {
/* cont doesn't need freeing here, since the reference
* now held in def->controllers */
return NULL;
@@ -1721,9 +1719,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver,
goto error;
releaseaddr = true;
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest unexpectedly quit during hotplug"));
+ if (virDomainObjCheckActive(vm) < 0) {
goto error;
}
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 5aecdddff0..950c106d5b 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5678,9 +5678,7 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
goto endjob;
}
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest unexpectedly quit"));
+ if (virDomainObjCheckActive(vm) < 0) {
qemuMigrationDstErrorReport(driver, vm->def->name);
goto endjob;
}
@@ -5941,9 +5939,7 @@ qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm,
}
}
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest unexpectedly quit"));
+ if (virDomainObjCheckActive(vm) < 0) {
/* nothing to tear down */
return -1;
}
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 0ff938a577..308c85999e 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -695,9 +695,7 @@ qemuMonitorOpen(virDomainObj *vm,
if (fd < 0)
return NULL;
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_OPERATION_FAILED, "%s",
- _("domain is not running"));
+ if (virDomainObjCheckActive(vm) < 0) {
return NULL;
}
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6fa47badd9..fdd95b9dab 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -235,10 +235,8 @@ qemuConnectAgent(virQEMUDriver *driver, virDomainObj *vm)
&agentCallbacks,
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VSERPORT_CHANGE));
- if (!virDomainObjIsActive(vm)) {
+ if (virDomainObjCheckActive(vm) < 0) {
qemuAgentClose(agent);
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest crashed while connecting to the guest
agent"));
return -1;
}
@@ -465,9 +463,7 @@ qemuProcessFakeReboot(void *opaque)
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
goto cleanup;
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest unexpectedly quit"));
+ if (virDomainObjCheckActive(vm) < 0) {
goto endjob;
}
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index cb2a7eb739..836f0cb724 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -308,9 +308,7 @@ qemuSnapshotCreateActiveInternal(virQEMUDriver *driver,
goto cleanup;
resume = true;
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest unexpectedly quit"));
+ if (virDomainObjCheckActive(vm) < 0) {
goto cleanup;
}
}
@@ -1395,9 +1393,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver,
QEMU_ASYNC_JOB_SNAPSHOT) < 0)
goto cleanup;
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest unexpectedly quit"));
+ if (virDomainObjCheckActive(vm) < 0) {
goto cleanup;
}
@@ -2105,9 +2101,7 @@ qemuSnapshotRevertActive(virDomainObj *vm,
virObjectEventStateQueue(driver->domainEventState, event2);
} else {
/* Transitions 2, 5, 8 */
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("guest unexpectedly quit"));
+ if (virDomainObjCheckActive(vm) < 0) {
return -1;
}
rc = qemuProcessStartCPUs(driver, vm,
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 4eca5c4a65..529b9e5a25 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3049,9 +3049,7 @@ static int testDomainGetVcpus(virDomainPtr domain,
if (!(privdom = testDomObjFromDomain(domain)))
return -1;
- if (!virDomainObjIsActive(privdom)) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot list vcpus for an inactive
domain"));
+ if (virDomainObjCheckActive(privdom) < 0) {
goto cleanup;
}
@@ -3119,9 +3117,7 @@ static int testDomainPinVcpuFlags(virDomainPtr domain,
def = privdom->def;
- if (!virDomainObjIsActive(privdom)) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s", _("cannot pin vcpus on an inactive
domain"));
+ if (virDomainObjCheckActive(privdom) < 0) {
goto cleanup;
}
@@ -9198,9 +9194,7 @@ testDomainCheckpointCreateXML(virDomainPtr domain,
goto cleanup;
}
- if (!virDomainObjIsActive(vm)) {
- virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
- _("cannot create checkpoint for inactive domain"));
+ if (virDomainObjCheckActive(vm) < 0) {
goto cleanup;
}
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index fc91b6dddf..0a84e8b6c3 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -958,10 +958,7 @@ vzDomainGetVcpus(virDomainPtr domain,
if (virDomainGetVcpusEnsureACL(domain->conn, dom->def) < 0)
goto cleanup;
- if (!virDomainObjIsActive(dom)) {
- virReportError(VIR_ERR_OPERATION_INVALID,
- "%s",
- _("cannot list vcpu pinning for an inactive domain"));
+ if (virDomainObjCheckActive(dom) < 0) {
goto cleanup;
}
Michal