Daniel P. Berrange
2008-Jul-24 11:36 UTC
[Xen-devel] RFC: Making QEMU honour ''readonly'' flag for disks
The Xen disk configuration syntax allows a block device to be marked as
readonly, exclusive writable or shared writeable. The xen hotplug scripts
will clash for clashing configs between domains, but it is upto the
backend drivers to actually enforce the readonly flag on I/O operations.
The paravirt backend disk driver does this fine, but QEMU''s emulated
backend driver does not.
So the basic problem is that even if you specify a disk is readonly, eg
"file:/var/lib/xen/images/readonly.img,hdb,r"
Fullyvirtualized guests without PV drivers, can write to readonly.img
QEMU has a concept of read-only in its block backend drivers, it sets this
flag based on the permissions of the underlying media. There is no way to
force a driver to be read-only, independant of the media permissions.
This proof of concept patch I''ve done against the RHEL-5 Xen tree
introduces
a new ''drv_read_only'' flag to QEMU''s BlockDriverState
struct, and if set to
non-zero, will cause the individual block backend drivers inside QEMU to
always open with O_RDONLY, and never try O_RDWR. Ultimately this would be
hooked up to the ''-drive'' parameter via a extra flag
'',ro'' in its args.
It then makes xenstore.c set this flag based on the ''mode''
parameter for
the disk in xenstore.
With this patch applied, if the guest OS attempts to write to the disk
they will get a IDE I/O error returned, and the underlying image will
not be touched.
What do people think of this approach shown in the patch below ? If it
sounds reasonable I''ll prepare a patch which applies against
Ian''s latest
''qemu-xen'' tree instead of my current RHEL-5 Xen based patch
Daniel
diff -rup xen-3.1.0-src.orig/tools/ioemu/block-bochs.c
xen-3.1.0-src.new/tools/ioemu/block-bochs.c
--- xen-3.1.0-src.orig/tools/ioemu/block-bochs.c 2007-05-18 10:45:21.000000000
-0400
+++ xen-3.1.0-src.new/tools/ioemu/block-bochs.c 2008-07-11 07:54:52.000000000
-0400
@@ -88,10 +88,11 @@ static int bochs_probe(const uint8_t *bu
static int bochs_open(BlockDriverState *bs, const char *filename)
{
BDRVBochsState *s = bs->opaque;
- int fd, i;
+ int fd = -1, i;
struct bochs_header bochs;
- fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE);
+ if (!bs->drv_read_only)
+ fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE);
if (fd < 0) {
fd = open(filename, O_RDONLY | O_BINARY | O_LARGEFILE);
if (fd < 0)
diff -rup xen-3.1.0-src.orig/tools/ioemu/block.c
xen-3.1.0-src.new/tools/ioemu/block.c
--- xen-3.1.0-src.orig/tools/ioemu/block.c 2008-07-10 05:41:58.000000000 -0400
+++ xen-3.1.0-src.new/tools/ioemu/block.c 2008-07-11 07:54:18.000000000 -0400
@@ -125,13 +125,14 @@ void bdrv_register(BlockDriver *bdrv)
}
/* create a new block device (by default it is empty) */
-BlockDriverState *bdrv_new(const char *device_name)
+BlockDriverState *bdrv_new(const char *device_name, int read_only)
{
BlockDriverState **pbs, *bs;
bs = qemu_mallocz(sizeof(BlockDriverState));
if(!bs)
return NULL;
+ bs->drv_read_only = read_only;
pstrcpy(bs->device_name, sizeof(bs->device_name), device_name);
if (device_name[0] != ''\0'') {
/* insert at the end */
@@ -287,7 +288,7 @@ int bdrv_open2(BlockDriverState *bs, con
instead of opening ''filename'' directly */
/* if there is a backing file, use it */
- bs1 = bdrv_new("");
+ bs1 = bdrv_new("", 0);
if (!bs1) {
return -1;
}
@@ -332,7 +333,7 @@ int bdrv_open2(BlockDriverState *bs, con
#endif
if (bs->backing_file[0] != ''\0'' &&
drv->bdrv_is_allocated) {
/* if there is a backing file, use it */
- bs->backing_hd = bdrv_new("");
+ bs->backing_hd = bdrv_new("", 0);
if (!bs->backing_hd) {
fail:
bdrv_close(bs);
@@ -690,7 +691,7 @@ static int raw_probe(const uint8_t *buf,
static int raw_open(BlockDriverState *bs, const char *filename)
{
BDRVRawState *s = bs->opaque;
- int fd;
+ int fd = -1;
int64_t size;
#ifdef _BSD
struct stat sb;
@@ -700,7 +701,8 @@ static int raw_open(BlockDriverState *bs
int rv;
#endif
- fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE);
+ if (!bs->drv_read_only)
+ fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE);
if (fd < 0) {
fd = open(filename, O_RDONLY | O_BINARY | O_LARGEFILE);
if (fd < 0)
diff -rup xen-3.1.0-src.orig/tools/ioemu/block-cow.c
xen-3.1.0-src.new/tools/ioemu/block-cow.c
--- xen-3.1.0-src.orig/tools/ioemu/block-cow.c 2007-05-18 10:45:21.000000000
-0400
+++ xen-3.1.0-src.new/tools/ioemu/block-cow.c 2008-07-11 07:57:24.000000000
-0400
@@ -65,11 +65,12 @@ static int cow_probe(const uint8_t *buf,
static int cow_open(BlockDriverState *bs, const char *filename)
{
BDRVCowState *s = bs->opaque;
- int fd;
+ int fd = -1;
struct cow_header_v2 cow_header;
int64_t size;
- fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE);
+ if (!bs->drv_read_only)
+ fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE);
if (fd < 0) {
fd = open(filename, O_RDONLY | O_BINARY | O_LARGEFILE);
if (fd < 0)
diff -rup xen-3.1.0-src.orig/tools/ioemu/block_int.h
xen-3.1.0-src.new/tools/ioemu/block_int.h
--- xen-3.1.0-src.orig/tools/ioemu/block_int.h 2007-05-18 10:45:21.000000000
-0400
+++ xen-3.1.0-src.new/tools/ioemu/block_int.h 2008-07-11 07:48:58.000000000
-0400
@@ -46,6 +46,7 @@ struct BlockDriver {
struct BlockDriverState {
int64_t total_sectors;
+ int drv_read_only; /* if true, all media is forced read only */
int read_only; /* if true, the media is read only */
int inserted; /* if true, the media is present */
int removable; /* if true, the media can be removed */
diff -rup xen-3.1.0-src.orig/tools/ioemu/block-qcow.c
xen-3.1.0-src.new/tools/ioemu/block-qcow.c
--- xen-3.1.0-src.orig/tools/ioemu/block-qcow.c 2007-05-18 10:45:21.000000000
-0400
+++ xen-3.1.0-src.new/tools/ioemu/block-qcow.c 2008-07-11 07:57:50.000000000
-0400
@@ -92,14 +92,16 @@ static int qcow_probe(const uint8_t *buf
static int qcow_open(BlockDriverState *bs, const char *filename)
{
BDRVQcowState *s = bs->opaque;
- int fd, len, i, shift;
+ int fd = -1, len, i, shift;
QCowHeader header;
- fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE);
+ if (!bs->drv_read_only)
+ fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE);
if (fd < 0) {
fd = open(filename, O_RDONLY | O_BINARY | O_LARGEFILE);
if (fd < 0)
return -1;
+ bs->read_only = 1;
}
s->fd = fd;
if (read(fd, &header, sizeof(header)) != sizeof(header))
diff -rup xen-3.1.0-src.orig/tools/ioemu/block-vmdk.c
xen-3.1.0-src.new/tools/ioemu/block-vmdk.c
--- xen-3.1.0-src.orig/tools/ioemu/block-vmdk.c 2007-05-18 10:45:21.000000000
-0400
+++ xen-3.1.0-src.new/tools/ioemu/block-vmdk.c 2008-07-11 07:55:36.000000000
-0400
@@ -92,11 +92,12 @@ static int vmdk_probe(const uint8_t *buf
static int vmdk_open(BlockDriverState *bs, const char *filename)
{
BDRVVmdkState *s = bs->opaque;
- int fd, i;
+ int fd = -1, i;
uint32_t magic;
int l1_size;
- fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE);
+ if (!bs->drv_read_only)
+ fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE);
if (fd < 0) {
fd = open(filename, O_RDONLY | O_BINARY | O_LARGEFILE);
if (fd < 0)
diff -rup xen-3.1.0-src.orig/tools/ioemu/block-vpc.c
xen-3.1.0-src.new/tools/ioemu/block-vpc.c
--- xen-3.1.0-src.orig/tools/ioemu/block-vpc.c 2007-05-18 10:45:21.000000000
-0400
+++ xen-3.1.0-src.new/tools/ioemu/block-vpc.c 2008-07-11 07:56:00.000000000
-0400
@@ -89,10 +89,11 @@ static int vpc_probe(const uint8_t *buf,
static int vpc_open(BlockDriverState *bs, const char *filename)
{
BDRVVPCState *s = bs->opaque;
- int fd, i;
+ int fd = -1, i;
struct vpc_subheader header;
- fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE);
+ if (!bs->drv_read_only)
+ fd = open(filename, O_RDWR | O_BINARY | O_LARGEFILE);
if (fd < 0) {
fd = open(filename, O_RDONLY | O_BINARY | O_LARGEFILE);
if (fd < 0)
diff -rup xen-3.1.0-src.orig/tools/ioemu/block-vvfat.c
xen-3.1.0-src.new/tools/ioemu/block-vvfat.c
--- xen-3.1.0-src.orig/tools/ioemu/block-vvfat.c 2007-05-18 10:45:21.000000000
-0400
+++ xen-3.1.0-src.new/tools/ioemu/block-vvfat.c 2008-07-11 07:57:04.000000000
-0400
@@ -2737,7 +2737,7 @@ static int enable_write_target(BDRVVVFAT
if (bdrv_create(&bdrv_qcow,
s->qcow_filename, s->sector_count, "fat:", 0) < 0)
return -1;
- s->qcow = bdrv_new("");
+ s->qcow = bdrv_new("", 0);
if (s->qcow == NULL || bdrv_open(s->qcow, s->qcow_filename, 0)
< 0)
return -1;
diff -rup xen-3.1.0-src.orig/tools/ioemu/hw/pc.c
xen-3.1.0-src.new/tools/ioemu/hw/pc.c
--- xen-3.1.0-src.orig/tools/ioemu/hw/pc.c 2007-05-18 10:45:21.000000000 -0400
+++ xen-3.1.0-src.new/tools/ioemu/hw/pc.c 2008-07-11 07:50:35.000000000 -0400
@@ -911,10 +911,10 @@ static void pc_init1(uint64_t ram_size,
BlockDriverState *bdrv;
scsi = lsi_scsi_init(pci_bus, -1);
- bdrv = bdrv_new("scsidisk");
+ bdrv = bdrv_new("scsidisk", 0);
bdrv_open(bdrv, "scsi_disk.img", 0);
lsi_scsi_attach(scsi, bdrv, -1);
- bdrv = bdrv_new("scsicd");
+ bdrv = bdrv_new("scsicd", 0);
bdrv_open(bdrv, "scsi_cd.iso", 0);
bdrv_set_type_hint(bdrv, BDRV_TYPE_CDROM);
lsi_scsi_attach(scsi, bdrv, -1);
diff -rup xen-3.1.0-src.orig/tools/ioemu/hw/usb-msd.c
xen-3.1.0-src.new/tools/ioemu/hw/usb-msd.c
--- xen-3.1.0-src.orig/tools/ioemu/hw/usb-msd.c 2008-07-10 05:41:58.000000000
-0400
+++ xen-3.1.0-src.new/tools/ioemu/hw/usb-msd.c 2008-07-11 07:50:27.000000000
-0400
@@ -391,7 +391,7 @@ USBDevice *usb_msd_init(const char *file
if (!s)
return NULL;
- bdrv = bdrv_new("usb");
+ bdrv = bdrv_new("usb", 0);
bdrv_open2(bdrv, filename, 0, drv);
s->dev.speed = USB_SPEED_FULL;
diff -rup xen-3.1.0-src.orig/tools/ioemu/qemu-img.c
xen-3.1.0-src.new/tools/ioemu/qemu-img.c
--- xen-3.1.0-src.orig/tools/ioemu/qemu-img.c 2007-05-18 10:45:21.000000000
-0400
+++ xen-3.1.0-src.new/tools/ioemu/qemu-img.c 2008-07-11 07:51:12.000000000 -0400
@@ -284,7 +284,7 @@ static BlockDriverState *bdrv_new_open(c
BlockDriver *drv;
char password[256];
- bs = bdrv_new("");
+ bs = bdrv_new("", 0);
if (!bs)
error("Not enough memory");
if (fmt) {
@@ -410,7 +410,7 @@ static int img_commit(int argc, char **a
help();
filename = argv[optind++];
- bs = bdrv_new("");
+ bs = bdrv_new("", 0);
if (!bs)
error("Not enough memory");
if (fmt) {
@@ -657,7 +657,7 @@ static int img_info(int argc, char **arg
help();
filename = argv[optind++];
- bs = bdrv_new("");
+ bs = bdrv_new("", 0);
if (!bs)
error("Not enough memory");
if (fmt) {
diff -rup xen-3.1.0-src.orig/tools/ioemu/vl.c xen-3.1.0-src.new/tools/ioemu/vl.c
--- xen-3.1.0-src.orig/tools/ioemu/vl.c 2008-07-10 05:41:58.000000000 -0400
+++ xen-3.1.0-src.new/tools/ioemu/vl.c 2008-07-11 07:51:36.000000000 -0400
@@ -6522,7 +6522,7 @@ int main(int argc, char **argv)
/* we always create the cdrom drive, even if no disk is there */
bdrv_init();
if (cdrom_index >= 0) {
- bs_table[cdrom_index] = bdrv_new("cdrom");
+ bs_table[cdrom_index] = bdrv_new("cdrom", 0);
bdrv_set_type_hint(bs_table[cdrom_index], BDRV_TYPE_CDROM);
}
@@ -6532,7 +6532,7 @@ int main(int argc, char **argv)
if (!bs_table[i]) {
char buf[64];
snprintf(buf, sizeof(buf), "hd%c", i +
''a'');
- bs_table[i] = bdrv_new(buf);
+ bs_table[i] = bdrv_new(buf, 0);
}
if (bdrv_open(bs_table[i], hd_filename[i], snapshot) < 0) {
fprintf(stderr, "qemu: could not open hard disk image
''%s''\n",
@@ -6548,7 +6548,7 @@ int main(int argc, char **argv)
#endif /* !CONFIG_DM */
/* we always create at least one floppy disk */
- fd_table[0] = bdrv_new("fda");
+ fd_table[0] = bdrv_new("fda", 0);
bdrv_set_type_hint(fd_table[0], BDRV_TYPE_FLOPPY);
for(i = 0; i < MAX_FD; i++) {
@@ -6556,7 +6556,7 @@ int main(int argc, char **argv)
if (!fd_table[i]) {
char buf[64];
snprintf(buf, sizeof(buf), "fd%c", i +
''a'');
- fd_table[i] = bdrv_new(buf);
+ fd_table[i] = bdrv_new(buf, 0);
bdrv_set_type_hint(fd_table[i], BDRV_TYPE_FLOPPY);
}
if (fd_filename[i] != ''\0'') {
diff -rup xen-3.1.0-src.orig/tools/ioemu/vl.h xen-3.1.0-src.new/tools/ioemu/vl.h
--- xen-3.1.0-src.orig/tools/ioemu/vl.h 2008-07-10 05:41:58.000000000 -0400
+++ xen-3.1.0-src.new/tools/ioemu/vl.h 2008-07-11 07:47:49.000000000 -0400
@@ -545,7 +545,7 @@ BlockDriver *bdrv_find_format(const char
int bdrv_create(BlockDriver *drv,
const char *filename, int64_t size_in_sectors,
const char *backing_file, int flags);
-BlockDriverState *bdrv_new(const char *device_name);
+BlockDriverState *bdrv_new(const char *device_name, int read_only);
void bdrv_delete(BlockDriverState *bs);
int bdrv_open(BlockDriverState *bs, const char *filename, int snapshot);
int bdrv_open2(BlockDriverState *bs, const char *filename, int snapshot,
diff -rup xen-3.1.0-src.orig/tools/ioemu/xenstore.c
xen-3.1.0-src.new/tools/ioemu/xenstore.c
--- xen-3.1.0-src.orig/tools/ioemu/xenstore.c 2008-07-10 05:41:58.000000000
-0400
+++ xen-3.1.0-src.new/tools/ioemu/xenstore.c 2008-07-11 08:20:36.000000000 -0400
@@ -81,9 +81,9 @@ void xenstore_parse_domain_config(int do
{
char **e = NULL;
char *buf = NULL, *path;
- char *fpath = NULL, *bpath = NULL,
+ char *fpath = NULL, *bpath = NULL, *mode = NULL,
*dev = NULL, *params = NULL, *type = NULL, *format = NULL;
- int i, is_scsi;
+ int i, is_scsi, is_readonly = 0;
unsigned int len, num, hd_index;
BlockDriver *drv;
@@ -141,6 +141,15 @@ void xenstore_parse_domain_config(int do
params = xs_read(xsh, XBT_NULL, buf, &len);
if (params == NULL)
continue;
+ 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;
+ fprintf(stderr, "Read only %s %d\n", params, is_readonly);
if (pasprintf(&buf, "%s/format", bpath) == -1)
continue;
free(format);
@@ -177,7 +186,7 @@ void xenstore_parse_domain_config(int do
}
}
- bs_table[hd_index + (is_scsi ? MAX_DISKS : 0)] = bdrv_new(dev);
+ bs_table[hd_index + (is_scsi ? MAX_DISKS : 0)] = bdrv_new(dev,
is_readonly);
/* check if it is a cdrom */
if (type && !strcmp(type, "cdrom")) {
bdrv_set_type_hint(bs_table[hd_index], BDRV_TYPE_CDROM);
@@ -203,6 +212,7 @@ void xenstore_parse_domain_config(int do
out:
free(type);
free(format);
+ free(mode);
free(params);
free(dev);
free(bpath);
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Jackson
2008-Jul-24 13:37 UTC
Re: [Xen-devel] RFC: Making QEMU honour ''readonly'' flag for disks
Daniel P. Berrange writes ("[Xen-devel] RFC: Making QEMU honour
''readonly'' flag for disks"):> This proof of concept patch I''ve done against the RHEL-5 Xen tree
introduces
> a new ''drv_read_only'' flag to QEMU''s
BlockDriverState struct, and if set to
> non-zero, will cause the individual block backend drivers inside QEMU to
> always open with O_RDONLY, and never try O_RDWR. Ultimately this would be
> hooked up to the ''-drive'' parameter via a extra flag
'',ro'' in its args.
> It then makes xenstore.c set this flag based on the
''mode'' parameter for
> the disk in xenstore.
I think this is a good idea but you should discuss it with upstream to
try to minimise the difference between the patch that goes into our
tree and the one that goes into upstream.
Normally I would say that this kind of thing is rather much to be
doing after feature freeze but I think you could (and I would) argue
that the missing check is a security defect.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Daniel P. Berrange
2008-Jul-24 13:41 UTC
Re: [Xen-devel] RFC: Making QEMU honour ''readonly'' flag for disks
On Thu, Jul 24, 2008 at 02:37:17PM +0100, Ian Jackson wrote:> Daniel P. Berrange writes ("[Xen-devel] RFC: Making QEMU honour ''readonly'' flag for disks"): > > This proof of concept patch I''ve done against the RHEL-5 Xen tree introduces > > a new ''drv_read_only'' flag to QEMU''s BlockDriverState struct, and if set to > > non-zero, will cause the individual block backend drivers inside QEMU to > > always open with O_RDONLY, and never try O_RDWR. Ultimately this would be > > hooked up to the ''-drive'' parameter via a extra flag '',ro'' in its args. > > It then makes xenstore.c set this flag based on the ''mode'' parameter for > > the disk in xenstore. > > I think this is a good idea but you should discuss it with upstream to > try to minimise the difference between the patch that goes into our > tree and the one that goes into upstream.Ok, I''ll do a generic patch for upstream QEMU and then a xenstore integration add on. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2008-Jul-24 13:43 UTC
Re: [Xen-devel] RFC: Making QEMU honour ''readonly'' flag for disks
Daniel P. Berrange writes ("Re: [Xen-devel] RFC: Making QEMU honour
''readonly'' flag for disks"):> Ok, I''ll do a generic patch for upstream QEMU and then a xenstore
integration
> add on.
That would be ideal. If you post the former to the qemu list I can
pick it up from there.
Thanks!
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Jackson
2008-Jul-30 09:01 UTC
Re: [Xen-devel] RFC: Making QEMU honour ''readonly'' flag for disks
Daniel P. Berrange writes ("[Xen-devel] RFC: Making QEMU honour
''readonly'' flag for disks"):> The Xen disk configuration syntax allows a block device to be marked as
> readonly, exclusive writable or shared writeable. The xen hotplug scripts
> will clash for clashing configs between domains, but it is upto the
> backend drivers to actually enforce the readonly flag on I/O operations.
> The paravirt backend disk driver does this fine, but QEMU''s
emulated
> backend driver does not.
I still think this is a fix we should have but your patch is very
intrusive. Is there some reason why you didn''t just invent
BDRV_O_RDONLY_NO__ACTUALLY__READONLY ? A new parameter to bdrv_new
seems quite wrong.
(It''s a shame that the existing BDRV_O_RDONLY does something strange
and probably wrong, but we probably don''t want to fix that in our
branch.)
Would you be willing to prepare a revised patch along those lines ?
If you don''t want to deal with upstream I can put your change in the
pile with the others ...
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Daniel P. Berrange
2008-Jul-30 09:16 UTC
Re: [Xen-devel] RFC: Making QEMU honour ''readonly'' flag for disks
On Wed, Jul 30, 2008 at 10:01:29AM +0100, Ian Jackson wrote:> Daniel P. Berrange writes ("[Xen-devel] RFC: Making QEMU honour ''readonly'' flag for disks"): > > The Xen disk configuration syntax allows a block device to be marked as > > readonly, exclusive writable or shared writeable. The xen hotplug scripts > > will clash for clashing configs between domains, but it is upto the > > backend drivers to actually enforce the readonly flag on I/O operations. > > The paravirt backend disk driver does this fine, but QEMU''s emulated > > backend driver does not. > > I still think this is a fix we should have but your patch is very > intrusive. Is there some reason why you didn''t just invent > BDRV_O_RDONLY_NO__ACTUALLY__READONLY ? A new parameter to bdrv_new > seems quite wrong. > > (It''s a shame that the existing BDRV_O_RDONLY does something strange > and probably wrong, but we probably don''t want to fix that in our > branch.)Yeah, I''m actually attempting to fix that craziness, which is why I''ve not posted an update yet. I think it may actually be easier to fix that it appears.... Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel P. Berrange
2008-Jul-31 11:45 UTC
Re: [Xen-devel] RFC: Making QEMU honour ''readonly'' flag for disks
On Wed, Jul 30, 2008 at 10:16:08AM +0100, Daniel P. Berrange wrote:> On Wed, Jul 30, 2008 at 10:01:29AM +0100, Ian Jackson wrote: > > Daniel P. Berrange writes ("[Xen-devel] RFC: Making QEMU honour ''readonly'' flag for disks"): > > > The Xen disk configuration syntax allows a block device to be marked as > > > readonly, exclusive writable or shared writeable. The xen hotplug scripts > > > will clash for clashing configs between domains, but it is upto the > > > backend drivers to actually enforce the readonly flag on I/O operations. > > > The paravirt backend disk driver does this fine, but QEMU''s emulated > > > backend driver does not. > > > > I still think this is a fix we should have but your patch is very > > intrusive. Is there some reason why you didn''t just invent > > BDRV_O_RDONLY_NO__ACTUALLY__READONLY ? A new parameter to bdrv_new > > seems quite wrong. > > > > (It''s a shame that the existing BDRV_O_RDONLY does something strange > > and probably wrong, but we probably don''t want to fix that in our > > branch.) > > Yeah, I''m actually attempting to fix that craziness, which is why > I''ve not posted an update yet. I think it may actually be easier > to fix that it appears....Take a look at my patch on qemu-devel subject of "PATCH: Control over drive open modes for backing file" If that gets into QEMU, the xen specific bit will merely involve changing the xenstore.c file to pass correct flags to bdrv_open() instead of using the default ''0''. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel