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>

diff --git a/tests/virsh-output.out b/tests/virsh-output.out
index 8fbe02a2323c498d743c12e6087afe193a6d0121..a93492e8f87324d950daf961f5a19543b601cfdd 100644 (file)
--- 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 2eaac9ad8cd0aea951de843d3ccf71c0f22dab2c..18e9ab0b46c7b2977a91f0fcaecefd1837738ebc 100644 (file)
--- 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);