* [Qemu-devel] [PATCH] block: Fix .bdrv_open flags
@ 2016-01-11 18:32 Kevin Wolf
2016-01-12 5:58 ` Denis V. Lunev
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Kevin Wolf @ 2016-01-11 18:32 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, borntraeger, qemu-devel, stefanha, den
bdrv_common_open() modified bs->open_flags after inferring the set of
options to pass to the driver's .bdrv_open callback. This means that the
cache options were correctly set in bs->open_flags (and therefore
correctly displayed in 'info block'), but the image would actually be
opened with the default cache mode instead.
This patch removes the flags parameter to bdrv_common_open() (except for
BDRV_O_NO_BACKING it's the same as bs->open_flags anyway, and having two
names for the same thing is confusing), and moves the assignment of
open_flags down to immediately before calling into the block drivers. In
all other places, bs->open_flags is now used consistently.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/block.c b/block.c
index 01655de..ef37d51 100644
--- a/block.c
+++ b/block.c
@@ -905,7 +905,7 @@ static QemuOptsList bdrv_runtime_opts = {
* Removes all processed options from *options.
*/
static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
- QDict *options, int flags, Error **errp)
+ QDict *options, Error **errp)
{
int ret, open_flags;
const char *filename;
@@ -943,7 +943,8 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
goto fail_opts;
}
- trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name);
+ trace_bdrv_open_common(bs, filename ?: "", bs->open_flags,
+ drv->format_name);
node_name = qemu_opt_get(opts, "node-name");
bdrv_assign_node_name(bs, node_name, &local_err);
@@ -955,8 +956,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
bs->request_alignment = 512;
bs->zero_beyond_eof = true;
- open_flags = bdrv_open_flags(bs, flags);
- bs->read_only = !(open_flags & BDRV_O_RDWR);
+ bs->read_only = !(bs->open_flags & BDRV_O_RDWR);
if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) {
error_setg(errp,
@@ -969,7 +969,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
}
assert(bs->copy_on_read == 0); /* bdrv_new() and bdrv_close() make it so */
- if (flags & BDRV_O_COPY_ON_READ) {
+ if (bs->open_flags & BDRV_O_COPY_ON_READ) {
if (!bs->read_only) {
bdrv_enable_copy_on_read(bs);
} else {
@@ -994,6 +994,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
bdrv_set_enable_write_cache(bs, bs->open_flags & BDRV_O_CACHE_WB);
/* Open the image, either directly or using a protocol */
+ open_flags = bdrv_open_flags(bs, bs->open_flags);
if (drv->bdrv_file_open) {
assert(file == NULL);
assert(!drv->bdrv_needs_filename || filename != NULL);
@@ -1660,7 +1661,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
assert(!(flags & BDRV_O_PROTOCOL) || !file);
/* Open the image */
- ret = bdrv_open_common(bs, file, options, flags, &local_err);
+ ret = bdrv_open_common(bs, file, options, &local_err);
if (ret < 0) {
goto fail;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] block: Fix .bdrv_open flags
2016-01-11 18:32 [Qemu-devel] [PATCH] block: Fix .bdrv_open flags Kevin Wolf
@ 2016-01-12 5:58 ` Denis V. Lunev
2016-01-12 7:28 ` Christian Borntraeger
2016-01-14 14:09 ` Stefan Hajnoczi
2 siblings, 0 replies; 4+ messages in thread
From: Denis V. Lunev @ 2016-01-12 5:58 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: borntraeger, qemu-devel, stefanha
On 01/11/2016 09:32 PM, Kevin Wolf wrote:
> bdrv_common_open() modified bs->open_flags after inferring the set of
> options to pass to the driver's .bdrv_open callback. This means that the
> cache options were correctly set in bs->open_flags (and therefore
> correctly displayed in 'info block'), but the image would actually be
> opened with the default cache mode instead.
>
> This patch removes the flags parameter to bdrv_common_open() (except for
> BDRV_O_NO_BACKING it's the same as bs->open_flags anyway, and having two
> names for the same thing is confusing), and moves the assignment of
> open_flags down to immediately before calling into the block drivers. In
> all other places, bs->open_flags is now used consistently.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/block.c b/block.c
> index 01655de..ef37d51 100644
> --- a/block.c
> +++ b/block.c
> @@ -905,7 +905,7 @@ static QemuOptsList bdrv_runtime_opts = {
> * Removes all processed options from *options.
> */
> static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
> - QDict *options, int flags, Error **errp)
> + QDict *options, Error **errp)
> {
> int ret, open_flags;
> const char *filename;
> @@ -943,7 +943,8 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
> goto fail_opts;
> }
>
> - trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name);
> + trace_bdrv_open_common(bs, filename ?: "", bs->open_flags,
> + drv->format_name);
>
> node_name = qemu_opt_get(opts, "node-name");
> bdrv_assign_node_name(bs, node_name, &local_err);
> @@ -955,8 +956,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
>
> bs->request_alignment = 512;
> bs->zero_beyond_eof = true;
> - open_flags = bdrv_open_flags(bs, flags);
> - bs->read_only = !(open_flags & BDRV_O_RDWR);
> + bs->read_only = !(bs->open_flags & BDRV_O_RDWR);
>
> if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) {
> error_setg(errp,
> @@ -969,7 +969,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
> }
>
> assert(bs->copy_on_read == 0); /* bdrv_new() and bdrv_close() make it so */
> - if (flags & BDRV_O_COPY_ON_READ) {
> + if (bs->open_flags & BDRV_O_COPY_ON_READ) {
> if (!bs->read_only) {
> bdrv_enable_copy_on_read(bs);
> } else {
> @@ -994,6 +994,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
> bdrv_set_enable_write_cache(bs, bs->open_flags & BDRV_O_CACHE_WB);
>
> /* Open the image, either directly or using a protocol */
> + open_flags = bdrv_open_flags(bs, bs->open_flags);
> if (drv->bdrv_file_open) {
> assert(file == NULL);
> assert(!drv->bdrv_needs_filename || filename != NULL);
> @@ -1660,7 +1661,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
> assert(!(flags & BDRV_O_PROTOCOL) || !file);
>
> /* Open the image */
> - ret = bdrv_open_common(bs, file, options, flags, &local_err);
> + ret = bdrv_open_common(bs, file, options, &local_err);
> if (ret < 0) {
> goto fail;
> }
Reviewed-by: Denis V. Lunev <den@openvz.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] block: Fix .bdrv_open flags
2016-01-11 18:32 [Qemu-devel] [PATCH] block: Fix .bdrv_open flags Kevin Wolf
2016-01-12 5:58 ` Denis V. Lunev
@ 2016-01-12 7:28 ` Christian Borntraeger
2016-01-14 14:09 ` Stefan Hajnoczi
2 siblings, 0 replies; 4+ messages in thread
From: Christian Borntraeger @ 2016-01-12 7:28 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: den, qemu-devel, stefanha
On 01/11/2016 07:32 PM, Kevin Wolf wrote:
> bdrv_common_open() modified bs->open_flags after inferring the set of
> options to pass to the driver's .bdrv_open callback. This means that the
> cache options were correctly set in bs->open_flags (and therefore
> correctly displayed in 'info block'), but the image would actually be
> opened with the default cache mode instead.
>
> This patch removes the flags parameter to bdrv_common_open() (except for
> BDRV_O_NO_BACKING it's the same as bs->open_flags anyway, and having two
> names for the same thing is confusing), and moves the assignment of
> open_flags down to immediately before calling into the block drivers. In
> all other places, bs->open_flags is now used consistently.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> block.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/block.c b/block.c
> index 01655de..ef37d51 100644
> --- a/block.c
> +++ b/block.c
> @@ -905,7 +905,7 @@ static QemuOptsList bdrv_runtime_opts = {
> * Removes all processed options from *options.
> */
> static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
> - QDict *options, int flags, Error **errp)
> + QDict *options, Error **errp)
> {
> int ret, open_flags;
> const char *filename;
> @@ -943,7 +943,8 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
> goto fail_opts;
> }
>
> - trace_bdrv_open_common(bs, filename ?: "", flags, drv->format_name);
> + trace_bdrv_open_common(bs, filename ?: "", bs->open_flags,
> + drv->format_name);
>
> node_name = qemu_opt_get(opts, "node-name");
> bdrv_assign_node_name(bs, node_name, &local_err);
> @@ -955,8 +956,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
>
> bs->request_alignment = 512;
> bs->zero_beyond_eof = true;
> - open_flags = bdrv_open_flags(bs, flags);
> - bs->read_only = !(open_flags & BDRV_O_RDWR);
> + bs->read_only = !(bs->open_flags & BDRV_O_RDWR);
>
> if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) {
> error_setg(errp,
> @@ -969,7 +969,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
> }
>
> assert(bs->copy_on_read == 0); /* bdrv_new() and bdrv_close() make it so */
> - if (flags & BDRV_O_COPY_ON_READ) {
> + if (bs->open_flags & BDRV_O_COPY_ON_READ) {
> if (!bs->read_only) {
> bdrv_enable_copy_on_read(bs);
> } else {
> @@ -994,6 +994,7 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
> bdrv_set_enable_write_cache(bs, bs->open_flags & BDRV_O_CACHE_WB);
>
> /* Open the image, either directly or using a protocol */
> + open_flags = bdrv_open_flags(bs, bs->open_flags);
> if (drv->bdrv_file_open) {
> assert(file == NULL);
> assert(!drv->bdrv_needs_filename || filename != NULL);
> @@ -1660,7 +1661,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
> assert(!(flags & BDRV_O_PROTOCOL) || !file);
>
> /* Open the image */
> - ret = bdrv_open_common(bs, file, options, flags, &local_err);
> + ret = bdrv_open_common(bs, file, options, &local_err);
> if (ret < 0) {
> goto fail;
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] block: Fix .bdrv_open flags
2016-01-11 18:32 [Qemu-devel] [PATCH] block: Fix .bdrv_open flags Kevin Wolf
2016-01-12 5:58 ` Denis V. Lunev
2016-01-12 7:28 ` Christian Borntraeger
@ 2016-01-14 14:09 ` Stefan Hajnoczi
2 siblings, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2016-01-14 14:09 UTC (permalink / raw)
To: Kevin Wolf; +Cc: borntraeger, qemu-devel, qemu-block, den
[-- Attachment #1: Type: text/plain, Size: 960 bytes --]
On Mon, Jan 11, 2016 at 07:32:36PM +0100, Kevin Wolf wrote:
> bdrv_common_open() modified bs->open_flags after inferring the set of
> options to pass to the driver's .bdrv_open callback. This means that the
> cache options were correctly set in bs->open_flags (and therefore
> correctly displayed in 'info block'), but the image would actually be
> opened with the default cache mode instead.
>
> This patch removes the flags parameter to bdrv_common_open() (except for
> BDRV_O_NO_BACKING it's the same as bs->open_flags anyway, and having two
> names for the same thing is confusing), and moves the assignment of
> open_flags down to immediately before calling into the block drivers. In
> all other places, bs->open_flags is now used consistently.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-01-14 16:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-11 18:32 [Qemu-devel] [PATCH] block: Fix .bdrv_open flags Kevin Wolf
2016-01-12 5:58 ` Denis V. Lunev
2016-01-12 7:28 ` Christian Borntraeger
2016-01-14 14:09 ` Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.