From: Zenghui Yu <yuzenghui@huawei.com>

Quote docs/devel/style.rst (section "Automatic memory deallocation"):

* Variables declared with g_auto* MUST always be initialized,

otherwise the cleanup function will use uninitialized stack memory

Initialize @name properly to get rid of the compilation error (using

gcc-7.3.0 on CentOS):

../hw/remote/proxy.c: In function 'pci_proxy_dev_realize':

/usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: 'name' may be used uninitialized in this function [-Werror=maybe-uninitialized]

g_free (*pp);

^~~~~~~~~~~~

../hw/remote/proxy.c:350:30: note: 'name' was declared here

g_autofree char *name;

^~~~

Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>

Reviewed-by: Jagannathan Raman <jag.raman@oracle.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com>

Message-id: 20210312112143.1369-1-yuzenghui@huawei.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

hw/remote/memory.c | 5 ++---

hw/remote/proxy.c | 3 +--

2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/remote/memory.c b/hw/remote/memory.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/remote/memory.c

+++ b/hw/remote/memory.c

@@ -XXX,XX +XXX,XX @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp)

remote_sysmem_reset();

- for (region = 0; region < msg->num_fds; region++) {

- g_autofree char *name;

+ for (region = 0; region < msg->num_fds; region++, suffix++) {

+ g_autofree char *name = g_strdup_printf("remote-mem-%u", suffix);

subregion = g_new(MemoryRegion, 1);

- name = g_strdup_printf("remote-mem-%u", suffix++);

memory_region_init_ram_from_fd(subregion, NULL,

name, sysmem_info->sizes[region],

true, msg->fds[region],

diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/remote/proxy.c

+++ b/hw/remote/proxy.c

@@ -XXX,XX +XXX,XX @@ static void probe_pci_info(PCIDevice *dev, Error **errp)

PCI_BASE_ADDRESS_SPACE_IO : PCI_BASE_ADDRESS_SPACE_MEMORY;

if (size) {

- g_autofree char *name;

+ g_autofree char *name = g_strdup_printf("bar-region-%d", i);

pdev->region[i].dev = pdev;

pdev->region[i].present = true;

if (type == PCI_BASE_ADDRESS_SPACE_MEMORY) {

pdev->region[i].memory = true;

}

- name = g_strdup_printf("bar-region-%d", i);

memory_region_init_io(&pdev->region[i].mr, OBJECT(pdev),

&proxy_mr_ops, &pdev->region[i],

name, size);

--

2.31.1

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Document the following functions return the bitmap size

if no matching bit is found:

- find_first_bit

- find_next_bit

- find_last_bit

- find_first_zero_bit

- find_next_zero_bit

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Message-id: 20210510200758.2623154-2-philmd@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

include/qemu/bitops.h | 15 ++++++++++++---

1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h

index XXXXXXX..XXXXXXX 100644

--- a/include/qemu/bitops.h

+++ b/include/qemu/bitops.h

@@ -XXX,XX +XXX,XX @@ static inline int test_bit(long nr, const unsigned long *addr)

* @addr: The address to start the search at

* @size: The maximum size to search

*

- * Returns the bit number of the first set bit, or size.

+ * Returns the bit number of the last set bit,

+ * or @size if there is no set bit in the bitmap.

*/

unsigned long find_last_bit(const unsigned long *addr,

unsigned long size);

@@ -XXX,XX +XXX,XX @@ unsigned long find_last_bit(const unsigned long *addr,

* @addr: The address to base the search on

* @offset: The bitnumber to start searching at

* @size: The bitmap size in bits

+ *

+ * Returns the bit number of the next set bit,

+ * or @size if there are no further set bits in the bitmap.

*/

unsigned long find_next_bit(const unsigned long *addr,

unsigned long size,

@@ -XXX,XX +XXX,XX @@ unsigned long find_next_bit(const unsigned long *addr,

* @addr: The address to base the search on

* @offset: The bitnumber to start searching at

* @size: The bitmap size in bits

+ *

+ * Returns the bit number of the next cleared bit,

+ * or @size if there are no further clear bits in the bitmap.

*/

unsigned long find_next_zero_bit(const unsigned long *addr,

@@ -XXX,XX +XXX,XX @@ unsigned long find_next_zero_bit(const unsigned long *addr,

* @addr: The address to start the search at

* @size: The maximum size to search

*

- * Returns the bit number of the first set bit.

+ * Returns the bit number of the first set bit,

+ * or @size if there is no set bit in the bitmap.

*/

static inline unsigned long find_first_bit(const unsigned long *addr,

unsigned long size)

@@ -XXX,XX +XXX,XX @@ static inline unsigned long find_first_bit(const unsigned long *addr,

* @addr: The address to start the search at

* @size: The maximum size to search

*

- * Returns the bit number of the first cleared bit.

+ * Returns the bit number of the first cleared bit,

+ * or @size if there is no clear bit in the bitmap.

*/

static inline unsigned long find_first_zero_bit(const unsigned long *addr,

unsigned long size)

--

2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

The lifetime of the timer is well-known (it cannot outlive

qemu_co_sleep_ns_wakeable, because it's deleted by the time the

coroutine resumes), so it is not necessary to place it on the heap.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Message-id: 20210517100548.28806-2-pbonzini@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

util/qemu-coroutine-sleep.c | 9 ++++-----

1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c

index XXXXXXX..XXXXXXX 100644

--- a/util/qemu-coroutine-sleep.c

+++ b/util/qemu-coroutine-sleep.c

@@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";

struct QemuCoSleepState {

Coroutine *co;

- QEMUTimer *ts;

+ QEMUTimer ts;

QemuCoSleepState **user_state_pointer;

};

@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)

if (sleep_state->user_state_pointer) {

*sleep_state->user_state_pointer = NULL;

}

- timer_del(sleep_state->ts);

+ timer_del(&sleep_state->ts);

aio_co_wake(sleep_state->co);

}

@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,

AioContext *ctx = qemu_get_current_aio_context();

QemuCoSleepState state = {

.co = qemu_coroutine_self(),

- .ts = aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, &state),

.user_state_pointer = sleep_state,

};

@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,

abort();

}

+ aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);

if (sleep_state) {

*sleep_state = &state;

}

- timer_mod(state.ts, qemu_clock_get_ns(type) + ns);

+ timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);

qemu_coroutine_yield();

if (sleep_state) {

/*

@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,

*/

assert(*sleep_state == NULL);

}

- timer_free(state.ts);

}

--

2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

Simplify the code by removing conditionals. qemu_co_sleep_ns

can simply point the argument to an on-stack temporary.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Message-id: 20210517100548.28806-3-pbonzini@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

include/qemu/coroutine.h | 5 +++--

util/qemu-coroutine-sleep.c | 18 +++++-------------

2 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h

index XXXXXXX..XXXXXXX 100644

--- a/include/qemu/coroutine.h

+++ b/include/qemu/coroutine.h

@@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleepState QemuCoSleepState;

/**

* Yield the coroutine for a given duration. During this yield, @sleep_state

- * (if not NULL) is set to an opaque pointer, which may be used for

+ * is set to an opaque pointer, which may be used for

* qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the

* timer fires. Don't save the obtained value to other variables and don't call

* qemu_co_sleep_wake from another aio context.

@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,

QemuCoSleepState **sleep_state);

static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)

{

- qemu_co_sleep_ns_wakeable(type, ns, NULL);

+ QemuCoSleepState *unused = NULL;

+ qemu_co_sleep_ns_wakeable(type, ns, &unused);

}

/**

diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c

index XXXXXXX..XXXXXXX 100644

--- a/util/qemu-coroutine-sleep.c

+++ b/util/qemu-coroutine-sleep.c

@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)

qemu_co_sleep_ns__scheduled, NULL);

assert(scheduled == qemu_co_sleep_ns__scheduled);

- if (sleep_state->user_state_pointer) {

- *sleep_state->user_state_pointer = NULL;

- }

+ *sleep_state->user_state_pointer = NULL;

timer_del(&sleep_state->ts);

aio_co_wake(sleep_state->co);

}

@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,

}

aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state);

- if (sleep_state) {

- *sleep_state = &state;

- }

+ *sleep_state = &state;

timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);

qemu_coroutine_yield();

- if (sleep_state) {

- /*

- * Note that *sleep_state is cleared during qemu_co_sleep_wake

- * before resuming this coroutine.

- */

- assert(*sleep_state == NULL);

- }

+

+ /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */

+ assert(*sleep_state == NULL);

}

--

2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

This simplification is enabled by the previous patch. Now aio_co_wake

will only be called once, therefore we do not care about a spurious

firing of the timer after a qemu_co_sleep_wake.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Message-id: 20210517100548.28806-5-pbonzini@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

util/qemu-coroutine-sleep.c | 8 ++++----

1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c

index XXXXXXX..XXXXXXX 100644

--- a/util/qemu-coroutine-sleep.c

+++ b/util/qemu-coroutine-sleep.c

@@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns";

struct QemuCoSleepState {

Coroutine *co;

- QEMUTimer ts;

QemuCoSleepState **user_state_pointer;

};

@@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state)

