block: do not set BDS read_only if copy_on_read enabled

A few block drivers will set the BDS read_only flag from their
.bdrv_open() function.  This means the bs->read_only flag could
be set after we enable copy_on_read, as the BDRV_O_COPY_ON_READ
flag check occurs prior to the call to bdrv->bdrv_open().

This adds an error return to bdrv_set_read_only(), and an error will be
return if we try to set the BDS to read_only while copy_on_read is
enabled.

This patch also changes the behavior of vvfat.  Before, vvfat could
override the drive 'readonly' flag with its own, internal 'rw' flag.

For instance, this -drive parameter would result in a writable image:

"-drive format=vvfat,dir=/tmp/vvfat,rw,if=virtio,readonly=on"

This is not correct.  Now, attempting to use the above -drive parameter
will result in an error (i.e., 'rw' is incompatible with 'readonly=on').

Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 0c5b4c1cc2c651471b131f21376dfd5ea24d2196.1491597120.git.jcody@redhat.com
This commit is contained in:
Jeff Cody 2017-04-07 16:55:26 -04:00
parent fe5241bfe3
commit e2b8247a32
7 changed files with 49 additions and 11 deletions

10
block.c
View File

@ -192,9 +192,17 @@ void path_combine(char *dest, int dest_size,
} }
} }
void bdrv_set_read_only(BlockDriverState *bs, bool read_only) int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp)
{ {
/* Do not set read_only if copy_on_read is enabled */
if (bs->copy_on_read && read_only) {
error_setg(errp, "Can't set node '%s' to r/o with copy-on-read enabled",
bdrv_get_device_or_node_name(bs));
return -EINVAL;
}
bs->read_only = read_only; bs->read_only = read_only;
return 0;
} }
void bdrv_get_full_backing_filename_from_filename(const char *backed, void bdrv_get_full_backing_filename_from_filename(const char *backed,

View File

@ -110,7 +110,10 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
return -EINVAL; return -EINVAL;
} }
bdrv_set_read_only(bs, true); /* no write support yet */ ret = bdrv_set_read_only(bs, true, errp); /* no write support yet */
if (ret < 0) {
return ret;
}
ret = bdrv_pread(bs->file, 0, &bochs, sizeof(bochs)); ret = bdrv_pread(bs->file, 0, &bochs, sizeof(bochs));
if (ret < 0) { if (ret < 0) {

View File

@ -72,7 +72,10 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
return -EINVAL; return -EINVAL;
} }
bdrv_set_read_only(bs, true); ret = bdrv_set_read_only(bs, true, errp);
if (ret < 0) {
return ret;
}
/* read header */ /* read header */
ret = bdrv_pread(bs->file, 128, &s->block_size, 4); ret = bdrv_pread(bs->file, 128, &s->block_size, 4);

View File

@ -419,8 +419,12 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
return -EINVAL; return -EINVAL;
} }
ret = bdrv_set_read_only(bs, true, errp);
if (ret < 0) {
return ret;
}
block_module_load_one("dmg-bz2"); block_module_load_one("dmg-bz2");
bdrv_set_read_only(bs, true);
s->n_chunks = 0; s->n_chunks = 0;
s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL; s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;

View File

@ -635,13 +635,22 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
goto failed_shutdown; goto failed_shutdown;
} }
/* rbd_open is always r/w */
r = rbd_open(s->io_ctx, s->name, &s->image, s->snap); r = rbd_open(s->io_ctx, s->name, &s->image, s->snap);
if (r < 0) { if (r < 0) {
error_setg_errno(errp, -r, "error reading header from %s", s->name); error_setg_errno(errp, -r, "error reading header from %s", s->name);
goto failed_open; goto failed_open;
} }
bdrv_set_read_only(bs, (s->snap != NULL)); /* If we are using an rbd snapshot, we must be r/o, otherwise
* leave as-is */
if (s->snap != NULL) {
r = bdrv_set_read_only(bs, true, &local_err);
if (r < 0) {
error_propagate(errp, local_err);
goto failed_open;
}
}
qemu_opts_del(opts); qemu_opts_del(opts);
return 0; return 0;

View File

@ -1156,8 +1156,6 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
s->current_cluster=0xffffffff; s->current_cluster=0xffffffff;
/* read only is the default for safety */
bdrv_set_read_only(bs, true);
s->qcow = NULL; s->qcow = NULL;
s->qcow_filename = NULL; s->qcow_filename = NULL;
s->fat2 = NULL; s->fat2 = NULL;
@ -1169,11 +1167,24 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags,
s->sector_count = cyls * heads * secs - (s->first_sectors_number - 1); s->sector_count = cyls * heads * secs - (s->first_sectors_number - 1);
if (qemu_opt_get_bool(opts, "rw", false)) { if (qemu_opt_get_bool(opts, "rw", false)) {
ret = enable_write_target(bs, errp); if (!bdrv_is_read_only(bs)) {
if (ret < 0) { ret = enable_write_target(bs, errp);
if (ret < 0) {
goto fail;
}
} else {
ret = -EPERM;
error_setg(errp,
"Unable to set VVFAT to 'rw' when drive is read-only");
goto fail;
}
} else {
/* read only is the default for safety */
ret = bdrv_set_read_only(bs, true, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
goto fail; goto fail;
} }
bdrv_set_read_only(bs, false);
} }
bs->total_sectors = cyls * heads * secs; bs->total_sectors = cyls * heads * secs;

View File

@ -434,7 +434,7 @@ int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
int64_t sector_num, int nb_sectors, int *pnum); int64_t sector_num, int nb_sectors, int *pnum);
bool bdrv_is_read_only(BlockDriverState *bs); bool bdrv_is_read_only(BlockDriverState *bs);
void bdrv_set_read_only(BlockDriverState *bs, bool read_only); int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp);
bool bdrv_is_sg(BlockDriverState *bs); bool bdrv_is_sg(BlockDriverState *bs);
bool bdrv_is_inserted(BlockDriverState *bs); bool bdrv_is_inserted(BlockDriverState *bs);
int bdrv_media_changed(BlockDriverState *bs); int bdrv_media_changed(BlockDriverState *bs);