qemu-devel
[Top][All Lists]
Advanced

[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
> 





reply via email to

[Prev in Thread] Current Thread [Next in Thread]