On Wed, 2012-03-14 at 08:41 +0100, Paolo Bonzini wrote: > Il 13/03/2012 20:13, Richard Laager ha scritto: > >>> > > * For SCSI, report an unmap_granularity to the guest as follows: > >>> > > max(logical_block_size, discard_granularity) / > >>> > > logical_block_size > >> > > >> > This is more or less already in place later in the series. > > I didn't see it. Which patch number? > > Patch 11:
I was saying QEMU should pass the discard_granularity to the guest as
OPTIMAL UNMAP GRANULARITY. This would almost surely need to be done in
hw/scsi-disk.c, roughly around this change from your patch 10:
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1295,8 +1295,11 @@ static int scsi_disk_emulate_command(SCSIDiskReq *r)
outbuf[13] = get_physical_block_exp(&s->qdev.conf);
/* set TPE bit if the format supports discard */
- if (s->qdev.conf.discard_granularity) {
+ if (s->qdev.type == TYPE_DISK && s->qdev.conf.discard_granularity)
{
outbuf[14] = 0x80;
+ if (s->qdev.conf.discard_zeroes_data) {
+ outbuf[14] |= 0x40;
+ }
}
/* Protection, exponent and lowest lba field left blank. */
The code from patch 11 is more along the lines of what I think QEMU
should have in the block layer:
> + } else if (discard_granularity < s->qdev.conf.logical_block_size) {
> + error_report("scsi-block: invalid discard_granularity");
> + return -1;
>
> + } else if (discard_granularity & (discard_granularity - 1)) {
> + error_report("scsi-block: discard_granularity not a power of two");
> + return -1;
> + }
However, I think the first check is unnecessarily restrictive. As long
as discard_granularity is a power of two, if it's less than the block
size (which is also a power of two), the block size will always be a
multiple of discard_granularity, so there's no problem.
I'd also like to explicitly allow discard_granularity = 1, which is what
fallocate() provides.
> It is worse in that we do not want the hardware parameters exposed to the
> guest to change behind the scenes, except if you change the machine type
> or if you use the default unversioned type.
You're saying that discard_granularity and discard_zeros_data need to be
properties of the machine type? If you start with that as a requirement,
I can see why you want to always report discard_granularity=512 &
discard_zeros_data=1. But that design has many downsides. I'm not
convinced that discard_granularity and discard_zeros_data need to be
properties of the machine type. Why do you feel that's necessary? What's
the harm in those properties changing across QEMU startups (i.e. guest
boots)?
--
Richard
signature.asc
Description: This is a digitally signed message part
