Re: [Libguestfs] [PATCH] Revert "add_drive_ro adds readonly=on option if available." (RHBZ#617200).

> From 004c0345d6fd9f141978f5163e053a67dba31cd0 Mon Sep 17 00:00:00 2001 From: Richard Jones<rjones(a)redhat.com> Date: Thu, 22 Jul 2010 14:39:36 +0100 Subject: [PATCH] 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. --- src/generator.ml | 4 +--- src/guestfs.c | 22 ++++------------------ 2 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/src/guestfs.c b/src/guestfs.c index 85a042a..d6c8d60 100644 --- 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) { + size_t len = strlen (filename) + 64; + char buf[len]; + 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; } - 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); }