* [Qemu-devel] [PATCH] SCSI allow removable SBC disks @ 2012-07-30 4:43 Ronnie Sahlberg 2012-07-30 4:43 ` [Qemu-devel] [PATCH] SCSI/BLOCK: Allow (scsi) disks to be REMOVABLE Ronnie Sahlberg 0 siblings, 1 reply; 3+ messages in thread From: Ronnie Sahlberg @ 2012-07-30 4:43 UTC (permalink / raw) To: qemu-devel, pbonzini List, Paolo Please find a small patch that adds a new "drive" keyword "removable". When used together with SCSI DISK devices, this makes the emulation support removable scsi disk. Example: -drive file=./scsi-disk.img,if=scsi,removable Emulation of SCSI DISK with removable media currently pass all my tests for SCSI removable media and behaviour of STARTSTOPUNIT/ALLOWPREVENTMEDIUMREMOVAL commands : iscsi-test --dataloss --test="*prevent*eject*" iscsi://127.0.0.1:32601/iqn.qemu.scsi-test/3 0381_preventallow_eject: ======================== Try to set PREVENTALLOW ... [OK] Try to eject the media ... Eject failed. [OK] Load the media again in case it was ejected ... [OK] Clear the PREVENTALLOW again ... [OK] TEST T0381_preventallow_eject [OK] iscsi-test --dataloss --test="*nomedia*" iscsi://127.0.0.1:32601/iqn.qemu.scsi-test/3 0370_nomedia: ============ Try to eject the media ... [OK] Test TESTUNITREADY ... [OK] Test SYNCHRONIZECACHE10 ... [OK] Test SYNCHRONIZECACHE16 ... [OK] Test READ10 ... [OK] Test READ12 ... [OK] Test READ16 ... [OK] Test READCAPACITY10 ... [OK] Test READCAPACITY16 ... [OK] Test GETLBASTATUS ... [OK] Test PREFETCH10 ... [OK] Test PREFETCH16 ... [OK] Test VERIFY10 ... [OK] Test VERIFY12 ... [OK] Test VERIFY16 ... [OK] Test WRITE10 ... [OK] Test WRITE12 ... [OK] Test WRITE16 ... [OK] Test WRITEVERIFY10 ... [OK] Test WRITEVERIFY12 ... [OK] Test WRITEVERIFY16 ... [OK] Test ORWRITE ... [OK] Test COMPAREWRITE ... [OK] Test WRITESAME10 ... [OK] Test WRITESAME16 ... [OK] Test UNMAP ... [OK] Try to mount the media again ... [OK] TEST T0370_nomedia [OK] ^ permalink raw reply [flat|nested] 3+ messages in thread
* [Qemu-devel] [PATCH] SCSI/BLOCK: Allow (scsi) disks to be REMOVABLE 2012-07-30 4:43 [Qemu-devel] [PATCH] SCSI allow removable SBC disks Ronnie Sahlberg @ 2012-07-30 4:43 ` Ronnie Sahlberg 2012-07-30 7:37 ` Paolo Bonzini 0 siblings, 1 reply; 3+ messages in thread From: Ronnie Sahlberg @ 2012-07-30 4:43 UTC (permalink / raw) To: qemu-devel, pbonzini; +Cc: Ronnie Sahlberg 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". Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> --- 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 copy-on-read=@var{copy-on-read} @var{copy-on-read} is "on" or "off" and enables whether to copy read backing file sectors into the image file. +@item 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 -- 1.7.3.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] SCSI/BLOCK: Allow (scsi) disks to be REMOVABLE 2012-07-30 4:43 ` [Qemu-devel] [PATCH] SCSI/BLOCK: Allow (scsi) disks to be REMOVABLE Ronnie Sahlberg @ 2012-07-30 7:37 ` Paolo Bonzini 0 siblings, 0 replies; 3+ messages in thread From: Paolo Bonzini @ 2012-07-30 7:37 UTC (permalink / raw) To: Ronnie Sahlberg; +Cc: qemu-devel 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 <ronniesahlberg@gmail.com> > --- > 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 copy-on-read=@var{copy-on-read} > @var{copy-on-read} is "on" or "off" and enables whether to copy read backing > file sectors into the image file. > +@item 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 > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-07-30 7:37 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-07-30 4:43 [Qemu-devel] [PATCH] SCSI allow removable SBC disks Ronnie Sahlberg 2012-07-30 4:43 ` [Qemu-devel] [PATCH] SCSI/BLOCK: Allow (scsi) disks to be REMOVABLE Ronnie Sahlberg 2012-07-30 7:37 ` Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).