| Message ID | 1360328775-13144-3-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Feb 08, 2013 at 02:06:12PM +0100, Paolo Bonzini wrote: > @@ -1489,6 +1496,10 @@ QemuOptsList qemu_drive_opts = { > .type = QEMU_OPT_STRING, > .help = "disk image", > },{ > + .name = "discard", > + .type = QEMU_OPT_STRING, > + .help = "discard operation (ignore/off, unmap/on)", off/on aliases will be confusing when we plan to add "anchor" in the future, making this non-boolean. Please support ignore/unmap only.
Il 13/02/2013 11:39, Stefan Hajnoczi ha scritto: >> > .type = QEMU_OPT_STRING, >> > .help = "disk image", >> > },{ >> > + .name = "discard", >> > + .type = QEMU_OPT_STRING, >> > + .help = "discard operation (ignore/off, unmap/on)", > off/on aliases will be confusing when we plan to add "anchor" in the > future, making this non-boolean. Please support ignore/unmap only. My problem here is that "unmap" will not be the default but at the same time it will be what people want to use. Hence, it is important to have an easy to remember and discover option; "discard=on" is immediately understandable, while "discard=unmap" is not. Paolo
On Wed, Feb 20, 2013 at 03:09:51PM +0100, Paolo Bonzini wrote: > Il 13/02/2013 11:39, Stefan Hajnoczi ha scritto: > >> > .type = QEMU_OPT_STRING, > >> > .help = "disk image", > >> > },{ > >> > + .name = "discard", > >> > + .type = QEMU_OPT_STRING, > >> > + .help = "discard operation (ignore/off, unmap/on)", > > off/on aliases will be confusing when we plan to add "anchor" in the > > future, making this non-boolean. Please support ignore/unmap only. > > My problem here is that "unmap" will not be the default but at the same > time it will be what people want to use. Hence, it is important to have > an easy to remember and discover option; "discard=on" is immediately > understandable, while "discard=unmap" is not. I understand. The risk is that we end up confusing users rather than helping, because the actual ignore/anchor/unmap modes need to be understood to use this feature properly. If you and Kevin like the off/on alias, go ahead. It's not critical. Stefan
On Thu, Feb 21, 2013 at 09:48:56AM +0100, Stefan Hajnoczi wrote: > On Wed, Feb 20, 2013 at 03:09:51PM +0100, Paolo Bonzini wrote: > > Il 13/02/2013 11:39, Stefan Hajnoczi ha scritto: > > >> > .type = QEMU_OPT_STRING, > > >> > .help = "disk image", > > >> > },{ > > >> > + .name = "discard", > > >> > + .type = QEMU_OPT_STRING, > > >> > + .help = "discard operation (ignore/off, unmap/on)", > > > off/on aliases will be confusing when we plan to add "anchor" in the > > > future, making this non-boolean. Please support ignore/unmap only. > > > > My problem here is that "unmap" will not be the default but at the same > > time it will be what people want to use. Hence, it is important to have > > an easy to remember and discover option; "discard=on" is immediately > > understandable, while "discard=unmap" is not. > > I understand. The risk is that we end up confusing users rather than > helping, because the actual ignore/anchor/unmap modes need to be > understood to use this feature properly. > > If you and Kevin like the off/on alias, go ahead. It's not critical. I actually think both are good points and am not sure which one is more important. In such cases I tend to just accept whatever a proposed patch contains. Kevin
diff --git a/blockdev.c b/blockdev.c index 63e6f1e..b4ae36a 100644 --- a/blockdev.c +++ b/blockdev.c @@ -378,6 +378,13 @@ DriveInfo *drive_init(QemuOpts *opts, BlockInterfaceType block_default_type) } } + if ((buf = qemu_opt_get(opts, "discard")) != NULL) { + if (bdrv_parse_discard_flags(buf, &bdrv_flags) != 0) { + error_report("invalid discard option"); + return NULL; + } + } + bdrv_flags |= BDRV_O_CACHE_WB; if ((buf = qemu_opt_get(opts, "cache")) != NULL) { if (bdrv_parse_cache_flags(buf, &bdrv_flags) != 0) { @@ -1489,6 +1496,10 @@ QemuOptsList qemu_drive_opts = { .type = QEMU_OPT_STRING, .help = "disk image", },{ + .name = "discard", + .type = QEMU_OPT_STRING, + .help = "discard operation (ignore/off, unmap/on)", + },{ .name = "cache", .type = QEMU_OPT_STRING, .help = "host cache usage (none, writeback, writethrough, " diff --git a/qemu-options.hx b/qemu-options.hx index 046bdc0..d5a99c7 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -185,6 +185,8 @@ These options have the same definition as they have in @option{-hdachs}. @var{cache} is "none", "writeback", "unsafe", "directsync" or "writethrough" and controls how the host cache is used to access block data. @item aio=@var{aio} @var{aio} is "threads", or "native" and selects between pthread based disk I/O and native Linux AIO. +@item discard=@var{discard} +@var{discard} is one of "ignore" (or "off") or "unmap" (or "on") and controls whether @dfn{discard} (also known as @dfn{trim} or @dfn{unmap}) requests are ignored or passed to the filesystem. Some machine types may not support discard requests. @item format=@var{format} Specify which disk @var{format} will be used rather than detecting the format. Can be used to specifiy format=raw to avoid interpreting
Add support for BDRV_O_UNMAP from the QEMU command-line. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- blockdev.c | 11 +++++++++++ qemu-options.hx | 2 ++ 2 files changed, 13 insertions(+)