assert(scheduled == qemu_co_sleep_ns__scheduled);

*sleep_state->user_state_pointer = NULL;

- timer_del(&sleep_state->ts);

aio_co_wake(sleep_state->co);

}

}

@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,

QemuCoSleepState **sleep_state)

{

AioContext *ctx = qemu_get_current_aio_context();

+ QEMUTimer ts;

QemuCoSleepState state = {

.co = qemu_coroutine_self(),

.user_state_pointer = sleep_state,

@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns,

abort();

}

- aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state);

+ aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state);

*sleep_state = &state;

- timer_mod(&state.ts, qemu_clock_get_ns(type) + ns);

+ timer_mod(&ts, qemu_clock_get_ns(type) + ns);

qemu_coroutine_yield();

+ timer_del(&ts);

/* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */

assert(*sleep_state == NULL);

--

2.31.1

From: Paolo Bonzini <pbonzini@redhat.com>

Allow using QemuCoSleep to sleep forever until woken by qemu_co_sleep_wake.

This makes the logic of qemu_co_sleep_ns_wakeable easy to understand.

In the future we will introduce an API that can work even if the

sleep and wake happen from different threads. For now, initializing

w->to_wake after timer_mod is fine because the timer can only fire in

the same AioContext.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Message-id: 20210517100548.28806-7-pbonzini@redhat.com

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

