The domuuid, net-uuid, and pool-uuid commands call vshError() when
GetUUIDString() fails, but unconditionally return true, which
vshCommandRun() maps to EXIT_SUCCESS. This means scripts checking
$? see success despite the error.

Return false on failure so the exit code correctly reflects the
error, consistent with other virsh commands.

Signed-off-by: Lucas Amaral <lucaaamaral@gmail.com>
---
Found by code audit. The bug dates back to the original cmdDomuuid
implementation in d47ddf5b67 (May 2006) and was replicated in
cmdNetworkUuid and cmdPoolUuid when those were added later.

In practice GetUUIDString() is unlikely to fail from virsh (the
UUID is populated during the preceding lookup), so this is a
defensive correctness fix rather than a user-visible regression.
The restructuring uses the early-return-on-error pattern already
used by the adjacent object lookup (e.g., virshCommandOptDomainBy
a few lines above).

Tested on CentOS Stream 9 — full test suite passes (307 OK,
1 Expected Fail, 0 Fail).

 tools/virsh-domain.c  | 7 ++++---
 tools/virsh-network.c | 7 ++++---
 tools/virsh-pool.c    | 7 ++++---
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index cb9dd069b6..a7f6a97dd7 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11041,11 +11041,12 @@ cmdDomuuid(vshControl *ctl, const vshCmd *cmd)
                                         VIRSH_BYNAME|VIRSH_BYID)))
         return false;
 
-    if (virDomainGetUUIDString(dom, uuid) != -1)
-        vshPrint(ctl, "%s\n", uuid);
-    else
+    if (virDomainGetUUIDString(dom, uuid) == -1) {
         vshError(ctl, "%s", _("failed to get domain UUID"));
+        return false;
+    }
 
+    vshPrint(ctl, "%s\n", uuid);
     return true;
 }
 
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 2e9613e01b..a725be289f 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -1389,11 +1389,12 @@ cmdNetworkUuid(vshControl *ctl, const vshCmd *cmd)
                                              VIRSH_BYNAME)))
         return false;
 
-    if (virNetworkGetUUIDString(network, uuid) != -1)
-        vshPrint(ctl, "%s\n", uuid);
-    else
+    if (virNetworkGetUUIDString(network, uuid) == -1) {
         vshError(ctl, "%s", _("failed to get network UUID"));
+        return false;
+    }
 
+    vshPrint(ctl, "%s\n", uuid);
     return true;
 }
 
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index 2010ef1356..e06a6cf7c8 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -1720,11 +1720,12 @@ cmdPoolUuid(vshControl *ctl, const vshCmd *cmd)
     if (!(pool = virshCommandOptPoolBy(ctl, cmd, "pool", NULL, VIRSH_BYNAME)))
         return false;
 
-    if (virStoragePoolGetUUIDString(pool, uuid) != -1)
-        vshPrint(ctl, "%s\n", uuid);
-    else
+    if (virStoragePoolGetUUIDString(pool, uuid) == -1) {
         vshError(ctl, "%s", _("failed to get pool UUID"));
+        return false;
+    }
 
+    vshPrint(ctl, "%s\n", uuid);
     return true;
 }
 
-- 
2.52.0

On 2/16/26 15:48, Lucas Amaral wrote:
> The domuuid, net-uuid, and pool-uuid commands call vshError() when
> GetUUIDString() fails, but unconditionally return true, which
> vshCommandRun() maps to EXIT_SUCCESS. This means scripts checking
> $? see success despite the error.
> 
> Return false on failure so the exit code correctly reflects the
> error, consistent with other virsh commands.
> 
> Signed-off-by: Lucas Amaral <lucaaamaral@gmail.com>
> ---
> Found by code audit. The bug dates back to the original cmdDomuuid
> implementation in d47ddf5b67 (May 2006) and was replicated in
> cmdNetworkUuid and cmdPoolUuid when those were added later.
> 
> In practice GetUUIDString() is unlikely to fail from virsh (the
> UUID is populated during the preceding lookup), so this is a
> defensive correctness fix rather than a user-visible regression.
> The restructuring uses the early-return-on-error pattern already
> used by the adjacent object lookup (e.g., virshCommandOptDomainBy
> a few lines above).
> 
> Tested on CentOS Stream 9 — full test suite passes (307 OK,
> 1 Expected Fail, 0 Fail).
> 
>  tools/virsh-domain.c  | 7 ++++---
>  tools/virsh-network.c | 7 ++++---
>  tools/virsh-pool.c    | 7 ++++---
>  3 files changed, 12 insertions(+), 9 deletions(-)


Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and merged. Congratulations on your first libvirt contribution!

Michal