Revert "add_drive_ro adds readonly=on option if available." (RHBZ#617200).

Adding the readonly=on option is not so clever.  This causes
qemu to present the disk as read-only to the guest.  (The
expected behaviour of snapshots=on,readonly=on was that it
would open the disk O_RDONLY but present a writable disk to
the guest).

Since the guest sees a read-only disk, we are unable to do any
recovery if a filesystem on the disk is inconsistent.  This basically
prevents most accesses to live disk images.

What we really want is a qemu option which presents a writable
disk to the guest, but only opens the disk on the host side with
O_RDONLY, to alleviate the udev bug RHBZ#571714.

This reverts commit 676462684e05dd8341dd695762dd99a87d8ec022.

diff --git a/src/generator.ml b/src/generator.ml
index 69f561c..372d01e 100755 (executable)
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -559,15 +559,13 @@ handle is closed.  We don't currently have any method to enable
 changes to be committed, although qemu can support this.
 
 This is equivalent to the qemu parameter
 changes to be committed, although qemu can support this.
 
 This is equivalent to the qemu parameter

-C<-drive file=filename,snapshot=on,readonly=on,if=...>.
+C<-drive file=filename,snapshot=on,if=...>.

 
 C<if=...> is set at compile time by the configuration option
 C<./configure --with-drive-if=...>.  In the rare case where you
 might need to change this at run time, use C<guestfs_add_drive_with_if>
 or C<guestfs_add_drive_ro_with_if>.
 
 
 C<if=...> is set at compile time by the configuration option
 C<./configure --with-drive-if=...>.  In the rare case where you
 might need to change this at run time, use C<guestfs_add_drive_with_if>
 or C<guestfs_add_drive_ro_with_if>.
 

-C<readonly=on> is only added where qemu supports this option.
-

 Note that this call checks for the existence of C<filename>.  This
 stops you from specifying other types of drive which are supported
 by qemu such as C<nbd:> and C<http:> URLs.  To specify those, use
 Note that this call checks for the existence of C<filename>.  This
 stops you from specifying other types of drive which are supported
 by qemu such as C<nbd:> and C<http:> URLs.  To specify those, use

diff --git a/src/guestfs.c b/src/guestfs.c
index 85a042a..d6c8d60 100644 (file)
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -836,6 +836,9 @@ int
 guestfs__add_drive_ro_with_if (guestfs_h *g, const char *filename,
                                const char *drive_if)
 {
 guestfs__add_drive_ro_with_if (guestfs_h *g, const char *filename,
                                const char *drive_if)
 {

+  size_t len = strlen (filename) + 64;
+  char buf[len];
+

   if (strchr (filename, ',') != NULL) {
     error (g, _("filename cannot contain ',' (comma) character"));
     return -1;
   if (strchr (filename, ',') != NULL) {
     error (g, _("filename cannot contain ',' (comma) character"));
     return -1;


@@ -846,24 +849,7 @@ guestfs__add_drive_ro_with_if (guestfs_h *g, const char *filename,
     return -1;
   }
 
     return -1;
   }
 

-  if (qemu_supports (g, NULL) == -1)
-    return -1;
-
-  /* Only SCSI and virtio drivers support readonly mode.
-   * This is only supported as a QEMU feature since 2010/01.
-   */
-  int supports_ro = 0;
-  if ((STREQ (drive_if, "scsi") || STREQ (drive_if, "virtio")) &&
-      qemu_supports (g, "readonly=on"))
-    supports_ro = 1;
-
-  size_t len = strlen (filename) + 100;
-  char buf[len];
-
-  snprintf (buf, len, "file=%s,snapshot=on,%sif=%s",
-            filename,
-            supports_ro ? "readonly=on," : "",
-            drive_if);
+  snprintf (buf, len, "file=%s,snapshot=on,if=%s", filename, drive_if);

 
   return guestfs__config (g, "-drive", buf);
 }
 
   return guestfs__config (g, "-drive", buf);
 }