---

include/qemu/coroutine.h | 5 +++++

util/qemu-coroutine-sleep.c | 26 +++++++++++++++++++-------

2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h

index XXXXXXX..XXXXXXX 100644

--- a/include/qemu/coroutine.h

+++ b/include/qemu/coroutine.h

@@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleep {

void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,

QEMUClockType type, int64_t ns);

+/**

+ * Yield the coroutine until the next call to qemu_co_sleep_wake.

+ */

+void coroutine_fn qemu_co_sleep(QemuCoSleep *w);

+

static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns)

{

QemuCoSleep w = { 0 };

diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c

index XXXXXXX..XXXXXXX 100644

--- a/util/qemu-coroutine-sleep.c

+++ b/util/qemu-coroutine-sleep.c

@@ -XXX,XX +XXX,XX @@ static void co_sleep_cb(void *opaque)

qemu_co_sleep_wake(w);

}

-void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,

- QEMUClockType type, int64_t ns)

+void coroutine_fn qemu_co_sleep(QemuCoSleep *w)

{

Coroutine *co = qemu_coroutine_self();

- AioContext *ctx = qemu_get_current_aio_context();

- QEMUTimer ts;

const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL,

qemu_co_sleep_ns__scheduled);

@@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,

}

w->to_wake = co;

- aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w),

- timer_mod(&ts, qemu_clock_get_ns(type) + ns);

qemu_coroutine_yield();

- timer_del(&ts);

/* w->to_wake is cleared before resuming this coroutine. */

assert(w->to_wake == NULL);

}

+

+void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w,

+ QEMUClockType type, int64_t ns)

+{

+ AioContext *ctx = qemu_get_current_aio_context();

+ QEMUTimer ts;

+

+ aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w);

+ timer_mod(&ts, qemu_clock_get_ns(type) + ns);

+

+ /*

+ * The timer will fire in the current AiOContext, so the callback

+ * must happen after qemu_co_sleep yields and there is no race

+ * between timer_mod and qemu_co_sleep.

+ */

+ qemu_co_sleep(w);

+ timer_del(&ts);

+}

--

2.31.1