| Message ID | 1360328775-13144-5-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Feb 08, 2013 at 02:06:14PM +0100, Paolo Bonzini wrote: > Because discard is now a host parameter, we can always fake it as enabled > in the guest. This is an extension of the current choice to ignore > "not supported" errors from the host when discard_granularity is set > to nonzero. > > The default granularity is set to the logical block size or 4k, whichever > is largest, because cluster sizes below 4k are rarely used and 4K is a > typical block size for files. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> I wonder if it would make sense to default to 64k so that a qcow2 image with default options can make use of it. On the other hand, it might just mean that discard requests are already dropped in the guest kernel instead of qemu, then we don't really win much. Kevin
Il 20/02/2013 15:13, Kevin Wolf ha scritto: > On Fri, Feb 08, 2013 at 02:06:14PM +0100, Paolo Bonzini wrote: >> Because discard is now a host parameter, we can always fake it as enabled >> in the guest. This is an extension of the current choice to ignore >> "not supported" errors from the host when discard_granularity is set >> to nonzero. >> >> The default granularity is set to the logical block size or 4k, whichever >> is largest, because cluster sizes below 4k are rarely used and 4K is a >> typical block size for files. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > I wonder if it would make sense to default to 64k so that a qcow2 image > with default options can make use of it. On the other hand, it might > just mean that discard requests are already dropped in the guest kernel > instead of qemu, then we don't really win much. In theory it might provide better alignment, but only if we set a "maximum unmap length" in the block limits VPD page. Since we don't, there should be no difference. Luckily this is just a guest visible parameter and can be versioned in the machine types, so it's not "another cache=writethrough" that took years to correct. Paolo
diff --git a/hw/block-common.h b/hw/block-common.h index bb808f7..dd11532 100644 --- a/hw/block-common.h +++ b/hw/block-common.h @@ -50,7 +50,7 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0), \ DEFINE_PROP_INT32("bootindex", _state, _conf.bootindex, -1), \ DEFINE_PROP_UINT32("discard_granularity", _state, \ - _conf.discard_granularity, 0) + _conf.discard_granularity, -1) #define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \ DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \ diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index c436b38..fd06da7 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -143,7 +143,10 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus); IDEState *s = bus->ifs + dev->unit; - if (dev->conf.discard_granularity && dev->conf.discard_granularity != 512) { + if (dev->conf.discard_granularity == -1) { + dev->conf.discard_granularity = 512; + } else if (dev->conf.discard_granularity && + dev->conf.discard_granularity != 512) { error_report("discard_granularity must be 512 for ide"); return -1; } diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 28e75bb..e11a5bb 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -41,9 +41,11 @@ do { printf("scsi-disk: " fmt , ## __VA_ARGS__); } while (0) #include <scsi/sg.h> #endif -#define SCSI_DMA_BUF_SIZE 131072 -#define SCSI_MAX_INQUIRY_LEN 256 -#define SCSI_MAX_MODE_LEN 256 +#define SCSI_DMA_BUF_SIZE 131072 +#define SCSI_MAX_INQUIRY_LEN 256 +#define SCSI_MAX_MODE_LEN 256 + +#define DEFAULT_DISCARD_GRANULARITY 4096 typedef struct SCSIDiskState SCSIDiskState; @@ -2059,6 +2061,11 @@ static int scsi_initfn(SCSIDevice *dev) return -1; } + if (s->qdev.conf.discard_granularity == -1) { + s->qdev.conf.discard_granularity = + MAX(s->qdev.conf.logical_block_size, DEFAULT_DISCARD_GRANULARITY); + } + if (!s->version) { s->version = g_strdup(qemu_get_version()); }
Because discard is now a host parameter, we can always fake it as enabled in the guest. This is an extension of the current choice to ignore "not supported" errors from the host when discard_granularity is set to nonzero. The default granularity is set to the logical block size or 4k, whichever is largest, because cluster sizes below 4k are rarely used and 4K is a typical block size for files. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- hw/block-common.h | 2 +- hw/ide/qdev.c | 5 ++++- hw/scsi-disk.c | 13 ++++++++++--- 3 files changed, 15 insertions(+), 5 deletions(-)