| Message ID | 1431105726-3682-6-git-send-email-kwolf@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/08/2015 11:21 AM, Kevin Wolf wrote: > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > blockdev.c | 24 ++++++++++++------------ > include/block/block.h | 8 ++++++++ > 2 files changed, 20 insertions(+), 12 deletions(-) > Reviewed-by: Eric Blake <eblake@redhat.com>
On 08.05.2015 19:21, Kevin Wolf wrote: > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > blockdev.c | 24 ++++++++++++------------ > include/block/block.h | 8 ++++++++ > 2 files changed, 20 insertions(+), 12 deletions(-) Any reason for not making it part of the BLOCK_OPT_* macros in block_int.h? Max
Am 11.05.2015 um 16:40 hat Max Reitz geschrieben: > On 08.05.2015 19:21, Kevin Wolf wrote: > >Signed-off-by: Kevin Wolf <kwolf@redhat.com> > >--- > > blockdev.c | 24 ++++++++++++------------ > > include/block/block.h | 8 ++++++++ > > 2 files changed, 20 insertions(+), 12 deletions(-) > > Any reason for not making it part of the BLOCK_OPT_* macros in block_int.h? Those aren't bdrv_open(), but bdrv_create() options, which should be abundantly clear from the fact that they start in BLOCK instead of BDRV. *cough* I guess I can move the new options to block_int.h indeed, but when it comes to renaming to make the difference clearer, I think I'd prefer renaming the create options. Any opinions on that? Kevin
On 11.05.2015 16:51, Kevin Wolf wrote: > Am 11.05.2015 um 16:40 hat Max Reitz geschrieben: >> On 08.05.2015 19:21, Kevin Wolf wrote: >>> Signed-off-by: Kevin Wolf <kwolf@redhat.com> >>> --- >>> blockdev.c | 24 ++++++++++++------------ >>> include/block/block.h | 8 ++++++++ >>> 2 files changed, 20 insertions(+), 12 deletions(-) >> Any reason for not making it part of the BLOCK_OPT_* macros in block_int.h? > Those aren't bdrv_open(), but bdrv_create() options, which should be > abundantly clear from the fact that they start in BLOCK instead of BDRV. > *cough* Oh, right, how could I have missed that. *cough cough* > I guess I can move the new options to block_int.h indeed, but when it > comes to renaming to make the difference clearer, I think I'd prefer > renaming the create options. Sounds good to me. Also, block.h is a more appropriate place, considering this is not an internal option of the block layer, but something for the outer interface. Max
On 08.05.2015 19:21, Kevin Wolf wrote: > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > blockdev.c | 24 ++++++++++++------------ > include/block/block.h | 8 ++++++++ > 2 files changed, 20 insertions(+), 12 deletions(-) Reviewed-by: Max Reitz <mreitz@redhat.com>
On Fri, May 08, 2015 at 07:21:37PM +0200, Kevin Wolf wrote: > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > blockdev.c | 24 ++++++++++++------------ > include/block/block.h | 8 ++++++++ > 2 files changed, 20 insertions(+), 12 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index 5eaf77e..77cbe72 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -391,13 +391,13 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, > } > } > > - if (qemu_opt_get_bool(opts, "cache.writeback", true)) { > + if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_WB, true)) { > bdrv_flags |= BDRV_O_CACHE_WB; > } > - if (qemu_opt_get_bool(opts, "cache.direct", false)) { > + if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_DIRECT, false)) { > bdrv_flags |= BDRV_O_NOCACHE; > } > - if (qemu_opt_get_bool(opts, "cache.no-flush", false)) { > + if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_NO_FLUSH, false)) { > bdrv_flags |= BDRV_O_NO_FLUSH; > } > > @@ -733,16 +733,16 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) > } > > /* Specific options take precedence */ > - if (!qemu_opt_get(all_opts, "cache.writeback")) { > - qemu_opt_set_bool(all_opts, "cache.writeback", > + if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_WB)) { > + qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_WB, > !!(flags & BDRV_O_CACHE_WB), &error_abort); > } > - if (!qemu_opt_get(all_opts, "cache.direct")) { > - qemu_opt_set_bool(all_opts, "cache.direct", > + if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_DIRECT)) { > + qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_DIRECT, > !!(flags & BDRV_O_NOCACHE), &error_abort); > } > - if (!qemu_opt_get(all_opts, "cache.no-flush")) { > - qemu_opt_set_bool(all_opts, "cache.no-flush", > + if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_NO_FLUSH)) { > + qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_NO_FLUSH, > !!(flags & BDRV_O_NO_FLUSH), &error_abort); > } > qemu_opt_unset(all_opts, "cache"); > @@ -3106,15 +3106,15 @@ QemuOptsList qemu_common_drive_opts = { > .type = QEMU_OPT_STRING, > .help = "discard operation (ignore/off, unmap/on)", > },{ > - .name = "cache.writeback", > + .name = BDRV_OPT_CACHE_WB, > .type = QEMU_OPT_BOOL, > .help = "enables writeback mode for any caches", > },{ > - .name = "cache.direct", > + .name = BDRV_OPT_CACHE_DIRECT, > .type = QEMU_OPT_BOOL, > .help = "enables use of O_DIRECT (bypass the host page cache)", > },{ > - .name = "cache.no-flush", > + .name = BDRV_OPT_CACHE_NO_FLUSH, > .type = QEMU_OPT_BOOL, > .help = "ignore any flush requests for the device", > },{ > diff --git a/include/block/block.h b/include/block/block.h > index 7d1a717..86bdac8 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -90,6 +90,14 @@ typedef struct HDGeometry { > > #define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH) > > + > +/* Option names of options parsed by the block layer */ > + > +#define BDRV_OPT_CACHE_WB "cache.writeback" > +#define BDRV_OPT_CACHE_DIRECT "cache.direct" > +#define BDRV_OPT_CACHE_NO_FLUSH "cache.no-flush" > + > + > #define BDRV_SECTOR_BITS 9 > #define BDRV_SECTOR_SIZE (1ULL << BDRV_SECTOR_BITS) > #define BDRV_SECTOR_MASK ~(BDRV_SECTOR_SIZE - 1) > -- > 1.8.3.1 > > Reviewed-by: Jeff Cody <jcody@redhat.com>
On Fri 08 May 2015 07:21:37 PM CEST, Kevin Wolf wrote: > Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Berto
diff --git a/blockdev.c b/blockdev.c index 5eaf77e..77cbe72 100644 --- a/blockdev.c +++ b/blockdev.c @@ -391,13 +391,13 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, } } - if (qemu_opt_get_bool(opts, "cache.writeback", true)) { + if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_WB, true)) { bdrv_flags |= BDRV_O_CACHE_WB; } - if (qemu_opt_get_bool(opts, "cache.direct", false)) { + if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_DIRECT, false)) { bdrv_flags |= BDRV_O_NOCACHE; } - if (qemu_opt_get_bool(opts, "cache.no-flush", false)) { + if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_NO_FLUSH, false)) { bdrv_flags |= BDRV_O_NO_FLUSH; } @@ -733,16 +733,16 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type) } /* Specific options take precedence */ - if (!qemu_opt_get(all_opts, "cache.writeback")) { - qemu_opt_set_bool(all_opts, "cache.writeback", + if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_WB)) { + qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_WB, !!(flags & BDRV_O_CACHE_WB), &error_abort); } - if (!qemu_opt_get(all_opts, "cache.direct")) { - qemu_opt_set_bool(all_opts, "cache.direct", + if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_DIRECT)) { + qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_DIRECT, !!(flags & BDRV_O_NOCACHE), &error_abort); } - if (!qemu_opt_get(all_opts, "cache.no-flush")) { - qemu_opt_set_bool(all_opts, "cache.no-flush", + if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_NO_FLUSH)) { + qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_NO_FLUSH, !!(flags & BDRV_O_NO_FLUSH), &error_abort); } qemu_opt_unset(all_opts, "cache"); @@ -3106,15 +3106,15 @@ QemuOptsList qemu_common_drive_opts = { .type = QEMU_OPT_STRING, .help = "discard operation (ignore/off, unmap/on)", },{ - .name = "cache.writeback", + .name = BDRV_OPT_CACHE_WB, .type = QEMU_OPT_BOOL, .help = "enables writeback mode for any caches", },{ - .name = "cache.direct", + .name = BDRV_OPT_CACHE_DIRECT, .type = QEMU_OPT_BOOL, .help = "enables use of O_DIRECT (bypass the host page cache)", },{ - .name = "cache.no-flush", + .name = BDRV_OPT_CACHE_NO_FLUSH, .type = QEMU_OPT_BOOL, .help = "ignore any flush requests for the device", },{ diff --git a/include/block/block.h b/include/block/block.h index 7d1a717..86bdac8 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -90,6 +90,14 @@ typedef struct HDGeometry { #define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH) + +/* Option names of options parsed by the block layer */ + +#define BDRV_OPT_CACHE_WB "cache.writeback" +#define BDRV_OPT_CACHE_DIRECT "cache.direct" +#define BDRV_OPT_CACHE_NO_FLUSH "cache.no-flush" + + #define BDRV_SECTOR_BITS 9 #define BDRV_SECTOR_SIZE (1ULL << BDRV_SECTOR_BITS) #define BDRV_SECTOR_MASK ~(BDRV_SECTOR_SIZE - 1)
Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- blockdev.c | 24 ++++++++++++------------ include/block/block.h | 8 ++++++++ 2 files changed, 20 insertions(+), 12 deletions(-)