Am 12.08.2012 04:48, schrieb Kevin Shanahan:
> So qmp_change_blockdev uses bdrv_is_read_only() to check whether to
> try and open the backing file read only, which uses the ->read_only
> member of struct BlockDriverState to decide whether to pass the
> BDRV_O_RDRW flag to qmp_bdrv_open_encypted() and then bdrv_open().
>
> I would assume we want to set this flag in drive_init() when the block
> driver state is initialised. How about a patch like this instead?
>
> diff --git a/blockdev.c b/blockdev.c
> index 8669142..ba22064 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -526,6 +526,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
> if_name[type], mediastr, unit_id);
> }
> dinfo->bdrv = bdrv_new(dinfo->id);
> + dinfo->bdrv->read_only = ro;
> dinfo->devaddr = devaddr;
> dinfo->type = type;
> dinfo->bus = bus_id;
Ah, yes, this looks much more like the proper fix. Basically we need to
set everything that is retained after a 'change' command. We have this
code in qmp_change_blockdev():
bdrv_flags = bdrv_is_read_only(bs) ? 0 : BDRV_O_RDWR;
bdrv_flags |= bdrv_is_snapshot(bs) ? BDRV_O_SNAPSHOT : 0;
qmp_bdrv_open_encrypted(bs, filename, bdrv_flags, drv, NULL, errp);
bdrv_is_read_only() is covered by your patch, bdrv_is_snapshot()
additionally requires bs->open_flags to be right.
Markus, how will this look in the -blockdev world? There seem to be
properties that belong to host state, but are not coupled to a medium.
Kevin
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list