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@xxxxxxxxxx>
>> ---
>>
>> 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
