From 5c9fc43e690a6c3dac140af13590eba9dc44722f Mon Sep 17 00:00:00 2001
From: Peter Krempa <pkrempa@redhat.com>
Date: Thu, 19 Nov 2020 11:01:22 +0100
Subject: [PATCH] virsh: cmdAttachDisk: Don't assume type='block' if file can't
 be examined

'virsh attach-disk' uses stat() to determine if the 'source' is a
regular file. If stat fails though it assumes that the file is block.

Since it's way more common to have regular files and the detection does
not work at all when accessing a remote host, modify the default to
assume type='file' by default.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 tests/virsh-output.out |  4 ++--
 tools/virsh-domain.c   | 30 +++++++++++++++++-------------
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/tests/virsh-output.out b/tests/virsh-output.out
index 8fbe02a232..a93492e8f8 100644
--- a/tests/virsh-output.out
+++ b/tests/virsh-output.out
@@ -10,8 +10,8 @@ error: command 'attach-disk' requires <target> option
 error: command 'attach-disk' requires <target> option
 + attach_disk --target hda
 + ./virsh attach-disk --print-xml --domain testdom --target hda --source /nonexistent/file
-<disk type='block'>
-  <source dev='/nonexistent/file'/>
+<disk type='file'>
+  <source file='/nonexistent/file'/>
   <target dev='hda'/>
 </disk>
 
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 2eaac9ad8c..18e9ab0b46 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -577,7 +577,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
     const char *targetbus = NULL;
     const char *alias = NULL;
     struct DiskAddress diskAddr;
-    bool isFile = false;
+    bool isBlock = false;
     int ret;
     unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
     const char *stype = NULL;
@@ -617,15 +617,15 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
         return false;
 
     if (!stype) {
-        if (driver && (STREQ(driver, "file") || STREQ(driver, "tap"))) {
-            isFile = true;
-        } else {
-            if (source && !stat(source, &st))
-                isFile = S_ISREG(st.st_mode) ? true : false;
-        }
-    } else if (STREQ(stype, "file")) {
-        isFile = true;
-    } else if (STRNEQ(stype, "block")) {
+        if (STRNEQ_NULLABLE(driver, "file") &&
+            STRNEQ_NULLABLE(driver, "tap") &&
+            source &&
+            stat(source, &st) == 0 &&
+            S_ISBLK(st.st_mode))
+            isBlock = true;
+    } else if (STREQ(stype, "block")) {
+        isBlock = true;
+    } else if (STRNEQ(stype, "file")) {
         vshError(ctl, _("Unknown source type: '%s'"), stype);
         return false;
     }
@@ -642,8 +642,12 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
         return false;
 
     /* Make XML of disk */
-    virBufferAsprintf(&buf, "<disk type='%s'",
-                      isFile ? "file" : "block");
+    virBufferAddLit(&buf, "<disk");
+    if (isBlock)
+        virBufferAddLit(&buf, " type='block'");
+    else
+        virBufferAddLit(&buf, " type='file'");
+
     if (type)
         virBufferAsprintf(&buf, " device='%s'", type);
     if (vshCommandOptBool(cmd, "rawio"))
@@ -670,7 +674,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
 
     if (source)
         virBufferAsprintf(&buf, "<source %s='%s'/>\n",
-                          isFile ? "file" : "dev", source);
+                          !isBlock ? "file" : "dev", source);
     virBufferAsprintf(&buf, "<target dev='%s'", target);
     if (targetbus)
         virBufferAsprintf(&buf, " bus='%s'", targetbus);