[PATCH] qemu: Use virDomainObjCheckActive() more

Michal Prívozník mprivozn at redhat.com
Mon Feb 21 14:31:26 UTC 2022


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 at 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




More information about the libvir-list mailing list