Re: [Qemu-devel] [PATCH 2/7] scsi-disk: support WRITE SAME (16) with unmap bit

2010-12-16 Thread Kevin Wolf 
Am 10.12.2010 16:00, schrieb Christoph Hellwig:
 Support discards via the WRITE SAME command with the unmap bit set, and
 tell the initiator about the support for it via the block limit and the
 new thin provisioning EVPD pages.  Also fix the comment which incorrectly
 describedthe block limits EVPD page.
 
 Signed-off-by: Christoph Hellwig h...@lst.de
 
 Index: qemu/hw/scsi-disk.c
 ===
 --- qemu.orig/hw/scsi-disk.c  2010-12-07 09:35:44.203009851 +0100
 +++ qemu/hw/scsi-disk.c   2010-12-07 09:42:07.984255141 +0100
 @@ -424,7 +424,8 @@ static int scsi_disk_emulate_inquiry(SCS
  outbuf[buflen++] = 0x80; // unit serial number
  outbuf[buflen++] = 0x83; // device identification
  if (bdrv_get_type_hint(s-bs) != BDRV_TYPE_CDROM) {
 -outbuf[buflen++] = 0xb0; // block device characteristics
 +outbuf[buflen++] = 0xb0; // block limits
 +outbuf[buflen++] = 0xb2; // thin provisioning
  }
  outbuf[pages] = buflen - pages - 1; // number of pages
  break;
 @@ -466,8 +467,10 @@ static int scsi_disk_emulate_inquiry(SCS
  buflen += id_len;
  break;
  }
 -case 0xb0: /* block device characteristics */
 +case 0xb0: /* block limits */
  {
 +unsigned int unmap_sectors =
 +s-qdev.conf.discard_granularity / s-qdev.blocksize;
  unsigned int min_io_size =
  s-qdev.conf.min_io_size / s-qdev.blocksize;
  unsigned int opt_io_size =
 @@ -492,6 +495,21 @@ static int scsi_disk_emulate_inquiry(SCS
  outbuf[13] = (opt_io_size  16)  0xff;
  outbuf[14] = (opt_io_size  8)  0xff;
  outbuf[15] = opt_io_size  0xff;
 +
 +/* optimal unmap granularity */
 +outbuf[28] = (unmap_sectors  24)  0xff;
 +outbuf[29] = (unmap_sectors  16)  0xff;
 +outbuf[30] = (unmap_sectors  8)  0xff;
 +outbuf[31] = unmap_sectors  0xff;
 +break;
 +}
 +case 0xb2: /* thin provisioning */
 +{
 +outbuf[3] = buflen = 8;
 +outbuf[4] = 0;
 +outbuf[5] = 0x40; /* write same with unmap supported */
 +outbuf[6] = 0;
 +outbuf[7] = 0;
  break;
  }
  default:
 @@ -959,6 +977,11 @@ static int scsi_disk_emulate_command(SCS
  outbuf[11] = 0;
  outbuf[12] = 0;
  outbuf[13] = get_physical_block_exp(s-qdev.conf);
 +
 +/* set TPE bit if the format supports discard */
 +if (s-qdev.conf.discard_granularity)
 +outbuf[14] = 0x80;

Missing braces.

 +
  /* Protection, exponent and lowest lba field left blank. */
  buflen = req-cmd.xfer;
  break;
 @@ -1123,6 +1146,34 @@ static int32_t scsi_send_command(SCSIDev
  goto illegal_lba;
  }
  break;
 +case WRITE_SAME_16:
 +len = r-req.cmd.xfer / d-blocksize;
 +
 +DPRINTF(WRITE SAME(16) (sector % PRId64 , count %d)\n,
 +r-req.cmd.lba, len);
 +
 +if (r-req.cmd.lba  s-max_lba) {
 +goto illegal_lba;
 +}
 +
 +/*
 + * We only support WRITE SAME with the unmap bit set for now.
 + */
 +if (!(buf[1]  0x8)) {
 +goto fail;
 +}
 +
 +rc = bdrv_discard(s-bs, r-req.cmd.lba * s-cluster_size,
 +  len * s-cluster_size);
 +if (rc  0) {
 +/* XXX: better error code ?*/
 +goto fail;
 +}
 +
 +scsi_req_set_status(r, GOOD, NO_SENSE);
 +scsi_req_complete(r-req);
 +scsi_remove_request(r);

Isn't this the same as scsi_command_complete()?

 +return 0;

And if you break; here instead of returning (like all other commands)
and remove the three lines above completely, I think it would just do
the right thing.

Kevin

Re: [Qemu-devel] [PATCH 2/7] scsi-disk: support WRITE SAME (16) with unmap bit

2010-12-16 Thread Christoph Hellwig 
On Thu, Dec 16, 2010 at 04:48:15PM +0100, Kevin Wolf wrote:
  +scsi_req_set_status(r, GOOD, NO_SENSE);
  +scsi_req_complete(r-req);
  +scsi_remove_request(r);
 
 Isn't this the same as scsi_command_complete()?

Yes.

 
  +return 0;
 
 And if you break; here instead of returning (like all other commands)
 and remove the three lines above completely, I think it would just do
 the right thing.

Yes, that looks doable.

[Qemu-devel] [PATCH 2/7] scsi-disk: support WRITE SAME (16) with unmap bit

2010-12-10 Thread Christoph Hellwig 
Support discards via the WRITE SAME command with the unmap bit set, and
tell the initiator about the support for it via the block limit and the
new thin provisioning EVPD pages.  Also fix the comment which incorrectly
describedthe block limits EVPD page.

Signed-off-by: Christoph Hellwig h...@lst.de

Index: qemu/hw/scsi-disk.c
===
--- qemu.orig/hw/scsi-disk.c2010-12-07 09:35:44.203009851 +0100
+++ qemu/hw/scsi-disk.c 2010-12-07 09:42:07.984255141 +0100
@@ -424,7 +424,8 @@ static int scsi_disk_emulate_inquiry(SCS
 outbuf[buflen++] = 0x80; // unit serial number
 outbuf[buflen++] = 0x83; // device identification
 if (bdrv_get_type_hint(s-bs) != BDRV_TYPE_CDROM) {
-outbuf[buflen++] = 0xb0; // block device characteristics
+outbuf[buflen++] = 0xb0; // block limits
+outbuf[buflen++] = 0xb2; // thin provisioning
 }
 outbuf[pages] = buflen - pages - 1; // number of pages
 break;
@@ -466,8 +467,10 @@ static int scsi_disk_emulate_inquiry(SCS
 buflen += id_len;
 break;
 }
-case 0xb0: /* block device characteristics */
+case 0xb0: /* block limits */
 {
+unsigned int unmap_sectors =
+s-qdev.conf.discard_granularity / s-qdev.blocksize;
 unsigned int min_io_size =
 s-qdev.conf.min_io_size / s-qdev.blocksize;
 unsigned int opt_io_size =
@@ -492,6 +495,21 @@ static int scsi_disk_emulate_inquiry(SCS
 outbuf[13] = (opt_io_size  16)  0xff;
 outbuf[14] = (opt_io_size  8)  0xff;
 outbuf[15] = opt_io_size  0xff;
+
+/* optimal unmap granularity */
+outbuf[28] = (unmap_sectors  24)  0xff;
+outbuf[29] = (unmap_sectors  16)  0xff;
+outbuf[30] = (unmap_sectors  8)  0xff;
+outbuf[31] = unmap_sectors  0xff;
+break;
+}
+case 0xb2: /* thin provisioning */
+{
+outbuf[3] = buflen = 8;
+outbuf[4] = 0;
+outbuf[5] = 0x40; /* write same with unmap supported */
+outbuf[6] = 0;
+outbuf[7] = 0;
 break;
 }
 default:
@@ -959,6 +977,11 @@ static int scsi_disk_emulate_command(SCS
 outbuf[11] = 0;
 outbuf[12] = 0;
 outbuf[13] = get_physical_block_exp(s-qdev.conf);
+
+/* set TPE bit if the format supports discard */
+if (s-qdev.conf.discard_granularity)
+outbuf[14] = 0x80;
+
 /* Protection, exponent and lowest lba field left blank. */
 buflen = req-cmd.xfer;
 break;
@@ -1123,6 +1146,34 @@ static int32_t scsi_send_command(SCSIDev
 goto illegal_lba;
 }
 break;
+case WRITE_SAME_16:
+len = r-req.cmd.xfer / d-blocksize;
+
+DPRINTF(WRITE SAME(16) (sector % PRId64 , count %d)\n,
+r-req.cmd.lba, len);
+
+if (r-req.cmd.lba  s-max_lba) {
+goto illegal_lba;
+}
+
+/*
+ * We only support WRITE SAME with the unmap bit set for now.
+ */
+if (!(buf[1]  0x8)) {
+goto fail;
+}
+
+rc = bdrv_discard(s-bs, r-req.cmd.lba * s-cluster_size,
+  len * s-cluster_size);
+if (rc  0) {
+/* XXX: better error code ?*/
+goto fail;
+}
+
+scsi_req_set_status(r, GOOD, NO_SENSE);
+scsi_req_complete(r-req);
+scsi_remove_request(r);
+return 0;
 default:
 DPRINTF(Unknown SCSI command (%2.2x)\n, buf[0]);
 fail:
Index: qemu/hw/scsi-defs.h
===
--- qemu.orig/hw/scsi-defs.h2010-12-07 09:35:44.219005032 +0100
+++ qemu/hw/scsi-defs.h 2010-12-07 09:38:35.726003986 +0100
@@ -84,6 +84,7 @@
 #define MODE_SENSE_10 0x5a
 #define PERSISTENT_RESERVE_IN 0x5e
 #define PERSISTENT_RESERVE_OUT 0x5f
+#define WRITE_SAME_16 0x93
 #define MAINTENANCE_IN0xa3
 #define MAINTENANCE_OUT   0xa4
 #define MOVE_MEDIUM   0xa5