Re: [Qemu-block] [Qemu-devel] [PATCH v8 15/35] qom: Swap 'name' next to visitor in ObjectPropertyAccessor
Hi On Wed, Dec 23, 2015 at 5:30 PM, Eric Blake wrote: > On 12/21/2015 10:08 AM, Eric Blake wrote: >> Similar to the previous patch, it's nice to have all functions >> n the tree that involve a visitor and a name for conversion to > > s/^n/in/ I was about to say that, with that Reviewed-by: Marc-André Lureau > >> or from QAPI to consistently stick the 'name' parameter next >> to the Visitor parameter. >> > > -- > Eric Blake eblake redhat com+1-919-301-3266 > Libvirt virtualization library http://libvirt.org > -- Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [PATCH v8 14/35] qapi: Swap visit_* arguments for consistent 'name' placement
Reviewed-by: Marc-André Lureau Hi
On Mon, Dec 21, 2015 at 6:08 PM, Eric Blake wrote:
> JSON uses "name":value, but many of our visitor interfaces were
> called with visit_type_FOO(v, &value, name, errp). This can be
> a bit confusing to have to mentally swap the parameter order to
> match JSON order. It's particularly bad for visit_start_struct(),
> where the 'name' parameter is smack in the middle of the
> otherwise-related group of 'obj, kind, size' parameters! It's
> time to do a global swap of the parameter ordering, so that the
> 'name' parameter is always immediately after the Visitor argument.
>
fwiw, I do agree.
> Additional reasons in favor of the swap: name is always an input
> parameter, while &value is sometimes an output parameter (depending
> on whether the caller is using an input visitor); and it is nicer
> to list input parameters first. Also, the existing include/qjson.h
> prefers listing 'name' first in json_prop_*(), and I have plans to
> unify that file with the qapi visitors; listing 'name' first in
> qapi will minimize churn to the (admittedly few) qjson.h clients.
>
> The next patches will then fix object.h, visitor-impl.h, and those
> clients to match.
>
> Done by first patching scripts/qapi*.py by hand to make generated
> files do what I want, then by running the following Coccinelle
> script to affect the rest of the code base:
> $ spatch --sp-file script `git grep -l '\bvisit_' -- '**/*.[ch]'`
> I then had to apply some touchups (Coccinelle insisted on TAB
> indentation in visitor.h, and botched the signature of
> visit_type_enum() by rewriting 'const char *const strings[]' to
> the syntactically invalid 'const char*const[] strings').
>
> // Part 1: Swap declaration order
> @@
> type TV, TErr, TObj, T1, T2;
> identifier OBJ, ARG1, ARG2;
> @@
> void visit_start_struct
> -(TV v, TObj OBJ, T1 ARG1, const char *name, T2 ARG2, TErr errp)
> +(TV v, const char *name, TObj OBJ, T1 ARG1, T2 ARG2, TErr errp)
> { ... }
>
> @@
> type bool, TV, T1;
> identifier ARG1;
> @@
> bool visit_optional
> -(TV v, T1 ARG1, const char *name)
> +(TV v, const char *name, T1 ARG1)
> { ... }
>
> @@
> type TV, TErr, TObj, T1;
> identifier OBJ, ARG1;
> @@
> void visit_get_next_type
> -(TV v, TObj OBJ, T1 ARG1, const char *name, TErr errp)
> +(TV v, const char *name, TObj OBJ, T1 ARG1, TErr errp)
> { ... }
>
> @@
> type TV, TErr, TObj, T1, T2;
> identifier OBJ, ARG1, ARG2;
> @@
> void visit_type_enum
> -(TV v, TObj OBJ, T1 ARG1, T2 ARG2, const char *name, TErr errp)
> +(TV v, const char *name, TObj OBJ, T1 ARG1, T2 ARG2, TErr errp)
> { ... }
>
> @@
> type TV, TErr, TObj;
> identifier OBJ;
> identifier VISIT_TYPE =~ "^visit_type_";
> @@
> void VISIT_TYPE
> -(TV v, TObj OBJ, const char *name, TErr errp)
> +(TV v, const char *name, TObj OBJ, TErr errp)
> { ... }
>
> // Part 2: swap caller order
> @@
> expression V, NAME, OBJ, ARG1, ARG2, ERR;
> identifier VISIT_TYPE =~ "^visit_type_";
> @@
> (
> -visit_start_struct(V, OBJ, ARG1, NAME, ARG2, ERR)
> +visit_start_struct(V, NAME, OBJ, ARG1, ARG2, ERR)
> |
> -visit_optional(V, ARG1, NAME)
> +visit_optional(V, NAME, ARG1)
> |
> -visit_get_next_type(V, OBJ, ARG1, NAME, ERR)
> +visit_get_next_type(V, NAME, OBJ, ARG1, ERR)
> |
> -visit_type_enum(V, OBJ, ARG1, ARG2, NAME, ERR)
> +visit_type_enum(V, NAME, OBJ, ARG1, ARG2, ERR)
> |
> -VISIT_TYPE(V, OBJ, NAME, ERR)
> +VISIT_TYPE(V, NAME, OBJ, ERR)
> )
>
> Signed-off-by: Eric Blake
The result looks good and passes the tests
Reviewed-by: Marc-André Lureau
However, docs/qapi-code-gen.txt should be updated in a follow-up patch.
--
Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [PATCH 1/7] util: Add UUID API
Hi On Thu, Aug 4, 2016 at 4:44 PM Paolo Bonzini wrote: > > > On 03/08/2016 04:36, Fam Zheng wrote: > > On Tue, 08/02 15:45, Paolo Bonzini wrote: > >> > >> > >> - Original Message - > >>> From: "Fam Zheng" > >>> To: qemu-de...@nongnu.org > >>> Cc: f...@redhat.com, berra...@redhat.com, pbonz...@redhat.com, > kw...@redhat.com, mre...@redhat.com, > >>> mdr...@linux.vnet.ibm.com, arm...@redhat.com, s...@weilnetz.de, > qemu-block@nongnu.org > >>> Sent: Tuesday, August 2, 2016 11:18:32 AM > >>> Subject: [PATCH 1/7] util: Add UUID API > >>> > >>> A number of different places across the code base use CONFIG_UUID. Some > >>> of them are soft dependency, some are not built if libuuid is not > >>> available, some come with dummy fallback, some throws runtime error. > For info, there has been glib UUID proposal for a while: https://bugzilla.gnome.org/show_bug.cgi?id=639078 Although quite ready, the discussion stopped because some dev believe g_uuid_string_random() is all you need. Anyway, if ever accepted, it would take another 5y or so to be acceptable for qemu :) -- Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [PATCH] hmp: Remove dead code in hmp_qemu_io()
On Thu, Sep 15, 2016 at 7:50 PM Kevin Wolf wrote:
> blk can never be NULL, drop the check. This fixes a Coverity warning.
>
> Signed-off-by: Kevin Wolf
>
Reviewed-by: Marc-André Lureau
> ---
> hmp.c | 14 +-
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index ad33b44..0a16aef 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1924,6 +1924,7 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
> {
> BlockBackend *blk;
> BlockBackend *local_blk = NULL;
> +AioContext *aio_context;
> const char* device = qdict_get_str(qdict, "device");
> const char* command = qdict_get_str(qdict, "command");
> Error *err = NULL;
> @@ -1939,17 +1940,12 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
> }
> }
>
> -if (blk) {
> -AioContext *aio_context = blk_get_aio_context(blk);
> -aio_context_acquire(aio_context);
> +aio_context = blk_get_aio_context(blk);
> +aio_context_acquire(aio_context);
>
> -qemuio_command(blk, command);
> +qemuio_command(blk, command);
>
> -aio_context_release(aio_context);
> -} else {
> -error_set(&err, ERROR_CLASS_DEVICE_NOT_FOUND,
> - "Device '%s' not found", device);
> -}
> +aio_context_release(aio_context);
>
> fail:
> blk_unref(local_blk);
> --
> 1.8.3.1
>
>
> --
Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [PATCH v6 08/15] qstring: Add qstring_wrap_str()
--git a/tests/check-qstring.c b/tests/check-qstring.c
> index 11823c2..0c427c7 100644
> --- a/tests/check-qstring.c
> +++ b/tests/check-qstring.c
> @@ -34,6 +34,20 @@ static void qstring_from_str_test(void)
> QDECREF(qstring);
> }
>
> +static void qstring_wrap_str_test(void)
> +{
> +QString *qstring;
> +char *str = g_strdup("QEMU");
> +
> +qstring = qstring_wrap_str(str);
> +g_assert(qstring != NULL);
> +g_assert(qstring->base.refcnt == 1);
> +g_assert(strcmp("QEMU", qstring->string) == 0);
> +g_assert(qobject_type(QOBJECT(qstring)) == QTYPE_QSTRING);
> +
> +QDECREF(qstring);
> +}
> +
> static void qstring_destroy_test(void)
> {
> QString *qstring = qstring_from_str("destroy test");
> @@ -119,6 +133,7 @@ int main(int argc, char **argv)
> g_test_init(&argc, &argv, NULL);
>
> g_test_add_func("/public/from_str", qstring_from_str_test);
> +g_test_add_func("/public/wrap_str", qstring_wrap_str_test);
> g_test_add_func("/public/destroy", qstring_destroy_test);
> g_test_add_func("/public/get_str", qstring_get_str_test);
> g_test_add_func("/public/append_chr", qstring_append_chr_test);
> --
> 2.7.4
>
>
> --
Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [PATCH v6 08/15] qstring: Add qstring_wrap_str()
Hi
On Tue, Oct 11, 2016 at 7:05 PM Eric Blake wrote:
> On 10/11/2016 10:04 AM, Eric Blake wrote:
> > On 10/11/2016 06:08 AM, Marc-André Lureau wrote:
> >
> >>> +++ b/block.c
> >>> @@ -1640,7 +1640,8 @@ static BlockDriverState
> >>> *bdrv_append_temp_snapshot(BlockDriverState *bs,
> >>> qdict_put(snapshot_options, "file.driver",
> >>>qstring_from_str("file"));
> >>> qdict_put(snapshot_options, "file.filename",
> >>> - qstring_from_str(tmp_filename));
> >>> + qstring_wrap_str(tmp_filename));
> >>> +tmp_filename = NULL;
> >>> qdict_put(snapshot_options, "driver",
> >>>qstring_from_str("qcow2"));
> >>>
> >>>
> >> You could also remove g_free(tmp_filename) from the normal return path
> >> (this may please static analyzers).
> >
> > No. g_free(NULL) is safe, but we can also reach the 'out' label with
> > tmp_filename still malloc'd prior to the place where we transfer it
> > here, so the g_free() in the cleanup label is still required. The
> > assignment to NULL here prevents a double free. The patch is correct
> as-is.
>
> Spoke too soon. I see what you're saying - the normal return path now
> has a dead g_free(NULL). It won't cause any grief to the static
> analyzers, but it is a useless no-op function call, so I can indeed trim
> it (the one before the label, not the one after).
>
static analyzers could catch the fact that you always call g_free(NULL) in
this code path, please remove it.
> --
> Eric Blake eblake redhat com+1-919-301-3266 <+1%20919-301-3266>
> Libvirt virtualization library http://libvirt.org
>
> --
Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [PATCH for-2.8] qapi: Document DEVICE_TRAY_MOVED addition
On Tue, Dec 6, 2016 at 7:04 PM Eric Blake wrote:
> Commit 2d76e72 failed to add a versioning tag to 'id'.
>
> I audited all qapi*.json files from v2.7.0 to the current
> state of the tree, and didn't find any other additions where
> we failed to use a version tag.
>
> Signed-off-by: Eric Blake
>
Reviewed-by: Marc-André Lureau
---
>
> Doc-only, so safe for 2.8. But we're also close enough to -rc3
> that I'm fine if it has to go in later via -stable.
>
> qapi/block.json | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qapi/block.json b/qapi/block.json
> index 937df05..8e9f590 100644
> --- a/qapi/block.json
> +++ b/qapi/block.json
> @@ -199,7 +199,7 @@
> # reasons, but it can be empty ("") if the image does not
> # have a device name associated.
> #
> -# @id: The name or QOM path of the guest device
> +# @id: The name or QOM path of the guest device (since 2.8)
> #
> # @tray-open: true if the tray has been opened or false if it has been
> closed
> #
> --
> 2.9.3
>
>
> --
Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [PATCH v3] opts: produce valid command line in qemu_opts_print
On Tue, Jul 7, 2015 at 4:42 PM, Kővágó, Zoltán 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 you can remove the first "and" here > * do not print separator before the first element > * do not quote string arguments > * properly escape commas (,) for QEMU Reviewed-by: Marc-André Lureau -- Marc-André Lureau
[Qemu-block] [PATCH] iscsi: use scsi_create_task()
The function does the same initialization, and matches with
scsi_free_scsi_task() usage, and qemu doesn't need to know the
allocator details.
Signed-off-by: Marc-André Lureau
---
block/iscsi.c | 27 ---
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 5daa201181..a6bcd8eb13 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1013,6 +1013,7 @@ static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
struct iscsi_context *iscsi = iscsilun->iscsi;
struct iscsi_data data;
IscsiAIOCB *acb;
+int xfer_dir;
acb = qemu_aio_get(&iscsi_aiocb_info, bs, cb, opaque);
@@ -1034,30 +1035,26 @@ static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
return NULL;
}
-acb->task = malloc(sizeof(struct scsi_task));
-if (acb->task == NULL) {
-error_report("iSCSI: Failed to allocate task for scsi command. %s",
- iscsi_get_error(iscsi));
-qemu_aio_unref(acb);
-return NULL;
-}
-memset(acb->task, 0, sizeof(struct scsi_task));
-
switch (acb->ioh->dxfer_direction) {
case SG_DXFER_TO_DEV:
-acb->task->xfer_dir = SCSI_XFER_WRITE;
+xfer_dir = SCSI_XFER_WRITE;
break;
case SG_DXFER_FROM_DEV:
-acb->task->xfer_dir = SCSI_XFER_READ;
+xfer_dir = SCSI_XFER_READ;
break;
default:
-acb->task->xfer_dir = SCSI_XFER_NONE;
+xfer_dir = SCSI_XFER_NONE;
break;
}
-acb->task->cdb_size = acb->ioh->cmd_len;
-memcpy(&acb->task->cdb[0], acb->ioh->cmdp, acb->ioh->cmd_len);
-acb->task->expxferlen = acb->ioh->dxfer_len;
+acb->task = scsi_create_task(acb->ioh->cmd_len, acb->ioh->cmdp,
+ xfer_dir, acb->ioh->dxfer_len);
+if (acb->task == NULL) {
+error_report("iSCSI: Failed to allocate task for scsi command. %s",
+ iscsi_get_error(iscsi));
+qemu_aio_unref(acb);
+return NULL;
+}
data.size = 0;
qemu_mutex_lock(&iscsilun->mutex);
--
2.13.0.91.g00982b8dd
[Qemu-block] [PATCH 03/31] vhdx: use QEMU_ALIGN_DOWN
I used the clang-tidy qemu-round check to generate the fix: https://github.com/elmarco/clang-tools-extra Signed-off-by: Marc-André Lureau --- block/vhdx-log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/vhdx-log.c b/block/vhdx-log.c index 3f4c2aa095..6125ea4c6d 100644 --- a/block/vhdx-log.c +++ b/block/vhdx-log.c @@ -884,7 +884,7 @@ static int vhdx_log_write(BlockDriverState *bs, BDRVVHDXState *s, } sector_offset = offset % VHDX_LOG_SECTOR_SIZE; -file_offset = (offset / VHDX_LOG_SECTOR_SIZE) * VHDX_LOG_SECTOR_SIZE; +file_offset = QEMU_ALIGN_DOWN(offset, VHDX_LOG_SECTOR_SIZE); aligned_length = length; -- 2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 07/31] dmg: use DIV_ROUND_UP
I used the clang-tidy qemu-round check to generate the fix: https://github.com/elmarco/clang-tools-extra Signed-off-by: Marc-André Lureau --- block/dmg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/dmg.c b/block/dmg.c index 900ae5a678..6c0711f563 100644 --- a/block/dmg.c +++ b/block/dmg.c @@ -111,7 +111,7 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk, uncompressed_sectors = s->sectorcounts[chunk]; break; case 1: /* copy */ -uncompressed_sectors = (s->lengths[chunk] + 511) / 512; +uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512); break; case 2: /* zero */ /* as the all-zeroes block may be large, it is treated specially: the -- 2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 09/31] vpc: use DIV_ROUND_UP
I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra
Signed-off-by: Marc-André Lureau
---
block/vpc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/vpc.c b/block/vpc.c
index 4240ba9d1c..f52a7c0f0f 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -760,7 +760,7 @@ static int calculate_geometry(int64_t total_sectors,
uint16_t* cyls,
} else {
*secs_per_cyl = 17;
cyls_times_heads = total_sectors / *secs_per_cyl;
-*heads = (cyls_times_heads + 1023) / 1024;
+*heads = DIV_ROUND_UP(cyls_times_heads, 1024);
if (*heads < 4) {
*heads = 4;
@@ -813,7 +813,7 @@ static int create_dynamic_disk(BlockBackend *blk, uint8_t
*buf,
offset = 3 * 512;
memset(buf, 0xFF, 512);
-for (i = 0; i < (num_bat_entries * 4 + 511) / 512; i++) {
+for (i = 0; i < DIV_ROUND_UP(num_bat_entries * 4, 512); i++) {
ret = blk_pwrite(blk, offset, buf, 512, 0);
if (ret < 0) {
goto fail;
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 10/31] vvfat: use DIV_ROUND_UP
I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra
Signed-off-by: Marc-André Lureau
---
block/vvfat.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index 426ca70e35..877f71dcdc 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -433,7 +433,7 @@ static inline direntry_t*
create_long_filename(BDRVVVFATState* s,const char* fil
{
char buffer[258];
int length=short2long_name(buffer,filename),
-number_of_entries=(length+25)/26,i;
+number_of_entries=DIV_ROUND_UP(length, 26),i;
direntry_t* entry;
for(i=0;i offset && c >=2 && !fat_eof(s, c)));
ret = vvfat_read(s->bs, cluster2sector(s, c),
- (uint8_t*)cluster, (rest_size + 0x1ff) / 0x200);
+ (uint8_t*)cluster, DIV_ROUND_UP(rest_size, 0x200));
if (ret < 0) {
qemu_close(fd);
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 08/31] qcow2: use DIV_ROUND_UP
I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra
Signed-off-by: Marc-André Lureau
---
block/qcow2-cluster.c | 2 +-
block/qcow2-refcount.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index d779ea19cf..da9008815c 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -61,7 +61,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t
min_size,
new_l1_size = 1;
}
while (min_size > new_l1_size) {
-new_l1_size = (new_l1_size * 3 + 1) / 2;
+new_l1_size = DIV_ROUND_UP(new_l1_size * 3, 2);
}
}
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 7c06061aae..75107cf093 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -293,7 +293,7 @@ static unsigned int next_refcount_table_size(BDRVQcow2State
*s,
MAX(1, s->refcount_table_size >> (s->cluster_bits - 3));
while (min_clusters > refcount_table_clusters) {
-refcount_table_clusters = (refcount_table_clusters * 3 + 1) / 2;
+refcount_table_clusters = DIV_ROUND_UP(refcount_table_clusters * 3, 2);
}
return refcount_table_clusters << (s->cluster_bits - 3);
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 07/35] blockjob: mark coroutine_fn
/home/elmarco/src/qemu/blockjob.c:820:9: error: calling function
'qemu_coroutine_yield' requires holding role '_coroutine_fn' exclusively
[-Werror,-Wthread-safety-analysis]
qemu_coroutine_yield();
^
/home/elmarco/src/qemu/blockjob.c:824:5: error: calling function
'block_job_pause_point' requires holding role '_coroutine_fn' exclusively
[-Werror,-Wthread-safety-analysis]
block_job_pause_point(job);
^
Signed-off-by: Marc-André Lureau
---
include/block/blockjob_int.h | 4 ++--
blockjob.c | 6 --
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index f13ad05c0d..a3bc01fd51 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -145,7 +145,7 @@ void *block_job_create(const char *job_id, const
BlockJobDriver *driver,
* Put the job to sleep (assuming that it wasn't canceled) for @ns
* nanoseconds. Canceling the job will interrupt the wait immediately.
*/
-void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns);
+void coroutine_fn block_job_sleep_ns(BlockJob *job, QEMUClockType type,
int64_t ns);
/**
* block_job_yield:
@@ -153,7 +153,7 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type,
int64_t ns);
*
* Yield the block job coroutine.
*/
-void block_job_yield(BlockJob *job);
+void coroutine_fn block_job_yield(BlockJob *job);
/**
* block_job_pause_all:
diff --git a/blockjob.c b/blockjob.c
index 70a78188b7..96424323c1 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -788,7 +788,8 @@ bool block_job_is_cancelled(BlockJob *job)
return job->cancelled;
}
-void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
+void coroutine_fn
+block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
{
assert(job->busy);
@@ -806,7 +807,8 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType type,
int64_t ns)
block_job_pause_point(job);
}
-void block_job_yield(BlockJob *job)
+void coroutine_fn
+block_job_yield(BlockJob *job)
{
assert(job->busy);
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 02/35] WIP: coroutine: manually tag the fast-paths
Some functions are both regular and coroutine. They may call coroutine
functions in some cases, if it is known to be running in a coroutine.
Signed-off-by: Marc-André Lureau
---
block.c | 2 ++
block/block-backend.c | 2 ++
block/io.c | 16 +++-
block/sheepdog.c| 2 ++
block/throttle-groups.c | 10 --
migration/rdma.c| 2 ++
6 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index 694396281b..b08c006da4 100644
--- a/block.c
+++ b/block.c
@@ -443,7 +443,9 @@ int bdrv_create(BlockDriver *drv, const char* filename,
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
+co_role_acquire(_coroutine_fn);
bdrv_create_co_entry(&cco);
+co_role_release(_coroutine_fn);
} else {
co = qemu_coroutine_create(bdrv_create_co_entry, &cco);
qemu_coroutine_enter(co);
diff --git a/block/block-backend.c b/block/block-backend.c
index 0df3457a09..56fc0a4d1e 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1072,7 +1072,9 @@ static int blk_prw(BlockBackend *blk, int64_t offset,
uint8_t *buf,
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
+co_role_acquire(_coroutine_fn);
co_entry(&rwco);
+co_role_release(_coroutine_fn);
} else {
Coroutine *co = qemu_coroutine_create(co_entry, &rwco);
bdrv_coroutine_enter(blk_bs(blk), co);
diff --git a/block/io.c b/block/io.c
index 2de7c77983..14b88c8609 100644
--- a/block/io.c
+++ b/block/io.c
@@ -229,7 +229,9 @@ static void coroutine_fn
bdrv_co_yield_to_drain(BlockDriverState *bs)
void bdrv_drained_begin(BlockDriverState *bs)
{
if (qemu_in_coroutine()) {
+co_role_acquire(_coroutine_fn);
bdrv_co_yield_to_drain(bs);
+co_role_release(_coroutine_fn);
return;
}
@@ -616,7 +618,9 @@ static int bdrv_prwv_co(BdrvChild *child, int64_t offset,
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
+co_role_acquire(_coroutine_fn);
bdrv_rw_co_entry(&rwco);
+co_role_release(_coroutine_fn);
} else {
co = qemu_coroutine_create(bdrv_rw_co_entry, &rwco);
bdrv_coroutine_enter(child->bs, co);
@@ -1901,7 +1905,9 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs,
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
+co_role_acquire(_coroutine_fn);
bdrv_get_block_status_above_co_entry(&data);
+co_role_release(_coroutine_fn);
} else {
co = qemu_coroutine_create(bdrv_get_block_status_above_co_entry,
&data);
@@ -2027,7 +2033,11 @@ bdrv_rw_vmstate(BlockDriverState *bs, QEMUIOVector
*qiov, int64_t pos,
bool is_read)
{
if (qemu_in_coroutine()) {
-return bdrv_co_rw_vmstate(bs, qiov, pos, is_read);
+int ret;
+co_role_acquire(_coroutine_fn);
+ret = bdrv_co_rw_vmstate(bs, qiov, pos, is_read);
+co_role_release(_coroutine_fn);
+return ret;
} else {
BdrvVmstateCo data = {
.bs = bs,
@@ -2259,7 +2269,9 @@ int bdrv_flush(BlockDriverState *bs)
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
+co_role_acquire(_coroutine_fn);
bdrv_flush_co_entry(&flush_co);
+co_role_release(_coroutine_fn);
} else {
co = qemu_coroutine_create(bdrv_flush_co_entry, &flush_co);
bdrv_coroutine_enter(bs, co);
@@ -2406,7 +2418,9 @@ int bdrv_pdiscard(BlockDriverState *bs, int64_t offset,
int bytes)
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
+co_role_acquire(_coroutine_fn);
bdrv_pdiscard_co_entry(&rwco);
+co_role_release(_coroutine_fn);
} else {
co = qemu_coroutine_create(bdrv_pdiscard_co_entry, &rwco);
bdrv_coroutine_enter(bs, co);
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 08d7b11e9d..83bc43dde4 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -726,7 +726,9 @@ static int do_req(int sockfd, BlockDriverState *bs,
SheepdogReq *hdr,
};
if (qemu_in_coroutine()) {
+co_role_acquire(_coroutine_fn);
do_co_req(&srco);
+co_role_release(_coroutine_fn);
} else {
co = qemu_coroutine_create(do_co_req, &srco);
if (bs) {
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index da2b490c38..8778f78965 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -304,9 +304,15 @@ static void schedule_next_request(BlockBackend *blk, bool
is_write)
/* If it doesn't have to wait, queue it for immediate execution */
if (!must_wait) {
+bool handled = false;
+
[Qemu-block] [PATCH 11/35] qcow2: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/qcow2.h | 6 --
block/qcow.c | 4 +++-
block/qcow2-cluster.c | 11 +++
block/qcow2.c | 15 ++-
4 files changed, 24 insertions(+), 12 deletions(-)
diff --git a/block/qcow2.h b/block/qcow2.h
index 87b15eb4aa..a32b47b7f6 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -550,14 +550,16 @@ int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t
sector_num,
int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
unsigned int *bytes, uint64_t *cluster_offset);
-int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn
+qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
unsigned int *bytes, uint64_t *host_offset,
QCowL2Meta **m);
uint64_t qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
uint64_t offset,
int compressed_size);
-int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
+int coroutine_fn
+qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
int qcow2_cluster_discard(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, enum qcow2_discard_type type,
bool full_discard);
diff --git a/block/qcow.c b/block/qcow.c
index 7bd94dcd46..d84ae7fb74 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -796,7 +796,9 @@ static void qcow_close(BlockDriverState *bs)
error_free(s->migration_blocker);
}
-static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
+
+static int coroutine_fn
+qcow_create(const char *filename, QemuOpts *opts, Error **errp)
{
int header_size, backing_filename_len, l1_size, shift, i;
QCowHeader header;
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 3d341fd9cb..964d23aee8 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -761,7 +761,8 @@ uint64_t
qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs,
return cluster_offset;
}
-static int perform_cow(BlockDriverState *bs, QCowL2Meta *m)
+static int coroutine_fn
+perform_cow(BlockDriverState *bs, QCowL2Meta *m)
{
BDRVQcow2State *s = bs->opaque;
Qcow2COWRegion *start = &m->cow_start;
@@ -890,7 +891,7 @@ fail:
return ret;
}
-int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
+int coroutine_fn qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta
*m)
{
BDRVQcow2State *s = bs->opaque;
int i, j = 0, l2_index, ret;
@@ -1014,7 +1015,8 @@ out:
* information on cluster allocation may be invalid now. The caller
* must start over anyway, so consider *cur_bytes undefined.
*/
-static int handle_dependencies(BlockDriverState *bs, uint64_t guest_offset,
+static int coroutine_fn
+handle_dependencies(BlockDriverState *bs, uint64_t guest_offset,
uint64_t *cur_bytes, QCowL2Meta **m)
{
BDRVQcow2State *s = bs->opaque;
@@ -1413,7 +1415,8 @@ fail:
*
* Return 0 on success and -errno in error cases
*/
-int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn
+qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
unsigned int *bytes, uint64_t *host_offset,
QCowL2Meta **m)
{
diff --git a/block/qcow2.c b/block/qcow2.c
index 2f94f0326e..6ecf1489dc 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2079,7 +2079,8 @@ static int qcow2_change_backing_file(BlockDriverState *bs,
return qcow2_update_header(bs);
}
-static int preallocate(BlockDriverState *bs)
+static int coroutine_fn
+preallocate(BlockDriverState *bs)
{
uint64_t bytes;
uint64_t offset;
@@ -2140,7 +2141,8 @@ static int preallocate(BlockDriverState *bs)
return 0;
}
-static int qcow2_create2(const char *filename, int64_t total_size,
+static int coroutine_fn
+qcow2_create2(const char *filename, int64_t total_size,
const char *backing_file, const char *backing_format,
int flags, size_t cluster_size, PreallocMode prealloc,
QemuOpts *opts, int version, int refcount_order,
@@ -2390,7 +2392,8 @@ out:
return ret;
}
-static int qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+qcow2_create(const char *filename, QemuOpts *opts, Error **errp)
{
char *backing_file = NULL;
char *backing_fmt = NULL;
@@ -3011,7 +3014,8 @@ static void dump_refcounts(BlockDriverState *bs)
}
#endif
-static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
+static int coroutine_fn
+qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
int64_t pos)
{
BDRVQcow2State *s = bs->opaque;
@@ -3021,7
[Qemu-block] [PATCH 08/35] block: all bdrv_aio callbacks are coroutine_fn
Signed-off-by: Marc-André Lureau
---
include/block/block_int.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 15fa602150..93eb2a9528 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -133,15 +133,15 @@ struct BlockDriver {
void (*bdrv_refresh_filename)(BlockDriverState *bs, QDict *options);
/* aio */
-BlockAIOCB *(*bdrv_aio_readv)(BlockDriverState *bs,
+BlockAIOCB * coroutine_fn (*bdrv_aio_readv)(BlockDriverState *bs,
int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
BlockCompletionFunc *cb, void *opaque);
-BlockAIOCB *(*bdrv_aio_writev)(BlockDriverState *bs,
+BlockAIOCB * coroutine_fn (*bdrv_aio_writev)(BlockDriverState *bs,
int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
BlockCompletionFunc *cb, void *opaque);
-BlockAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
+BlockAIOCB * coroutine_fn (*bdrv_aio_flush)(BlockDriverState *bs,
BlockCompletionFunc *cb, void *opaque);
-BlockAIOCB *(*bdrv_aio_pdiscard)(BlockDriverState *bs,
+BlockAIOCB * coroutine_fn (*bdrv_aio_pdiscard)(BlockDriverState *bs,
int64_t offset, int bytes,
BlockCompletionFunc *cb, void *opaque);
@@ -247,7 +247,7 @@ struct BlockDriver {
void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
/* to control generic scsi devices */
-BlockAIOCB *(*bdrv_aio_ioctl)(BlockDriverState *bs,
+BlockAIOCB * coroutine_fn (*bdrv_aio_ioctl)(BlockDriverState *bs,
unsigned long int req, void *buf,
BlockCompletionFunc *cb, void *opaque);
int coroutine_fn (*bdrv_co_ioctl)(BlockDriverState *bs,
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 15/35] backup: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
include/block/block_backup.h | 4 ++--
block/backup.c | 9 ++---
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/include/block/block_backup.h b/include/block/block_backup.h
index 8a759477a3..415cf8519d 100644
--- a/include/block/block_backup.h
+++ b/include/block/block_backup.h
@@ -27,12 +27,12 @@ typedef struct CowRequest {
CoQueue wait_queue; /* coroutines blocked on this request */
} CowRequest;
-void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num,
+void coroutine_fn backup_wait_for_overlapping_requests(BlockJob *job, int64_t
sector_num,
int nb_sectors);
void backup_cow_request_begin(CowRequest *req, BlockJob *job,
int64_t sector_num,
int nb_sectors);
-void backup_cow_request_end(CowRequest *req);
+void coroutine_fn backup_cow_request_end(CowRequest *req);
void backup_do_checkpoint(BlockJob *job, Error **errp);
diff --git a/block/backup.c b/block/backup.c
index 5387fbd84e..58ddd80b3f 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -84,7 +84,8 @@ static void cow_request_begin(CowRequest *req, BackupBlockJob
*job,
}
/* Forget about a completed request */
-static void cow_request_end(CowRequest *req)
+static void coroutine_fn
+cow_request_end(CowRequest *req)
{
QLIST_REMOVE(req, list);
qemu_co_queue_restart_all(&req->wait_queue);
@@ -275,7 +276,8 @@ void backup_do_checkpoint(BlockJob *job, Error **errp)
bitmap_zero(backup_job->done_bitmap, len);
}
-void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num,
+void coroutine_fn
+backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num,
int nb_sectors)
{
BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common);
@@ -304,7 +306,8 @@ void backup_cow_request_begin(CowRequest *req, BlockJob
*job,
cow_request_begin(req, backup_job, start, end);
}
-void backup_cow_request_end(CowRequest *req)
+void coroutine_fn
+backup_cow_request_end(CowRequest *req)
{
cow_request_end(req);
}
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 21/35] rbd: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/rbd.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/block/rbd.c b/block/rbd.c
index 9da02cdceb..7b4d548cd2 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -348,7 +348,8 @@ static QemuOptsList runtime_opts = {
},
};
-static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp)
{
Error *local_err = NULL;
int64_t bytes = 0;
@@ -861,7 +862,8 @@ failed:
return NULL;
}
-static BlockAIOCB *qemu_rbd_aio_readv(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+qemu_rbd_aio_readv(BlockDriverState *bs,
int64_t sector_num,
QEMUIOVector *qiov,
int nb_sectors,
@@ -873,7 +875,8 @@ static BlockAIOCB *qemu_rbd_aio_readv(BlockDriverState *bs,
RBD_AIO_READ);
}
-static BlockAIOCB *qemu_rbd_aio_writev(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+qemu_rbd_aio_writev(BlockDriverState *bs,
int64_t sector_num,
QEMUIOVector *qiov,
int nb_sectors,
@@ -886,7 +889,8 @@ static BlockAIOCB *qemu_rbd_aio_writev(BlockDriverState *bs,
}
#ifdef LIBRBD_SUPPORTS_AIO_FLUSH
-static BlockAIOCB *qemu_rbd_aio_flush(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+qemu_rbd_aio_flush(BlockDriverState *bs,
BlockCompletionFunc *cb,
void *opaque)
{
@@ -1063,7 +1067,8 @@ static int qemu_rbd_snap_list(BlockDriverState *bs,
}
#ifdef LIBRBD_SUPPORTS_DISCARD
-static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+qemu_rbd_aio_pdiscard(BlockDriverState *bs,
int64_t offset,
int bytes,
BlockCompletionFunc *cb,
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 20/35] quorum: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/quorum.c | 25 -
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/block/quorum.c b/block/quorum.c
index 55ba916655..b086d70daa 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -264,7 +264,8 @@ static void quorum_report_bad_versions(BDRVQuorumState *s,
}
}
-static void quorum_rewrite_entry(void *opaque)
+static void coroutine_fn
+quorum_rewrite_entry(void *opaque)
{
QuorumCo *co = opaque;
QuorumAIOCB *acb = co->acb;
@@ -282,7 +283,8 @@ static void quorum_rewrite_entry(void *opaque)
}
}
-static bool quorum_rewrite_bad_versions(QuorumAIOCB *acb,
+static bool coroutine_fn
+quorum_rewrite_bad_versions(QuorumAIOCB *acb,
QuorumVoteValue *value)
{
QuorumVoteVersion *version;
@@ -497,7 +499,8 @@ static int quorum_vote_error(QuorumAIOCB *acb)
return ret;
}
-static void quorum_vote(QuorumAIOCB *acb)
+static void coroutine_fn
+quorum_vote(QuorumAIOCB *acb)
{
bool quorum = true;
int i, j, ret;
@@ -577,7 +580,8 @@ free_exit:
quorum_free_vote_list(&acb->votes);
}
-static void read_quorum_children_entry(void *opaque)
+static void coroutine_fn
+read_quorum_children_entry(void *opaque)
{
QuorumCo *co = opaque;
QuorumAIOCB *acb = co->acb;
@@ -605,7 +609,7 @@ static void read_quorum_children_entry(void *opaque)
}
}
-static int read_quorum_children(QuorumAIOCB *acb)
+static int coroutine_fn read_quorum_children(QuorumAIOCB *acb)
{
BDRVQuorumState *s = acb->bs->opaque;
int i, ret;
@@ -648,7 +652,8 @@ static int read_quorum_children(QuorumAIOCB *acb)
return ret;
}
-static int read_fifo_child(QuorumAIOCB *acb)
+static int coroutine_fn
+read_fifo_child(QuorumAIOCB *acb)
{
BDRVQuorumState *s = acb->bs->opaque;
int n, ret;
@@ -669,7 +674,8 @@ static int read_fifo_child(QuorumAIOCB *acb)
return ret;
}
-static int quorum_co_preadv(BlockDriverState *bs, uint64_t offset,
+static int coroutine_fn
+quorum_co_preadv(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov, int flags)
{
BDRVQuorumState *s = bs->opaque;
@@ -689,7 +695,7 @@ static int quorum_co_preadv(BlockDriverState *bs, uint64_t
offset,
return ret;
}
-static void write_quorum_entry(void *opaque)
+static void coroutine_fn write_quorum_entry(void *opaque)
{
QuorumCo *co = opaque;
QuorumAIOCB *acb = co->acb;
@@ -715,7 +721,8 @@ static void write_quorum_entry(void *opaque)
}
}
-static int quorum_co_pwritev(BlockDriverState *bs, uint64_t offset,
+static int coroutine_fn
+quorum_co_pwritev(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov, int flags)
{
BDRVQuorumState *s = bs->opaque;
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 10/35] vmdk: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/vmdk.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 55581b03fe..f8422e8971 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1334,7 +1334,8 @@ static int64_t coroutine_fn
vmdk_co_get_block_status(BlockDriverState *bs,
return ret;
}
-static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
+static int coroutine_fn
+vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
int64_t offset_in_cluster, QEMUIOVector *qiov,
uint64_t qiov_offset, uint64_t n_bytes,
uint64_t offset)
@@ -1406,7 +1407,8 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t
cluster_offset,
return ret;
}
-static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
+static int coroutine_fn
+vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
int64_t offset_in_cluster, QEMUIOVector *qiov,
int bytes)
{
@@ -1551,7 +1553,8 @@ fail:
*
* Returns: error code with 0 for success.
*/
-static int vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
+static int coroutine_fn
+vmdk_pwritev(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov,
bool zeroed, bool zero_dry_run)
{
@@ -1857,7 +1860,8 @@ static int filename_decompose(const char *filename, char
*path, char *prefix,
return VMDK_OK;
}
-static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
{
int idx = 0;
BlockBackend *new_blk = NULL;
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 23/35] ssh: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/ssh.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/block/ssh.c b/block/ssh.c
index 52964416da..03a8ebe6f7 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -813,7 +813,8 @@ static QemuOptsList ssh_create_opts = {
}
};
-static int ssh_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+ssh_create(const char *filename, QemuOpts *opts, Error **errp)
{
int r, ret;
int64_t total_size = 0;
@@ -1029,7 +1030,8 @@ static coroutine_fn int ssh_co_readv(BlockDriverState *bs,
return ret;
}
-static int ssh_write(BDRVSSHState *s, BlockDriverState *bs,
+static int coroutine_fn
+ssh_write(BDRVSSHState *s, BlockDriverState *bs,
int64_t offset, size_t size,
QEMUIOVector *qiov)
{
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 24/35] null: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/null.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/block/null.c b/block/null.c
index 876f90965b..4c8afe16d7 100644
--- a/block/null.c
+++ b/block/null.c
@@ -167,7 +167,8 @@ static inline BlockAIOCB *null_aio_common(BlockDriverState
*bs,
return &acb->common;
}
-static BlockAIOCB *null_aio_readv(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+null_aio_readv(BlockDriverState *bs,
int64_t sector_num, QEMUIOVector *qiov,
int nb_sectors,
BlockCompletionFunc *cb,
@@ -182,7 +183,8 @@ static BlockAIOCB *null_aio_readv(BlockDriverState *bs,
return null_aio_common(bs, cb, opaque);
}
-static BlockAIOCB *null_aio_writev(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+null_aio_writev(BlockDriverState *bs,
int64_t sector_num, QEMUIOVector *qiov,
int nb_sectors,
BlockCompletionFunc *cb,
@@ -191,7 +193,8 @@ static BlockAIOCB *null_aio_writev(BlockDriverState *bs,
return null_aio_common(bs, cb, opaque);
}
-static BlockAIOCB *null_aio_flush(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+null_aio_flush(BlockDriverState *bs,
BlockCompletionFunc *cb,
void *opaque)
{
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 13/35] nbd: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/nbd-client.h | 10 +-
block/nbd-client.c | 24
block/nbd.c| 3 ++-
nbd/server.c | 3 ++-
4 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/block/nbd-client.h b/block/nbd-client.h
index 49636bc621..473d1f88fd 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -42,13 +42,13 @@ int nbd_client_init(BlockDriverState *bs,
Error **errp);
void nbd_client_close(BlockDriverState *bs);
-int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes);
-int nbd_client_co_flush(BlockDriverState *bs);
-int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset,
int bytes);
+int coroutine_fn nbd_client_co_flush(BlockDriverState *bs);
+int coroutine_fn nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov, int flags);
-int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
+int coroutine_fn nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t
offset,
int bytes, BdrvRequestFlags flags);
-int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov, int flags);
void nbd_client_detach_aio_context(BlockDriverState *bs);
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 02e928142e..63c0210c37 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -111,7 +111,8 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
s->read_reply_co = NULL;
}
-static int nbd_co_send_request(BlockDriverState *bs,
+static int coroutine_fn
+nbd_co_send_request(BlockDriverState *bs,
NBDRequest *request,
QEMUIOVector *qiov)
{
@@ -158,7 +159,8 @@ static int nbd_co_send_request(BlockDriverState *bs,
return rc;
}
-static void nbd_co_receive_reply(NBDClientSession *s,
+static void coroutine_fn
+nbd_co_receive_reply(NBDClientSession *s,
NBDRequest *request,
NBDReply *reply,
QEMUIOVector *qiov)
@@ -185,7 +187,8 @@ static void nbd_co_receive_reply(NBDClientSession *s,
}
}
-static void nbd_coroutine_end(BlockDriverState *bs,
+static void coroutine_fn
+nbd_coroutine_end(BlockDriverState *bs,
NBDRequest *request)
{
NBDClientSession *s = nbd_get_client_session(bs);
@@ -204,7 +207,8 @@ static void nbd_coroutine_end(BlockDriverState *bs,
qemu_co_mutex_unlock(&s->send_mutex);
}
-int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn
+nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov, int flags)
{
NBDClientSession *client = nbd_get_client_session(bs);
@@ -229,7 +233,8 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t
offset,
return -reply.error;
}
-int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
+int coroutine_fn
+nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
uint64_t bytes, QEMUIOVector *qiov, int flags)
{
NBDClientSession *client = nbd_get_client_session(bs);
@@ -258,7 +263,8 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t
offset,
return -reply.error;
}
-int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
+int coroutine_fn
+nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
int bytes, BdrvRequestFlags flags)
{
ssize_t ret;
@@ -292,7 +298,8 @@ int nbd_client_co_pwrite_zeroes(BlockDriverState *bs,
int64_t offset,
return -reply.error;
}
-int nbd_client_co_flush(BlockDriverState *bs)
+int coroutine_fn
+nbd_client_co_flush(BlockDriverState *bs)
{
NBDClientSession *client = nbd_get_client_session(bs);
NBDRequest request = { .type = NBD_CMD_FLUSH };
@@ -316,7 +323,8 @@ int nbd_client_co_flush(BlockDriverState *bs)
return -reply.error;
}
-int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
+int coroutine_fn
+nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes)
{
NBDClientSession *client = nbd_get_client_session(bs);
NBDRequest request = {
diff --git a/block/nbd.c b/block/nbd.c
index d529305330..8689b27d7d 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -465,7 +465,8 @@ static int nbd_open(BlockDriverState *bs, QDict *options,
int flags,
return ret;
}
-static int nbd_co_flush(BlockDriverState *bs)
+static int coroutine_fn
+nbd_co_flush(BlockDriverState *bs)
{
return nbd_client_co_flush(bs);
}
diff --git a/nbd/server.c b/nbd/server.c
index
[Qemu-block] [PATCH 09/35] block: bdrv_create() and bdrv_debug_event() are coroutine_fn
Called from coroutine.
Signed-off-by: Marc-André Lureau
---
include/block/block_int.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 93eb2a9528..a183c72b7c 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -126,7 +126,7 @@ struct BlockDriver {
int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
Error **errp);
void (*bdrv_close)(BlockDriverState *bs);
-int (*bdrv_create)(const char *filename, QemuOpts *opts, Error **errp);
+int coroutine_fn (*bdrv_create)(const char *filename, QemuOpts *opts,
Error **errp);
int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
int (*bdrv_make_empty)(BlockDriverState *bs);
@@ -267,7 +267,7 @@ struct BlockDriver {
BlockDriverAmendStatusCB *status_cb,
void *cb_opaque);
-void (*bdrv_debug_event)(BlockDriverState *bs, BlkdebugEvent event);
+void coroutine_fn (*bdrv_debug_event)(BlockDriverState *bs, BlkdebugEvent
event);
/* TODO Better pass a option string/QDict/QemuOpts to add any rule? */
int (*bdrv_debug_breakpoint)(BlockDriverState *bs, const char *event,
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 16/35] crypto: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/crypto.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/crypto.c b/block/crypto.c
index 10e5ddccaa..0e30a4ea06 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -568,7 +568,8 @@ static int block_crypto_open_luks(BlockDriverState *bs,
bs, options, flags, errp);
}
-static int block_crypto_create_luks(const char *filename,
+static int coroutine_fn
+block_crypto_create_luks(const char *filename,
QemuOpts *opts,
Error **errp)
{
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 12/35] raw: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/raw-format.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/block/raw-format.c b/block/raw-format.c
index 0d185fe41b..402d3b9fba 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -361,7 +361,8 @@ static void raw_lock_medium(BlockDriverState *bs, bool
locked)
bdrv_lock_medium(bs->file->bs, locked);
}
-static int raw_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
+static int coroutine_fn
+raw_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf)
{
BDRVRawState *s = bs->opaque;
if (s->offset || s->has_size) {
@@ -375,7 +376,8 @@ static int raw_has_zero_init(BlockDriverState *bs)
return bdrv_has_zero_init(bs->file->bs);
}
-static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+raw_create(const char *filename, QemuOpts *opts, Error **errp)
{
return bdrv_create_file(filename, opts, errp);
}
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 19/35] nfs: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/nfs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/nfs.c b/block/nfs.c
index c3c5de0113..3f393a95a4 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -679,7 +679,8 @@ static QemuOptsList nfs_create_opts = {
}
};
-static int nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
{
int ret = 0;
int64_t total_size = 0;
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 18/35] gluster: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/gluster.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/gluster.c b/block/gluster.c
index addceed6eb..dea8ab43a5 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -965,7 +965,8 @@ static coroutine_fn int
qemu_gluster_co_pwrite_zeroes(BlockDriverState *bs,
}
#endif
-static int qemu_gluster_create(const char *filename,
+static int coroutine_fn
+qemu_gluster_create(const char *filename,
QemuOpts *opts, Error **errp)
{
BlockdevOptionsGluster *gconf;
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 27/35] file-posix: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/file-posix.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 3927fabf06..adafbbb6a0 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1483,7 +1483,8 @@ static int aio_worker(void *arg)
return ret;
}
-static int paio_submit_co(BlockDriverState *bs, int fd,
+static int coroutine_fn
+paio_submit_co(BlockDriverState *bs, int fd,
int64_t offset, QEMUIOVector *qiov,
int bytes, int type)
{
@@ -1599,7 +1600,8 @@ static void raw_aio_unplug(BlockDriverState *bs)
#endif
}
-static BlockAIOCB *raw_aio_flush(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+raw_aio_flush(BlockDriverState *bs,
BlockCompletionFunc *cb, void *opaque)
{
BDRVRawState *s = bs->opaque;
@@ -1835,7 +1837,8 @@ static int64_t
raw_get_allocated_file_size(BlockDriverState *bs)
return (int64_t)st.st_blocks * 512;
}
-static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+raw_create(const char *filename, QemuOpts *opts, Error **errp)
{
int fd;
int result = 0;
@@ -2526,7 +2529,8 @@ hdev_open_Mac_error:
#if defined(__linux__)
-static BlockAIOCB *hdev_aio_ioctl(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+hdev_aio_ioctl(BlockDriverState *bs,
unsigned long int req, void *buf,
BlockCompletionFunc *cb, void *opaque)
{
@@ -2592,7 +2596,8 @@ static coroutine_fn int
hdev_co_pwrite_zeroes(BlockDriverState *bs,
return -ENOTSUP;
}
-static int hdev_create(const char *filename, QemuOpts *opts,
+static int coroutine_fn
+hdev_create(const char *filename, QemuOpts *opts,
Error **errp)
{
int fd;
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 17/35] curl: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/curl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/curl.c b/block/curl.c
index 2a244e2439..d3719dc086 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -855,7 +855,8 @@ out_noclean:
return -EINVAL;
}
-static void curl_setup_preadv(BlockDriverState *bs, CURLAIOCB *acb)
+static void coroutine_fn
+curl_setup_preadv(BlockDriverState *bs, CURLAIOCB *acb)
{
CURLState *state;
int running;
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 32/35] qed: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/qed.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/qed.c b/block/qed.c
index 385381a78a..dd2859a1c9 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -622,7 +622,8 @@ out:
return ret;
}
-static int bdrv_qed_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+bdrv_qed_create(const char *filename, QemuOpts *opts, Error **errp)
{
uint64_t image_size = 0;
uint32_t cluster_size = QED_DEFAULT_CLUSTER_SIZE;
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 25/35] mirror: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/mirror.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index 68744a17e8..2f0a9946d9 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -224,7 +224,8 @@ static int mirror_cow_align(MirrorBlockJob *s,
return ret;
}
-static inline void mirror_wait_for_io(MirrorBlockJob *s)
+static inline void coroutine_fn
+mirror_wait_for_io(MirrorBlockJob *s)
{
assert(!s->waiting_for_io);
s->waiting_for_io = true;
@@ -239,7 +240,8 @@ static inline void mirror_wait_for_io(MirrorBlockJob *s)
* (new_end - sector_num) if tail is rounded up or down due to
* alignment or buffer limit.
*/
-static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num,
+static int coroutine_fn
+mirror_do_read(MirrorBlockJob *s, int64_t sector_num,
int nb_sectors)
{
BlockBackend *source = s->common.blk;
@@ -490,7 +492,8 @@ static void mirror_free_init(MirrorBlockJob *s)
* mirror_resume() because mirror_run() will begin iterating again
* when the job is resumed.
*/
-static void mirror_wait_for_all_io(MirrorBlockJob *s)
+static void coroutine_fn
+mirror_wait_for_all_io(MirrorBlockJob *s)
{
while (s->in_flight > 0) {
mirror_wait_for_io(s);
@@ -605,7 +608,8 @@ static void mirror_exit(BlockJob *job, void *opaque)
bdrv_unref(src);
}
-static void mirror_throttle(MirrorBlockJob *s)
+static void coroutine_fn
+mirror_throttle(MirrorBlockJob *s)
{
int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
@@ -984,7 +988,8 @@ static void mirror_complete(BlockJob *job, Error **errp)
block_job_enter(&s->common);
}
-static void mirror_pause(BlockJob *job)
+static void coroutine_fn
+mirror_pause(BlockJob *job)
{
MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 26/35] iscsi: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/iscsi.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 54067e2620..e16311cb4a 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1005,7 +1005,8 @@ static void iscsi_ioctl_handle_emulated(IscsiAIOCB *acb,
int req, void *buf)
qemu_bh_schedule(acb->bh);
}
-static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
+static BlockAIOCB * coroutine_fn
+iscsi_aio_ioctl(BlockDriverState *bs,
unsigned long int req, void *buf,
BlockCompletionFunc *cb, void *opaque)
{
@@ -2107,7 +2108,8 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t
offset, Error **errp)
return 0;
}
-static int iscsi_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+iscsi_create(const char *filename, QemuOpts *opts, Error **errp)
{
int ret = 0;
int64_t total_size = 0;
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 34/35] vhdx: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/vhdx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/vhdx.c b/block/vhdx.c
index 8b270b57c9..56b54f3ed7 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1787,7 +1787,8 @@ exit:
*. ~ --- ~ ~ ~ ---.
* 1MB
*/
-static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
{
int ret = 0;
uint64_t image_size = (uint64_t) 2 * GiB;
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 22/35] sheepdog: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/sheepdog.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 83bc43dde4..64ff275db9 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -481,7 +481,8 @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s,
SheepdogAIOCB *acb,
return aio_req;
}
-static void wait_for_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB
*acb)
+static void coroutine_fn
+wait_for_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *acb)
{
SheepdogAIOCB *cb;
@@ -494,7 +495,8 @@ retry:
}
}
-static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
+static void coroutine_fn
+sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s,
QEMUIOVector *qiov, int64_t sector_num, int
nb_sectors,
int type)
{
@@ -1954,7 +1956,8 @@ static int parse_block_size_shift(BDRVSheepdogState *s,
QemuOpts *opt)
return 0;
}
-static int sd_create(const char *filename, QemuOpts *opts,
+static int coroutine_fn
+sd_create(const char *filename, QemuOpts *opts,
Error **errp)
{
Error *err = NULL;
@@ -2431,7 +2434,8 @@ static void coroutine_fn sd_co_rw_vector(SheepdogAIOCB
*acb)
}
}
-static void sd_aio_complete(SheepdogAIOCB *acb)
+static void coroutine_fn
+sd_aio_complete(SheepdogAIOCB *acb)
{
if (acb->aiocb_type == AIOCB_FLUSH_CACHE) {
return;
@@ -2905,7 +2909,8 @@ cleanup:
return ret;
}
-static int sd_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
+static int coroutine_fn
+sd_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
int64_t pos)
{
BDRVSheepdogState *s = bs->opaque;
@@ -2920,7 +2925,8 @@ static int sd_save_vmstate(BlockDriverState *bs,
QEMUIOVector *qiov,
return ret;
}
-static int sd_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
+static int coroutine_fn
+sd_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
int64_t pos)
{
BDRVSheepdogState *s = bs->opaque;
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 33/35] vdi: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/vdi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/vdi.c b/block/vdi.c
index 79af47763b..53cd7f64d8 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -716,7 +716,8 @@ vdi_co_pwritev(BlockDriverState *bs, uint64_t offset,
uint64_t bytes,
return ret;
}
-static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+vdi_create(const char *filename, QemuOpts *opts, Error **errp)
{
int ret = 0;
uint64_t bytes = 0;
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 31/35] parallels: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/parallels.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/parallels.c b/block/parallels.c
index 8be46a7d48..213e42b9d2 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -472,7 +472,8 @@ static int parallels_check(BlockDriverState *bs,
BdrvCheckResult *res,
}
-static int parallels_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+parallels_create(const char *filename, QemuOpts *opts, Error **errp)
{
int64_t total_size, cl_size;
uint8_t tmp[BDRV_SECTOR_SIZE];
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 29/35] block: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/blkdebug.c | 15 ++-
block/blkverify.c | 3 ++-
block/io.c| 9 ++---
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index a1b24b9b0d..d55e2e69c8 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -483,7 +483,8 @@ out:
return ret;
}
-static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes)
+static int coroutine_fn
+rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes)
{
BDRVBlkdebugState *s = bs->opaque;
BlkdebugRule *rule = NULL;
@@ -563,7 +564,8 @@ blkdebug_co_pwritev(BlockDriverState *bs, uint64_t offset,
uint64_t bytes,
return bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags);
}
-static int blkdebug_co_flush(BlockDriverState *bs)
+static int coroutine_fn
+blkdebug_co_flush(BlockDriverState *bs)
{
int err = rule_check(bs, 0, 0);
@@ -656,7 +658,8 @@ static void blkdebug_close(BlockDriverState *bs)
g_free(s->config_file);
}
-static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
+static void coroutine_fn
+suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
{
BDRVBlkdebugState *s = bs->opaque;
BlkdebugSuspendedReq r;
@@ -681,7 +684,8 @@ static void suspend_request(BlockDriverState *bs,
BlkdebugRule *rule)
g_free(r.tag);
}
-static bool process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
+static bool coroutine_fn
+process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
bool injected)
{
BDRVBlkdebugState *s = bs->opaque;
@@ -712,7 +716,8 @@ static bool process_rule(BlockDriverState *bs, struct
BlkdebugRule *rule,
return injected;
}
-static void blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event)
+static void coroutine_fn
+blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event)
{
BDRVBlkdebugState *s = bs->opaque;
struct BlkdebugRule *rule, *next;
diff --git a/block/blkverify.c b/block/blkverify.c
index 06369f9eac..d0c946173a 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -255,7 +255,8 @@ blkverify_co_pwritev(BlockDriverState *bs, uint64_t offset,
uint64_t bytes,
return blkverify_co_prwv(bs, &r, offset, bytes, qiov, qiov, flags, true);
}
-static int blkverify_co_flush(BlockDriverState *bs)
+static int coroutine_fn
+blkverify_co_flush(BlockDriverState *bs)
{
BDRVBlkverifyState *s = bs->opaque;
diff --git a/block/io.c b/block/io.c
index 14b88c8609..a53a86df3e 100644
--- a/block/io.c
+++ b/block/io.c
@@ -366,7 +366,8 @@ void bdrv_drain_all(void)
*
* This function should be called when a tracked request is completing.
*/
-static void tracked_request_end(BdrvTrackedRequest *req)
+static void coroutine_fn
+tracked_request_end(BdrvTrackedRequest *req)
{
if (req->serialising) {
atomic_dec(&req->bs->serialising_in_flight);
@@ -381,7 +382,8 @@ static void tracked_request_end(BdrvTrackedRequest *req)
/**
* Add an active request to the tracked requests list
*/
-static void tracked_request_begin(BdrvTrackedRequest *req,
+static void coroutine_fn
+tracked_request_begin(BdrvTrackedRequest *req,
BlockDriverState *bs,
int64_t offset,
unsigned int bytes,
@@ -2430,7 +2432,8 @@ int bdrv_pdiscard(BlockDriverState *bs, int64_t offset,
int bytes)
return rwco.ret;
}
-int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf)
+int coroutine_fn
+bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf)
{
BlockDriver *drv = bs->drv;
CoroutineIOCompletion co = {
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 35/35] vpc: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
block/vpc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/block/vpc.c b/block/vpc.c
index 4240ba9d1c..1b4aba20bd 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -872,7 +872,8 @@ static int create_fixed_disk(BlockBackend *blk, uint8_t
*buf,
return ret;
}
-static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
+static int coroutine_fn
+vpc_create(const char *filename, QemuOpts *opts, Error **errp)
{
uint8_t buf[1024];
VHDFooter *footer = (VHDFooter *) buf;
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH 30/35] block-backend: mark coroutine_fn
Signed-off-by: Marc-André Lureau
---
include/sysemu/block-backend.h | 4 ++--
block/block-backend.c | 36
2 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 1e05281fff..2f967037af 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -165,8 +165,8 @@ int blk_co_ioctl(BlockBackend *blk, unsigned long int req,
void *buf);
int blk_ioctl(BlockBackend *blk, unsigned long int req, void *buf);
BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned long int req, void *buf,
BlockCompletionFunc *cb, void *opaque);
-int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes);
-int blk_co_flush(BlockBackend *blk);
+int coroutine_fn blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes);
+int coroutine_fn blk_co_flush(BlockBackend *blk);
int blk_flush(BlockBackend *blk);
int blk_commit_all(void);
void blk_drain(BlockBackend *blk);
diff --git a/block/block-backend.c b/block/block-backend.c
index 56fc0a4d1e..a48aa4f900 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1032,7 +1032,8 @@ typedef struct BlkRwCo {
BdrvRequestFlags flags;
} BlkRwCo;
-static void blk_read_entry(void *opaque)
+static void coroutine_fn
+blk_read_entry(void *opaque)
{
BlkRwCo *rwco = opaque;
@@ -1040,7 +1041,8 @@ static void blk_read_entry(void *opaque)
rwco->qiov, rwco->flags);
}
-static void blk_write_entry(void *opaque)
+static void coroutine_fn
+blk_write_entry(void *opaque)
{
BlkRwCo *rwco = opaque;
@@ -1195,7 +1197,8 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk,
int64_t offset, int bytes,
return &acb->common;
}
-static void blk_aio_read_entry(void *opaque)
+static void coroutine_fn
+blk_aio_read_entry(void *opaque)
{
BlkAioEmAIOCB *acb = opaque;
BlkRwCo *rwco = &acb->rwco;
@@ -1206,7 +1209,8 @@ static void blk_aio_read_entry(void *opaque)
blk_aio_complete(acb);
}
-static void blk_aio_write_entry(void *opaque)
+static void coroutine_fn
+blk_aio_write_entry(void *opaque)
{
BlkAioEmAIOCB *acb = opaque;
BlkRwCo *rwco = &acb->rwco;
@@ -1288,7 +1292,8 @@ BlockAIOCB *blk_aio_pwritev(BlockBackend *blk, int64_t
offset,
blk_aio_write_entry, flags, cb, opaque);
}
-static void blk_aio_flush_entry(void *opaque)
+static void coroutine_fn
+blk_aio_flush_entry(void *opaque)
{
BlkAioEmAIOCB *acb = opaque;
BlkRwCo *rwco = &acb->rwco;
@@ -1303,7 +1308,8 @@ BlockAIOCB *blk_aio_flush(BlockBackend *blk,
return blk_aio_prwv(blk, 0, 0, NULL, blk_aio_flush_entry, 0, cb, opaque);
}
-static void blk_aio_pdiscard_entry(void *opaque)
+static void coroutine_fn
+blk_aio_pdiscard_entry(void *opaque)
{
BlkAioEmAIOCB *acb = opaque;
BlkRwCo *rwco = &acb->rwco;
@@ -1339,7 +1345,8 @@ int blk_co_ioctl(BlockBackend *blk, unsigned long int
req, void *buf)
return bdrv_co_ioctl(blk_bs(blk), req, buf);
}
-static void blk_ioctl_entry(void *opaque)
+static void coroutine_fn
+blk_ioctl_entry(void *opaque)
{
BlkRwCo *rwco = opaque;
rwco->ret = blk_co_ioctl(rwco->blk, rwco->offset,
@@ -1351,7 +1358,8 @@ int blk_ioctl(BlockBackend *blk, unsigned long int req,
void *buf)
return blk_prw(blk, req, buf, 0, blk_ioctl_entry, 0);
}
-static void blk_aio_ioctl_entry(void *opaque)
+static void coroutine_fn
+blk_aio_ioctl_entry(void *opaque)
{
BlkAioEmAIOCB *acb = opaque;
BlkRwCo *rwco = &acb->rwco;
@@ -1376,7 +1384,8 @@ BlockAIOCB *blk_aio_ioctl(BlockBackend *blk, unsigned
long int req, void *buf,
return blk_aio_prwv(blk, req, 0, &qiov, blk_aio_ioctl_entry, 0, cb,
opaque);
}
-int blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
+int coroutine_fn
+blk_co_pdiscard(BlockBackend *blk, int64_t offset, int bytes)
{
int ret = blk_check_byte_request(blk, offset, bytes);
if (ret < 0) {
@@ -1386,7 +1395,8 @@ int blk_co_pdiscard(BlockBackend *blk, int64_t offset,
int bytes)
return bdrv_co_pdiscard(blk_bs(blk), offset, bytes);
}
-int blk_co_flush(BlockBackend *blk)
+int coroutine_fn
+blk_co_flush(BlockBackend *blk)
{
if (!blk_is_available(blk)) {
return -ENOMEDIUM;
@@ -1395,7 +1405,8 @@ int blk_co_flush(BlockBackend *blk)
return bdrv_co_flush(blk_bs(blk));
}
-static void blk_flush_entry(void *opaque)
+static void coroutine_fn
+blk_flush_entry(void *opaque)
{
BlkRwCo *rwco = opaque;
rwco->ret = blk_co_flush(rwco->blk);
@@ -1785,7 +1796,8 @@ int blk_truncate(BlockBackend *blk, int64_t offset, Error
**errp)
return bdrv_truncate(blk->root, offset, errp);
}
-static void blk_pdiscard_entry(void *opaque)
+static void coroutine_fn
+blk_pdiscard_entry(void *opaque)
{
BlkRwCo *rwco = opaque;
rwco->ret = blk_co_pdiscard(rwco->blk, rwco->offset, rwco->qiov->size);
--
2.13.1.395.gf7b71de06
Re: [Qemu-block] [PATCH 0/2] block: make .bdrv_create() a coroutine_fn
- Original Message - > The BlockDriver->bdrv_create() function is always called from coroutine > context. These patches rename it and clean up qcow2 code that is currently > calling CoMutex functions outside coroutine_fn. > > Stefan Hajnoczi (2): > block: rename .bdrv_create() to .bdrv_co_create() > qcow2: make qcow2_co_create2() a coroutine_fn > I came to the same changes with my series, so Reviewed-by: Marc-André Lureau > include/block/block_int.h | 3 ++- > block.c | 4 ++-- > block/crypto.c| 8 > block/file-posix.c| 15 --- > block/file-win32.c| 3 ++- > block/gluster.c | 12 ++-- > block/iscsi.c | 7 --- > block/nfs.c | 5 +++-- > block/parallels.c | 6 -- > block/qcow.c | 5 +++-- > block/qcow2.c | 22 -- > block/qed.c | 6 -- > block/raw-format.c| 5 +++-- > block/rbd.c | 6 -- > block/sheepdog.c | 10 +- > block/ssh.c | 5 +++-- > block/vdi.c | 5 +++-- > block/vhdx.c | 5 +++-- > block/vmdk.c | 5 +++-- > block/vpc.c | 5 +++-- > 20 files changed, 81 insertions(+), 61 deletions(-) > > -- > 2.9.4 > >
Re: [Qemu-block] [Qemu-devel] [PATCH 08/35] block: all bdrv_aio callbacks are coroutine_fn
- Original Message -
> On 05/07/2017 00:03, Marc-André Lureau wrote:
> > Signed-off-by: Marc-André Lureau
> > ---
> > include/block/block_int.h | 10 +-
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/block/block_int.h b/include/block/block_int.h
> > index 15fa602150..93eb2a9528 100644
> > --- a/include/block/block_int.h
> > +++ b/include/block/block_int.h
> > @@ -133,15 +133,15 @@ struct BlockDriver {
> > void (*bdrv_refresh_filename)(BlockDriverState *bs, QDict *options);
> >
> > /* aio */
> > -BlockAIOCB *(*bdrv_aio_readv)(BlockDriverState *bs,
> > +BlockAIOCB * coroutine_fn (*bdrv_aio_readv)(BlockDriverState *bs,
> > int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
> > BlockCompletionFunc *cb, void *opaque);
> > -BlockAIOCB *(*bdrv_aio_writev)(BlockDriverState *bs,
> > +BlockAIOCB * coroutine_fn (*bdrv_aio_writev)(BlockDriverState *bs,
> > int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
> > BlockCompletionFunc *cb, void *opaque);
> > -BlockAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
> > +BlockAIOCB * coroutine_fn (*bdrv_aio_flush)(BlockDriverState *bs,
> > BlockCompletionFunc *cb, void *opaque);
> > -BlockAIOCB *(*bdrv_aio_pdiscard)(BlockDriverState *bs,
> > +BlockAIOCB * coroutine_fn (*bdrv_aio_pdiscard)(BlockDriverState *bs,
> > int64_t offset, int bytes,
> > BlockCompletionFunc *cb, void *opaque);
> >
> > @@ -247,7 +247,7 @@ struct BlockDriver {
> > void (*bdrv_lock_medium)(BlockDriverState *bs, bool locked);
> >
> > /* to control generic scsi devices */
> > -BlockAIOCB *(*bdrv_aio_ioctl)(BlockDriverState *bs,
> > +BlockAIOCB * coroutine_fn (*bdrv_aio_ioctl)(BlockDriverState *bs,
> > unsigned long int req, void *buf,
> > BlockCompletionFunc *cb, void *opaque);
> > int coroutine_fn (*bdrv_co_ioctl)(BlockDriverState *bs,
> >
>
>
> They are, but it's an implementation detail. Why is this patch necessary?
I didn't think this would be controversial :) well, the checks I added to clang
verify function pointer share the coroutine attribute.
The function themself are/need to be coroutine_fn (as they will call
coroutine_fn too)
Re: [Qemu-block] [Qemu-devel] [PATCH 08/35] block: all bdrv_aio callbacks are coroutine_fn
Hi - Original Message - > > > On 05/07/2017 16:21, Marc-André Lureau wrote: > >> > >> They are, but it's an implementation detail. Why is this patch necessary? > > I didn't think this would be controversial :) well, the checks I added to > > clang verify function pointer share the coroutine attribute. > > > > The function themself are/need to be coroutine_fn (as they will call > > coroutine_fn too) > > It's not controversial, I would not have expected the functions to call > coroutine_fn. :) How do they do that? > For example, null_co_readv() calls null_co_common() which calls co_aio_sleep_ns()
Re: [Qemu-block] [Qemu-devel] [PATCH 08/35] block: all bdrv_aio callbacks are coroutine_fn
Hi - Original Message - > > > On 05/07/2017 18:06, Marc-André Lureau wrote: > >>> coroutine_fn too) > >> It's not controversial, I would not have expected the functions to call > >> coroutine_fn. :) How do they do that? > >> > > For example, null_co_readv() calls null_co_common() which calls > > co_aio_sleep_ns() > > But these are bdrv_co_*, not bdrv_aio_*. Oops, right. Indeed, it's not needed, but to avoid coroutine annotation mismatch, we would have to remove a few: static coroutine_fn BlockAIOCB *raw_aio_pdiscard(BlockDriverState *bs, static coroutine_fn BlockAIOCB *hdev_aio_pdiscard(BlockDriverState *bs, Only those 2, it seems.
Re: [Qemu-block] [Qemu-devel] [PATCH 02/35] WIP: coroutine: manually tag the fast-paths
Hi
- Original Message -
> On Wed, Jul 05, 2017 at 12:03:13AM +0200, Marc-André Lureau wrote:
> > Some functions are both regular and coroutine. They may call coroutine
> > functions in some cases, if it is known to be running in a coroutine.
> >
> > Signed-off-by: Marc-André Lureau
> > ---
> > block.c | 2 ++
> > block/block-backend.c | 2 ++
> > block/io.c | 16 +++-
> > block/sheepdog.c| 2 ++
> > block/throttle-groups.c | 10 --
> > migration/rdma.c| 2 ++
> > 6 files changed, 31 insertions(+), 3 deletions(-)
> >
> > diff --git a/block.c b/block.c
> > index 694396281b..b08c006da4 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -443,7 +443,9 @@ int bdrv_create(BlockDriver *drv, const char* filename,
> >
> > if (qemu_in_coroutine()) {
> > /* Fast-path if already in coroutine context */
> > +co_role_acquire(_coroutine_fn);
> > bdrv_create_co_entry(&cco);
> > +co_role_release(_coroutine_fn);
> > } else {
> > co = qemu_coroutine_create(bdrv_create_co_entry, &cco);
> > qemu_coroutine_enter(co);
>
> I guess the clever analysis for clang would be to detect that if
> (qemu_in_coroutine()) means we have the _coroutine_fn role. It's
> similar to how Coverity sees an if (ptr) and knows whether the pointer
> is NULL/non-NULL in the branches.
>
> But this patch is okay too :-).
Right, I though about using try_acquire_capability, similarly needed for
try_lock etc. However, I don't see how to automatically release the capability
when going out of scope. Apparently there are some known limitations around
these patterns. I would love to hear from compilers folks what they think about
-Wthread-safety and if it can be added to gcc with various kind of improvements.
Re: [Qemu-block] [Qemu-devel] [PULL 30/34] virtio-bus: have callers tolerate new host notifier api
qbus), i, false);
> +if (rc == -ENOSYS) {
> +k->set_host_notifier(qbus->parent, i, false);
> +}
> }
> k->set_guest_notifiers(qbus->parent, vs->conf.num_queues + 2, false);
> fail_guest_notifiers:
> @@ -174,7 +181,7 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
> BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
> VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s);
> -int i;
> +int i, rc;
>
> if (!s->dataplane_started || s->dataplane_stopping) {
> return;
> @@ -198,7 +205,10 @@ void virtio_scsi_dataplane_stop(VirtIOSCSI *s)
> aio_context_release(s->ctx);
>
> for (i = 0; i < vs->conf.num_queues + 2; i++) {
> -k->set_host_notifier(qbus->parent, i, false);
> +rc = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
> +if (rc == -ENOSYS) {
> +k->set_host_notifier(qbus->parent, i, false);
> +}
> }
>
> /* Clean up guest notifier (irq) */
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 81cc5b0..bce1b6e 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1110,14 +1110,18 @@ int vhost_dev_enable_notifiers(struct vhost_dev
> *hdev, VirtIODevice *vdev)
> VirtioBusState *vbus = VIRTIO_BUS(qbus);
> VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
> int i, r, e;
> -if (!k->set_host_notifier) {
> +if (!k->set_host_notifier || !k->ioeventfd_started) {
> fprintf(stderr, "binding does not support host notifiers\n");
> r = -ENOSYS;
> goto fail;
> }
>
> for (i = 0; i < hdev->nvqs; ++i) {
> -r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);
> +r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index +
> i,
> + true);
> +if (r == -ENOSYS) {
> +r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, true);
> +}
> if (r < 0) {
> fprintf(stderr, "vhost VQ %d notifier binding failed: %d\n", i,
> -r);
> goto fail_vq;
> @@ -1127,7 +1131,11 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev,
> VirtIODevice *vdev)
> return 0;
> fail_vq:
> while (--i >= 0) {
> -e = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
> +e = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index +
> i,
> + false);
> +if (e == -ENOSYS) {
> +e = k->set_host_notifier(qbus->parent, hdev->vq_index + i,
> false);
> + }
> if (e < 0) {
> fprintf(stderr, "vhost VQ %d notifier cleanup error: %d\n", i,
> -r);
> fflush(stderr);
> @@ -1151,7 +1159,11 @@ void vhost_dev_disable_notifiers(struct vhost_dev
> *hdev, VirtIODevice *vdev)
> int i, r;
>
> for (i = 0; i < hdev->nvqs; ++i) {
> -r = k->set_host_notifier(qbus->parent, hdev->vq_index + i, false);
> +r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index +
> i,
> + false);
> +if (r == -ENOSYS) {
> +r = k->set_host_notifier(qbus->parent, hdev->vq_index + i,
> false);
> +}
> if (r < 0) {
> fprintf(stderr, "vhost VQ %d notifier cleanup failed: %d\n", i,
> -r);
> fflush(stderr);
> --
> MST
>
>
--
Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [PATCH v4 1/8] nbd/client: fix drop_sync [CVE-2017-2630]
On Tue, Feb 21, 2017 at 6:49 AM Eric Blake wrote:
> From: Vladimir Sementsov-Ogievskiy
>
> Comparison symbol is misused. It may lead to memory corruption.
> Introduced in commit 7d3123e.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> Message-Id: <20170203154757.36140-6-vsement...@virtuozzo.com>
> [eblake: add CVE details]
> Signed-off-by: Eric Blake
>
Reviewed-by: Marc-André Lureau
> ---
> nbd/client.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/nbd/client.c b/nbd/client.c
> index ffb0743..0d16cd1 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -94,7 +94,7 @@ static ssize_t drop_sync(QIOChannel *ioc, size_t size)
> char small[1024];
> char *buffer;
>
> -buffer = sizeof(small) < size ? small : g_malloc(MIN(65536, size));
> +buffer = sizeof(small) > size ? small : g_malloc(MIN(65536, size));
> while (size > 0) {
> ssize_t count = read_sync(ioc, buffer, MIN(65536, size));
>
> --
> 2.9.3
>
>
> --
Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [PATCH v4 2/8] nbd: Create struct for tracking export info
On Tue, Feb 21, 2017 at 6:49 AM Eric Blake wrote:
> The NBD Protocol is introducing some additional information
> about exports, such as minimum request size and alignment, as
> well as an advertised maximum request size. It will be easier
> to feed this information back to the block layer if we gather
> all the information into a struct, rather than adding yet more
> pointer parameters during negotiation.
>
> Signed-off-by: Eric Blake
>
Reviewed-by: Marc-André Lureau
>
> ---
> v4: rebase to master
> v3: new patch
> ---
> block/nbd-client.h | 3 +--
> include/block/nbd.h | 15 +++
> block/nbd-client.c | 18 --
> block/nbd.c | 2 +-
> nbd/client.c| 47 +--
> qemu-nbd.c | 10 --
> 6 files changed, 50 insertions(+), 45 deletions(-)
>
> diff --git a/block/nbd-client.h b/block/nbd-client.h
> index f8d6006..098b65c 100644
> --- a/block/nbd-client.h
> +++ b/block/nbd-client.h
> @@ -20,8 +20,7 @@
> typedef struct NBDClientSession {
> QIOChannelSocket *sioc; /* The master data channel */
> QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS)
> */
> -uint16_t nbdflags;
> -off_t size;
> +NBDExportInfo info;
>
> CoMutex send_mutex;
> CoQueue free_sema;
> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index 3e373f0..8cc9cbe 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -123,16 +123,23 @@ enum {
> * aren't overflowing some other buffer. */
> #define NBD_MAX_NAME_SIZE 256
>
> +/* Details collected by NBD_OPT_EXPORT_NAME and NBD_OPT_GO */
> +struct NBDExportInfo {
> +uint64_t size;
> +uint16_t flags;
> +};
> +typedef struct NBDExportInfo NBDExportInfo;
> +
> ssize_t nbd_wr_syncv(QIOChannel *ioc,
> struct iovec *iov,
> size_t niov,
> size_t length,
> bool do_read);
> -int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t
> *flags,
> +int nbd_receive_negotiate(QIOChannel *ioc, const char *name,
>QCryptoTLSCreds *tlscreds, const char *hostname,
> - QIOChannel **outioc,
> - off_t *size, Error **errp);
> -int nbd_init(int fd, QIOChannelSocket *sioc, uint16_t flags, off_t size);
> + QIOChannel **outioc, NBDExportInfo *info,
> + Error **errp);
> +int nbd_init(int fd, QIOChannelSocket *sioc, NBDExportInfo *info);
> ssize_t nbd_send_request(QIOChannel *ioc, NBDRequest *request);
> ssize_t nbd_receive_reply(QIOChannel *ioc, NBDReply *reply);
> int nbd_client(int fd);
> diff --git a/block/nbd-client.c b/block/nbd-client.c
> index 06f1532..32d7c90 100644
> --- a/block/nbd-client.c
> +++ b/block/nbd-client.c
> @@ -258,7 +258,7 @@ int nbd_client_co_pwritev(BlockDriverState *bs,
> uint64_t offset,
> ssize_t ret;
>
> if (flags & BDRV_REQ_FUA) {
> -assert(client->nbdflags & NBD_FLAG_SEND_FUA);
> +assert(client->info.flags & NBD_FLAG_SEND_FUA);
> request.flags |= NBD_CMD_FLAG_FUA;
> }
>
> @@ -287,12 +287,12 @@ int nbd_client_co_pwrite_zeroes(BlockDriverState
> *bs, int64_t offset,
> };
> NBDReply reply;
>
> -if (!(client->nbdflags & NBD_FLAG_SEND_WRITE_ZEROES)) {
> +if (!(client->info.flags & NBD_FLAG_SEND_WRITE_ZEROES)) {
> return -ENOTSUP;
> }
>
> if (flags & BDRV_REQ_FUA) {
> -assert(client->nbdflags & NBD_FLAG_SEND_FUA);
> +assert(client->info.flags & NBD_FLAG_SEND_FUA);
> request.flags |= NBD_CMD_FLAG_FUA;
> }
> if (!(flags & BDRV_REQ_MAY_UNMAP)) {
> @@ -317,7 +317,7 @@ int nbd_client_co_flush(BlockDriverState *bs)
> NBDReply reply;
> ssize_t ret;
>
> -if (!(client->nbdflags & NBD_FLAG_SEND_FLUSH)) {
> +if (!(client->info.flags & NBD_FLAG_SEND_FLUSH)) {
> return 0;
> }
>
> @@ -346,7 +346,7 @@ int nbd_client_co_pdiscard(BlockDriverState *bs,
> int64_t offset, int count)
> NBDReply reply;
> ssize_t ret;
>
> -if (!(client->nbdflags & NBD_FLAG_SEND_TRIM)) {
> +if (!(client->info.flags & NBD_FLAG_SEND_TRIM)) {
> return 0;
> }
>
> @@ -405,19 +405,17 @@ int nbd_client_init(BlockDriverState *bs,
> qio_channel_set_blocking(QIO_CHANNEL(sioc), true, NULL);
>
> ret = nbd_receive_negotiate(QIO_CHANNEL(sioc), export,
> -&clien
Re: [Qemu-block] [Qemu-devel] [PATCH v4 3/8] block: Add blk_get_opt_transfer()
On Tue, Feb 21, 2017 at 6:45 AM Eric Blake wrote:
> The NBD protocol would like to advertise the optimal I/O
> size to the client; but it would be a layering violation to
> peek into blk_bs(blk)->bl, when we only have a BB.
>
> This copies the existing blk_get_max_transfer() in reading
> a value from the top BDS; where that value was picked via
> bdrv_refresh_limits() to reflect the overall constraints of
> the entire BDS chain.
>
> Signed-off-by: Eric Blake
>
Reviewed-by: Marc-André Lureau
>
> ---
> v4: retitle, as part of rebasing to byte limits
> v3: new patch
> ---
> include/sysemu/block-backend.h | 1 +
> block/block-backend.c | 12
> 2 files changed, 13 insertions(+)
>
> diff --git a/include/sysemu/block-backend.h
> b/include/sysemu/block-backend.h
> index 6444e41..882480a 100644
> --- a/include/sysemu/block-backend.h
> +++ b/include/sysemu/block-backend.h
> @@ -174,6 +174,7 @@ void blk_lock_medium(BlockBackend *blk, bool locked);
> void blk_eject(BlockBackend *blk, bool eject_flag);
> int blk_get_flags(BlockBackend *blk);
> uint32_t blk_get_max_transfer(BlockBackend *blk);
> +uint32_t blk_get_opt_transfer(BlockBackend *blk);
> int blk_get_max_iov(BlockBackend *blk);
> void blk_set_guest_block_size(BlockBackend *blk, int align);
> void *blk_try_blockalign(BlockBackend *blk, size_t size);
> diff --git a/block/block-backend.c b/block/block-backend.c
> index efbf398..4d91ff8 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -1426,6 +1426,18 @@ uint32_t blk_get_max_transfer(BlockBackend *blk)
> return MIN_NON_ZERO(max, INT_MAX);
> }
>
> +/* Returns the optimum transfer length, in bytes; may be 0 if no optimum
> */
> +uint32_t blk_get_opt_transfer(BlockBackend *blk)
> +{
> +BlockDriverState *bs = blk_bs(blk);
> +
> +if (bs) {
> +return bs->bl.opt_transfer;
> +} else {
> +return 0;
> +}
> +}
> +
> int blk_get_max_iov(BlockBackend *blk)
> {
> return blk->root->bs->bl.max_iov;
> --
> 2.9.3
>
>
> --
Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [PATCH v4 4/8] nbd: Expose and debug more NBD constants
On Tue, Feb 21, 2017 at 6:54 AM Eric Blake wrote:
> The NBD protocol has several constants defined in various extensions
> that we are about to implement. Expose them to the code, along with
> an easy way to map various constants to strings during diagnostic
> messages.
>
> Doing this points out a debug message in server.c that got
> parameters mixed up.
>
> Signed-off-by: Eric Blake
>
Reviewed-by: Marc-André Lureau
>
> ---
> v4: new patch
> ---
> include/block/nbd.h | 34 +++---
> nbd/nbd-internal.h | 9 +++
> nbd/client.c| 56 ---
> nbd/common.c| 69
> +
> nbd/server.c| 17 +++--
> 5 files changed, 145 insertions(+), 40 deletions(-)
>
> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index 8cc9cbe..c84e022 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (C) 2016 Red Hat, Inc.
> + * Copyright (C) 2016-2017 Red Hat, Inc.
> * Copyright (C) 2005 Anthony Liguori
> *
> * Network Block Device
> @@ -83,18 +83,36 @@ typedef struct NBDReply NBDReply;
> #define NBD_FLAG_C_FIXED_NEWSTYLE (1 << 0) /* Fixed newstyle protocol. */
> #define NBD_FLAG_C_NO_ZEROES (1 << 1) /* End handshake without
> zeroes. */
>
> -/* Reply types. */
> +/* Option requests. */
> +#define NBD_OPT_EXPORT_NAME (1)
> +#define NBD_OPT_ABORT (2)
> +#define NBD_OPT_LIST(3)
> +/* #define NBD_OPT_PEEK_EXPORT (4) not in use */
> +#define NBD_OPT_STARTTLS(5)
> +#define NBD_OPT_INFO(6)
> +#define NBD_OPT_GO (7)
> +
> +/* Option reply types. */
> #define NBD_REP_ERR(value) ((UINT32_C(1) << 31) | (value))
>
> #define NBD_REP_ACK (1) /* Data sending finished.
> */
> #define NBD_REP_SERVER (2) /* Export description. */
> +#define NBD_REP_INFO(3) /* NBD_OPT_INFO/GO. */
>
> -#define NBD_REP_ERR_UNSUP NBD_REP_ERR(1) /* Unknown option */
> -#define NBD_REP_ERR_POLICY NBD_REP_ERR(2) /* Server denied */
> -#define NBD_REP_ERR_INVALID NBD_REP_ERR(3) /* Invalid length */
> -#define NBD_REP_ERR_PLATFORMNBD_REP_ERR(4) /* Not compiled in */
> -#define NBD_REP_ERR_TLS_REQDNBD_REP_ERR(5) /* TLS required */
> -#define NBD_REP_ERR_SHUTDOWNNBD_REP_ERR(7) /* Server shutting down */
> +#define NBD_REP_ERR_UNSUP NBD_REP_ERR(1) /* Unknown option */
> +#define NBD_REP_ERR_POLICY NBD_REP_ERR(2) /* Server denied */
> +#define NBD_REP_ERR_INVALID NBD_REP_ERR(3) /* Invalid length */
> +#define NBD_REP_ERR_PLATFORMNBD_REP_ERR(4) /* Not compiled in */
> +#define NBD_REP_ERR_TLS_REQDNBD_REP_ERR(5) /* TLS required */
> +#define NBD_REP_ERR_UNKNOWN NBD_REP_ERR(6) /* Export unknown */
> +#define NBD_REP_ERR_SHUTDOWNNBD_REP_ERR(7) /* Server shutting
> down */
> +#define NBD_REP_ERR_BLOCK_SIZE_REQD NBD_REP_ERR(8) /* Need
> INFO_BLOCK_SIZE */
> +
> +/* Info types, used during NBD_REP_INFO */
> +#define NBD_INFO_EXPORT 0
> +#define NBD_INFO_NAME 1
> +#define NBD_INFO_DESCRIPTION2
> +#define NBD_INFO_BLOCK_SIZE 3
>
> /* Request flags, sent from client to server during transmission phase */
> #define NBD_CMD_FLAG_FUA(1 << 0) /* 'force unit access' during
> write */
> diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
> index f43d990..aa5b2fd 100644
> --- a/nbd/nbd-internal.h
> +++ b/nbd/nbd-internal.h
> @@ -76,12 +76,6 @@
> #define NBD_SET_TIMEOUT _IO(0xab, 9)
> #define NBD_SET_FLAGS _IO(0xab, 10)
>
> -#define NBD_OPT_EXPORT_NAME (1)
> -#define NBD_OPT_ABORT (2)
> -#define NBD_OPT_LIST(3)
> -#define NBD_OPT_PEEK_EXPORT (4)
> -#define NBD_OPT_STARTTLS(5)
> -
> /* NBD errors are based on errno numbers, so there is a 1:1 mapping,
> * but only a limited set of errno values is specified in the protocol.
> * Everything else is squashed to EINVAL.
> @@ -122,5 +116,8 @@ struct NBDTLSHandshakeData {
>
> void nbd_tls_handshake(QIOTask *task,
> void *opaque);
> +const char *nbd_opt_lookup(uint32_t opt);
> +const char *nbd_rep_lookup(uint32_t rep);
> +const char *nbd_info_lookup(uint16_t info);
>
> #endif
> diff --git a/nbd/client.c b/nbd/client.c
> index 69f0e09..f96539b 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (C) 2016 Red Hat, Inc.
> + * Copyright (C) 2016-2017 Red Hat, Inc.
>
Re: [Qemu-block] [for-2.9 2/8] char: Fix socket with "type": "vsock" address
- Original Message -
>
> Hi
>
> - Original Message -
> > Watch this:
> >
> > $ qemu-system-x86_64 -nodefaults -S -display none -qmp stdio
> > {"QMP": {"version": {"qemu": {"micro": 91, "minor": 8, "major": 2},
> > "package": " (v2.8.0-1195-gf84141e-dirty)"}, "capabilities": []}}
> > { "execute": "qmp_capabilities" }
> > {"return": {}}
> > { "execute": "chardev-add", "arguments": { "id": "chr0", "backend": {
> > "type": "socket", "data": { "addr": { "type": "vsock", "data": { "cid":
> > "CID", "port": "P" }}
> > Aborted (core dumped)
> >
> > Crashes because SocketAddress_to_str() is blissfully unaware of
> > SOCKET_ADDRESS_KIND_VSOCK. Fix that. Pick the output format to match
> > socket_parse(), just like the existing formats.
> >
> > Cc: Stefan Hajnoczi
> > Cc: Paolo Bonzini
> > Cc: Marc-André Lureau
> > Signed-off-by: Markus Armbruster
>
> Reviewed-by: Marc-André Lureau
>
> > ---
> > chardev/char-socket.c | 4
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> > index 6344b07..36ab0d6 100644
> > --- a/chardev/char-socket.c
> > +++ b/chardev/char-socket.c
> > @@ -357,6 +357,10 @@ static char *SocketAddress_to_str(const char *prefix,
> > SocketAddress *addr,
> > return g_strdup_printf("%sfd:%s%s", prefix, addr->u.fd.data->str,
> > is_listen ? ",server" : "");
> > break;
> > +case SOCKET_ADDRESS_KIND_VSOCK:
> > +return g_strdup_printf("%svsock:%s:%s", prefix,
> > + addr->u.vsock.data->cid,
> > + addr->u.vsock.data->port);
> > default:
> > abort();
>
> ooch.. may I suggest we don't abort() here? g_return_val_if_fail() perhaps a
> more judicious choice?
-> g_return_if_reached()
>
Re: [Qemu-block] [for-2.9 2/8] char: Fix socket with "type": "vsock" address
Hi
- Original Message -
> Watch this:
>
> $ qemu-system-x86_64 -nodefaults -S -display none -qmp stdio
> {"QMP": {"version": {"qemu": {"micro": 91, "minor": 8, "major": 2},
> "package": " (v2.8.0-1195-gf84141e-dirty)"}, "capabilities": []}}
> { "execute": "qmp_capabilities" }
> {"return": {}}
> { "execute": "chardev-add", "arguments": { "id": "chr0", "backend": {
> "type": "socket", "data": { "addr": { "type": "vsock", "data": { "cid":
> "CID", "port": "P" }}
> Aborted (core dumped)
>
> Crashes because SocketAddress_to_str() is blissfully unaware of
> SOCKET_ADDRESS_KIND_VSOCK. Fix that. Pick the output format to match
> socket_parse(), just like the existing formats.
>
> Cc: Stefan Hajnoczi
> Cc: Paolo Bonzini
> Cc: Marc-André Lureau
> Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
> ---
> chardev/char-socket.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 6344b07..36ab0d6 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -357,6 +357,10 @@ static char *SocketAddress_to_str(const char *prefix,
> SocketAddress *addr,
> return g_strdup_printf("%sfd:%s%s", prefix, addr->u.fd.data->str,
> is_listen ? ",server" : "");
> break;
> +case SOCKET_ADDRESS_KIND_VSOCK:
> +return g_strdup_printf("%svsock:%s:%s", prefix,
> + addr->u.vsock.data->cid,
> + addr->u.vsock.data->port);
> default:
> abort();
ooch.. may I suggest we don't abort() here? g_return_val_if_fail() perhaps a
more judicious choice?
Re: [Qemu-block] [Qemu-devel] [RFC for-3.0 0/4] block: Add qcow2-rust block driver
woot! Happy birthday! On Tue, Apr 18, 2017 at 7:58 PM Max Reitz wrote: > The issues of using C are well understood and nobody likes it. Let's use > a better language. C++ is not a better language, Rust is. Everybody > loves Rust. Rust is good. Rust is hip. It will attract developers, it > will improve code quality, it will improve performance, it will even > improve your marriage. Rust is the future and the future is now. > > As the block layer, let's show our commitment to the future by replacing > one of our core parts, the the LEGACY (Bah! Yuck! Ugh!) qcow2 driver, by > a shiny (Oooh! Aaah!) Rust driver. Much better. My VMs now run thrice as > fast. Promise. > > > Max Reitz (4): > block: Add Rust interface > block/qcow2-rust: Add qcow2-rust block driver > block/qcow2-rust: Add partial write support > block/qcow2-rust: Register block driver > > configure |2 + > Makefile |6 +- > Makefile.target|2 +- > block/Makefile.objs|1 + > block/qcow2-rust.c | 38 ++ > block/rust/Cargo.toml | 10 + > block/rust/src/interface/c_constants.rs|8 + > block/rust/src/interface/c_functions.rs| 37 + > block/rust/src/interface/c_structs.rs | 587 > block/rust/src/interface/mod.rs| 1012 > > block/rust/src/lib.rs | 12 + > block/rust/src/qcow2/allocation.rs | 162 + > block/rust/src/qcow2/io.rs | 439 > block/rust/src/qcow2/mod.rs| 347 ++ > block/rust/src/qcow2/on_disk_structures.rs | 59 ++ > block/rust/src/qcow2/refcount.rs | 96 +++ > 16 files changed, 2816 insertions(+), 2 deletions(-) > create mode 100644 block/qcow2-rust.c > create mode 100644 block/rust/Cargo.toml > create mode 100644 block/rust/src/interface/c_constants.rs > create mode 100644 block/rust/src/interface/c_functions.rs > create mode 100644 block/rust/src/interface/c_structs.rs > create mode 100644 block/rust/src/interface/mod.rs > create mode 100644 block/rust/src/lib.rs > create mode 100644 block/rust/src/qcow2/allocation.rs > create mode 100644 block/rust/src/qcow2/io.rs > create mode 100644 block/rust/src/qcow2/mod.rs > create mode 100644 block/rust/src/qcow2/on_disk_structures.rs > create mode 100644 block/rust/src/qcow2/refcount.rs > > --- > > Yes, I'm late. I thought somebody else would do something but nobody > did. It's still April, though, so I think I'm all good. Also, it's my > birthday today so you're not allowed to complain! > > I could have waited until next year, but I'm afraid it would no longer > have been funny by then because we might truly have Rust code in master > then... > > -- > 2.12.2 > > > -- Marc-André Lureau
[Qemu-block] [PATCH v2 03/29] vhdx: use QEMU_ALIGN_DOWN
I used the clang-tidy qemu-round check to generate the fix: https://github.com/elmarco/clang-tools-extra Signed-off-by: Marc-André Lureau --- block/vhdx-log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/vhdx-log.c b/block/vhdx-log.c index 01278f3fc9..ad70706b99 100644 --- a/block/vhdx-log.c +++ b/block/vhdx-log.c @@ -884,7 +884,7 @@ static int vhdx_log_write(BlockDriverState *bs, BDRVVHDXState *s, } sector_offset = offset % VHDX_LOG_SECTOR_SIZE; -file_offset = (offset / VHDX_LOG_SECTOR_SIZE) * VHDX_LOG_SECTOR_SIZE; +file_offset = QEMU_ALIGN_DOWN(offset, VHDX_LOG_SECTOR_SIZE); aligned_length = length; -- 2.13.1.395.gf7b71de06
[Qemu-block] [PATCH v2 07/29] dmg: use DIV_ROUND_UP
I used the clang-tidy qemu-round check to generate the fix: https://github.com/elmarco/clang-tools-extra Signed-off-by: Marc-André Lureau Reviewed-by: Stefan Hajnoczi --- block/dmg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/dmg.c b/block/dmg.c index 900ae5a678..6c0711f563 100644 --- a/block/dmg.c +++ b/block/dmg.c @@ -111,7 +111,7 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk, uncompressed_sectors = s->sectorcounts[chunk]; break; case 1: /* copy */ -uncompressed_sectors = (s->lengths[chunk] + 511) / 512; +uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512); break; case 2: /* zero */ /* as the all-zeroes block may be large, it is treated specially: the -- 2.13.1.395.gf7b71de06
[Qemu-block] [PATCH v2 09/29] vpc: use DIV_ROUND_UP
I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra
Signed-off-by: Marc-André Lureau
---
block/vpc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/vpc.c b/block/vpc.c
index 9a6f8173a5..c88dc72491 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -760,7 +760,7 @@ static int calculate_geometry(int64_t total_sectors,
uint16_t* cyls,
} else {
*secs_per_cyl = 17;
cyls_times_heads = total_sectors / *secs_per_cyl;
-*heads = (cyls_times_heads + 1023) / 1024;
+*heads = DIV_ROUND_UP(cyls_times_heads, 1024);
if (*heads < 4) {
*heads = 4;
@@ -813,7 +813,7 @@ static int create_dynamic_disk(BlockBackend *blk, uint8_t
*buf,
offset = 3 * 512;
memset(buf, 0xFF, 512);
-for (i = 0; i < (num_bat_entries * 4 + 511) / 512; i++) {
+for (i = 0; i < DIV_ROUND_UP(num_bat_entries * 4, 512); i++) {
ret = blk_pwrite(blk, offset, buf, 512, 0);
if (ret < 0) {
goto fail;
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH v2 10/29] vvfat: use DIV_ROUND_UP
I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra
Signed-off-by: Marc-André Lureau
---
block/vvfat.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index 4fd28e1e87..c08c4fb525 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -437,7 +437,7 @@ static direntry_t *create_long_filename(BDRVVVFATState *s,
const char *filename)
return NULL;
}
-number_of_entries = (length * 2 + 25) / 26;
+number_of_entries = DIV_ROUND_UP(length * 2, 26);
for(i=0;idirectory));
@@ -2520,7 +2520,7 @@ static int commit_one_file(BDRVVVFATState* s,
(size > offset && c >=2 && !fat_eof(s, c)));
ret = vvfat_read(s->bs, cluster2sector(s, c),
-(uint8_t*)cluster, (rest_size + 0x1ff) / 0x200);
+(uint8_t*)cluster, DIV_ROUND_UP(rest_size, 0x200));
if (ret < 0) {
qemu_close(fd);
--
2.13.1.395.gf7b71de06
[Qemu-block] [PATCH v2 08/29] qcow2: use DIV_ROUND_UP
I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra
Signed-off-by: Marc-André Lureau
---
block/qcow2-cluster.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index f06c08f64c..30db942dde 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -61,7 +61,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t
min_size,
new_l1_size = 1;
}
while (min_size > new_l1_size) {
-new_l1_size = (new_l1_size * 3 + 1) / 2;
+new_l1_size = DIV_ROUND_UP(new_l1_size * 3, 2);
}
}
--
2.13.1.395.gf7b71de06
Re: [Qemu-block] [Qemu-devel] [PATCH 27/29] tests/hbitmap: use ARRAY_SIZE macro
On Mon, Jul 17, 2017 at 11:10 PM, Philippe Mathieu-Daudé
wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> tests/test-hbitmap.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/test-hbitmap.c b/tests/test-hbitmap.c
> index 1acb353889..628e72122b 100644
> --- a/tests/test-hbitmap.c
> +++ b/tests/test-hbitmap.c
> @@ -813,7 +813,7 @@ static void test_hbitmap_serialize_basic(TestHBitmapData
> *data,
> size_t buf_size;
> uint8_t *buf;
> uint64_t positions[] = { 0, 1, L1 - 1, L1, L2 - 1, L2, L2 + 1, L3 - 1 };
> -int num_positions = sizeof(positions) / sizeof(positions[0]);
> +int num_positions = ARRAY_SIZE(positions);
>
> hbitmap_test_init(data, L3, 0);
> g_assert(hbitmap_is_serializable(data->hb));
> @@ -838,7 +838,7 @@ static void test_hbitmap_serialize_part(TestHBitmapData
> *data,
> size_t buf_size;
> uint8_t *buf;
> uint64_t positions[] = { 0, 1, L1 - 1, L1, L2 - 1, L2, L2 + 1, L3 - 1 };
> -int num_positions = sizeof(positions) / sizeof(positions[0]);
> +int num_positions = ARRAY_SIZE(positions);
>
> hbitmap_test_init(data, L3, 0);
> buf_size = L2;
> @@ -880,7 +880,7 @@ static void test_hbitmap_serialize_zeroes(TestHBitmapData
> *data,
> int64_t next;
> uint64_t min_l1 = MAX(L1, 64);
> uint64_t positions[] = { 0, min_l1, L2, L3 - min_l1};
> -int num_positions = sizeof(positions) / sizeof(positions[0]);
> + int num_positions = ARRAY_SIZE(positions);
>
> hbitmap_test_init(data, L3, 0);
>
> --
> 2.13.2
>
>
--
Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [PATCH 23/29] async: use ARRAY_SIZE macro
On Mon, Jul 17, 2017 at 11:09 PM, Philippe Mathieu-Daudé
wrote:
> Applied using the Coccinelle semantic patch scripts/coccinelle/use_osdep.cocci
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> util/aio-posix.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/util/aio-posix.c b/util/aio-posix.c
> index 2d51239ec6..e89fd7024a 100644
> --- a/util/aio-posix.c
> +++ b/util/aio-posix.c
> @@ -119,7 +119,7 @@ static int aio_epoll(AioContext *ctx, GPollFD *pfds,
> }
> if (timeout <= 0 || ret > 0) {
> ret = epoll_wait(ctx->epollfd, events,
> - sizeof(events) / sizeof(events[0]),
> + ARRAY_SIZE(events),
> timeout);
> if (ret <= 0) {
> goto out;
> --
> 2.13.2
>
>
--
Marc-André Lureau
[Qemu-block] [PATCH 02/26] qobject: replace dump_qobject() by qobject_to_string()
The dump functions is generally useful for any qobject user, for
testing, debugging etc.
The callback-based output is replaced by string allocation. Trading
efficiency for ease-of-use is okay here.
Signed-off-by: Marc-André Lureau
---
include/qapi/qmp/qdict.h | 2 ++
include/qapi/qmp/qlist.h | 2 ++
include/qapi/qmp/qobject.h | 7
block/qapi.c | 90 +++---
qobject/qdict.c| 30
qobject/qlist.c| 23
qobject/qobject.c | 21 +++
tests/check-qjson.c| 19 ++
8 files changed, 108 insertions(+), 86 deletions(-)
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 363e431106..c9c4038435 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -86,4 +86,6 @@ QObject *qdict_crumple(const QDict *src, Error **errp);
void qdict_join(QDict *dest, QDict *src, bool overwrite);
+char *qdict_to_string(QDict *dict, int indent);
+
#endif /* QDICT_H */
diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index c4b5fdad9b..c93ac3e15b 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -60,6 +60,8 @@ size_t qlist_size(const QList *qlist);
QList *qobject_to_qlist(const QObject *obj);
void qlist_destroy_obj(QObject *obj);
+char *qlist_to_string(QList *list, int indent);
+
static inline const QListEntry *qlist_first(const QList *qlist)
{
return QTAILQ_FIRST(&qlist->head);
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index eab29edd12..3365eb73c9 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -105,4 +105,11 @@ static inline QNull *qnull(void)
return &qnull_;
}
+char *qobject_to_string_indent(QObject *obj, int indent);
+
+static inline char *qobject_to_string(QObject *obj)
+{
+return qobject_to_string_indent(obj, 0);
+}
+
#endif /* QOBJECT_H */
diff --git a/block/qapi.c b/block/qapi.c
index d2b18ee9df..08b5a666d1 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -623,101 +623,19 @@ void bdrv_snapshot_dump(fprintf_function func_fprintf,
void *f,
}
}
-static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
- QDict *dict);
-static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
- QList *list);
-
-static void dump_qobject(fprintf_function func_fprintf, void *f,
- int comp_indent, QObject *obj)
-{
-switch (qobject_type(obj)) {
-case QTYPE_QNUM: {
-QNum *value = qobject_to_qnum(obj);
-char *tmp = qnum_to_string(value);
-func_fprintf(f, "%s", tmp);
-g_free(tmp);
-break;
-}
-case QTYPE_QSTRING: {
-QString *value = qobject_to_qstring(obj);
-func_fprintf(f, "%s", qstring_get_str(value));
-break;
-}
-case QTYPE_QDICT: {
-QDict *value = qobject_to_qdict(obj);
-dump_qdict(func_fprintf, f, comp_indent, value);
-break;
-}
-case QTYPE_QLIST: {
-QList *value = qobject_to_qlist(obj);
-dump_qlist(func_fprintf, f, comp_indent, value);
-break;
-}
-case QTYPE_QBOOL: {
-QBool *value = qobject_to_qbool(obj);
-func_fprintf(f, "%s", qbool_get_bool(value) ? "true" : "false");
-break;
-}
-default:
-abort();
-}
-}
-
-static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
- QList *list)
-{
-const QListEntry *entry;
-int i = 0;
-
-for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
-QType type = qobject_type(entry->value);
-bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
-func_fprintf(f, "%*s[%i]:%c", indentation * 4, "", i,
- composite ? '\n' : ' ');
-dump_qobject(func_fprintf, f, indentation + 1, entry->value);
-if (!composite) {
-func_fprintf(f, "\n");
-}
-}
-}
-
-static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
- QDict *dict)
-{
-const QDictEntry *entry;
-
-for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry)) {
-QType type = qobject_type(entry->value);
-bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
-char *key = g_malloc(strlen(entry->key) + 1);
-int i;
-
-/* replace dashes with spaces in key (variable) names */
-for (i = 0; entry->key[i]; i++) {
-key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
-}
-key[i] = 0;
-func_fprin
[Qemu-block] [PATCH v2] iscsi: use scsi_create_task()
The function does the same initialization, and matches with
scsi_free_scsi_task() usage, and qemu doesn't need to know the
allocator details.
Signed-off-by: Marc-André Lureau
---
block/iscsi.c | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
v2:
- set cdb_size if API_VERSION < 20150510
diff --git a/block/iscsi.c b/block/iscsi.c
index d557c99668..9449c90631 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1013,6 +1013,7 @@ static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
struct iscsi_context *iscsi = iscsilun->iscsi;
struct iscsi_data data;
IscsiAIOCB *acb;
+int xfer_dir;
acb = qemu_aio_get(&iscsi_aiocb_info, bs, cb, opaque);
@@ -1034,31 +1035,30 @@ static BlockAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
return NULL;
}
-acb->task = malloc(sizeof(struct scsi_task));
-if (acb->task == NULL) {
-error_report("iSCSI: Failed to allocate task for scsi command. %s",
- iscsi_get_error(iscsi));
-qemu_aio_unref(acb);
-return NULL;
-}
-memset(acb->task, 0, sizeof(struct scsi_task));
-
switch (acb->ioh->dxfer_direction) {
case SG_DXFER_TO_DEV:
-acb->task->xfer_dir = SCSI_XFER_WRITE;
+xfer_dir = SCSI_XFER_WRITE;
break;
case SG_DXFER_FROM_DEV:
-acb->task->xfer_dir = SCSI_XFER_READ;
+xfer_dir = SCSI_XFER_READ;
break;
default:
-acb->task->xfer_dir = SCSI_XFER_NONE;
+xfer_dir = SCSI_XFER_NONE;
break;
}
-acb->task->cdb_size = acb->ioh->cmd_len;
-memcpy(&acb->task->cdb[0], acb->ioh->cmdp, acb->ioh->cmd_len);
-acb->task->expxferlen = acb->ioh->dxfer_len;
+acb->task = scsi_create_task(acb->ioh->cmd_len, acb->ioh->cmdp,
+ xfer_dir, acb->ioh->dxfer_len);
+if (acb->task == NULL) {
+error_report("iSCSI: Failed to allocate task for scsi command. %s",
+ iscsi_get_error(iscsi));
+qemu_aio_unref(acb);
+return NULL;
+}
+#if LIBISCSI_API_VERSION < 20150510
+acb->task->cdb_size = acb->ioh->cmd_len; /* fixed in libiscsi 1.13.0 */
+#endif
data.size = 0;
qemu_mutex_lock(&iscsilun->mutex);
if (acb->task->xfer_dir == SCSI_XFER_WRITE) {
--
2.14.0.rc0.1.g40ca67566
Re: [Qemu-block] [Qemu-devel] [PATCH v2] iscsi: use scsi_create_task()
On Fri, Jul 28, 2017 at 6:58 PM Paolo Bonzini wrote:
> On 28/07/2017 18:30, Marc-André Lureau wrote:
> > The function does the same initialization, and matches with
> > scsi_free_scsi_task() usage, and qemu doesn't need to know the
> > allocator details.
> >
> > Signed-off-by: Marc-André Lureau
> > ---
> > block/iscsi.c | 30 +++---
> > 1 file changed, 15 insertions(+), 15 deletions(-)
>
> Stupid question: what's the benefit?
scsi_create/scsi_free is a library API. If they have their own allocator,
we better use it, or it may easily break, no?
> Change malloc to g_new0 in the
> existing code, and we even make it shorter...
>
replacing malloc with g_new is the subject of another upcoming series :) (
https://github.com/elmarco/clang-tools-extra/blob/master/clang-tidy/qemu/UseGnewCheck.cpp
)
>
> Paolo
>
> > v2:
> > - set cdb_size if API_VERSION < 20150510
> >
> > diff --git a/block/iscsi.c b/block/iscsi.c
> > index d557c99668..9449c90631 100644
> > --- a/block/iscsi.c
> > +++ b/block/iscsi.c
> > @@ -1013,6 +1013,7 @@ static BlockAIOCB
> *iscsi_aio_ioctl(BlockDriverState *bs,
> > struct iscsi_context *iscsi = iscsilun->iscsi;
> > struct iscsi_data data;
> > IscsiAIOCB *acb;
> > +int xfer_dir;
> >
> > acb = qemu_aio_get(&iscsi_aiocb_info, bs, cb, opaque);
> >
> > @@ -1034,31 +1035,30 @@ static BlockAIOCB
> *iscsi_aio_ioctl(BlockDriverState *bs,
> > return NULL;
> > }
> >
> > -acb->task = malloc(sizeof(struct scsi_task));
> > -if (acb->task == NULL) {
> > -error_report("iSCSI: Failed to allocate task for scsi command.
> %s",
> > - iscsi_get_error(iscsi));
> > -qemu_aio_unref(acb);
> > -return NULL;
> > -}
> > -memset(acb->task, 0, sizeof(struct scsi_task));
> > -
> > switch (acb->ioh->dxfer_direction) {
> > case SG_DXFER_TO_DEV:
> > -acb->task->xfer_dir = SCSI_XFER_WRITE;
> > +xfer_dir = SCSI_XFER_WRITE;
> > break;
> > case SG_DXFER_FROM_DEV:
> > -acb->task->xfer_dir = SCSI_XFER_READ;
> > +xfer_dir = SCSI_XFER_READ;
> > break;
> > default:
> > -acb->task->xfer_dir = SCSI_XFER_NONE;
> > +xfer_dir = SCSI_XFER_NONE;
> > break;
> > }
> >
> > -acb->task->cdb_size = acb->ioh->cmd_len;
> > -memcpy(&acb->task->cdb[0], acb->ioh->cmdp, acb->ioh->cmd_len);
> > -acb->task->expxferlen = acb->ioh->dxfer_len;
> > +acb->task = scsi_create_task(acb->ioh->cmd_len, acb->ioh->cmdp,
> > + xfer_dir, acb->ioh->dxfer_len);
> > + if (acb->task == NULL) {
> > +error_report("iSCSI: Failed to allocate task for scsi command.
> %s",
> > + iscsi_get_error(iscsi));
> > +qemu_aio_unref(acb);
> > +return NULL;
> > +}
> >
> > +#if LIBISCSI_API_VERSION < 20150510
> > +acb->task->cdb_size = acb->ioh->cmd_len; /* fixed in libiscsi
> 1.13.0 */
> > +#endif
> > data.size = 0;
> > qemu_mutex_lock(&iscsilun->mutex);
> > if (acb->task->xfer_dir == SCSI_XFER_WRITE) {
> >
>
>
> --
Marc-André Lureau
Re: [Qemu-block] [RFC PATCH 02/56] qdict: New helpers to put and get unsigned integers
Hi
Is this based on
https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg01894.html ?
If so, you should probably keep me signed-off.
My patch had also a test :)
- Original Message -
> Signed-off-by: Markus Armbruster
> ---
> include/qapi/qmp/qdict.h | 5 +
> qobject/qdict.c | 43 ---
> 2 files changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
> index 363e431..3b52481 100644
> --- a/include/qapi/qmp/qdict.h
> +++ b/include/qapi/qmp/qdict.h
> @@ -56,6 +56,8 @@ void qdict_destroy_obj(QObject *obj);
> /* Helpers for int, bool, and string */
> #define qdict_put_int(qdict, key, value) \
> qdict_put(qdict, key, qnum_from_int(value))
> +#define qdict_put_uint(qdict, key, value) \
> +qdict_put(qdict, key, qnum_from_uint(value))
> #define qdict_put_bool(qdict, key, value) \
> qdict_put(qdict, key, qbool_from_bool(value))
> #define qdict_put_str(qdict, key, value) \
> @@ -64,12 +66,15 @@ void qdict_destroy_obj(QObject *obj);
> /* High level helpers */
> double qdict_get_double(const QDict *qdict, const char *key);
> int64_t qdict_get_int(const QDict *qdict, const char *key);
> +uint64_t qdict_get_uint(const QDict *qdict, const char *key);
> bool qdict_get_bool(const QDict *qdict, const char *key);
> QList *qdict_get_qlist(const QDict *qdict, const char *key);
> QDict *qdict_get_qdict(const QDict *qdict, const char *key);
> const char *qdict_get_str(const QDict *qdict, const char *key);
> int64_t qdict_get_try_int(const QDict *qdict, const char *key,
>int64_t def_value);
> +uint64_t qdict_get_try_uint(const QDict *qdict, const char *key,
> +uint64_t def_value);
> bool qdict_get_try_bool(const QDict *qdict, const char *key, bool
> def_value);
> const char *qdict_get_try_str(const QDict *qdict, const char *key);
>
> diff --git a/qobject/qdict.c b/qobject/qdict.c
> index d795079..be919b9 100644
> --- a/qobject/qdict.c
> +++ b/qobject/qdict.c
> @@ -189,10 +189,9 @@ double qdict_get_double(const QDict *qdict, const char
> *key)
> }
>
> /**
> - * qdict_get_int(): Get an integer mapped by @key
> + * qdict_get_int(): Get a signed integer mapped by @key
> *
> - * This function assumes that @key exists and it stores a
> - * QNum representable as int.
> + * @qdict must map @key to an integer QNum that fits into int64_t.
> *
> * Return integer mapped by @key.
> */
> @@ -202,6 +201,18 @@ int64_t qdict_get_int(const QDict *qdict, const char
> *key)
> }
>
> /**
> + * qdict_get_uint(): Get an unsigned integer mapped by 'key'
> + *
> + * @qdict must map @key to an integer QNum that fits into uint64_t.
> + *
> + * Return integer mapped by 'key'.
> + */
> +uint64_t qdict_get_uint(const QDict *qdict, const char *key)
> +{
> +return qnum_get_uint(qobject_to_qnum(qdict_get(qdict, key)));
> +}
> +
> +/**
> * qdict_get_bool(): Get a bool mapped by @key
> *
> * This function assumes that @key exists and it stores a
> @@ -245,11 +256,10 @@ const char *qdict_get_str(const QDict *qdict, const
> char *key)
> }
>
> /**
> - * qdict_get_try_int(): Try to get integer mapped by @key
> + * qdict_get_try_int(): Try to get signed integer mapped by @key
> *
> - * Return integer mapped by @key, if it is not present in the
> - * dictionary or if the stored object is not a QNum representing an
> - * integer, @def_value will be returned.
> + * If @qdict maps @key to an integer QNum that fits into int64_t,
> + * return it. Else return @def_value.
> */
> int64_t qdict_get_try_int(const QDict *qdict, const char *key,
>int64_t def_value)
> @@ -265,6 +275,25 @@ int64_t qdict_get_try_int(const QDict *qdict, const char
> *key,
> }
>
> /**
> + * qdict_get_try_uint(): Try to get unsigned integer mapped by 'key'
> + *
> + * If @qdict maps @key to an integer QNum that fits into uint64_t,
> + * return it. Else return @def_value.
> + */
> +uint64_t qdict_get_try_uint(const QDict *qdict, const char *key,
> +uint64_t def_value)
> +{
> +QNum *qnum = qobject_to_qnum(qdict_get(qdict, key));
> +uint64_t val;
> +
> +if (!qnum || !qnum_get_try_uint(qnum, &val)) {
> +return def_value;
> +}
> +
> +return val;
> +}
> +
> +/**
> * qdict_get_try_bool(): Try to get a bool mapped by @key
> *
> * Return bool mapped by @key, if it is not present in the
> --
> 2.7.5
>
>
Re: [Qemu-block] [Qemu-devel] [RFC PATCH 04/56] char: Make ringbuf-read size unsigned in QAPI/QMP
Hi
On Mon, Aug 7, 2017 at 4:45 PM, Markus Armbruster wrote:
> Sizes should use QAPI type 'size' (uint64_t). ringbuf-read parameter
> @size is 'int' (int64_t). qmp_ringbuf_read() rejects negative values,
> then implicitly converts to size_t.
>
> Change the parameter to 'size' and drop the check for negative values.
>
> ringbuf-read now accepts size values between 2^63 and 2^64-1. It
> accepts negative values as before, because that's how the QObject
> input visitor works for backward compatibility.
>
Negative values over json will be implicitly converted to positive
values with this change, right? Or are they rejected earlier?
If so that is a change of behaviour that I am not sure is worth doing
now (without explicit protocol break), but I don't mind.
> The HMP command's size parameter remains uint32_t, as HMP args_type
> strings can't do uint64_t byte counts: 'l' is signed, and 'o'
> multiplies by 2^20.
>
> Signed-off-by: Markus Armbruster
> ---
> chardev/char-ringbuf.c | 11 +++
> qapi-schema.json | 2 +-
> 2 files changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/chardev/char-ringbuf.c b/chardev/char-ringbuf.c
> index df52b04..a9205ea 100644
> --- a/chardev/char-ringbuf.c
> +++ b/chardev/char-ringbuf.c
> @@ -65,10 +65,10 @@ static int ringbuf_chr_write(Chardev *chr, const uint8_t
> *buf, int len)
> return len;
> }
>
> -static int ringbuf_chr_read(Chardev *chr, uint8_t *buf, int len)
> +static int ringbuf_chr_read(Chardev *chr, uint8_t *buf, size_t len)
> {
> RingBufChardev *d = RINGBUF_CHARDEV(chr);
> -int i;
> +size_t i;
>
> qemu_mutex_lock(&chr->chr_write_lock);
> for (i = 0; i < len && d->cons != d->prod; i++) {
> @@ -151,7 +151,7 @@ void qmp_ringbuf_write(const char *device, const char
> *data,
> }
> }
>
> -char *qmp_ringbuf_read(const char *device, int64_t size,
> +char *qmp_ringbuf_read(const char *device, uint64_t size,
> bool has_format, enum DataFormat format,
> Error **errp)
> {
> @@ -171,11 +171,6 @@ char *qmp_ringbuf_read(const char *device, int64_t size,
> return NULL;
> }
>
> -if (size <= 0) {
> -error_setg(errp, "size must be greater than zero");
> -return NULL;
> -}
> -
> count = ringbuf_count(chr);
> size = size > count ? count : size;
> read_data = g_malloc(size + 1);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index febe70e..18ec301 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -543,7 +543,7 @@
> #
> ##
> { 'command': 'ringbuf-read',
> - 'data': {'device': 'str', 'size': 'int', '*format': 'DataFormat'},
> + 'data': {'device': 'str', 'size': 'size', '*format': 'DataFormat'},
>'returns': 'str' }
>
> ##
> --
> 2.7.5
>
>
--
Marc-André Lureau
[Qemu-block] [PATCH v2 10/54] block: use qemu_enum_parse() in blkdebug_debug_breakpoint
This slightly change the error report message from "Invalid event
name" to "Invalid parameter".
Signed-off-by: Marc-André Lureau
---
block/blkdebug.c | 28
1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index c19ab28f07..50edda2a31 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -32,6 +32,7 @@
#include "qapi/qmp/qbool.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qstring.h"
+#include "qapi/util.h"
#include "sysemu/qtest.h"
typedef struct BDRVBlkdebugState {
@@ -149,20 +150,6 @@ static QemuOptsList *config_groups[] = {
NULL
};
-static int get_event_by_name(const char *name, BlkdebugEvent *event)
-{
-int i;
-
-for (i = 0; i < BLKDBG__MAX; i++) {
-if (!strcmp(BlkdebugEvent_lookup[i], name)) {
-*event = i;
-return 0;
-}
-}
-
-return -1;
-}
-
struct add_rule_data {
BDRVBlkdebugState *s;
int action;
@@ -173,7 +160,7 @@ static int add_rule(void *opaque, QemuOpts *opts, Error
**errp)
struct add_rule_data *d = opaque;
BDRVBlkdebugState *s = d->s;
const char* event_name;
-BlkdebugEvent event;
+int event;
struct BlkdebugRule *rule;
int64_t sector;
@@ -182,8 +169,9 @@ static int add_rule(void *opaque, QemuOpts *opts, Error
**errp)
if (!event_name) {
error_setg(errp, "Missing event name for rule");
return -1;
-} else if (get_event_by_name(event_name, &event) < 0) {
-error_setg(errp, "Invalid event name \"%s\"", event_name);
+}
+event = qapi_enum_parse(BlkdebugEvent_lookup, event_name, BLKDBG__MAX, -1,
errp);
+if (event < 0) {
return -1;
}
@@ -743,13 +731,13 @@ static int blkdebug_debug_breakpoint(BlockDriverState
*bs, const char *event,
{
BDRVBlkdebugState *s = bs->opaque;
struct BlkdebugRule *rule;
-BlkdebugEvent blkdebug_event;
+int blkdebug_event;
-if (get_event_by_name(event, &blkdebug_event) < 0) {
+blkdebug_event = qapi_enum_parse(BlkdebugEvent_lookup, event, BLKDBG__MAX,
-1, NULL);
+if (blkdebug_event < 0) {
return -ENOENT;
}
-
rule = g_malloc(sizeof(*rule));
*rule = (struct BlkdebugRule) {
.event = blkdebug_event,
--
2.14.1.146.gd35faa819
[Qemu-block] [PATCH v2 11/54] quorum: use qapi_enum_parse() in quorum_open
Signed-off-by: Marc-André Lureau
---
block/quorum.c | 27 ---
1 file changed, 8 insertions(+), 19 deletions(-)
diff --git a/block/quorum.c b/block/quorum.c
index d04da4f430..e4271caa7a 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -22,6 +22,7 @@
#include "qapi/qmp/qjson.h"
#include "qapi/qmp/qlist.h"
#include "qapi/qmp/qstring.h"
+#include "qapi/util.h"
#include "qapi-event.h"
#include "crypto/hash.h"
@@ -867,24 +868,6 @@ static QemuOptsList quorum_runtime_opts = {
},
};
-static int parse_read_pattern(const char *opt)
-{
-int i;
-
-if (!opt) {
-/* Set quorum as default */
-return QUORUM_READ_PATTERN_QUORUM;
-}
-
-for (i = 0; i < QUORUM_READ_PATTERN__MAX; i++) {
-if (!strcmp(opt, QuorumReadPattern_lookup[i])) {
-return i;
-}
-}
-
-return -EINVAL;
-}
-
static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
Error **errp)
{
@@ -925,7 +908,13 @@ static int quorum_open(BlockDriverState *bs, QDict
*options, int flags,
goto exit;
}
-ret = parse_read_pattern(qemu_opt_get(opts, QUORUM_OPT_READ_PATTERN));
+if (!qemu_opt_get(opts, QUORUM_OPT_READ_PATTERN)) {
+ret = QUORUM_READ_PATTERN_QUORUM;
+} else {
+ret = qapi_enum_parse(QuorumReadPattern_lookup,
+ qemu_opt_get(opts, QUORUM_OPT_READ_PATTERN),
+ QUORUM_READ_PATTERN__MAX, -EINVAL, NULL);
+}
if (ret < 0) {
error_setg(&local_err, "Please set read-pattern as fifo or quorum");
goto exit;
--
2.14.1.146.gd35faa819
[Qemu-block] [PATCH v2 06/54] qapi: introduce qapi_enum_lookup()
This will help with the introduction of a new structure to handle
enum lookup.
Signed-off-by: Marc-André Lureau
---
include/qapi/util.h | 2 +
backends/hostmem.c | 3 +-
block/backup.c | 3 +-
block/file-posix.c | 5 ++-
block/file-win32.c | 2 +-
block/gluster.c | 4 +-
block/iscsi.c | 3 +-
block/nfs.c | 3 +-
block/qcow2.c | 5 ++-
block/qed.c | 3 +-
block/rbd.c | 3 +-
block/sheepdog.c| 3 +-
blockdev.c | 6 ++-
blockjob.c | 9 +++--
chardev/char.c | 7 +++-
crypto/block-luks.c | 17 ++---
crypto/block.c | 5 ++-
crypto/cipher-afalg.c | 2 +-
crypto/cipher-builtin.c | 8 ++--
crypto/cipher-gcrypt.c | 4 +-
crypto/cipher-nettle.c | 8 ++--
crypto/cipher.c | 1 +
crypto/hmac-gcrypt.c| 2 +-
crypto/hmac-glib.c | 3 +-
crypto/hmac-nettle.c| 3 +-
crypto/pbkdf-gcrypt.c | 3 +-
crypto/pbkdf-nettle.c | 4 +-
hmp.c | 74 +++--
hw/block/fdc.c | 9 +++--
hw/char/escc.c | 3 +-
hw/core/qdev-properties.c | 9 +++--
hw/input/virtio-input-hid.c | 5 ++-
migration/colo-failover.c | 6 ++-
migration/colo.c| 17 +
migration/global_state.c| 2 +-
monitor.c | 24 +++-
net/net.c | 5 ++-
qapi/qapi-util.c| 7
qapi/qmp-dispatch.c | 4 +-
tests/test-qobject-output-visitor.c | 4 +-
tests/test-string-output-visitor.c | 6 ++-
tpm.c | 4 +-
ui/input.c | 13 ---
ui/vnc.c| 8 ++--
vl.c| 7 ++--
45 files changed, 206 insertions(+), 122 deletions(-)
diff --git a/include/qapi/util.h b/include/qapi/util.h
index 7436ed815c..60733b6a80 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -11,6 +11,8 @@
#ifndef QAPI_UTIL_H
#define QAPI_UTIL_H
+const char *qapi_enum_lookup(const char * const lookup[], int val);
+
int qapi_enum_parse(const char * const lookup[], const char *buf,
int max, int def, Error **errp);
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 4606b73849..c4f795475c 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -13,6 +13,7 @@
#include "sysemu/hostmem.h"
#include "hw/boards.h"
#include "qapi/error.h"
+#include "qapi/util.h"
#include "qapi/visitor.h"
#include "qapi-types.h"
#include "qapi-visit.h"
@@ -304,7 +305,7 @@ host_memory_backend_memory_complete(UserCreatable *uc,
Error **errp)
return;
} else if (maxnode == 0 && backend->policy != MPOL_DEFAULT) {
error_setg(errp, "host-nodes must be set for policy %s",
- HostMemPolicy_lookup[backend->policy]);
+qapi_enum_lookup(HostMemPolicy_lookup, backend->policy));
return;
}
diff --git a/block/backup.c b/block/backup.c
index 504a089150..a700cc0315 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -19,6 +19,7 @@
#include "block/blockjob_int.h"
#include "block/block_backup.h"
#include "qapi/error.h"
+#include "qapi/util.h"
#include "qapi/qmp/qerror.h"
#include "qemu/ratelimit.h"
#include "qemu/cutils.h"
@@ -596,7 +597,7 @@ BlockJob *backup_job_create(const char *job_id,
BlockDriverState *bs,
error_setg(errp,
"a sync_bitmap was provided to backup_run, "
"but received an incompatible sync_mode (%s)",
- MirrorSyncMode_lookup[sync_mode]);
+ qapi_enum_lookup(MirrorSyncMode_lookup, sync_mode));
return NULL;
}
diff --git a/block/file-posix.c b/block/file-posix.c
index cb3bfce147..48200aef0b 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1725,7 +1725,7 @@ static int raw_regular_truncate(int fd, int64_t offset,
PreallocMode prealloc,
default:
result = -ENOTSUP;
error_setg(errp, "Unsupported preallocation mode: %s",
- PreallocMode_lookup[prealloc]);
+ qapi_enum_lookup(PreallocMode_lookup, prealloc));
return result;
}
@@ -1760,7 +1760,8 @@ static int raw_truncate(BlockDriverState *bs, int64_
[Qemu-block] [PATCH v2 12/54] qapi: change enum lookup structure
Store the length in the lookup table, i.e. change it from
const char *const[] to struct { int n, const char *const s[] }.
The following conditional enum entry change will create "hole"
elements in the generated lookup array, that should be skipped.
Signed-off-by: Marc-André Lureau
---
include/qapi/visitor.h | 2 +-
scripts/qapi.py | 11 +--
scripts/qapi-types.py | 6
scripts/qapi-visit.py | 2 +-
include/hw/qdev-core.h | 2 +-
include/qapi/util.h | 6 ++--
include/qom/object.h| 4 +--
qapi/qapi-visit-core.c | 24 +++---
backends/hostmem.c | 4 +--
block.c | 3 +-
block/backup.c | 2 +-
block/blkdebug.c| 4 +--
block/file-posix.c | 17 +-
block/file-win32.c | 6 ++--
block/gluster.c | 12 +++
block/iscsi.c | 2 +-
block/nfs.c | 2 +-
block/parallels.c | 10 --
block/qcow2.c | 14
block/qed.c | 2 +-
block/quorum.c | 4 +--
block/rbd.c | 2 +-
block/sheepdog.c| 2 +-
blockdev.c | 7 ++--
blockjob.c | 6 ++--
chardev/char.c | 4 +--
crypto/block-luks.c | 38 +-
crypto/block.c | 4 +--
crypto/cipher-afalg.c | 2 +-
crypto/cipher-builtin.c | 8 ++---
crypto/cipher-gcrypt.c | 4 +--
crypto/cipher-nettle.c | 8 ++---
crypto/hmac-gcrypt.c| 2 +-
crypto/hmac-glib.c | 2 +-
crypto/hmac-nettle.c| 2 +-
crypto/pbkdf-gcrypt.c | 2 +-
crypto/pbkdf-nettle.c | 2 +-
crypto/secret.c | 2 +-
crypto/tlscreds.c | 2 +-
hmp.c | 64 +
hw/block/fdc.c | 6 ++--
hw/char/escc.c | 2 +-
hw/core/qdev-properties.c | 10 +++---
hw/input/virtio-input-hid.c | 4 +--
migration/colo-failover.c | 4 +--
migration/colo.c| 14
migration/global_state.c| 5 ++-
monitor.c | 20 ++--
net/filter.c| 2 +-
net/net.c | 4 +--
qapi/qapi-util.c| 13
qapi/qmp-dispatch.c | 2 +-
qemu-img.c | 6 ++--
qemu-nbd.c | 3 +-
qom/object.c| 16 +-
tests/check-qom-proplist.c | 7 +++-
tests/test-qapi-util.c | 17 --
tests/test-qobject-input-visitor.c | 8 ++---
tests/test-qobject-output-visitor.c | 2 +-
tests/test-string-input-visitor.c | 4 +--
tests/test-string-output-visitor.c | 4 +--
tpm.c | 4 +--
ui/input-legacy.c | 6 ++--
ui/input.c | 12 +++
ui/vnc.c| 6 ++--
vl.c| 6 ++--
66 files changed, 241 insertions(+), 248 deletions(-)
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 0f3b8cb459..62a51a54cb 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -469,7 +469,7 @@ bool visit_optional(Visitor *v, const char *name, bool
*present);
* that visit_type_str() must have no unwelcome side effects.
*/
void visit_type_enum(Visitor *v, const char *name, int *obj,
- const char *const strings[], Error **errp);
+ const QEnumLookup *lookup, Error **errp);
/*
* Check if visitor is an input visitor.
diff --git a/scripts/qapi.py b/scripts/qapi.py
index a3ac799535..314d7e0365 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1851,7 +1851,7 @@ def guardend(name):
def gen_enum_lookup(name, values, prefix=None):
ret = mcgen('''
-const char *const %(c_name)s_lookup[] = {
+static const char *const %(c_name)s_array[] = {
''',
c_name=c_name(name))
for value in values:
@@ -1865,8 +1865,13 @@ const char *const %(c_name)s_lookup[] = {
ret += mcgen('''
[%(max_index)s] = NULL,
};
+
+const QEnumLookup %(c_name)s_lookup = {
+.array = %(c_name)s_array,
+.size = %(max_index)s
+};
''',
- max_index=max_index)
+ max_index=max_index, c_name=c_name(name))
return ret
@@ -1896,7 +1901,7 @@ typedef enum %(c_name)s {
ret += mcgen('''
-exte
[Qemu-block] [PATCH v2 13/54] qapi: drop the sentinel in enum array
Now that all usages have been converted to user lookup helpers.
Signed-off-by: Marc-André Lureau
---
scripts/qapi.py | 1 -
block/parallels.c | 1 -
ui/input-legacy.c | 2 +-
3 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 314d7e0365..73adb90379 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1863,7 +1863,6 @@ static const char *const %(c_name)s_array[] = {
max_index = c_enum_const(name, '_MAX', prefix)
ret += mcgen('''
-[%(max_index)s] = NULL,
};
const QEnumLookup %(c_name)s_lookup = {
diff --git a/block/parallels.c b/block/parallels.c
index f870bbac3e..d5de692c9c 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -72,7 +72,6 @@ typedef enum ParallelsPreallocMode {
static const char *prealloc_mode_array[] = {
"falloc",
"truncate",
-NULL,
};
static QEnumLookup prealloc_mode_lookup = {
diff --git a/ui/input-legacy.c b/ui/input-legacy.c
index c597bdc711..d50a18a505 100644
--- a/ui/input-legacy.c
+++ b/ui/input-legacy.c
@@ -61,7 +61,7 @@ int index_from_key(const char *key, size_t key_length)
{
int i;
-for (i = 0; QKeyCode_lookup.array[i] != NULL; i++) {
+for (i = 0; i < QKeyCode_lookup.size; i++) {
if (!strncmp(key, QKeyCode_lookup.array[i], key_length) &&
!QKeyCode_lookup.array[i][key_length]) {
break;
--
2.14.1.146.gd35faa819
Re: [Qemu-block] [Qemu-devel] [PATCH 08/10] scsi: build qemu-pr-helper
targ);
> +exit(EXIT_FAILURE);
> +}
> +break;
> +}
> +case 'g': {
> +unsigned long res;
> +struct group *groupinfo = getgrnam(optarg);
> +if (groupinfo) {
> +gid = groupinfo->gr_gid;
> +} else if (qemu_strtoul(optarg, NULL, 10, &res) == 0 &&
> + (gid_t)res == res) {
> +gid = res;
> +} else {
> +error_report("invalid group '%s'", optarg);
> +exit(EXIT_FAILURE);
> +}
> +break;
> +}
> +#else
> +case 'u':
> +case 'g':
> +error_report("-%c not supported by this %s", ch, argv[0]);
> +exit(1);
> +#endif
> +case 'd':
> +daemonize = true;
> +break;
> +case 'q':
> +quiet = 1;
> +break;
> +case 'T':
> +g_free(trace_file);
> +trace_file = trace_opt_parse(optarg);
> +break;
> +case 'V':
> +version(argv[0]);
> +exit(0);
EXIT_SUCCESS for consistency
> +break;
> +case 'h':
> +usage(argv[0]);
> +exit(0);
same here
> +break;
> +case '?':
> +error_report("Try `%s --help' for more information.", argv[0]);
> +exit(EXIT_FAILURE);
> +}
> +}
> +
> +/* set verbosity */
> +verbose = !quiet;
> +
> +if (!trace_init_backends()) {
> +exit(1);
EXIT_FAILURE
> +}
> +trace_init_file(trace_file);
> +qemu_set_log(LOG_TRACE);
> +
> +#ifdef CONFIG_MPATH
> +dm_init();
> +multipath_pr_init();
> +#endif
> +
> +socket_activation = check_socket_activation();
> +if (socket_activation == 0) {
> +SocketAddress saddr = {
> +.type = SOCKET_ADDRESS_TYPE_UNIX,
> +.u.q_unix.path = g_strdup(socket_path)
> +};
> +server_ioc = qio_channel_socket_new();
> +if (qio_channel_socket_listen_sync(server_ioc, &saddr, &local_err) <
> 0) {
> +object_unref(OBJECT(server_ioc));
> +error_report_err(local_err);
> +return 1;
> +}
> +g_free(saddr.u.q_unix.path);
> +} else {
> +/* Using socket activation - check user didn't use -p etc. */
> +const char *err_msg = socket_activation_validate_opts();
> +if (err_msg != NULL) {
> +error_report("%s", err_msg);
> +exit(EXIT_FAILURE);
> +}
> +
> +/* Can only listen on a single socket. */
> +if (socket_activation > 1) {
> +error_report("%s does not support socket activation with
> LISTEN_FDS > 1",
> + argv[0]);
> +exit(EXIT_FAILURE);
> +}
> +server_ioc = qio_channel_socket_new_fd(FIRST_SOCKET_ACTIVATION_FD,
> + &local_err);
> +if (server_ioc == NULL) {
> +error_report("Failed to use socket activation: %s",
> + error_get_pretty(local_err));
> +exit(EXIT_FAILURE);
> +}
> +socket_path = NULL;
> +}
> +
> +if (qemu_init_main_loop(&local_err)) {
> +error_report_err(local_err);
> +exit(EXIT_FAILURE);
> +}
> +
> +server_watch = qio_channel_add_watch(QIO_CHANNEL(server_ioc),
> + G_IO_IN,
> + accept_client,
> + NULL, NULL);
> +
> +#ifdef CONFIG_LIBCAP
> +if (drop_privileges() < 0) {
> +error_report("Failed to drop privileges: %s", strerror(errno));
> +exit(EXIT_FAILURE);
> +}
> +#endif
> +
> +if (daemonize) {
> +if (daemon(0, 0) < 0) {
> +error_report("Failed to daemonize: %s", strerror(errno));
> +exit(EXIT_FAILURE);
> +}
> +}
> +
> +state = RUNNING;
> +do {
> +main_loop_wait(false);
> +if (state == TERMINATE) {
> +state = TERMINATING;
> +close_server_socket();
> +}
> +} while (num_active_sockets > 0);
> +
> +exit(EXIT_SUCCESS);
> +}
> --
> 2.13.5
>
>
>
--
Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [RFC PATCH 04/56] char: Make ringbuf-read size unsigned in QAPI/QMP
Hi
On Tue, Aug 22, 2017 at 3:00 PM, Markus Armbruster wrote:
> Marc-André Lureau writes:
>
>> Hi
>>
>> On Mon, Aug 7, 2017 at 4:45 PM, Markus Armbruster wrote:
>>> Sizes should use QAPI type 'size' (uint64_t). ringbuf-read parameter
>>> @size is 'int' (int64_t). qmp_ringbuf_read() rejects negative values,
>>> then implicitly converts to size_t.
>>>
>>> Change the parameter to 'size' and drop the check for negative values.
>>>
>>> ringbuf-read now accepts size values between 2^63 and 2^64-1. It
>>> accepts negative values as before, because that's how the QObject
>>> input visitor works for backward compatibility.
>>>
>>
>> Negative values over json will be implicitly converted to positive
>> values with this change, right? Or are they rejected earlier?
>
> Yes. For details, see my reply to Juan's review of PATCH 15.
>
>> If so that is a change of behaviour that I am not sure is worth doing
>> now (without explicit protocol break), but I don't mind.
So before this change:
(QEMU) ringbuf-read device=foo size=-1
{"error": {"class": "GenericError", "desc": "size must be greater than zero"}}
after:
(QEMU) ringbuf-read device=foo size=-1
{"return": ""}
Is this really what we want?
--
Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [PATCH v2 06/54] qapi: introduce qapi_enum_lookup()
Hi
- Original Message -
> Marc-André Lureau writes:
>
> > This will help with the introduction of a new structure to handle
> > enum lookup.
> >
> > Signed-off-by: Marc-André Lureau
> > ---
> [...]
> > 45 files changed, 206 insertions(+), 122 deletions(-)
>
> Hmm.
>
> > diff --git a/include/qapi/util.h b/include/qapi/util.h
> > index 7436ed815c..60733b6a80 100644
> > --- a/include/qapi/util.h
> > +++ b/include/qapi/util.h
> > @@ -11,6 +11,8 @@
> > #ifndef QAPI_UTIL_H
> > #define QAPI_UTIL_H
> >
> > +const char *qapi_enum_lookup(const char * const lookup[], int val);
> > +
> > int qapi_enum_parse(const char * const lookup[], const char *buf,
> > int max, int def, Error **errp);
> >
> > diff --git a/backends/hostmem.c b/backends/hostmem.c
> > index 4606b73849..c4f795475c 100644
> > --- a/backends/hostmem.c
> > +++ b/backends/hostmem.c
> > @@ -13,6 +13,7 @@
> > #include "sysemu/hostmem.h"
> > #include "hw/boards.h"
> > #include "qapi/error.h"
> > +#include "qapi/util.h"
> > #include "qapi/visitor.h"
> > #include "qapi-types.h"
> > #include "qapi-visit.h"
> > @@ -304,7 +305,7 @@ host_memory_backend_memory_complete(UserCreatable *uc,
> > Error **errp)
> > return;
> > } else if (maxnode == 0 && backend->policy != MPOL_DEFAULT) {
> > error_setg(errp, "host-nodes must be set for policy %s",
> > - HostMemPolicy_lookup[backend->policy]);
> > +qapi_enum_lookup(HostMemPolicy_lookup, backend->policy));
> > return;
> > }
> >
>
> Lookup becomes even more verbose.
>
> Could we claw back some readability with macros? Say in addition to
>
> typedef enum FOO {
> ...
> } FOO;
>
> extern const char *const FOO_lookup[];
>
> generate
>
> #define FOO_str(val) qapi_enum_lookup(FOO_lookup, (val))
>
> Needs a matching qapi-code-gen.txt update.
>
> With that, this patch hunk would become
>
>error_setg(errp, "host-nodes must be set for policy %s",
> - HostMemPolicy_lookup[backend->policy]);
> + HostMemPolicy_str(backend->policy);
>
> Perhaps we could even throw in some type checking:
>
> #define FOO_str(val) (type_check(typeof((val)), FOO) \
> + qapi_enum_lookup(FOO_lookup, (val)))
>
> What do you think? Want me to explore a fixup patch you could squash
> in?
>
Yes, I had similar thoughts, but didn't spent too much time finding the best
proposal, that wasn't my main goal in the series.
Indeed, I would prefer that further improvements be done as follow-up. Or if
you have a ready solution, I can squash it there. I can picture the conflicts
with the next patch though...
> [Skipping lots of mechanical changes...]
>
> I think you missed test-qobject-input-visitor.c and
> string-input-visitor.c.
>
> In test-qobject-input-visitor.c's test_visitor_in_enum():
>
> v = visitor_input_test_init(data, "%s", EnumOne_lookup[i]);
>
> You update it in PATCH 12, but the code only works as long as EnumOne
> has no holes. Mapping the enum to string like we do everywhere else in
> this patch would be cleaner.
>
ok
> The loop control also subscripts EnumOne_lookup[i]. You take care of
> that one in PATCH 12. That's okay.
>
> Same for test-string-input-visitor.c's test_visitor_in_enum().
>
> There's one more in test_native_list_integer_helper():
>
> g_string_append_printf(gstr_union, "{ 'type': '%s', 'data': [ %s ] }",
>UserDefNativeListUnionKind_lookup[kind],
>gstr_list->str);
>
> Same story.
>
> The patch doesn't touch the _lookup[] subscripts you're going to replace
> by qapi_enum_parse() in PATCH 07-11. Understand, but I'd reorder the
> patches: first replace by qapi_enum_parse(), because DRY (no need to
> explain more at that point). Then get rid of all the remaining
> subscripting into _lookup[], i.e. this patch (explain it helps the next
> one), then PATCH 12.
>
ok
[Qemu-block] [PULL 03/29] vhdx: use QEMU_ALIGN_DOWN
I used the clang-tidy qemu-round check to generate the fix: https://github.com/elmarco/clang-tools-extra Signed-off-by: Marc-André Lureau Reviewed-by: Eric Blake Reviewed-by: Richard Henderson --- block/vhdx-log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/vhdx-log.c b/block/vhdx-log.c index 14b724ef7b..0ac4863b25 100644 --- a/block/vhdx-log.c +++ b/block/vhdx-log.c @@ -902,7 +902,7 @@ static int vhdx_log_write(BlockDriverState *bs, BDRVVHDXState *s, } sector_offset = offset % VHDX_LOG_SECTOR_SIZE; -file_offset = (offset / VHDX_LOG_SECTOR_SIZE) * VHDX_LOG_SECTOR_SIZE; +file_offset = QEMU_ALIGN_DOWN(offset, VHDX_LOG_SECTOR_SIZE); aligned_length = length; -- 2.14.1.146.gd35faa819
[Qemu-block] [PULL 10/29] vvfat: use DIV_ROUND_UP
I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra
Signed-off-by: Marc-André Lureau
Reviewed-by: Eric Blake
Reviewed-by: Richard Henderson
---
block/vvfat.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/vvfat.c b/block/vvfat.c
index a9e207f7f0..c54fa94651 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -449,7 +449,7 @@ static direntry_t *create_long_filename(BDRVVVFATState *s,
const char *filename)
return NULL;
}
-number_of_entries = (length * 2 + 25) / 26;
+number_of_entries = DIV_ROUND_UP(length * 2, 26);
for(i=0;idirectory));
@@ -2554,7 +2554,7 @@ static int commit_one_file(BDRVVVFATState* s,
(size > offset && c >=2 && !fat_eof(s, c)));
ret = vvfat_read(s->bs, cluster2sector(s, c),
-(uint8_t*)cluster, (rest_size + 0x1ff) / 0x200);
+(uint8_t*)cluster, DIV_ROUND_UP(rest_size, 0x200));
if (ret < 0) {
qemu_close(fd);
--
2.14.1.146.gd35faa819
[Qemu-block] [PULL 08/29] qcow2: use DIV_ROUND_UP
I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra
Signed-off-by: Marc-André Lureau
Reviewed-by: Eric Blake
Reviewed-by: Richard Henderson
---
block/qcow2-cluster.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index f06c08f64c..30db942dde 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -61,7 +61,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t
min_size,
new_l1_size = 1;
}
while (min_size > new_l1_size) {
-new_l1_size = (new_l1_size * 3 + 1) / 2;
+new_l1_size = DIV_ROUND_UP(new_l1_size * 3, 2);
}
}
--
2.14.1.146.gd35faa819
[Qemu-block] [PULL 07/29] dmg: use DIV_ROUND_UP
I used the clang-tidy qemu-round check to generate the fix: https://github.com/elmarco/clang-tools-extra Signed-off-by: Marc-André Lureau Reviewed-by: Stefan Hajnoczi Reviewed-by: Richard Henderson --- block/dmg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/dmg.c b/block/dmg.c index 900ae5a678..6c0711f563 100644 --- a/block/dmg.c +++ b/block/dmg.c @@ -111,7 +111,7 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk, uncompressed_sectors = s->sectorcounts[chunk]; break; case 1: /* copy */ -uncompressed_sectors = (s->lengths[chunk] + 511) / 512; +uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512); break; case 2: /* zero */ /* as the all-zeroes block may be large, it is treated specially: the -- 2.14.1.146.gd35faa819
[Qemu-block] [PULL 09/29] vpc: use DIV_ROUND_UP
I used the clang-tidy qemu-round check to generate the fix:
https://github.com/elmarco/clang-tools-extra
Signed-off-by: Marc-André Lureau
Reviewed-by: Eric Blake
Reviewed-by: Richard Henderson
---
block/vpc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/vpc.c b/block/vpc.c
index 82911ebead..1576d7b595 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -783,7 +783,7 @@ static int calculate_geometry(int64_t total_sectors,
uint16_t* cyls,
} else {
*secs_per_cyl = 17;
cyls_times_heads = total_sectors / *secs_per_cyl;
-*heads = (cyls_times_heads + 1023) / 1024;
+*heads = DIV_ROUND_UP(cyls_times_heads, 1024);
if (*heads < 4) {
*heads = 4;
@@ -836,7 +836,7 @@ static int create_dynamic_disk(BlockBackend *blk, uint8_t
*buf,
offset = 3 * 512;
memset(buf, 0xFF, 512);
-for (i = 0; i < (num_bat_entries * 4 + 511) / 512; i++) {
+for (i = 0; i < DIV_ROUND_UP(num_bat_entries * 4, 512); i++) {
ret = blk_pwrite(blk, offset, buf, 512, 0);
if (ret < 0) {
goto fail;
--
2.14.1.146.gd35faa819
Re: [Qemu-block] [Qemu-devel] [PATCH 4/8] qemu-options: Move -iscsi under "Block device options"
On Mon, Oct 2, 2017 at 4:03 PM, Markus Armbruster wrote:
> -iscsi ended up under the "Device URL Syntax" heading by a sequence of
> errors, as explained in the previous commit. Move it under the "Block
> device options" heading. Nothing left under "Device URL Syntax";
> drop the heading.
>
> Cc: Ronnie Sahlberg
> Cc: Kevin Wolf
> Cc: Max Reitz
> Cc: qemu-block@nongnu.org
> Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
> ---
> qemu-options.hx | 15 +++
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index f112281d37..c647fdde62 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1172,6 +1172,13 @@ STEXI
> Create synthetic file system image
> ETEXI
>
> +DEF("iscsi", HAS_ARG, QEMU_OPTION_iscsi,
> +"-iscsi [user=user][,password=password]\n"
> +" [,header-digest=CRC32C|CR32C-NONE|NONE-CRC32C|NONE\n"
> +" [,initiator-name=initiator-iqn][,id=target-iqn]\n"
> +" [,timeout=timeout]\n"
> +"iSCSI session parameters\n", QEMU_ARCH_ALL)
> +
> STEXI
> @end table
> ETEXI
> @@ -2811,14 +2818,6 @@ STEXI
> ETEXI
> DEFHEADING()
>
> -DEFHEADING(Device URL Syntax:)
> -DEF("iscsi", HAS_ARG, QEMU_OPTION_iscsi,
> -"-iscsi [user=user][,password=password]\n"
> -" [,header-digest=CRC32C|CR32C-NONE|NONE-CRC32C|NONE\n"
> -" [,initiator-name=initiator-iqn][,id=target-iqn]\n"
> -" [,timeout=timeout]\n"
> -"iSCSI session parameters\n", QEMU_ARCH_ALL)
> -
> DEFHEADING(Bluetooth(R) options:)
> STEXI
> @table @option
> --
> 2.13.6
>
>
--
Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [PATCH 3/8] qemu-options qemu-doc: Move "Device URL Syntax" to qemu-doc
On Mon, Oct 2, 2017 at 4:03 PM, Markus Armbruster wrote: > Commit 0f5314a (v1.0) added section "Device URL Syntax" to > qemu-options.hx. It's enclosed in STEXI..ETEXI, thus affects only > qemu-options.texi, not --help. It appears as a subsection under > section "Invocation". Similarly, qemu.1 has it as a subsection under > "OPTIONS". > > Commit f9dadc9 (v1.1.0) dropped new option -iscsi into the middle of > this section. No effect on qemu-options.texi. It appears in --help > run together with the "Bluetooth(R) options:" header. > > Commit c70a01e (v1.5.0) gives it is own heading in --help by moving > commit 0f5314a's DEFHEADING(Device URL Syntax:) outside STEXI..ETEXI. > Trouble is the heading makes no sense for -iscsi. > > Move all of the "Device URL Syntax" Texinfo to qemu-doc.texi. Mark it > for inclusion in qemu.1 with '@c man begin NOTES'. This turns it into > a separate section outside the list of options both in qemu-doc and in > qemu.1. > > There's substantial overlap with the existing qemu-doc section "Disk > Images". Mark with a TODO comment. > > Output of --help will be fixed next. > > Cc: Ronnie Sahlberg > Cc: Kevin Wolf > Cc: Max Reitz > Cc: qemu-block@nongnu.org > Signed-off-by: Markus Armbruster Reviewed-by: Marc-André Lureau > --- > qemu-doc.texi | 217 ++ > qemu-options.hx | 222 > > 2 files changed, 217 insertions(+), 222 deletions(-) > > diff --git a/qemu-doc.texi b/qemu-doc.texi > index ecd186a159..848e49966a 100644 > --- a/qemu-doc.texi > +++ b/qemu-doc.texi > @@ -245,6 +245,223 @@ targets do not need a disk image. > > @c man end > > +@node device_url > +@subsection Device URL Syntax > +@c TODO merge this with section Disk Images > + > +@c man begin NOTES > + > +In addition to using normal file images for the emulated storage devices, > +QEMU can also use networked resources such as iSCSI devices. These are > +specified using a special URL syntax. > + > +@table @option > +@item iSCSI > +iSCSI support allows QEMU to access iSCSI resources directly and use as > +images for the guest storage. Both disk and cdrom images are supported. > + > +Syntax for specifying iSCSI LUNs is > +``iscsi://[:]//'' > + > +By default qemu will use the iSCSI initiator-name > +'iqn.2008-11.org.linux-kvm[:]' but this can also be set from the > command > +line or a configuration file. > + > +Since version Qemu 2.4 it is possible to specify a iSCSI request timeout to > detect > +stalled requests and force a reestablishment of the session. The timeout > +is specified in seconds. The default is 0 which means no timeout. Libiscsi > +1.15.0 or greater is required for this feature. > + > +Example (without authentication): > +@example > +qemu-system-i386 -iscsi initiator-name=iqn.2001-04.com.example:my-initiator \ > + -cdrom iscsi://192.0.2.1/iqn.2001-04.com.example/2 \ > + -drive file=iscsi://192.0.2.1/iqn.2001-04.com.example/1 > +@end example > + > +Example (CHAP username/password via URL): > +@example > +qemu-system-i386 -drive > file=iscsi://user%password@@192.0.2.1/iqn.2001-04.com.example/1 > +@end example > + > +Example (CHAP username/password via environment variables): > +@example > +LIBISCSI_CHAP_USERNAME="user" \ > +LIBISCSI_CHAP_PASSWORD="password" \ > +qemu-system-i386 -drive file=iscsi://192.0.2.1/iqn.2001-04.com.example/1 > +@end example > + > +@item NBD > +QEMU supports NBD (Network Block Devices) both using TCP protocol as well > +as Unix Domain Sockets. > + > +Syntax for specifying a NBD device using TCP > +``nbd::[:exportname=]'' > + > +Syntax for specifying a NBD device using Unix Domain Sockets > +``nbd:unix:[:exportname=]'' > + > +Example for TCP > +@example > +qemu-system-i386 --drive file=nbd:192.0.2.1:3 > +@end example > + > +Example for Unix Domain Sockets > +@example > +qemu-system-i386 --drive file=nbd:unix:/tmp/nbd-socket > +@end example > + > +@item SSH > +QEMU supports SSH (Secure Shell) access to remote disks. > + > +Examples: > +@example > +qemu-system-i386 -drive file=ssh://user@@host/path/to/disk.img > +qemu-system-i386 -drive > file.driver=ssh,file.user=user,file.host=host,file.port=22,file.path=/path/to/disk.img > +@end example > + > +Currently authentication must be done using ssh-agent. Other > +authentication methods may be supported in future. > + > +@item Sheepdog > +Sheepdog is a distribut
Re: [Qemu-block] [Qemu-devel] [PATCH 5/8] qemu-options: Add missing -iscsi Texinfo documentation
On Mon, Oct 2, 2017 at 4:03 PM, Markus Armbruster wrote:
> Cc: Ronnie Sahlberg
> Cc: Kevin Wolf
> Cc: Max Reitz
> Cc: qemu-block@nongnu.org
> Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
(that's very short doc)
> ---
> qemu-options.hx | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index c647fdde62..8568ee388c 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1180,6 +1180,12 @@ DEF("iscsi", HAS_ARG, QEMU_OPTION_iscsi,
> "iSCSI session parameters\n", QEMU_ARCH_ALL)
>
> STEXI
> +@item -iscsi
> +@findex -iscsi
> +Configure iSCSI session parameters.
> +ETEXI
> +
> +STEXI
> @end table
> ETEXI
> DEFHEADING()
> --
> 2.13.6
>
>
--
Marc-André Lureau
[Qemu-block] [PATCH 5/6] ahci-test: fix opts leak of skip tests
Fixes the following ASAN report:
Direct leak of 128 byte(s) in 8 object(s) allocated from:
#0 0x7fefce311850 in malloc (/lib64/libasan.so.4+0xde850)
#1 0x7fefcdd5ef0c in g_malloc ../glib/gmem.c:94
#2 0x559b976faff0 in create_ahci_io_test
/home/elmarco/src/qemu/tests/ahci-test.c:1810
Signed-off-by: Marc-André Lureau
---
tests/ahci-test.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 7aa5af428c..1bd3cc7ca8 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1822,6 +1822,7 @@ static void create_ahci_io_test(enum IOMode type, enum
AddrMode addr,
if ((addr == ADDR_MODE_LBA48) && (offset == OFFSET_HIGH) &&
(mb_to_sectors(test_image_size_mb) <= 0xFFF)) {
g_test_message("%s: skipped; test image too small", name);
+g_free(opts);
g_free(name);
return;
}
--
2.16.1.73.g5832b7e9f2
[Qemu-block] [PATCH] blockjob: use qapi enum helpers
QAPI generator provide #define helpers for looking up enum string.
Signed-off-by: Marc-André Lureau
---
blockjob.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/blockjob.c b/blockjob.c
index ef3ed69ff1..11c9ce124d 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -75,10 +75,8 @@ static void block_job_state_transition(BlockJob *job,
BlockJobStatus s1)
assert(s1 >= 0 && s1 <= BLOCK_JOB_STATUS__MAX);
trace_block_job_state_transition(job, job->ret, BlockJobSTT[s0][s1] ?
"allowed" : "disallowed",
- qapi_enum_lookup(&BlockJobStatus_lookup,
- s0),
- qapi_enum_lookup(&BlockJobStatus_lookup,
- s1));
+ BlockJobStatus_str(s0),
+ BlockJobStatus_str(s1));
assert(BlockJobSTT[s0][s1]);
job->status = s1;
}
@@ -86,17 +84,15 @@ static void block_job_state_transition(BlockJob *job,
BlockJobStatus s1)
static int block_job_apply_verb(BlockJob *job, BlockJobVerb bv, Error **errp)
{
assert(bv >= 0 && bv <= BLOCK_JOB_VERB__MAX);
-trace_block_job_apply_verb(job, qapi_enum_lookup(&BlockJobStatus_lookup,
- job->status),
- qapi_enum_lookup(&BlockJobVerb_lookup, bv),
+trace_block_job_apply_verb(job, BlockJobStatus_str(job->status),
+ BlockJobVerb_str(bv),
BlockJobVerbTable[bv][job->status] ?
"allowed" : "prohibited");
if (BlockJobVerbTable[bv][job->status]) {
return 0;
}
error_setg(errp, "Job '%s' in state '%s' cannot accept command verb '%s'",
- job->id, qapi_enum_lookup(&BlockJobStatus_lookup, job->status),
- qapi_enum_lookup(&BlockJobVerb_lookup, bv));
+ job->id, BlockJobStatus_str(job->status), BlockJobVerb_str(bv));
return -EPERM;
}
--
2.17.0.rc1.1.g4c4f2b46a3
[Qemu-block] [PATCH] blockjob: leak fix, remove from txn when failing early
This fixes leaks found by ASAN such as:
GTESTER tests/test-blockjob
=
==31442==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x7f88483cba38 in __interceptor_calloc (/lib64/libasan.so.4+0xdea38)
#1 0x7f8845e1bd77 in g_malloc0 ../glib/gmem.c:129
#2 0x7f8845e1c04b in g_malloc0_n ../glib/gmem.c:360
#3 0x5584d2732498 in block_job_txn_new /home/elmarco/src/qemu/blockjob.c:172
#4 0x5584d2739b28 in block_job_create /home/elmarco/src/qemu/blockjob.c:973
#5 0x5584d270ae31 in mk_job /home/elmarco/src/qemu/tests/test-blockjob.c:34
#6 0x5584d270b1c1 in do_test_id
/home/elmarco/src/qemu/tests/test-blockjob.c:57
#7 0x5584d270b65c in test_job_ids
/home/elmarco/src/qemu/tests/test-blockjob.c:118
#8 0x7f8845e40b69 in test_case_run ../glib/gtestutils.c:2255
#9 0x7f8845e40f29 in g_test_run_suite_internal ../glib/gtestutils.c:2339
#10 0x7f8845e40fd2 in g_test_run_suite_internal ../glib/gtestutils.c:2351
#11 0x7f8845e411e9 in g_test_run_suite ../glib/gtestutils.c:2426
#12 0x7f8845e3fe72 in g_test_run ../glib/gtestutils.c:1692
#13 0x5584d270d6e2 in main /home/elmarco/src/qemu/tests/test-blockjob.c:377
#14 0x7f8843641f29 in __libc_start_main (/lib64/libc.so.6+0x20f29)
Add an assert to make sure that the job doesn't have associated txn before
free().
Signed-off-by: Marc-André Lureau
---
blockjob.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/blockjob.c b/blockjob.c
index 11c9ce124d..bb75386515 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -228,6 +228,7 @@ void block_job_unref(BlockJob *job)
{
if (--job->refcnt == 0) {
assert(job->status == BLOCK_JOB_STATUS_NULL);
+assert(!job->txn);
BlockDriverState *bs = blk_bs(job->blk);
QLIST_REMOVE(job, job_list);
bs->job = NULL;
@@ -479,6 +480,7 @@ static int block_job_finalize_single(BlockJob *job)
QLIST_REMOVE(job, txn_list);
block_job_txn_unref(job->txn);
+job->txn = NULL;
block_job_conclude(job);
return 0;
}
@@ -994,6 +996,9 @@ void block_job_pause_all(void)
void block_job_early_fail(BlockJob *job)
{
assert(job->status == BLOCK_JOB_STATUS_CREATED);
+QLIST_REMOVE(job, txn_list);
+block_job_txn_unref(job->txn);
+job->txn = NULL;
block_job_decommission(job);
}
--
2.17.0.rc1.1.g4c4f2b46a3
Re: [Qemu-block] [PATCH v2 1/1] blockjob: leak fix, remove from txn when failing early
On Wed, Mar 28, 2018 at 4:09 PM, Jeff Cody wrote:
> From: Marc-André Lureau
>
> This fixes leaks found by ASAN such as:
> GTESTER tests/test-blockjob
> =
> ==31442==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 24 byte(s) in 1 object(s) allocated from:
> #0 0x7f88483cba38 in __interceptor_calloc (/lib64/libasan.so.4+0xdea38)
> #1 0x7f8845e1bd77 in g_malloc0 ../glib/gmem.c:129
> #2 0x7f8845e1c04b in g_malloc0_n ../glib/gmem.c:360
> #3 0x5584d2732498 in block_job_txn_new
> /home/elmarco/src/qemu/blockjob.c:172
> #4 0x5584d2739b28 in block_job_create
> /home/elmarco/src/qemu/blockjob.c:973
> #5 0x5584d270ae31 in mk_job
> /home/elmarco/src/qemu/tests/test-blockjob.c:34
> #6 0x5584d270b1c1 in do_test_id
> /home/elmarco/src/qemu/tests/test-blockjob.c:57
> #7 0x5584d270b65c in test_job_ids
> /home/elmarco/src/qemu/tests/test-blockjob.c:118
> #8 0x7f8845e40b69 in test_case_run ../glib/gtestutils.c:2255
> #9 0x7f8845e40f29 in g_test_run_suite_internal ../glib/gtestutils.c:2339
> #10 0x7f8845e40fd2 in g_test_run_suite_internal ../glib/gtestutils.c:2351
> #11 0x7f8845e411e9 in g_test_run_suite ../glib/gtestutils.c:2426
> #12 0x7f8845e3fe72 in g_test_run ../glib/gtestutils.c:1692
> #13 0x5584d270d6e2 in main
> /home/elmarco/src/qemu/tests/test-blockjob.c:377
> #14 0x7f8843641f29 in __libc_start_main (/lib64/libc.so.6+0x20f29)
>
> Add an assert to make sure that the job doesn't have associated txn before
> free().
>
> [Jeff Cody: N.B., used updated patch provided by John Snow]
Looks good to me, so :)
Signed-off-by: Marc-André Lureau
thanks
>
> ---
> blockjob.c | 14 --
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/blockjob.c b/blockjob.c
> index ef3ed69ff1..c510a9fde5 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -204,6 +204,15 @@ void block_job_txn_add_job(BlockJobTxn *txn, BlockJob
> *job)
> block_job_txn_ref(txn);
> }
>
> +static void block_job_txn_del_job(BlockJob *job)
> +{
> +if (job->txn) {
> +QLIST_REMOVE(job, txn_list);
> +block_job_txn_unref(job->txn);
> +job->txn = NULL;
> +}
> +}
> +
> static void block_job_pause(BlockJob *job)
> {
> job->pause_count++;
> @@ -232,6 +241,7 @@ void block_job_unref(BlockJob *job)
> {
> if (--job->refcnt == 0) {
> assert(job->status == BLOCK_JOB_STATUS_NULL);
> +assert(!job->txn);
> BlockDriverState *bs = blk_bs(job->blk);
> QLIST_REMOVE(job, job_list);
> bs->job = NULL;
> @@ -392,6 +402,7 @@ static void block_job_decommission(BlockJob *job)
> job->busy = false;
> job->paused = false;
> job->deferred_to_main_loop = true;
> +block_job_txn_del_job(job);
> block_job_state_transition(job, BLOCK_JOB_STATUS_NULL);
> block_job_unref(job);
> }
> @@ -481,8 +492,7 @@ static int block_job_finalize_single(BlockJob *job)
> }
> }
>
> -QLIST_REMOVE(job, txn_list);
> -block_job_txn_unref(job->txn);
> +block_job_txn_del_job(job);
> block_job_conclude(job);
> return 0;
> }
> --
> 2.13.6
>
Re: [PATCH 04/10] chardev/char-fe: don't allow EAGAIN from blocking read
Hi
On Thu, Nov 11, 2021 at 7:44 PM Roman Kagan wrote:
> As its name suggests, ChardevClass.chr_sync_read is supposed to do a
> blocking read. The only implementation of it, tcp_chr_sync_read, does
> set the underlying io channel to the blocking mode indeed.
>
> Therefore a failure return with EAGAIN is not expected from this call.
>
> So do not retry it in qemu_chr_fe_read_all; instead place an assertion
> that it doesn't fail with EAGAIN.
>
The code was introduced in :
commit 7b0bfdf52d694c9a3a96505aa42ce3f8d63acd35
Author: Nikolay Nikolaev
Date: Tue May 27 15:03:48 2014 +0300
Add chardev API qemu_chr_fe_read_all
Also touched later by Daniel in:
commit 53628efbc8aa7a7ab5354d24b971f4d69452151d
Author: Daniel P. Berrangé
Date: Thu Mar 31 16:29:27 2016 +0100
char: fix broken EAGAIN retry on OS-X due to errno clobbering
> Signed-off-by: Roman Kagan
> ---
> chardev/char-fe.c | 7 ++-
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/chardev/char-fe.c b/chardev/char-fe.c
> index 7789f7be9c..f94efe928e 100644
> --- a/chardev/char-fe.c
> +++ b/chardev/char-fe.c
> @@ -68,13 +68,10 @@ int qemu_chr_fe_read_all(CharBackend *be, uint8_t
> *buf, int len)
> }
>
> while (offset < len) {
> -retry:
> res = CHARDEV_GET_CLASS(s)->chr_sync_read(s, buf + offset,
>len - offset);
> -if (res == -1 && errno == EAGAIN) {
> -g_usleep(100);
> -goto retry;
> -}
> +/* ->chr_sync_read should block */
> +assert(!(res < 0 && errno == EAGAIN));
>
>
While I agree with the rationale to clean this code a bit, I am not so sure
about replacing it with an assert(). In the past, when we did such things
we had unexpected regressions :)
A slightly better approach perhaps is g_warn_if_fail(), although it's not
very popular in qemu.
> if (res == 0) {
> break;
> --
> 2.33.1
>
>
>
--
Marc-André Lureau
Re: [PATCH 03/10] chardev/char-socket: tcp_chr_sync_read: don't clobber errno
On Thu, Nov 11, 2021 at 7:36 PM Roman Kagan wrote:
> After the return from tcp_chr_recv, tcp_chr_sync_read calls into a
> function which eventually makes a system call and may clobber errno.
>
> Make a copy of errno right after tcp_chr_recv and restore the errno on
> return from tcp_chr_sync_read.
>
> Signed-off-by: Roman Kagan
>
Reviewed-by: Marc-André Lureau
---
> chardev/char-socket.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 90054ce58c..cf7f2ba65a 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -581,6 +581,7 @@ static int tcp_chr_sync_read(Chardev *chr, const
> uint8_t *buf, int len)
> {
> SocketChardev *s = SOCKET_CHARDEV(chr);
> int size;
> +int saved_errno;
>
> if (s->state != TCP_CHARDEV_STATE_CONNECTED) {
> return 0;
> @@ -588,6 +589,7 @@ static int tcp_chr_sync_read(Chardev *chr, const
> uint8_t *buf, int len)
>
> qio_channel_set_blocking(s->ioc, true, NULL);
> size = tcp_chr_recv(chr, (void *) buf, len);
> +saved_errno = errno;
> if (s->state != TCP_CHARDEV_STATE_DISCONNECTED) {
> qio_channel_set_blocking(s->ioc, false, NULL);
> }
> @@ -596,6 +598,7 @@ static int tcp_chr_sync_read(Chardev *chr, const
> uint8_t *buf, int len)
> tcp_chr_disconnect(chr);
> }
>
> +errno = saved_errno;
> return size;
> }
>
> --
> 2.33.1
>
>
>
--
Marc-André Lureau
Re: [PATCH 02/10] chardev/char-socket: tcp_chr_recv: don't clobber errno
On Thu, Nov 11, 2021 at 7:38 PM Roman Kagan wrote:
> tcp_chr_recv communicates the specific error condition to the caller via
> errno. However, after setting it, it may call into some system calls or
> library functions which can clobber the errno.
>
> Avoid this by moving the errno assignment to the end of the function.
>
> Signed-off-by: Roman Kagan
>
Reviewed-by: Marc-André Lureau
---
> chardev/char-socket.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 836cfa0bc2..90054ce58c 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -346,13 +346,6 @@ static ssize_t tcp_chr_recv(Chardev *chr, char *buf,
> size_t len)
> NULL);
> }
>
> -if (ret == QIO_CHANNEL_ERR_BLOCK) {
> -errno = EAGAIN;
> -ret = -1;
> -} else if (ret == -1) {
> -errno = EIO;
> -}
> -
> if (msgfds_num) {
> /* close and clean read_msgfds */
> for (i = 0; i < s->read_msgfds_num; i++) {
> @@ -381,6 +374,13 @@ static ssize_t tcp_chr_recv(Chardev *chr, char *buf,
> size_t len)
> #endif
> }
>
> +if (ret == QIO_CHANNEL_ERR_BLOCK) {
> +errno = EAGAIN;
> +ret = -1;
> + } else if (ret == -1) {
> +errno = EIO;
> +}
> +
> return ret;
> }
>
> --
> 2.33.1
>
>
>
--
Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/3] migration: Replace strncpy() by strpadcpy(pad='\0')
On Tue, Dec 18, 2018 at 3:09 PM Philippe Mathieu-Daudé
wrote:
>
> GCC 8 added a -Wstringop-truncation warning:
>
> The -Wstringop-truncation warning added in GCC 8.0 via r254630 for
> bug 81117 is specifically intended to highlight likely unintended
> uses of the strncpy function that truncate the terminating NUL
> character from the source string.
>
> This new warning leads to compilation failures:
>
> CC migration/global_state.o
> qemu/migration/global_state.c: In function 'global_state_store_running':
> qemu/migration/global_state.c:45:5: error: 'strncpy' specified bound 100
> equals destination size [-Werror=stringop-truncation]
>strncpy((char *)global_state.runstate, state,
> sizeof(global_state.runstate));
>
> ^~~~
> make: *** [qemu/rules.mak:69: migration/global_state.o] Error 1
>
> The runstate name doesn't require the strings to be NUL-terminated,
> therefore strncpy is the right function to use here.
>
> We could add a #pragma GCC diagnostic ignored "-Wstringop-truncation"
> around, disable the warning globally using -Wno-stringop-truncation,
> but since QEMU provides the strpadcpy() which does the same purpose,
> simply use it to avoid the annoying warning.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> migration/global_state.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/migration/global_state.c b/migration/global_state.c
> index 8e8ab5c51e..c7e7618118 100644
> --- a/migration/global_state.c
> +++ b/migration/global_state.c
> @@ -42,8 +42,8 @@ int global_state_store(void)
> void global_state_store_running(void)
> {
> const char *state = RunState_str(RUN_STATE_RUNNING);
> -strncpy((char *)global_state.runstate,
> - state, sizeof(global_state.runstate));
> +strpadcpy((char *)global_state.runstate,
> + sizeof(global_state.runstate), state, '\0');
> }
>
> bool global_state_received(void)
> --
> 2.17.2
>
>
--
Marc-André Lureau
[Qemu-block] [PATCH v2 02/12] vhost-user: simplify vhost_user_init/vhost_user_cleanup
Take a VhostUserState* that can be pre-allocated, and initialize it
with the associated chardev.
Signed-off-by: Marc-André Lureau
Reviewed-by: Tiwei Bie
---
include/hw/virtio/vhost-user-blk.h | 2 +-
include/hw/virtio/vhost-user-scsi.h | 2 +-
include/hw/virtio/vhost-user.h | 2 +-
backends/cryptodev-vhost-user.c | 18 --
hw/block/vhost-user-blk.c | 22 --
hw/scsi/vhost-user-scsi.c | 20
hw/virtio/vhost-stub.c | 4 ++--
hw/virtio/vhost-user.c | 16
net/vhost-user.c| 13 -
9 files changed, 33 insertions(+), 66 deletions(-)
diff --git a/include/hw/virtio/vhost-user-blk.h
b/include/hw/virtio/vhost-user-blk.h
index d52944aeeb..a8a106eecb 100644
--- a/include/hw/virtio/vhost-user-blk.h
+++ b/include/hw/virtio/vhost-user-blk.h
@@ -36,7 +36,7 @@ typedef struct VHostUserBlk {
uint32_t queue_size;
uint32_t config_wce;
struct vhost_dev dev;
-VhostUserState *vhost_user;
+VhostUserState vhost_user;
} VHostUserBlk;
#endif
diff --git a/include/hw/virtio/vhost-user-scsi.h
b/include/hw/virtio/vhost-user-scsi.h
index e429cacd8e..738f9288bd 100644
--- a/include/hw/virtio/vhost-user-scsi.h
+++ b/include/hw/virtio/vhost-user-scsi.h
@@ -30,7 +30,7 @@
typedef struct VHostUserSCSI {
VHostSCSICommon parent_obj;
-VhostUserState *vhost_user;
+VhostUserState vhost_user;
} VHostUserSCSI;
#endif /* VHOST_USER_SCSI_H */
diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index fd660393a0..811e325f42 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -22,7 +22,7 @@ typedef struct VhostUserState {
VhostUserHostNotifier notifier[VIRTIO_QUEUE_MAX];
} VhostUserState;
-VhostUserState *vhost_user_init(void);
+bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp);
void vhost_user_cleanup(VhostUserState *user);
#endif
diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c
index d539f14d59..1052a5d0e9 100644
--- a/backends/cryptodev-vhost-user.c
+++ b/backends/cryptodev-vhost-user.c
@@ -47,7 +47,7 @@
typedef struct CryptoDevBackendVhostUser {
CryptoDevBackend parent_obj;
-VhostUserState *vhost_user;
+VhostUserState vhost_user;
CharBackend chr;
char *chr_name;
bool opened;
@@ -104,7 +104,7 @@ cryptodev_vhost_user_start(int queues,
continue;
}
-options.opaque = s->vhost_user;
+options.opaque = &s->vhost_user;
options.backend_type = VHOST_BACKEND_TYPE_USER;
options.cc = b->conf.peers.ccs[i];
s->vhost_crypto[i] = cryptodev_vhost_init(&options);
@@ -182,7 +182,6 @@ static void cryptodev_vhost_user_init(
size_t i;
Error *local_err = NULL;
Chardev *chr;
-VhostUserState *user;
CryptoDevBackendClient *cc;
CryptoDevBackendVhostUser *s =
CRYPTODEV_BACKEND_VHOST_USER(backend);
@@ -213,15 +212,10 @@ static void cryptodev_vhost_user_init(
}
}
-user = vhost_user_init();
-if (!user) {
-error_setg(errp, "Failed to init vhost_user");
+if (!vhost_user_init(&s->vhost_user, &s->chr, errp)) {
return;
}
-user->chr = &s->chr;
-s->vhost_user = user;
-
qemu_chr_fe_set_handlers(&s->chr, NULL, NULL,
cryptodev_vhost_user_event, NULL, s, NULL, true);
@@ -307,11 +301,7 @@ static void cryptodev_vhost_user_cleanup(
}
}
-if (s->vhost_user) {
-vhost_user_cleanup(s->vhost_user);
-g_free(s->vhost_user);
-s->vhost_user = NULL;
-}
+vhost_user_cleanup(&s->vhost_user);
}
static void cryptodev_vhost_user_set_chardev(Object *obj,
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 44ac814016..767c734a81 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -253,7 +253,6 @@ static void vhost_user_blk_device_realize(DeviceState *dev,
Error **errp)
{
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VHostUserBlk *s = VHOST_USER_BLK(vdev);
-VhostUserState *user;
struct vhost_virtqueue *vqs = NULL;
int i, ret;
@@ -272,15 +271,10 @@ static void vhost_user_blk_device_realize(DeviceState
*dev, Error **errp)
return;
}
-user = vhost_user_init();
-if (!user) {
-error_setg(errp, "vhost-user-blk: failed to init vhost_user");
+if (!vhost_user_init(&s->vhost_user, &s->chardev, errp)) {
return;
}
-user->chr = &s->chardev;
-s->vhost_user = user;
-
virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
sizeof(struct virtio_blk_config));
@@ -297,7 +291,7 @@ static void vhost_user_blk_device_realize(DeviceStat
[Qemu-block] [PATCH v3 03/13] vhost-user: simplify vhost_user_init/vhost_user_cleanup
Take a VhostUserState* that can be pre-allocated, and initialize it
with the associated chardev.
Signed-off-by: Marc-André Lureau
Reviewed-by: Tiwei Bie
---
include/hw/virtio/vhost-user-blk.h | 2 +-
include/hw/virtio/vhost-user-scsi.h | 2 +-
include/hw/virtio/vhost-user.h | 2 +-
backends/cryptodev-vhost-user.c | 18 --
hw/block/vhost-user-blk.c | 22 --
hw/scsi/vhost-user-scsi.c | 20
hw/virtio/vhost-stub.c | 4 ++--
hw/virtio/vhost-user.c | 16
net/vhost-user.c| 13 -
9 files changed, 33 insertions(+), 66 deletions(-)
diff --git a/include/hw/virtio/vhost-user-blk.h
b/include/hw/virtio/vhost-user-blk.h
index d52944aeeb..a8a106eecb 100644
--- a/include/hw/virtio/vhost-user-blk.h
+++ b/include/hw/virtio/vhost-user-blk.h
@@ -36,7 +36,7 @@ typedef struct VHostUserBlk {
uint32_t queue_size;
uint32_t config_wce;
struct vhost_dev dev;
-VhostUserState *vhost_user;
+VhostUserState vhost_user;
} VHostUserBlk;
#endif
diff --git a/include/hw/virtio/vhost-user-scsi.h
b/include/hw/virtio/vhost-user-scsi.h
index e429cacd8e..738f9288bd 100644
--- a/include/hw/virtio/vhost-user-scsi.h
+++ b/include/hw/virtio/vhost-user-scsi.h
@@ -30,7 +30,7 @@
typedef struct VHostUserSCSI {
VHostSCSICommon parent_obj;
-VhostUserState *vhost_user;
+VhostUserState vhost_user;
} VHostUserSCSI;
#endif /* VHOST_USER_SCSI_H */
diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index fd660393a0..811e325f42 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -22,7 +22,7 @@ typedef struct VhostUserState {
VhostUserHostNotifier notifier[VIRTIO_QUEUE_MAX];
} VhostUserState;
-VhostUserState *vhost_user_init(void);
+bool vhost_user_init(VhostUserState *user, CharBackend *chr, Error **errp);
void vhost_user_cleanup(VhostUserState *user);
#endif
diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c
index d539f14d59..1052a5d0e9 100644
--- a/backends/cryptodev-vhost-user.c
+++ b/backends/cryptodev-vhost-user.c
@@ -47,7 +47,7 @@
typedef struct CryptoDevBackendVhostUser {
CryptoDevBackend parent_obj;
-VhostUserState *vhost_user;
+VhostUserState vhost_user;
CharBackend chr;
char *chr_name;
bool opened;
@@ -104,7 +104,7 @@ cryptodev_vhost_user_start(int queues,
continue;
}
-options.opaque = s->vhost_user;
+options.opaque = &s->vhost_user;
options.backend_type = VHOST_BACKEND_TYPE_USER;
options.cc = b->conf.peers.ccs[i];
s->vhost_crypto[i] = cryptodev_vhost_init(&options);
@@ -182,7 +182,6 @@ static void cryptodev_vhost_user_init(
size_t i;
Error *local_err = NULL;
Chardev *chr;
-VhostUserState *user;
CryptoDevBackendClient *cc;
CryptoDevBackendVhostUser *s =
CRYPTODEV_BACKEND_VHOST_USER(backend);
@@ -213,15 +212,10 @@ static void cryptodev_vhost_user_init(
}
}
-user = vhost_user_init();
-if (!user) {
-error_setg(errp, "Failed to init vhost_user");
+if (!vhost_user_init(&s->vhost_user, &s->chr, errp)) {
return;
}
-user->chr = &s->chr;
-s->vhost_user = user;
-
qemu_chr_fe_set_handlers(&s->chr, NULL, NULL,
cryptodev_vhost_user_event, NULL, s, NULL, true);
@@ -307,11 +301,7 @@ static void cryptodev_vhost_user_cleanup(
}
}
-if (s->vhost_user) {
-vhost_user_cleanup(s->vhost_user);
-g_free(s->vhost_user);
-s->vhost_user = NULL;
-}
+vhost_user_cleanup(&s->vhost_user);
}
static void cryptodev_vhost_user_set_chardev(Object *obj,
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 44ac814016..767c734a81 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -253,7 +253,6 @@ static void vhost_user_blk_device_realize(DeviceState *dev,
Error **errp)
{
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VHostUserBlk *s = VHOST_USER_BLK(vdev);
-VhostUserState *user;
struct vhost_virtqueue *vqs = NULL;
int i, ret;
@@ -272,15 +271,10 @@ static void vhost_user_blk_device_realize(DeviceState
*dev, Error **errp)
return;
}
-user = vhost_user_init();
-if (!user) {
-error_setg(errp, "vhost-user-blk: failed to init vhost_user");
+if (!vhost_user_init(&s->vhost_user, &s->chardev, errp)) {
return;
}
-user->chr = &s->chardev;
-s->vhost_user = user;
-
virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
sizeof(struct virtio_blk_config));
@@ -297,7 +291,7 @@ static void vhost_user_blk_device_realize(DeviceStat
Re: [Qemu-block] [PATCH v4 01/17] util: add helper APIs for dealing with inotify in portable manner
Hi
On Fri, Feb 15, 2019 at 6:13 PM Daniel P. Berrangé wrote:
>
> The inotify userspace API for reading events is quite horrible, so it is
> useful to wrap it in a more friendly API to avoid duplicating code
> across many users in QEMU. Wrapping it also allows introduction of a
> platform portability layer, so that we can add impls for non-Linux based
> equivalents in future.
>
> Signed-off-by: Daniel P. Berrangé
Reviewed-by: Marc-André Lureau
> ---
> MAINTAINERS | 7 +
> Makefile.objs | 2 +-
> include/qemu/filemonitor.h| 128 +++
> tests/Makefile.include| 3 +
> tests/test-util-filemonitor.c | 685 ++
> util/Makefile.objs| 3 +
> util/filemonitor-inotify.c| 338 +
> util/filemonitor-stub.c | 59 +++
> util/trace-events | 9 +
> 9 files changed, 1233 insertions(+), 1 deletion(-)
> create mode 100644 include/qemu/filemonitor.h
> create mode 100644 tests/test-util-filemonitor.c
> create mode 100644 util/filemonitor-inotify.c
> create mode 100644 util/filemonitor-stub.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ffb029f63a..5989796fa9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2080,6 +2080,13 @@ F: include/qemu/sockets.h
> F: util/qemu-sockets.c
> F: qapi/sockets.json
>
> +File monitor
> +M: Daniel P. Berrange
> +S: Odd fixes
> +F: util/filemonitor*.c
> +F: include/qemu/filemonitor.h
> +F: tests/test-util-filemonitor.c
> +
> Throttling infrastructure
> M: Alberto Garcia
> S: Supported
> diff --git a/Makefile.objs b/Makefile.objs
> index b7aae33367..fee0ce9fc5 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -4,7 +4,7 @@ QAPI_MODULES += ui
>
> ###
> # Common libraries for tools and emulators
> -stub-obj-y = stubs/ crypto/
> +stub-obj-y = stubs/ util/ crypto/
> util-obj-y = util/ qobject/ qapi/
> util-obj-y += qapi/qapi-builtin-types.o
> util-obj-y += qapi/qapi-types.o
> diff --git a/include/qemu/filemonitor.h b/include/qemu/filemonitor.h
> new file mode 100644
> index 00..cd031832ed
> --- /dev/null
> +++ b/include/qemu/filemonitor.h
> @@ -0,0 +1,128 @@
> +/*
> + * QEMU file monitor helper
> + *
> + * Copyright (c) 2018 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see
> <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#ifndef QEMU_FILE_MONITOR_H
> +#define QEMU_FILE_MONITOR_H
> +
> +#include "qemu-common.h"
> +
> +
> +typedef struct QFileMonitor QFileMonitor;
> +
> +typedef enum {
> +/* File has been created in a dir */
> +QFILE_MONITOR_EVENT_CREATED,
> +/* File has been modified in a dir */
> +QFILE_MONITOR_EVENT_MODIFIED,
> +/* File has been deleted in a dir */
> +QFILE_MONITOR_EVENT_DELETED,
> +/* File has attributes changed */
> +QFILE_MONITOR_EVENT_ATTRIBUTES,
> +/* Dir is no longer being monitored (due to deletion) */
> +QFILE_MONITOR_EVENT_IGNORED,
> +} QFileMonitorEvent;
> +
> +
> +/**
> + * QFileMonitorHandler:
> + * @id: id from qemu_file_monitor_add_watch()
> + * @event: the file change that occurred
> + * @filename: the name of the file affected
> + * @opaque: opaque data provided to qemu_file_monitor_add_watch()
> + *
> + * Invoked whenever a file changes. If @event is
> + * QFILE_MONITOR_EVENT_IGNORED, @filename will be
> + * empty.
> + *
> + */
> +typedef void (*QFileMonitorHandler)(int id,
> +QFileMonitorEvent event,
> +const char *filename,
> +void *opaque);
> +
> +/**
> + * qemu_file_monitor_new:
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Create a handle for a file monitoring object.
> + *
> + * This object does locking internally to enable it to be
> + * safe to use from multiple threads
> + *
> + * If the platform does not
Re: [Qemu-block] [RFC PATCH 1/6] qdev: Fix latent bug with compat_props and onboard devices
Hi
On Mon, Feb 25, 2019 at 7:38 PM Markus Armbruster wrote:
>
> Compatibility properties started life as a qdev property thing: we
> supported them only for qdev properties, and implemented them with the
> machinery backing command line option -global.
>
> Recent commit fa0cb34d221 put them to use (tacitly) with memory
> backend objects (subtypes of TYPE_MEMORY_BACKEND). To make that
> possible, we first moved the work of applying them from the -global
> machinery into TYPE_DEVICE's .instance_post_init() method
> device_post_init(), in commits ea9ce8934c5 and b66bbee39f6, then made
> it available to TYPE_MEMORY_BACKEND's .instance_post_init() method
> host_memory_backend_post_init() as object_apply_compat_props(), in
> commit 1c3994f6d2a.
>
> Note the code smell: we now have function name starting with object_
> in hw/core/qdev.c. It has to be there rather than in qom/, because it
> calls qdev_get_machine() to find the current accelerator's and
> machine's compat_props.
>
> Turns out calling qdev_get_machine() there is problematic. If we
> qdev_create() from a machine's .instance_init() method, we call
> device_post_init() and thus qdev_get_machine() before main() can
> create "/machine" in QOM. qdev_get_machine() tries to get it with
> container_get(), which "helpfully" creates it as "container" object,
> and returns that. object_apply_compat_props() tries to paper over the
> problem by doing nothing when the value of qdev_get_machine() isn't a
> TYPE_MACHINE. But the damage is done already: when main() later
> attempts to create the real "/machine", it fails with "attempt to add
> duplicate property 'machine' to object (type 'container')", and
> aborts.
>
> Since no machine .instance_init() calls qdev_create() so far, the bug
> is latent. But since I want to do that, I get to fix the bug first.
>
> Observe that object_apply_compat_props() doesn't actually need the
> MachineState, only its the compat_props member of its MachineClass and
> AccelClass. This permits a simple fix: register MachineClass and
> AccelClass compat_props with the object_apply_compat_props() machinery
> right after these classes get selected.
>
> This is actually similar to how things worked before commits
> ea9ce8934c5 and b66bbee39f6, except we now register much earlier. The
> old code registered them only after the machine's .instance_init()
> ran, which would've broken compatibility properties for any devices
> created there.
>
> Cc: Marc-André Lureau
> Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
> ---
> accel/accel.c | 1 +
> hw/core/qdev.c | 48 --
> include/hw/qdev-core.h | 2 ++
> vl.c | 1 +
> 4 files changed, 41 insertions(+), 11 deletions(-)
>
> diff --git a/accel/accel.c b/accel/accel.c
> index 68b6d56323..4a1670e404 100644
> --- a/accel/accel.c
> +++ b/accel/accel.c
> @@ -66,6 +66,7 @@ static int accel_init_machine(AccelClass *acc, MachineState
> *ms)
> *(acc->allowed) = false;
> object_unref(OBJECT(accel));
> }
> +object_set_accelerator_compat_props(acc->compat_props);
> return ret;
> }
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index d59071b8ed..1a86c7990a 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -970,25 +970,51 @@ static void device_initfn(Object *obj)
> QLIST_INIT(&dev->gpios);
> }
>
> +/*
> + * Global property defaults
> + * Slot 0: accelerator's global property defaults
> + * Slot 1: machine's global property defaults
> + * Each is a GPtrArray of of GlobalProperty.
> + * Applied in order, later entries override earlier ones.
> + */
> +static GPtrArray *object_compat_props[2];
> +
> +/*
> + * Set machine's global property defaults to @compat_props.
> + * May be called at most once.
> + */
> +void object_set_machine_compat_props(GPtrArray *compat_props)
> +{
> +assert(!object_compat_props[1]);
> +object_compat_props[1] = compat_props;
> +}
> +
> +/*
> + * Set accelerator's global property defaults to @compat_props.
> + * May be called at most once.
> + */
> +void object_set_accelerator_compat_props(GPtrArray *compat_props)
> +{
> +assert(!object_compat_props[0]);
> +object_compat_props[0] = compat_props;
> +}
> +
> void object_apply_compat_props(Object *obj)
> {
> -if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
> -MachineState *m = MACHINE(qdev_get_machine());
> -MachineClass *mc = MACHINE_GET_CLASS(
Re: [Qemu-block] [Qemu-devel] [RFC PATCH 2/6] qom: Move compat_props machinery from qdev to QOM
Hi
On Mon, Feb 25, 2019 at 7:46 PM Markus Armbruster wrote:
>
> See the previous commit for rationale.
>
> Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
> ---
> hw/core/qdev.c | 39 ---
> include/hw/qdev-core.h | 4
> include/qom/object.h | 3 +++
> qom/object.c | 39 +++
> 4 files changed, 42 insertions(+), 43 deletions(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 1a86c7990a..25dffad3ed 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -970,45 +970,6 @@ static void device_initfn(Object *obj)
> QLIST_INIT(&dev->gpios);
> }
>
> -/*
> - * Global property defaults
> - * Slot 0: accelerator's global property defaults
> - * Slot 1: machine's global property defaults
> - * Each is a GPtrArray of of GlobalProperty.
> - * Applied in order, later entries override earlier ones.
> - */
> -static GPtrArray *object_compat_props[2];
> -
> -/*
> - * Set machine's global property defaults to @compat_props.
> - * May be called at most once.
> - */
> -void object_set_machine_compat_props(GPtrArray *compat_props)
> -{
> -assert(!object_compat_props[1]);
> -object_compat_props[1] = compat_props;
> -}
> -
> -/*
> - * Set accelerator's global property defaults to @compat_props.
> - * May be called at most once.
> - */
> -void object_set_accelerator_compat_props(GPtrArray *compat_props)
> -{
> -assert(!object_compat_props[0]);
> -object_compat_props[0] = compat_props;
> -}
> -
> -void object_apply_compat_props(Object *obj)
> -{
> -int i;
> -
> -for (i = 0; i < ARRAY_SIZE(object_compat_props); i++) {
> -object_apply_global_props(obj, object_compat_props[i],
> - &error_abort);
> -}
> -}
> -
> static void device_post_init(Object *obj)
> {
> /*
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index bced1f2666..f2f0006234 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -418,10 +418,6 @@ const char *qdev_fw_name(DeviceState *dev);
>
> Object *qdev_get_machine(void);
>
> -void object_set_machine_compat_props(GPtrArray *compat_props);
> -void object_set_accelerator_compat_props(GPtrArray *compat_props);
> -void object_apply_compat_props(Object *obj);
> -
> /* FIXME: make this a link<> */
> void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index e0262962b5..288cdddf44 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -677,6 +677,9 @@ Object *object_new_with_propv(const char *typename,
>
> void object_apply_global_props(Object *obj, const GPtrArray *props,
> Error **errp);
> +void object_set_machine_compat_props(GPtrArray *compat_props);
> +void object_set_accelerator_compat_props(GPtrArray *compat_props);
> +void object_apply_compat_props(Object *obj);
>
> /**
> * object_set_props:
> diff --git a/qom/object.c b/qom/object.c
> index b8c732063b..adb9b7fe91 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -408,6 +408,45 @@ void object_apply_global_props(Object *obj, const
> GPtrArray *props, Error **errp
> }
> }
>
> +/*
> + * Global property defaults
> + * Slot 0: accelerator's global property defaults
> + * Slot 1: machine's global property defaults
> + * Each is a GPtrArray of of GlobalProperty.
> + * Applied in order, later entries override earlier ones.
> + */
> +static GPtrArray *object_compat_props[2];
> +
> +/*
> + * Set machine's global property defaults to @compat_props.
> + * May be called at most once.
> + */
> +void object_set_machine_compat_props(GPtrArray *compat_props)
> +{
> +assert(!object_compat_props[1]);
> +object_compat_props[1] = compat_props;
> +}
> +
> +/*
> + * Set accelerator's global property defaults to @compat_props.
> + * May be called at most once.
> + */
> +void object_set_accelerator_compat_props(GPtrArray *compat_props)
> +{
> +assert(!object_compat_props[0]);
> +object_compat_props[0] = compat_props;
> +}
> +
> +void object_apply_compat_props(Object *obj)
> +{
> +int i;
> +
> +for (i = 0; i < ARRAY_SIZE(object_compat_props); i++) {
> +object_apply_global_props(obj, object_compat_props[i],
> + &error_abort);
> +}
> +}
> +
> static void object_initialize_with_type(void *data, size_t size, TypeImpl
> *type)
> {
> Object *obj = data;
> --
> 2.17.2
>
>
--
Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [RFC PATCH 3/6] vl: Fix latent bug with -global and onboard devices
On Mon, Feb 25, 2019 at 7:41 PM Markus Armbruster wrote:
>
> main() registers the user's -global only after we create the machine
> object, i.e. too late for devices created in the machine's
> .instance_init().
>
> Fortunately, we know the bug is only latent: the commit before
> previous fixed a bug that would've crashed any attempt to create a
> device in an .instance_init().
>
> Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
> ---
> vl.c | 19 ++-
> 1 file changed, 2 insertions(+), 17 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index c50c2d6178..e3fdce410f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2939,17 +2939,6 @@ static void user_register_global_props(void)
>global_init_func, NULL, NULL);
> }
>
> -/*
> - * Note: we should see that these properties are actually having a
> - * priority: accel < machine < user. This means e.g. when user
> - * specifies something in "-global", it'll always be used with highest
> - * priority than either machine/accelerator compat properties.
> - */
> -static void register_global_properties(MachineState *ms)
> -{
> -user_register_global_props();
> -}
> -
> int main(int argc, char **argv, char **envp)
> {
> int i;
> @@ -3943,6 +3932,8 @@ int main(int argc, char **argv, char **envp)
> */
> loc_set_none();
>
> +user_register_global_props();
> +
> replay_configure(icount_opts);
>
> if (incoming && !preconfig_exit_requested) {
> @@ -4248,12 +4239,6 @@ int main(int argc, char **argv, char **envp)
> machine_class->name, machine_class->deprecation_reason);
> }
>
> -/*
> - * Register all the global properties, including accel properties,
> - * machine properties, and user-specified ones.
> - */
> -register_global_properties(current_machine);
> -
> /*
> * Migration object can only be created after global properties
> * are applied correctly.
> --
> 2.17.2
>
>
--
Marc-André Lureau
Re: [Qemu-block] [Qemu-devel] [RFC PATCH 4/6] sysbus: Fix latent bug with onboard devices
Hi
On Mon, Feb 25, 2019 at 7:40 PM Markus Armbruster wrote:
>
> The first call of sysbus_get_default() creates the main system bus and
> stores it in QOM as "/machine/unattached/sysbus". This must not
> happen before main() creates "/machine", or else container_get() would
> "helpfully" create it as "container" object, and the real creation of
> "/machine" would later abort with "attempt to add duplicate property
> 'machine' to object (type 'container')". Has been that way ever since
> we wired up busses in QOM (commit f968fc6892d, v1.2.0).
>
> I believe the bug is latent. I got it to bite by trying to
> qdev_create() a sysbus device from a machine's .instance_init()
> method.
>
> The fix is obvious: store the main system bus in QOM right after
> creating "/machine".
>
> Signed-off-by: Markus Armbruster
makes sense to me, but I might miss some subtleties..
Reviewed-by: Marc-André Lureau
> ---
> hw/core/sysbus.c | 3 ---
> vl.c | 4
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index 9f9edbcab9..307cf90a51 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -357,9 +357,6 @@ static void main_system_bus_create(void)
> qbus_create_inplace(main_system_bus, system_bus_info.instance_size,
> TYPE_SYSTEM_BUS, NULL, "main-system-bus");
> OBJECT(main_system_bus)->free = g_free;
> -object_property_add_child(container_get(qdev_get_machine(),
> -"/unattached"),
> - "sysbus", OBJECT(main_system_bus), NULL);
> }
>
> BusState *sysbus_get_default(void)
> diff --git a/vl.c b/vl.c
> index e3fdce410f..6ce3d2d448 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3990,6 +3990,10 @@ int main(int argc, char **argv, char **envp)
> }
> object_property_add_child(object_get_root(), "machine",
>OBJECT(current_machine), &error_abort);
> +object_property_add_child(container_get(OBJECT(current_machine),
> +"/unattached"),
> + "sysbus", OBJECT(sysbus_get_default()),
> + NULL);
>
> if (machine_class->minimum_page_bits) {
> if
> (!set_preferred_target_page_bits(machine_class->minimum_page_bits)) {
> --
> 2.17.2
>
>
--
Marc-André Lureau
[Qemu-block] [PATCH] nbd: fix uninitialized variable warning
../block/nbd.c: In function 'nbd_co_request':
../block/nbd.c:745:8: error: 'local_reply.type' may be used uninitialized in
this function [-Werror=maybe-uninitialized]
if (chunk->type == NBD_REPLY_TYPE_NONE) {
^
../block/nbd.c:710:14: note: 'local_reply.type' was declared here
NBDReply local_reply;
^~~
../block/nbd.c:710:14: error: 'local_reply.flags' may be used uninitialized in
this function [-Werror=maybe-uninitialized]
../block/nbd.c:738:8: error: 'local_reply..magic' may be used
uninitialized in this function [-Werror=maybe-uninitialized]
if (nbd_reply_is_simple(reply) || s->quit) {
^
../block/nbd.c:710:14: note: 'local_reply..magic' was declared here
NBDReply local_reply;
^~~~~~~
cc1: all warnings being treated as errors
Signed-off-by: Marc-André Lureau
---
block/nbd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/nbd.c b/block/nbd.c
index 81edabbf35..02eef09728 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -707,7 +707,7 @@ static bool nbd_reply_chunk_iter_receive(BDRVNBDState *s,
void **payload)
{
int ret, request_ret;
-NBDReply local_reply;
+NBDReply local_reply = { 0, };
NBDStructuredReplyChunk *chunk;
Error *local_err = NULL;
if (s->quit) {
--
2.22.0.428.g6d5b264208
Re: [Qemu-block] [Qemu-devel] [PATCH] nbd: fix uninitialized variable warning
Hi
On Tue, Jul 16, 2019 at 1:19 PM Philippe Mathieu-Daudé
wrote:
>
> Hi Marc-André,
>
> On 7/16/19 10:42 AM, Marc-André Lureau wrote:
> > ../block/nbd.c: In function 'nbd_co_request':
> > ../block/nbd.c:745:8: error: 'local_reply.type' may be used uninitialized
> > in this function [-Werror=maybe-uninitialized]
> > if (chunk->type == NBD_REPLY_TYPE_NONE) {
> > ^
> > ../block/nbd.c:710:14: note: 'local_reply.type' was declared here
> > NBDReply local_reply;
> > ^~~
> > ../block/nbd.c:710:14: error: 'local_reply.flags' may be used uninitialized
> > in this function [-Werror=maybe-uninitialized]
> > ../block/nbd.c:738:8: error: 'local_reply..magic' may be used
> > uninitialized in this function [-Werror=maybe-uninitialized]
> > if (nbd_reply_is_simple(reply) || s->quit) {
> > ^
> > ../block/nbd.c:710:14: note: 'local_reply..magic' was declared here
> > NBDReply local_reply;
> > ^~~
> > cc1: all warnings being treated as errors
>
> Thomas reported this error when compiling with -O3 few months ago [1].
Thanks, I couldn't find a previous report.
> Are you using that, or the latest GCC emit a warning even with no -O3?
Right, the warning seems to appear with -O3 (I happen to have it with
mingw64-gcc-8.3.0-2.fc30.x86_64)
>
> Personally I'd add:
>
> Fixes: 86f8cdf3db8
Actually, it was there before, so I'll skip that.
> Reported-by: Thomas Huth
>
> > Signed-off-by: Marc-André Lureau
> > ---
> > block/nbd.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/block/nbd.c b/block/nbd.c
> > index 81edabbf35..02eef09728 100644
> > --- a/block/nbd.c
> > +++ b/block/nbd.c
> > @@ -707,7 +707,7 @@ static bool nbd_reply_chunk_iter_receive(BDRVNBDState
> > *s,
> > void **payload)
> > {
> > int ret, request_ret;
> > -NBDReply local_reply;
> > +NBDReply local_reply = { 0, };
>
> Yesterday [2] Peter said: "= {}" is our standard struct-zero-initializer
> so we should prefer that.
>
> With {}:
> Reviewed-by: Philippe Mathieu-Daudé
ok, thanks
>
> > NBDStructuredReplyChunk *chunk;
> > Error *local_err = NULL;
> > if (s->quit) {
> >
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg07007.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg03431.html
