Re: [Qemu-block] [PATCH] block/nfs: fix calculation of allocated file size
On Thu, Aug 20, 2015 at 12:46:47PM +0200, Peter Lieven wrote: st.st_blocks is always counted in 512 byte units. Do not use st.st_blksize as multiplicator which may be larger. Cc: qemu-sta...@nongnu.org Signed-off-by: Peter Lieven p...@kamp.de --- block/nfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/nfs.c b/block/nfs.c index c026ff6..02eb4e4 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -475,7 +475,7 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs) aio_poll(client-aio_context, true); } -return (task.ret 0 ? task.ret : st.st_blocks * st.st_blksize); +return (task.ret 0 ? task.ret : st.st_blocks * 512); } static int nfs_file_truncate(BlockDriverState *bs, int64_t offset) -- 1.9.1 Thanks, applied to my block tree: git://github.com/codyprime/qemu-kvm-jtc.git block -Jeff
Re: [Qemu-block] [PATCH] block: Override the driver in the filename with the user-specified one
On Wed 26 Aug 2015 04:53:06 PM CEST, Max Reitz wrote: Yet another thing is the problem described in the patch's commit message. Why and how is the driver option inherited by the snapshot? I think you're right and my description was wrong, this happens before the snapshot is created, when bdrv_image_create() opens the backing image in order to get its size. The command line is simply 'snapshot_blkdev virtio0 new.qcow2', the only requirement is that the original image is opened with driver-specific options. Berto
Re: [Qemu-block] [PATCHv2] block/nfs: cache allocated filesize for read-only files
Am 26.08.2015 um 17:31 schrieb Jeff Cody: On Mon, Aug 24, 2015 at 10:13:16PM +0200, Max Reitz wrote: On 24.08.2015 21:34, Peter Lieven wrote: Am 24.08.2015 um 20:39 schrieb Max Reitz: On 24.08.2015 10:06, Peter Lieven wrote: If the file is readonly its not expected to grow so save the blocking call to nfs_fstat_async and use the value saved at connection time. Also important the monitor (and thus the main loop) will not hang if block device info is queried and the NFS share is unresponsive. Signed-off-by: Peter Lieven p...@kamp.de --- v1-v2: update cache on reopen_prepare [Max] block/nfs.c | 35 +++ 1 file changed, 35 insertions(+) Reviewed-by: Max Reitz mre...@redhat.com I hope you're ready for the Stale actual-size value with cache=direct,read-only=on,format=raw files on NFS reports. :-) actually a good point, maybe the cache should only be used if !(bs-open_flags BDRV_O_NOCACHE) Good enough a point to fix it? ;-) Max It seems more inline with expected behavior, to add the cache checking in before using the size cache. Would you be opposed to a v3 with this check added in? Of course, will send it tomorrow. One other concern I have is similar to a concern Max raised earlier - about an external program modifying the raw image, while QEMU has it opened r/o. In particular, I wonder about an NFS server making an image either sparse / non-sparse. If it was exported read-only, it may be a valid assumption that this could be done safely, as it would not change the reported file size or contents, just the allocated size on disk. This might be a use case. But if I allow caching the allocated filesize might not always be correct. This is even the case on a NFS share mounted through the kernel where some attributes a cached for some time. Anyway, would it hurt here if the actual filesize was too small? In fact it was incorrect since libnfs support was added :-) Peter
Re: [Qemu-block] [PATCHv2] block/nfs: cache allocated filesize for read-only files
On Wed, Aug 26, 2015 at 08:49:06PM +0200, Peter Lieven wrote: Am 26.08.2015 um 17:31 schrieb Jeff Cody: On Mon, Aug 24, 2015 at 10:13:16PM +0200, Max Reitz wrote: On 24.08.2015 21:34, Peter Lieven wrote: Am 24.08.2015 um 20:39 schrieb Max Reitz: On 24.08.2015 10:06, Peter Lieven wrote: If the file is readonly its not expected to grow so save the blocking call to nfs_fstat_async and use the value saved at connection time. Also important the monitor (and thus the main loop) will not hang if block device info is queried and the NFS share is unresponsive. Signed-off-by: Peter Lieven p...@kamp.de --- v1-v2: update cache on reopen_prepare [Max] block/nfs.c | 35 +++ 1 file changed, 35 insertions(+) Reviewed-by: Max Reitz mre...@redhat.com I hope you're ready for the Stale actual-size value with cache=direct,read-only=on,format=raw files on NFS reports. :-) actually a good point, maybe the cache should only be used if !(bs-open_flags BDRV_O_NOCACHE) Good enough a point to fix it? ;-) Max It seems more inline with expected behavior, to add the cache checking in before using the size cache. Would you be opposed to a v3 with this check added in? Of course, will send it tomorrow. One other concern I have is similar to a concern Max raised earlier - about an external program modifying the raw image, while QEMU has it opened r/o. In particular, I wonder about an NFS server making an image either sparse / non-sparse. If it was exported read-only, it may be a valid assumption that this could be done safely, as it would not change the reported file size or contents, just the allocated size on disk. This might be a use case. But if I allow caching the allocated filesize might not always be correct. This is even the case on a NFS share mounted through the kernel where some attributes a cached for some time. Anyway, would it hurt here if the actual filesize was too small? In fact it was incorrect since libnfs support was added :-) Yeah, I'm not sure what harm it would cause in practice. It is a fairly edge use case to begin with, and a relatively benign side affect (especially since you added reopen() support). With the cache flag checking, I am comfortable adding my r-b. Jeff
[Qemu-block] [PATCH 4/5] block: Drop drv parameter from bdrv_fill_options()
Now that this parameter is effectively unused, we can drop it and change
the function accordingly.
Signed-off-by: Max Reitz mre...@redhat.com
---
block.c | 59 ++-
1 file changed, 22 insertions(+), 37 deletions(-)
diff --git a/block.c b/block.c
index 8aa5f25..434f43c 100644
--- a/block.c
+++ b/block.c
@@ -996,13 +996,13 @@ static QDict *parse_json_filename(const char *filename,
Error **errp)
* block driver has been specified explicitly.
*/
static int bdrv_fill_options(QDict **options, const char **pfilename,
- int *flags, BlockDriver *drv, Error **errp)
+ int *flags, Error **errp)
{
const char *filename = *pfilename;
const char *drvname;
bool protocol = *flags BDRV_O_PROTOCOL;
bool parse_filename = false;
-BlockDriver *tmp_drv;
+BlockDriver *drv = NULL;
Error *local_err = NULL;
/* Parse json: pseudo-protocol */
@@ -1021,15 +1021,15 @@ static int bdrv_fill_options(QDict **options, const
char **pfilename,
}
drvname = qdict_get_try_str(*options, driver);
-
-/* If the user has explicitly specified the driver, this choice should
- * override the BDRV_O_PROTOCOL flag */
-tmp_drv = drv;
-if (!tmp_drv drvname) {
-tmp_drv = bdrv_find_format(drvname);
-}
-if (tmp_drv) {
-protocol = tmp_drv-bdrv_file_open;
+if (drvname) {
+drv = bdrv_find_format(drvname);
+if (!drv) {
+error_setg(errp, Unknown driver '%s', drvname);
+return -ENOENT;
+}
+/* If the user has explicitly specified the driver, this choice should
+ * override the BDRV_O_PROTOCOL flag */
+protocol = drv-bdrv_file_open;
}
if (protocol) {
@@ -1053,33 +1053,18 @@ static int bdrv_fill_options(QDict **options, const
char **pfilename,
/* Find the right block driver */
filename = qdict_get_try_str(*options, filename);
-if (drv) {
-if (drvname) {
-error_setg(errp, Driver specified twice);
-return -EINVAL;
-}
-drvname = drv-format_name;
-qdict_put(*options, driver, qstring_from_str(drvname));
-} else {
-if (!drvname protocol) {
-if (filename) {
-drv = bdrv_find_protocol(filename, parse_filename, errp);
-if (!drv) {
-return -EINVAL;
-}
-
-drvname = drv-format_name;
-qdict_put(*options, driver, qstring_from_str(drvname));
-} else {
-error_setg(errp, Must specify either driver or file);
-return -EINVAL;
-}
-} else if (drvname) {
-drv = bdrv_find_format(drvname);
+if (!drvname protocol) {
+if (filename) {
+drv = bdrv_find_protocol(filename, parse_filename, errp);
if (!drv) {
-error_setg(errp, Unknown driver '%s', drvname);
-return -ENOENT;
+return -EINVAL;
}
+
+drvname = drv-format_name;
+qdict_put(*options, driver, qstring_from_str(drvname));
+} else {
+error_setg(errp, Must specify either driver or file);
+return -EINVAL;
}
}
@@ -1474,7 +1459,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs,
const char *filename,
flags = child_role-inherit_flags(parent-open_flags);
}
-ret = bdrv_fill_options(options, filename, flags, NULL, local_err);
+ret = bdrv_fill_options(options, filename, flags, local_err);
if (local_err) {
goto fail;
}
--
2.4.6
[Qemu-block] [PATCH 5/5] block: Drop bdrv_find_whitelisted_format()
It is unused by now, so we can drop it.
Signed-off-by: Max Reitz mre...@redhat.com
---
block.c | 7 ---
include/block/block.h | 2 --
2 files changed, 9 deletions(-)
diff --git a/block.c b/block.c
index 434f43c..461eb94 100644
--- a/block.c
+++ b/block.c
@@ -313,13 +313,6 @@ static int bdrv_is_whitelisted(BlockDriver *drv, bool
read_only)
return 0;
}
-BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
- bool read_only)
-{
-BlockDriver *drv = bdrv_find_format(format_name);
-return drv bdrv_is_whitelisted(drv, read_only) ? drv : NULL;
-}
-
typedef struct CreateCo {
BlockDriver *drv;
char *filename;
diff --git a/include/block/block.h b/include/block/block.h
index 67556c4..b075f67 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -193,8 +193,6 @@ BlockDriver *bdrv_find_protocol(const char *filename,
bool allow_protocol_prefix,
Error **errp);
BlockDriver *bdrv_find_format(const char *format_name);
-BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
- bool readonly);
int bdrv_create(BlockDriver *drv, const char* filename,
QemuOpts *opts, Error **errp);
int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp);
--
2.4.6
[Qemu-block] [PATCH 1/5] block: Always pass NULL as drv for bdrv_open()
Change all callers of bdrv_open() to pass the driver name in the options
QDict instead of passing its BlockDriver pointer.
Signed-off-by: Max Reitz mre...@redhat.com
---
block.c | 24 ++--
block/qcow2.c | 16 -
block/vvfat.c | 8 +--
blockdev.c| 72 +++
4 files changed, 57 insertions(+), 63 deletions(-)
diff --git a/block.c b/block.c
index d088ee0..193daf9 100644
--- a/block.c
+++ b/block.c
@@ -1385,11 +1385,13 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int
flags, Error **errp)
qstring_from_str(file));
qdict_put(snapshot_options, file.filename,
qstring_from_str(tmp_filename));
+qdict_put(snapshot_options, driver,
+ qstring_from_str(qcow2));
bs_snapshot = bdrv_new();
ret = bdrv_open(bs_snapshot, NULL, NULL, snapshot_options,
-flags, bdrv_qcow2, local_err);
+flags, NULL, local_err);
if (ret 0) {
error_propagate(errp, local_err);
goto out;
@@ -3739,7 +3741,6 @@ void bdrv_img_create(const char *filename, const char
*fmt,
const char *backing_fmt, *backing_file;
int64_t size;
BlockDriver *drv, *proto_drv;
-BlockDriver *backing_drv = NULL;
Error *local_err = NULL;
int ret = 0;
@@ -3813,14 +3814,6 @@ void bdrv_img_create(const char *filename, const char
*fmt,
}
backing_fmt = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT);
-if (backing_fmt) {
-backing_drv = bdrv_find_format(backing_fmt);
-if (!backing_drv) {
-error_setg(errp, Unknown backing file format '%s',
- backing_fmt);
-goto out;
-}
-}
// The size for the image must always be specified, with one exception:
// If we are using a backing file, we can obtain the size from there
@@ -3831,6 +3824,7 @@ void bdrv_img_create(const char *filename, const char
*fmt,
char *full_backing = g_new0(char, PATH_MAX);
int64_t size;
int back_flags;
+QDict *backing_options = NULL;
bdrv_get_full_backing_filename_from_filename(filename,
backing_file,
full_backing,
PATH_MAX,
@@ -3844,9 +3838,15 @@ void bdrv_img_create(const char *filename, const char
*fmt,
back_flags =
flags ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
+if (backing_fmt) {
+backing_options = qdict_new();
+qdict_put(backing_options, driver,
+ qstring_from_str(backing_fmt));
+}
+
bs = NULL;
-ret = bdrv_open(bs, full_backing, NULL, NULL, back_flags,
-backing_drv, local_err);
+ret = bdrv_open(bs, full_backing, NULL, backing_options,
+back_flags, NULL, local_err);
g_free(full_backing);
if (ret 0) {
goto out;
diff --git a/block/qcow2.c b/block/qcow2.c
index ea34ae2..867b43b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1873,8 +1873,10 @@ static int qcow2_create2(const char *filename, int64_t
total_size,
QemuOpts *opts, int version, int refcount_order,
Error **errp)
{
-/* Calculate cluster_bits */
int cluster_bits;
+QDict *options;
+
+/* Calculate cluster_bits */
cluster_bits = ctz32(cluster_size);
if (cluster_bits MIN_CLUSTER_BITS || cluster_bits MAX_CLUSTER_BITS ||
(1 cluster_bits) != cluster_size)
@@ -2032,9 +2034,11 @@ static int qcow2_create2(const char *filename, int64_t
total_size,
* refcount of the cluster that is occupied by the header and the refcount
* table)
*/
-ret = bdrv_open(bs, filename, NULL, NULL,
+options = qdict_new();
+qdict_put(options, driver, qstring_from_str(qcow2));
+ret = bdrv_open(bs, filename, NULL, options,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH,
-bdrv_qcow2, local_err);
+NULL, local_err);
if (ret 0) {
error_propagate(errp, local_err);
goto out;
@@ -2084,9 +2088,11 @@ static int qcow2_create2(const char *filename, int64_t
total_size,
bs = NULL;
/* Reopen the image without BDRV_O_NO_FLUSH to flush it before returning */
-ret = bdrv_open(bs, filename, NULL, NULL,
+options = qdict_new();
+qdict_put(options, driver, qstring_from_str(qcow2));
+ret = bdrv_open(bs, filename, NULL, options,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_BACKING,
-bdrv_qcow2, local_err);
+NULL, local_err);
if (local_err) {
error_propagate(errp, local_err);
goto out;
diff --git a/block/vvfat.c b/block/vvfat.c
index 2068697..bffe8ad 100644
---
[Qemu-block] [PATCH 3/5] block: Drop drv parameter from bdrv_open_inherit()
Now that this parameter is effectively unused, we can drop it and just
pass NULL to bdrv_fill_options().
Signed-off-by: Max Reitz mre...@redhat.com
---
block.c | 17 +++--
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/block.c b/block.c
index ac89487..8aa5f25 100644
--- a/block.c
+++ b/block.c
@@ -85,8 +85,7 @@ static QLIST_HEAD(, BlockDriver) bdrv_drivers =
static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
const char *reference, QDict *options, int flags,
BlockDriverState *parent,
- const BdrvChildRole *child_role,
- BlockDriver *drv, Error **errp);
+ const BdrvChildRole *child_role, Error **errp);
static void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
/* If non-zero, use only whitelisted block drivers */
@@ -1227,8 +1226,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict
*options, Error **errp)
assert(bs-backing_hd == NULL);
ret = bdrv_open_inherit(backing_hd,
*backing_filename ? backing_filename : NULL,
-NULL, options, 0, bs, child_backing,
-NULL, local_err);
+NULL, options, 0, bs, child_backing, local_err);
if (ret 0) {
bdrv_unref(backing_hd);
backing_hd = NULL;
@@ -1291,7 +1289,7 @@ BdrvChild *bdrv_open_child(const char *filename,
bs = NULL;
ret = bdrv_open_inherit(bs, filename, reference, image_options, 0,
-parent, child_role, NULL, errp);
+parent, child_role, errp);
if (ret 0) {
goto done;
}
@@ -1422,11 +1420,11 @@ out:
static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
const char *reference, QDict *options, int flags,
BlockDriverState *parent,
- const BdrvChildRole *child_role,
- BlockDriver *drv, Error **errp)
+ const BdrvChildRole *child_role, Error **errp)
{
int ret;
BlockDriverState *file = NULL, *bs;
+BlockDriver *drv = NULL;
const char *drvname;
Error *local_err = NULL;
int snapshot_flags = 0;
@@ -1476,13 +1474,12 @@ static int bdrv_open_inherit(BlockDriverState **pbs,
const char *filename,
flags = child_role-inherit_flags(parent-open_flags);
}
-ret = bdrv_fill_options(options, filename, flags, drv, local_err);
+ret = bdrv_fill_options(options, filename, flags, NULL, local_err);
if (local_err) {
goto fail;
}
/* Find the right image format driver */
-drv = NULL;
drvname = qdict_get_try_str(options, driver);
if (drvname) {
drv = bdrv_find_format(drvname);
@@ -1640,7 +1637,7 @@ int bdrv_open(BlockDriverState **pbs, const char
*filename,
const char *reference, QDict *options, int flags, Error **errp)
{
return bdrv_open_inherit(pbs, filename, reference, options, flags, NULL,
- NULL, NULL, errp);
+ NULL, errp);
}
typedef struct BlockReopenQueueEntry {
--
2.4.6
[Qemu-block] [PATCH 2/5] block: Drop drv parameter from bdrv_open()
Now that this parameter is effectively unused, we can drop it and just
pass NULL on to bdrv_open_inherit().
Signed-off-by: Max Reitz mre...@redhat.com
---
block.c | 9 -
block/block-backend.c | 2 +-
block/parallels.c | 2 +-
block/qcow.c | 2 +-
block/qcow2.c | 6 +++---
block/qed.c | 2 +-
block/sheepdog.c | 5 ++---
block/vdi.c | 2 +-
block/vhdx.c | 2 +-
block/vmdk.c | 7 +++
block/vpc.c | 2 +-
block/vvfat.c | 2 +-
blockdev.c| 8
include/block/block.h | 3 +--
14 files changed, 25 insertions(+), 29 deletions(-)
diff --git a/block.c b/block.c
index 193daf9..ac89487 100644
--- a/block.c
+++ b/block.c
@@ -1391,7 +1391,7 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int
flags, Error **errp)
bs_snapshot = bdrv_new();
ret = bdrv_open(bs_snapshot, NULL, NULL, snapshot_options,
-flags, NULL, local_err);
+flags, local_err);
if (ret 0) {
error_propagate(errp, local_err);
goto out;
@@ -1637,11 +1637,10 @@ close_and_fail:
}
int bdrv_open(BlockDriverState **pbs, const char *filename,
- const char *reference, QDict *options, int flags,
- BlockDriver *drv, Error **errp)
+ const char *reference, QDict *options, int flags, Error **errp)
{
return bdrv_open_inherit(pbs, filename, reference, options, flags, NULL,
- NULL, drv, errp);
+ NULL, NULL, errp);
}
typedef struct BlockReopenQueueEntry {
@@ -3846,7 +3845,7 @@ void bdrv_img_create(const char *filename, const char
*fmt,
bs = NULL;
ret = bdrv_open(bs, full_backing, NULL, backing_options,
-back_flags, NULL, local_err);
+back_flags, local_err);
g_free(full_backing);
if (ret 0) {
goto out;
diff --git a/block/block-backend.c b/block/block-backend.c
index aee8a12..c2e8732 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -126,7 +126,7 @@ BlockBackend *blk_new_open(const char *name, const char
*filename,
return NULL;
}
-ret = bdrv_open(blk-bs, filename, reference, options, flags, NULL, errp);
+ret = bdrv_open(blk-bs, filename, reference, options, flags, errp);
if (ret 0) {
blk_unref(blk);
return NULL;
diff --git a/block/parallels.c b/block/parallels.c
index 046b568..5cd6ec3 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -476,7 +476,7 @@ static int parallels_create(const char *filename, QemuOpts
*opts, Error **errp)
file = NULL;
ret = bdrv_open(file, filename, NULL, NULL,
-BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, local_err);
+BDRV_O_RDWR | BDRV_O_PROTOCOL, local_err);
if (ret 0) {
error_propagate(errp, local_err);
return ret;
diff --git a/block/qcow.c b/block/qcow.c
index 01fba54..6e35db1 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -793,7 +793,7 @@ static int qcow_create(const char *filename, QemuOpts
*opts, Error **errp)
qcow_bs = NULL;
ret = bdrv_open(qcow_bs, filename, NULL, NULL,
-BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, local_err);
+BDRV_O_RDWR | BDRV_O_PROTOCOL, local_err);
if (ret 0) {
error_propagate(errp, local_err);
goto cleanup;
diff --git a/block/qcow2.c b/block/qcow2.c
index 867b43b..a707d8d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1975,7 +1975,7 @@ static int qcow2_create2(const char *filename, int64_t
total_size,
bs = NULL;
ret = bdrv_open(bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
-NULL, local_err);
+local_err);
if (ret 0) {
error_propagate(errp, local_err);
return ret;
@@ -2038,7 +2038,7 @@ static int qcow2_create2(const char *filename, int64_t
total_size,
qdict_put(options, driver, qstring_from_str(qcow2));
ret = bdrv_open(bs, filename, NULL, options,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH,
-NULL, local_err);
+local_err);
if (ret 0) {
error_propagate(errp, local_err);
goto out;
@@ -2092,7 +2092,7 @@ static int qcow2_create2(const char *filename, int64_t
total_size,
qdict_put(options, driver, qstring_from_str(qcow2));
ret = bdrv_open(bs, filename, NULL, options,
BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_BACKING,
-NULL, local_err);
+local_err);
if (local_err) {
error_propagate(errp, local_err);
goto out;
diff --git a/block/qed.c b/block/qed.c
index 954ed00..a7ff1d9 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -583,7 +583,7 @@ static int qed_create(const char *filename, uint32_t
[Qemu-block] [PATCH 0/5] block: Drop drv parameter from bdrv_open()
We don't really need that parameter, so let's drop it. Doing so may even fix some bugs, see http://lists.nongnu.org/archive/html/qemu-block/2015-08/msg00171.html. In the course of writing this series, I had to decide whether the make sure all callers of bdrv_find_whitelisted_format() would still only accept whitelisted formats, which you'd think would be a good idea; but the only caller left was qmp_change_blockdev(), so I guess noone really cared about it anymore, instead relying on use_bdrv_whitelist alone. So I decided dropped bdrv_find_whitelisted_format() completely. If you feel this is a bad decision, feel free to argue but then I guess we'll have to reevaluate all bdrv_find_format() calls whether they should actually be bdrv_find_whitelisted_format() calls. Max Reitz (5): block: Always pass NULL as drv for bdrv_open() block: Drop drv parameter from bdrv_open() block: Drop drv parameter from bdrv_open_inherit() block: Drop drv parameter from bdrv_fill_options() block: Drop bdrv_find_whitelisted_format() block.c | 108 +++--- block/block-backend.c | 2 +- block/parallels.c | 2 +- block/qcow.c | 2 +- block/qcow2.c | 18 ++--- block/qed.c | 2 +- block/sheepdog.c | 5 +-- block/vdi.c | 2 +- block/vhdx.c | 2 +- block/vmdk.c | 7 ++-- block/vpc.c | 2 +- block/vvfat.c | 8 +++- blockdev.c| 72 + include/block/block.h | 5 +-- 14 files changed, 100 insertions(+), 137 deletions(-) -- 2.4.6
[Qemu-block] [PATCH 3/4] ide-test: add cdrom pio test
Add a simple read test for ATAPI devices,
using the PIO mechanism.
Signed-off-by: John Snow js...@redhat.com
---
tests/ide-test.c | 144 +++
1 file changed, 144 insertions(+)
diff --git a/tests/ide-test.c b/tests/ide-test.c
index 4a07e3a..90524e3 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -45,6 +45,12 @@
#define IDE_BASE 0x1f0
#define IDE_PRIMARY_IRQ 14
+#define ATAPI_BLOCK_SIZE 2048
+
+/* How many bytes to receive via ATAPI PIO at one time.
+ * Must be less than 0x. */
+#define BYTE_COUNT_LIMIT 5120
+
enum {
reg_data= 0x0,
reg_nsectors= 0x2,
@@ -80,6 +86,7 @@ enum {
CMD_WRITE_DMA = 0xca,
CMD_FLUSH_CACHE = 0xe7,
CMD_IDENTIFY= 0xec,
+CMD_PACKET = 0xa0,
CMDF_ABORT = 0x100,
CMDF_NO_BM = 0x200,
@@ -585,6 +592,140 @@ static void test_isa_retry_flush(const char *machine)
test_retry_flush(isapc);
}
+typedef struct Read10CDB {
+uint8_t opcode;
+uint8_t flags;
+uint32_t lba;
+uint8_t reserved;
+uint16_t nblocks;
+uint8_t control;
+uint16_t padding;
+} __attribute__((__packed__)) Read10CDB;
+
+static void send_scsi_cdb_read10(uint32_t lba, uint16_t nblocks)
+{
+Read10CDB pkt = { .padding = 0 };
+int i;
+
+/* Construct SCSI CDB packet */
+pkt.opcode = 0x28;
+pkt.lba = cpu_to_be32(lba);
+pkt.nblocks = cpu_to_be16(nblocks);
+
+/* Send Packet */
+for (i = 0; i sizeof(Read10CDB)/2; i++) {
+outw(IDE_BASE + reg_data, ((uint16_t *)pkt)[i]);
+}
+}
+
+static void nsleep(int64_t nsecs)
+{
+const struct timespec val = { .tv_nsec = nsecs };
+nanosleep(val, NULL);
+clock_set(nsecs);
+}
+
+static uint8_t ide_wait_clear(uint8_t flag)
+{
+int i;
+uint8_t data;
+
+/* Wait with a 5 second timeout */
+for (i = 0; i = 1250; i++) {
+data = inb(IDE_BASE + reg_status);
+if (!(data flag)) {
+return data;
+}
+nsleep(400);
+}
+g_assert_not_reached();
+return 0xff;
+}
+
+static void cdrom_pio_impl(int nblocks)
+{
+FILE *fh;
+size_t patt_len = ATAPI_BLOCK_SIZE * MAX(16, nblocks);
+char *pattern = g_malloc(patt_len);
+size_t rxsize = ATAPI_BLOCK_SIZE * nblocks;
+char *rx = g_malloc0(rxsize);
+int i, j;
+uint8_t data;
+uint16_t limit;
+
+/* Prepopulate the CDROM with an interesting pattern */
+generate_pattern(pattern, patt_len, ATAPI_BLOCK_SIZE);
+fh = fopen(tmp_path, w+);
+fwrite(pattern, ATAPI_BLOCK_SIZE, MAX(16, nblocks), fh);
+fclose(fh);
+
+ide_test_start(
+ -drive file=%s,if=ide,media=cdrom,cache=writeback,format=raw,
tmp_path);
+qtest_irq_intercept_in(global_qtest, ioapic);
+
+/* PACKET command on device 0 */
+outb(IDE_BASE + reg_device, 0);
+outb(IDE_BASE + reg_lba_middle, BYTE_COUNT_LIMIT 0xFF);
+outb(IDE_BASE + reg_lba_high, (BYTE_COUNT_LIMIT 8 0xFF));
+outb(IDE_BASE + reg_command, CMD_PACKET);
+/* HPD0: Check_Status_A State */
+nsleep(400);
+data = ide_wait_clear(BSY);
+/* HPD1: Send_Packet State */
+assert_bit_set(data, DRQ | DRDY);
+assert_bit_clear(data, ERR | DF | BSY);
+
+/* SCSI CDB (READ10) -- read n*2048 bytes from block 0 */
+send_scsi_cdb_read10(0, nblocks);
+
+/* HPD3: INTRQ_Wait */
+i = 0;
+do {
+data = get_irq(IDE_PRIMARY_IRQ);
+nsleep(400);
+i++;
+g_assert_cmpint(i, =, 1250);
+} while (!data);
+
+/* HPD2: Check_Status_B */
+data = ide_wait_clear(BSY);
+assert_bit_set(data, DRQ | DRDY);
+assert_bit_clear(data, ERR | DF | BSY);
+
+/* Read data back: occurs in bursts of 'BYTE_COUNT_LIMIT' bytes.
+ * If BYTE_COUNT_LIMIT is odd, we transfer BYTE_COUNT_LIMIT - 1 bytes.
+ * We allow an odd limit only when the remaining transfer size is
+ * less than BYTE_COUNT_LIMIT.
+ * For our purposes, we can only request even multiples, so do not
+ * attempt to read remainders. */
+limit = BYTE_COUNT_LIMIT ~1;
+for (i = 0; i DIV_ROUND_UP(rxsize, limit); i++) {
+size_t r = (rxsize - (limit * i)) / 2;
+for (j = 0; j MIN((limit / 2), r); j++) {
+((uint16_t *)rx)[(i * (limit/2)) + j] = inw(IDE_BASE + reg_data);
+}
+}
+data = ide_wait_clear(DRQ);
+assert_bit_set(data, DRDY);
+assert_bit_clear(data, DRQ | ERR | DF | BSY);
+
+g_assert_cmpint(memcmp(pattern, rx, rxsize), ==, 0);
+g_free(pattern);
+g_free(rx);
+test_bmdma_teardown();
+}
+
+static void test_cdrom_pio(void)
+{
+cdrom_pio_impl(1);
+}
+
+static void test_cdrom_pio_large(void)
+{
+/* Test a few loops of the PIO DRQ mechanism. */
+cdrom_pio_impl(BYTE_COUNT_LIMIT * 4 / ATAPI_BLOCK_SIZE);
+}
+
int main(int argc, char **argv)
{
const char *arch = qtest_get_arch();
@@ -628,6 +769,9 @@ int main(int argc, char **argv)
qtest_add_func(/ide/flush/retry_pci,
Re: [Qemu-block] [Qemu-devel] [PATCH v3] opts: produce valid command line in qemu_opts_print
2015-08-26 15:15 keltezéssel, Markus Armbruster írta: Stefan Hajnoczi stefa...@gmail.com writes: On Tue, Jul 7, 2015 at 3:42 PM, Kővágó, Zoltán dirty.ice...@gmail.com wrote: This will let us print options in a format that the user would actually write it on the command line (foo=bar,baz=asd,etc=def), without prepending a spurious comma at the beginning of the list, or quoting values unnecessarily. This patch provides the following changes: * write and id=, if the option has an id * do not print separator before the first element * do not quote string arguments * properly escape commas (,) for QEMU Signed-off-by: Kővágó, Zoltán dirty.ice...@gmail.com Reviewed-by: Markus Armbruster arm...@redhat.com Feel free to send a Ping email reply if your patches haven't been reviewed after about 1 week. Reviewed-by: Stefan Hajnoczi stefa...@redhat.com Route via qemu-trivial or qemu-block? Hmm, I originally intended qemu-trivial, qemu-block was added via scripts/get_maintainer.pl. Zoltan
Re: [Qemu-block] Creating snapshots with specific runtime options
On Tue, Aug 25, 2015 at 04:57:53PM +0300, Alberto Garcia wrote: As far as I can see there's no way to create a snapshot and either a) inherit the runtime options from the original image b) specify a new set of options This comment in external_snapshot_prepare() before calling bdrv_open() suggests that the problem is known but the discussion was postponed. /* TODO Inherit bs-options or only take explicit options with an * extended QMP command? */ I would like to retake this and make it possible. I discussed it briefly with Stefan on IRC and he said that Kevin might have some ideas. In principle extending the QMP command sounds as simple as adding 'options': 'BlockdevOptions' to 'blockdev-snapshot-sync', but it's surely more complicated than that :) Is the 'BlockdevOptions' API even stable? Some block drivers don't have BlockdevOptions support yet. I think that doesn't prevent us from passing BlockdevOptions to snapshot creation though.
Re: [Qemu-block] [PATCH] block: Override the driver in the filename with the user-specified one
On 25.08.2015 09:03, Alberto Garcia wrote: On Mon 24 Aug 2015 08:54:56 PM CEST, Max Reitz wrote: [bdrv_fill_options()] User-specified options should always have precedence over any other option. The thing is, we consider the filename to be specified by the user. For user-specified options like the lazy-refcounts case that I mentioned it makes sense, because that's the way the user wants to open it. For the image format it sounds counter-intuitive to me: the format is already set when the file is opened, the user doesn't have a choice there, or does she? The user can always override the option in the filename via the driver option in the QDict. If these options are not available for some reason (e.g. a backing file name), the filename is generally the only way for the user to set it (unless there is some special way to set the format, which there often is...). So it is actually correct that this option overrides the @drv parameter given to bdrv_open(), because that cannot be set by the user and is always set by qemu internally. Is that really the case? The drv parameter of bdrv_open() is being set by the user in a number of places: qmp_drive_backup(), qmp_drive_mirror(), qmp_change_blockdev() and qemu-img create. I consider that a bug. Case in point: Why is it only qemu-img create? Because all the other qemu-img functions use img_open(), which was was converted to blk_new_open() in 5bd313266bc5874dae9833be95e5dcfce787f1b7, whereas qemu-img create uses bdrv_img_create(), which uses the bdrv_open() @drv parameter for the backing file. blk_new_open() does not have a @drv parameter, which was intentional. Hm, now that gets me thinking. I basically said we should just drop the @drv parameter, and replace it by the driver QDict option. That means that in theory, they should actually be equal. Hm. So I think what we really want is to drop the @drv parameter from bdrv_open(), which I tried to do indirectly by introducing blk_new_open() and hoping to replace most bdrv_open() calls by that later on. But what we can do in the meantime probably is to apply your patch, because looking at all the bdrv_open() calls, there is always a very good reason for setting the @drv option which the user actually should not override. Yet another thing is the problem described in the patch's commit message. Why and how is the driver option inherited by the snapshot? I cannot see the filename being inherited, because the user always has to specify it explicitly (both in QMP's blockdev-snapshot-sync and in HMP's snapshot_blkdev). @options as given to bdrv_open() on the snapshot is either NULL or contains a node-name. Can you given an example of an (HMP) command line reproducing the problem? So I think the problem here is not in bdrv_fill_options(), but rather in blockdev.c:external_snapshot_prepare(). This function should not pass the driver as the @drv parameter to bdrv_open(), but rather set the driver option in @options in order to mark this a user-specified option. I guess in that case we should change that in all the other places I mentioned above. I think that would be something we will have to do eventually anyway. Your patch would probably solve the issue for now, and we can then drop it once we have dropped the @drv option from bdrv_open(). Max signature.asc Description: OpenPGP digital signature
[Qemu-block] [PATCH 0/4] ide: simple ATAPI tests
We don't have any CDROM tests yet. So, add some for the PCI/BMDMA HBA. For convenience, this branch is available at: https://github.com/jnsnow/qemu.git branch ide-atapi-test https://github.com/jnsnow/qemu/tree/ide-atapi-test This version is tagged ide-atapi-test-v1: https://github.com/jnsnow/qemu/releases/tag/ide-atapi-test-v1 John Snow (4): qtest/ahci: use generate_pattern everywhere qtest/ahci: export generate_pattern ide-test: add cdrom pio test ide-test: add cdrom dma test tests/ahci-test.c | 43 +- tests/ide-test.c | 232 ++ tests/libqos/libqos.c | 26 ++ tests/libqos/libqos.h | 1 + 4 files changed, 245 insertions(+), 57 deletions(-) -- 2.4.3
[Qemu-block] [PATCH 2/4] qtest/ahci: export generate_pattern
Share the pattern function for ide and ahci test.
Signed-off-by: John Snow js...@redhat.com
---
tests/ahci-test.c | 26 --
tests/libqos/libqos.c | 26 ++
tests/libqos/libqos.h | 1 +
3 files changed, 27 insertions(+), 26 deletions(-)
diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index b1a785c..59d387c 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -71,32 +71,6 @@ static void string_bswap16(uint16_t *s, size_t bytes)
}
}
-static void generate_pattern(void *buffer, size_t len, size_t cycle_len)
-{
-int i, j;
-unsigned char *tx = (unsigned char *)buffer;
-unsigned char p;
-size_t *sx;
-
-/* Write an indicative pattern that varies and is unique per-cycle */
-p = rand() % 256;
-for (i = 0; i len; i++) {
-tx[i] = p++ % 256;
-if (i % cycle_len == 0) {
-p = rand() % 256;
-}
-}
-
-/* force uniqueness by writing an id per-cycle */
-for (i = 0; i len / cycle_len; i++) {
-j = i * cycle_len;
-if (j + sizeof(*sx) = len) {
-sx = (size_t *)tx[j];
-*sx = i;
-}
-}
-}
-
/**
* Verify that the transfer did not corrupt our state at all.
*/
diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
index fce625b..8d7c5a9 100644
--- a/tests/libqos/libqos.c
+++ b/tests/libqos/libqos.c
@@ -212,3 +212,29 @@ void prepare_blkdebug_script(const char *debug_fn, const
char *event)
ret = fclose(debug_file);
g_assert(ret == 0);
}
+
+void generate_pattern(void *buffer, size_t len, size_t cycle_len)
+{
+int i, j;
+unsigned char *tx = (unsigned char *)buffer;
+unsigned char p;
+size_t *sx;
+
+/* Write an indicative pattern that varies and is unique per-cycle */
+p = rand() % 256;
+for (i = 0; i len; i++) {
+tx[i] = p++ % 256;
+if (i % cycle_len == 0) {
+p = rand() % 256;
+}
+}
+
+/* force uniqueness by writing an id per-cycle */
+for (i = 0; i len / cycle_len; i++) {
+j = i * cycle_len;
+if (j + sizeof(*sx) = len) {
+sx = (size_t *)tx[j];
+*sx = i;
+}
+}
+}
diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
index e1f14ea..492a651 100644
--- a/tests/libqos/libqos.h
+++ b/tests/libqos/libqos.h
@@ -24,6 +24,7 @@ void mkqcow2(const char *file, unsigned size_mb);
void set_context(QOSState *s);
void migrate(QOSState *from, QOSState *to, const char *uri);
void prepare_blkdebug_script(const char *debug_fn, const char *event);
+void generate_pattern(void *buffer, size_t len, size_t cycle_len);
static inline uint64_t qmalloc(QOSState *q, size_t bytes)
{
--
2.4.3
[Qemu-block] [PATCH 4/4] ide-test: add cdrom dma test
Now, test the DMA functionality of the ATAPI drive.
Signed-off-by: John Snow js...@redhat.com
---
tests/ide-test.c | 90
1 file changed, 72 insertions(+), 18 deletions(-)
diff --git a/tests/ide-test.c b/tests/ide-test.c
index 90524e3..f4a913d 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -53,6 +53,7 @@
enum {
reg_data= 0x0,
+reg_feature = 0x1,
reg_nsectors= 0x2,
reg_lba_low = 0x3,
reg_lba_middle = 0x4,
@@ -179,7 +180,8 @@ typedef struct PrdtEntry {
#define assert_bit_clear(data, mask) g_assert_cmphex((data) (mask), ==, 0)
static int send_dma_request(int cmd, uint64_t sector, int nb_sectors,
-PrdtEntry *prdt, int prdt_entries)
+PrdtEntry *prdt, int prdt_entries,
+void(*post_exec)(uint64_t sector, int nb_sectors))
{
QPCIDevice *dev;
uint16_t bmdma_base;
@@ -196,6 +198,9 @@ static int send_dma_request(int cmd, uint64_t sector, int
nb_sectors,
switch (cmd) {
case CMD_READ_DMA:
+case CMD_PACKET:
+/* Assuming we only test data reads w/ ATAPI, otherwise we need to know
+ * the SCSI command being sent in the packet, too. */
from_dev = true;
break;
case CMD_WRITE_DMA:
@@ -224,14 +229,22 @@ static int send_dma_request(int cmd, uint64_t sector, int
nb_sectors,
outl(bmdma_base + bmreg_prdt, guest_prdt);
/* ATA DMA command */
-outb(IDE_BASE + reg_nsectors, nb_sectors);
-
-outb(IDE_BASE + reg_lba_low,sector 0xff);
-outb(IDE_BASE + reg_lba_middle, (sector 8) 0xff);
-outb(IDE_BASE + reg_lba_high, (sector 16) 0xff);
+if (cmd == CMD_PACKET) {
+/* Enables ATAPI DMA; otherwise PIO is attempted */
+outb(IDE_BASE + reg_feature, 0x01);
+} else {
+outb(IDE_BASE + reg_nsectors, nb_sectors);
+outb(IDE_BASE + reg_lba_low,sector 0xff);
+outb(IDE_BASE + reg_lba_middle, (sector 8) 0xff);
+outb(IDE_BASE + reg_lba_high, (sector 16) 0xff);
+}
outb(IDE_BASE + reg_command, cmd);
+if (post_exec) {
+post_exec(sector, nb_sectors);
+}
+
/* Start DMA transfer */
outb(bmdma_base + bmreg_cmd, BM_CMD_START | (from_dev ? BM_CMD_WRITE : 0));
@@ -285,7 +298,8 @@ static void test_bmdma_simple_rw(void)
memset(buf, 0x55, len);
memwrite(guest_buf, buf, len);
-status = send_dma_request(CMD_WRITE_DMA, 0, 1, prdt, ARRAY_SIZE(prdt));
+status = send_dma_request(CMD_WRITE_DMA, 0, 1, prdt,
+ ARRAY_SIZE(prdt), NULL);
g_assert_cmphex(status, ==, BM_STS_INTR);
assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
@@ -293,14 +307,15 @@ static void test_bmdma_simple_rw(void)
memset(buf, 0xaa, len);
memwrite(guest_buf, buf, len);
-status = send_dma_request(CMD_WRITE_DMA, 1, 1, prdt, ARRAY_SIZE(prdt));
+status = send_dma_request(CMD_WRITE_DMA, 1, 1, prdt,
+ ARRAY_SIZE(prdt), NULL);
g_assert_cmphex(status, ==, BM_STS_INTR);
assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
/* Read and verify 0x55 pattern in sector 0 */
memset(cmpbuf, 0x55, len);
-status = send_dma_request(CMD_READ_DMA, 0, 1, prdt, ARRAY_SIZE(prdt));
+status = send_dma_request(CMD_READ_DMA, 0, 1, prdt, ARRAY_SIZE(prdt),
NULL);
g_assert_cmphex(status, ==, BM_STS_INTR);
assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
@@ -310,7 +325,7 @@ static void test_bmdma_simple_rw(void)
/* Read and verify 0xaa pattern in sector 1 */
memset(cmpbuf, 0xaa, len);
-status = send_dma_request(CMD_READ_DMA, 1, 1, prdt, ARRAY_SIZE(prdt));
+status = send_dma_request(CMD_READ_DMA, 1, 1, prdt, ARRAY_SIZE(prdt),
NULL);
g_assert_cmphex(status, ==, BM_STS_INTR);
assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
@@ -335,13 +350,13 @@ static void test_bmdma_short_prdt(void)
/* Normal request */
status = send_dma_request(CMD_READ_DMA, 0, 1,
- prdt, ARRAY_SIZE(prdt));
+ prdt, ARRAY_SIZE(prdt), NULL);
g_assert_cmphex(status, ==, 0);
assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
/* Abort the request before it completes */
status = send_dma_request(CMD_READ_DMA | CMDF_ABORT, 0, 1,
- prdt, ARRAY_SIZE(prdt));
+ prdt, ARRAY_SIZE(prdt), NULL);
g_assert_cmphex(status, ==, 0);
assert_bit_clear(inb(IDE_BASE + reg_status), DF | ERR);
}
@@ -360,13 +375,13 @@ static void test_bmdma_one_sector_short_prdt(void)
/* Normal request */
status = send_dma_request(CMD_READ_DMA, 0, 2,
- prdt, ARRAY_SIZE(prdt));
+ prdt, ARRAY_SIZE(prdt), NULL);
g_assert_cmphex(status, ==, 0);
[Qemu-block] [PATCH 1/4] qtest/ahci: use generate_pattern everywhere
Fix the pattern generation to actually be interesting,
and make sure all buffers in the ahci-test actually use it.
Signed-off-by: John Snow js...@redhat.com
---
tests/ahci-test.c | 23 ++-
1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 87d7691..b1a785c 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -80,9 +80,9 @@ static void generate_pattern(void *buffer, size_t len, size_t
cycle_len)
/* Write an indicative pattern that varies and is unique per-cycle */
p = rand() % 256;
-for (i = j = 0; i len; i++, j++) {
-tx[i] = p;
-if (j % cycle_len == 0) {
+for (i = 0; i len; i++) {
+tx[i] = p++ % 256;
+if (i % cycle_len == 0) {
p = rand() % 256;
}
}
@@ -1155,7 +1155,6 @@ static void ahci_migrate_simple(uint8_t cmd_read, uint8_t
cmd_write)
size_t bufsize = 4096;
unsigned char *tx = g_malloc(bufsize);
unsigned char *rx = g_malloc0(bufsize);
-unsigned i;
const char *uri = tcp:127.0.0.1:1234;
src = ahci_boot_and_enable(-m 1024 -M q35
@@ -1171,9 +1170,7 @@ static void ahci_migrate_simple(uint8_t cmd_read, uint8_t
cmd_write)
ahci_port_clear(src, px);
/* create pattern */
-for (i = 0; i bufsize; i++) {
-tx[i] = (bufsize - i);
-}
+generate_pattern(tx, bufsize, AHCI_SECTOR_SIZE);
/* Write, migrate, then read. */
ahci_io(src, px, cmd_write, tx, bufsize, 0);
@@ -1213,7 +1210,6 @@ static void ahci_halted_io_test(uint8_t cmd_read, uint8_t
cmd_write)
size_t bufsize = 4096;
unsigned char *tx = g_malloc(bufsize);
unsigned char *rx = g_malloc0(bufsize);
-unsigned i;
uint64_t ptr;
AHCICommand *cmd;
@@ -1231,11 +1227,8 @@ static void ahci_halted_io_test(uint8_t cmd_read,
uint8_t cmd_write)
port = ahci_port_select(ahci);
ahci_port_clear(ahci, port);
-for (i = 0; i bufsize; i++) {
-tx[i] = (bufsize - i);
-}
-
/* create DMA source buffer and write pattern */
+generate_pattern(tx, bufsize, AHCI_SECTOR_SIZE);
ptr = ahci_alloc(ahci, bufsize);
g_assert(ptr);
memwrite(ptr, tx, bufsize);
@@ -1282,7 +1275,6 @@ static void ahci_migrate_halted_io(uint8_t cmd_read,
uint8_t cmd_write)
size_t bufsize = 4096;
unsigned char *tx = g_malloc(bufsize);
unsigned char *rx = g_malloc0(bufsize);
-unsigned i;
uint64_t ptr;
AHCICommand *cmd;
const char *uri = tcp:127.0.0.1:1234;
@@ -1310,10 +1302,7 @@ static void ahci_migrate_halted_io(uint8_t cmd_read,
uint8_t cmd_write)
/* Initialize and prepare */
port = ahci_port_select(src);
ahci_port_clear(src, port);
-
-for (i = 0; i bufsize; i++) {
-tx[i] = (bufsize - i);
-}
+generate_pattern(tx, bufsize, AHCI_SECTOR_SIZE);
/* create DMA source buffer and write pattern */
ptr = ahci_alloc(src, bufsize);
--
2.4.3
Re: [Qemu-block] [PATCHv2] block/nfs: cache allocated filesize for read-only files
On Mon, Aug 24, 2015 at 10:13:16PM +0200, Max Reitz wrote: On 24.08.2015 21:34, Peter Lieven wrote: Am 24.08.2015 um 20:39 schrieb Max Reitz: On 24.08.2015 10:06, Peter Lieven wrote: If the file is readonly its not expected to grow so save the blocking call to nfs_fstat_async and use the value saved at connection time. Also important the monitor (and thus the main loop) will not hang if block device info is queried and the NFS share is unresponsive. Signed-off-by: Peter Lieven p...@kamp.de --- v1-v2: update cache on reopen_prepare [Max] block/nfs.c | 35 +++ 1 file changed, 35 insertions(+) Reviewed-by: Max Reitz mre...@redhat.com I hope you're ready for the Stale actual-size value with cache=direct,read-only=on,format=raw files on NFS reports. :-) actually a good point, maybe the cache should only be used if !(bs-open_flags BDRV_O_NOCACHE) Good enough a point to fix it? ;-) Max It seems more inline with expected behavior, to add the cache checking in before using the size cache. Would you be opposed to a v3 with this check added in? One other concern I have is similar to a concern Max raised earlier - about an external program modifying the raw image, while QEMU has it opened r/o. In particular, I wonder about an NFS server making an image either sparse / non-sparse. If it was exported read-only, it may be a valid assumption that this could be done safely, as it would not change the reported file size or contents, just the allocated size on disk.
Re: [Qemu-block] [PATCH] block/nfs: fix calculation of allocated file size
On Thu, Aug 20, 2015 at 12:46:47PM +0200, Peter Lieven wrote: st.st_blocks is always counted in 512 byte units. Do not use st.st_blksize as multiplicator which may be larger. Cc: qemu-sta...@nongnu.org Signed-off-by: Peter Lieven p...@kamp.de --- block/nfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/nfs.c b/block/nfs.c index c026ff6..02eb4e4 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -475,7 +475,7 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs) aio_poll(client-aio_context, true); } -return (task.ret 0 ? task.ret : st.st_blocks * st.st_blksize); +return (task.ret 0 ? task.ret : st.st_blocks * 512); } static int nfs_file_truncate(BlockDriverState *bs, int64_t offset) -- 1.9.1 Reviewed-by: Jeff Cody jc...@redhat.com
