xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] qemu-xen-3.4-testing: Fix read-only image file handling
@ 2010-06-03 14:04 Michal Novotny
  2010-06-03 14:12 ` Michal Novotny
  0 siblings, 1 reply; 44+ messages in thread
From: Michal Novotny @ 2010-06-03 14:04 UTC (permalink / raw)
  To: 'xen-devel@lists.xensource.com'

[-- Attachment #1: Type: text/plain, Size: 892 bytes --]

[Well, I did send an e-mail to the list using git but it's not here so 
I'm forwarding the e-mail to the list for sure:]

Hi,
this is the patch for qemu-xen-3.4-testing to fix the read-only
image file handling since the image file was always treated as
read-write which means that all the HVM guests were able to
write to all the disk images available in domain configuration
file no matter what the mode of the image was defined. This
patch fixes this functionality to honor the O_RDONLY in the
BDRV_O_ACCESS flag in block.c and also fixes the IDE and SCSI
interfaces that uses it.

It's been tested on RHEL-5 with xen-3.4-testing version of
upstream xen with xen-3.4-testing qemu implementation.

For SCSI devices the DATA PROTECT request sense has been added
as found at: http://en.wikipedia.org/wiki/SCSI_Request_Sense_Command

Michal

Signed-off-by: Michal Novotny <minovotn@redhat.com>

[-- Attachment #2: xen-qemu-fix-readonly-image-handling.patch --]
[-- Type: text/x-patch, Size: 6435 bytes --]

commit 38dffb8e986167c363f24fd770d77cbe3957f34c
Author: Michal Novotny <minovotn@redhat.com>
Date:   Thu Jun 3 15:44:31 2010 +0200

    qemu-xen: Fix read-only image file handling
    
    Hi,
    this is the patch for qemu-xen-3.4-testing to fix the read-only
    image file handling since the image file was always treated as
    read-write which means that all the HVM guests were able to
    write to all the disk images available in domain configuration
    file no matter what the mode of the image was defined. This
    patch fixes this functionality to honor the O_RDONLY in the
    BDRV_O_ACCESS flag in block.c and also fixes the IDE and SCSI
    interfaces that uses it.
    
    It's been tested on RHEL-5 with xen-3.4-testing version of
    upstream xen with xen-3.4-testing qemu implementation. The
    patch is applicable to qemu-xen-unstable.git as well with no
    modifications.
    
    Michal
    
    Signed-off-by: Michal Novotny <minovotn@redhat.com>

diff --git a/block.c b/block.c
index 88e70d3..05ff8cb 100644
--- a/block.c
+++ b/block.c
@@ -422,7 +422,7 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
     /* Note: for compatibility, we open disk image files as RDWR, and
        RDONLY as fallback */
     if (!(flags & BDRV_O_FILE))
-        open_flags = BDRV_O_RDWR | (flags & BDRV_O_CACHE_MASK);
+        open_flags = (flags & BDRV_O_ACCESS) | (flags & BDRV_O_CACHE_MASK);
     else
         open_flags = flags & ~(BDRV_O_FILE | BDRV_O_SNAPSHOT);
     ret = drv->bdrv_open(bs, filename, open_flags);
diff --git a/hw/ide.c b/hw/ide.c
index b38de55..791666b 100644
--- a/hw/ide.c
+++ b/hw/ide.c
@@ -2551,6 +2551,15 @@ static void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
         case WIN_WRITE_ONCE:
         case CFA_WRITE_SECT_WO_ERASE:
         case WIN_WRITE_VERIFY:
+            if (bdrv_is_read_only(s->bs)) {
+#if defined(DEBUG_IDE)
+                printf("Attempt to write on read-only device %s\n", s->bs->filename);
+#endif
+                s->status = WRERR_STAT;
+                s->error = ABRT_ERR;
+                ide_set_irq(s);
+                break;
+            }
 	    ide_cmd_lba48_transform(s, lba48);
             s->error = 0;
             s->status = SEEK_STAT | READY_STAT;
@@ -2573,6 +2582,15 @@ static void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
         case CFA_WRITE_MULTI_WO_ERASE:
             if (!s->mult_sectors)
                 goto abort_cmd;
+	    if (bdrv_is_read_only(s->bs)) {
+#if defined(DEBUG_IDE)
+                printf("Attempt to multiwrite on read-only device %s\n", s->bs->filename);
+#endif
+                s->status = WRERR_STAT;
+                s->error = ABRT_ERR;
+                ide_set_irq(s);
+                break;
+	    }
 	    ide_cmd_lba48_transform(s, lba48);
             s->error = 0;
             s->status = SEEK_STAT | READY_STAT;
@@ -2598,6 +2616,15 @@ static void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
         case WIN_WRITEDMA_ONCE:
             if (!s->bs)
                 goto abort_cmd;
+            if (bdrv_is_read_only(s->bs)) {
+#if defined(DEBUG_IDE)
+                printf("Attempt to DMA write to read-only device %s\n", s->bs->filename);
+#endif
+                s->status = WRERR_STAT;
+                s->error = ABRT_ERR;
+                ide_set_irq(s);
+                break;
+            }
 	    ide_cmd_lba48_transform(s, lba48);
             ide_sector_write_dma(s);
             s->media_changed = 1;
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 9745ca3..db808d4 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -35,6 +35,7 @@ do { fprintf(stderr, "scsi-disk: " fmt , ##args); } while (0)
 #define SENSE_NOT_READY       2
 #define SENSE_HARDWARE_ERROR  4
 #define SENSE_ILLEGAL_REQUEST 5
+#define SENSE_DATA_PROTECT    7
 
 #define STATUS_GOOD            0
 #define STATUS_CHECK_CONDITION 2
@@ -234,6 +235,9 @@ static int scsi_handle_write_error(SCSIRequest *r, int error)
             || action == BLOCK_ERR_STOP_ANY) {
         r->status |= SCSI_REQ_STATUS_RETRY;
         vm_stop(0);
+    } else if (error == SENSE_DATA_PROTECT) {
+        scsi_command_complete(r, STATUS_CHECK_CONDITION,
+                SENSE_DATA_PROTECT);
     } else {
         scsi_command_complete(r, STATUS_CHECK_CONDITION,
                 SENSE_HARDWARE_ERROR);
@@ -305,6 +309,11 @@ static int scsi_write_data(SCSIDevice *d, uint32_t tag)
         return 1;
     }
 
+    if (bdrv_is_read_only(r->dev->bdrv)) {
+        scsi_write_complete(r, SENSE_DATA_PROTECT);
+        return 1;
+    }
+
     if (r->aiocb)
         BADF("Data transfer already in progress\n");
 
diff --git a/xenstore.c b/xenstore.c
index b6d86ce..94ee0e2 100644
--- a/xenstore.c
+++ b/xenstore.c
@@ -343,6 +343,11 @@ void xenstore_parse_domain_config(int hvm_domid)
     BlockDriverState *bs;
     BlockDriver *format;
 
+    /* Read-only handling for image files */
+    char *mode = NULL;
+    int flags;
+    int is_readonly;
+
     /* paths controlled by untrustworthy guest, and values read from them */
     char *danger_path;
     char *danger_buf = NULL;
@@ -532,7 +537,24 @@ void xenstore_parse_domain_config(int hvm_domid)
 		}
 	    }
             pstrcpy(bs->filename, sizeof(bs->filename), params);
-            if (bdrv_open2(bs, params, BDRV_O_CACHE_WB /* snapshot and write-back */, format) < 0)
+
+            flags = BDRV_O_CACHE_WB; /* snapshot and write-back */
+            is_readonly = 0;
+            if (pasprintf(&buf, "%s/mode", bpath) == -1)
+                continue;
+            free(mode);
+            mode = xs_read(xsh, XBT_NULL, buf, &len);
+            if (mode == NULL)
+                continue;
+            if (strchr(mode, 'r') && !strchr(mode, 'w'))
+                is_readonly = 1;
+
+            if (!is_readonly)
+                flags |= BDRV_O_ACCESS & O_RDWR;
+
+            fprintf(stderr, "Using file %s in read-%s mode\n", bs->filename, is_readonly ? "only" : "write");
+
+            if (bdrv_open2(bs, params, flags, format) < 0)
                 fprintf(stderr, "qemu: could not open vbd '%s' or hard disk image '%s' (drv '%s' format '%s')\n", buf, params, drv ? drv : "?", format ? format->format_name : "0");
         }
 
@@ -679,6 +701,7 @@ void xenstore_parse_domain_config(int hvm_domid)
 
  out:
     free(danger_type);
+    free(mode);
     free(params);
     free(dev);
     free(bpath);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply related	[flat|nested] 44+ messages in thread
[parent not found: <C82EBB27.16AFC%keir.fraser@eu.citrix.com>]
end of thread, other threads:[~2010-06-08 11:15 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-03 14:04 [PATCH] qemu-xen-3.4-testing: Fix read-only image file handling Michal Novotny
2010-06-03 14:12 ` Michal Novotny
2010-06-04  9:37   ` Michal Novotny
2010-06-04 10:42     ` Pasi Kärkkäinen
2010-06-04 10:57       ` Michal Novotny
2010-06-04 13:00         ` Michal Novotny
2010-06-04 13:11           ` Keir Fraser
2010-06-04 13:12           ` Pasi Kärkkäinen
2010-06-07 11:21       ` Michal Novotny
2010-06-07 11:39         ` Keir Fraser
2010-06-07 13:09           ` Michal Novotny
2010-06-07 13:14             ` Pasi Kärkkäinen
2010-06-07 18:24             ` Jeremy Fitzhardinge
2010-06-07 11:45         ` Pasi Kärkkäinen
2010-06-07 13:10           ` Michal Novotny
2010-06-07 13:14             ` Michal Novotny
2010-06-07 13:19               ` Pasi Kärkkäinen
2010-06-07 13:21                 ` Michal Novotny
2010-06-07 13:25                   ` Michal Novotny
2010-06-07 13:36                     ` Michal Novotny
2010-06-07 13:46                       ` Keir Fraser
2010-06-07 15:00                         ` Michal Novotny
2010-06-07 15:17                           ` M A Young
2010-06-07 15:28                             ` Michal Novotny
2010-06-07 14:56                       ` Pasi Kärkkäinen
2010-06-07 15:02                         ` Michal Novotny
2010-06-07 16:12                       ` Konrad Rzeszutek Wilk
2010-06-07 16:27                         ` Michal Novotny
2010-06-07 16:34                           ` Keir Fraser
2010-06-07 16:37                             ` Michal Novotny
2010-06-07 17:56                               ` Keir Fraser
2010-06-08 10:04                                 ` Michal Novotny
2010-06-08 10:39                                   ` Michal Novotny
2010-06-08 10:50                                     ` Keir Fraser
2010-06-08 10:52                                       ` Michal Novotny
2010-06-08 11:03                                         ` Keir Fraser
2010-06-08 11:06                                           ` Michal Novotny
2010-06-08 10:53                                   ` Keir Fraser
2010-06-08 10:54                                     ` Michal Novotny
2010-06-08 11:11                                     ` M A Young
2010-06-08 11:15                                       ` Michal Novotny
2010-06-07 14:06                     ` Pasi Kärkkäinen
2010-06-07 13:25             ` M A Young
     [not found] <C82EBB27.16AFC%keir.fraser@eu.citrix.com>
2010-06-04 13:45 ` Michal Novotny

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).