[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] SCSI/BLOCK: Allow (scsi) disks to be REMOVABLE
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH] SCSI/BLOCK: Allow (scsi) disks to be REMOVABLE |
Date: |
Mon, 30 Jul 2012 09:37:37 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:13.0) Gecko/20120615 Thunderbird/13.0.1 |
Il 30/07/2012 06:43, Ronnie Sahlberg ha scritto:
> Add a new block device attribute : removable.
> Make all cdrom devices removable by default just like today
> but also allow (scsi) disk devices to become removable by a new
> -drive keyword : removable.
>
> Example
> -drive file=./scsi-disk.img,if=scsi,media=disk,removable
>
> Removable disks are currently only supported for SCSI but should be possible
> to extend to other bustypes that support removable media too.
> StartStopUnit scsi command has been updated to honour the allow/prevent
> medium removal flag for SBC devices in addition to MMC devices so that a
> scsi disk that is mounted by the guest can not be "ejected".
Nack, just use the removable property of -device scsi-hd. -drive should
only include host-visible properties. This is not true in many cases,
but let's not make things worse.
Paolo
> Signed-off-by: Ronnie Sahlberg <address@hidden>
> ---
> block.c | 10 ++++++++++
> block.h | 2 ++
> block_int.h | 1 +
> blockdev.c | 22 +++++++++++++++++-----
> hw/scsi-disk.c | 18 ++++++------------
> qemu-config.c | 4 ++++
> qemu-options.hx | 2 ++
> 7 files changed, 42 insertions(+), 17 deletions(-)
>
> diff --git a/block.c b/block.c
> index ce7eb8f..e9c0bfa 100644
> --- a/block.c
> +++ b/block.c
> @@ -2151,6 +2151,16 @@ int bdrv_is_read_only(BlockDriverState *bs)
> return bs->read_only;
> }
>
> +int bdrv_is_removable(BlockDriverState *bs)
> +{
> + return bs->removable;
> +}
> +
> +void bdrv_set_removable(BlockDriverState *bs, bool removable)
> +{
> + bs->removable = removable;
> +}
> +
> int bdrv_is_sg(BlockDriverState *bs)
> {
> return bs->sg;
> diff --git a/block.h b/block.h
> index c89590d..b6dd71f 100644
> --- a/block.h
> +++ b/block.h
> @@ -261,6 +261,8 @@ void bdrv_set_on_error(BlockDriverState *bs,
> BlockErrorAction on_read_error,
> BlockErrorAction on_write_error);
> BlockErrorAction bdrv_get_on_error(BlockDriverState *bs, int is_read);
> int bdrv_is_read_only(BlockDriverState *bs);
> +int bdrv_is_removable(BlockDriverState *bs);
> +void bdrv_set_removable(BlockDriverState *bs, bool removable);
> int bdrv_is_sg(BlockDriverState *bs);
> int bdrv_enable_write_cache(BlockDriverState *bs);
> void bdrv_set_enable_write_cache(BlockDriverState *bs, bool wce);
> diff --git a/block_int.h b/block_int.h
> index d72317f..3f6c771 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -266,6 +266,7 @@ struct BlockDriverState {
> size in sectors */
> int read_only; /* if true, the media is read only */
> int keep_read_only; /* if true, the media was requested to stay read
> only */
> + int removable; /* if true, the media is removable */
> int open_flags; /* flags used to open the file, re-used for re-open */
> int encrypted; /* if true, the media is encrypted */
> int valid_key; /* if true, a valid encryption key has been set */
> diff --git a/blockdev.c b/blockdev.c
> index 3d75015..034112c 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -288,6 +288,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
> int max_devs;
> int index;
> int ro = 0;
> + int removable = 0;
> int bdrv_flags = 0;
> int on_read_error, on_write_error;
> const char *devaddr;
> @@ -311,6 +312,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
>
> snapshot = qemu_opt_get_bool(opts, "snapshot", 0);
> ro = qemu_opt_get_bool(opts, "readonly", 0);
> + removable = qemu_opt_get_bool(opts, "removable", 0);
> copy_on_read = qemu_opt_get_bool(opts, "copy-on-read", false);
>
> file = qemu_opt_get(opts, "file");
> @@ -591,15 +593,25 @@ DriveInfo *drive_init(QemuOpts *opts, int
> default_to_scsi)
> if (media == MEDIA_CDROM) {
> /* CDROM is fine for any interface, don't check. */
> ro = 1;
> - } else if (ro == 1) {
> - if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY &&
> - type != IF_NONE && type != IF_PFLASH) {
> - error_report("readonly not supported by this bus type");
> - goto err;
> + removable = 1;
> + } else {
> + if (ro == 1) {
> + if (type != IF_SCSI && type != IF_VIRTIO && type != IF_FLOPPY &&
> + type != IF_NONE && type != IF_PFLASH) {
> + error_report("readonly not supported by this bus type");
> + goto err;
> + }
> + }
> + if (removable == 1) {
> + if (type != IF_SCSI) {
> + error_report("removable not supported by this bus type");
> + goto err;
> + }
> }
> }
>
> bdrv_flags |= ro ? 0 : BDRV_O_RDWR;
> + bdrv_set_removable(dinfo->bdrv, removable);
>
> if (ro && copy_on_read) {
> error_report("warning: disabling copy_on_read on readonly drive");
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index 5426990..4c394bd 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -58,8 +58,7 @@ typedef struct SCSIDiskReq {
> BlockAcctCookie acct;
> } SCSIDiskReq;
>
> -#define SCSI_DISK_F_REMOVABLE 0
> -#define SCSI_DISK_F_DPOFUA 1
> +#define SCSI_DISK_F_DPOFUA 0
>
> struct SCSIDiskState
> {
> @@ -668,7 +667,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req,
> uint8_t *outbuf)
> memset(outbuf, 0, buflen);
>
> outbuf[0] = s->qdev.type & 0x1f;
> - outbuf[1] = (s->features & (1 << SCSI_DISK_F_REMOVABLE)) ? 0x80 : 0;
> + outbuf[1] = bdrv_is_removable(s->qdev.conf.bs) ? 0x80 : 0;
> if (s->qdev.type == TYPE_ROM) {
> memcpy(&outbuf[16], "QEMU CD-ROM ", 16);
> } else {
> @@ -1251,7 +1250,7 @@ static int scsi_disk_emulate_start_stop(SCSIDiskReq *r)
> return 0;
> }
>
> - if (s->qdev.type == TYPE_ROM && loej) {
> + if (bdrv_is_removable(s->qdev.conf.bs) && loej) {
> if (!start && !s->tray_open && s->tray_locked) {
> scsi_check_condition(r,
> bdrv_is_inserted(s->qdev.conf.bs)
> @@ -1749,7 +1748,7 @@ static int scsi_initfn(SCSIDevice *dev)
> return -1;
> }
>
> - if (!(s->features & (1 << SCSI_DISK_F_REMOVABLE)) &&
> + if (!bdrv_is_removable(s->qdev.conf.bs) &&
> !bdrv_is_inserted(s->qdev.conf.bs)) {
> error_report("Device needs media, but drive is empty");
> return -1;
> @@ -1769,7 +1768,7 @@ static int scsi_initfn(SCSIDevice *dev)
> return -1;
> }
>
> - if (s->features & (1 << SCSI_DISK_F_REMOVABLE)) {
> + if (bdrv_is_removable(s->qdev.conf.bs)) {
> bdrv_set_dev_ops(s->qdev.conf.bs, &scsi_cd_block_ops, s);
> }
> bdrv_set_buffer_alignment(s->qdev.conf.bs, s->qdev.blocksize);
> @@ -1792,7 +1791,6 @@ static int scsi_cd_initfn(SCSIDevice *dev)
> SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
> s->qdev.blocksize = 2048;
> s->qdev.type = TYPE_ROM;
> - s->features |= 1 << SCSI_DISK_F_REMOVABLE;
> return scsi_initfn(&s->qdev);
> }
>
> @@ -1866,7 +1864,7 @@ static int get_device_type(SCSIDiskState *s)
> }
> s->qdev.type = buf[0];
> if (buf[1] & 0x80) {
> - s->features |= 1 << SCSI_DISK_F_REMOVABLE;
> + bdrv_set_removable(s->qdev.conf.bs, true);
> }
> return 0;
> }
> @@ -1967,8 +1965,6 @@ static SCSIRequest *scsi_block_new_request(SCSIDevice
> *d, uint32_t tag,
>
> static Property scsi_hd_properties[] = {
> DEFINE_SCSI_DISK_PROPERTIES(),
> - DEFINE_PROP_BIT("removable", SCSIDiskState, features,
> - SCSI_DISK_F_REMOVABLE, false),
> DEFINE_PROP_BIT("dpofua", SCSIDiskState, features,
> SCSI_DISK_F_DPOFUA, false),
> DEFINE_PROP_HEX64("wwn", SCSIDiskState, wwn, 0),
> @@ -2075,8 +2071,6 @@ static TypeInfo scsi_block_info = {
>
> static Property scsi_disk_properties[] = {
> DEFINE_SCSI_DISK_PROPERTIES(),
> - DEFINE_PROP_BIT("removable", SCSIDiskState, features,
> - SCSI_DISK_F_REMOVABLE, false),
> DEFINE_PROP_BIT("dpofua", SCSIDiskState, features,
> SCSI_DISK_F_DPOFUA, false),
> DEFINE_PROP_HEX64("wwn", SCSIDiskState, wwn, 0),
> diff --git a/qemu-config.c b/qemu-config.c
> index 5c3296b..ab12f5a 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -87,6 +87,10 @@ static QemuOptsList qemu_drive_opts = {
> .type = QEMU_OPT_BOOL,
> .help = "open drive file as read-only",
> },{
> + .name = "removable",
> + .type = QEMU_OPT_BOOL,
> + .help = "mark the device as removable",
> + },{
> .name = "iops",
> .type = QEMU_OPT_NUMBER,
> .help = "limit total I/O operations per second",
> diff --git a/qemu-options.hx b/qemu-options.hx
> index dc68e15..3748e38 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -196,6 +196,8 @@ Open drive @option{file} as read-only. Guest write
> attempts will fail.
> @item address@hidden
> @var{copy-on-read} is "on" or "off" and enables whether to copy read backing
> file sectors into the image file.
> address@hidden removable
> +Mark the drive @option{file} as removable. (CDROM devices are always
> removable).
> @end table
>
> By default, writethrough caching is used for all block device. This means
> that
>