Re: [PATCH] iotests/108: Fix when missing user_allow_other

2022-04-22 Thread Eric Blake
On Thu, Apr 21, 2022 at 04:24:35PM +0200, Hanna Reitz wrote:
> FUSE exports' allow-other option defaults to "auto", which means that it
> will try passing allow_other as a mount option, and fall back to not
> using it when an error occurs.  We make no effort to hide fusermount's
> error message (because it would be difficult, and because users might
> want to know about the fallback occurring), and so when allow_other does
> not work (primarily when /etc/fuse.conf does not contain
> user_allow_other), this error message will appear and break the
> reference output.
> 
> We do not need allow_other here, though, so we can just pass
> allow-other=off to fix that.
> 
> Reported-by: Markus Armbruster 
> Signed-off-by: Hanna Reitz 
> ---
>  tests/qemu-iotests/108 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I hit this today as well, and your fix works.

Tested-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org




Re: [PATCH v2] Only advertise aio=io_uring if support is actually available

2022-04-22 Thread Eric Blake
On Thu, Apr 21, 2022 at 02:27:55PM -0500, Eric Blake wrote:
> On Thu, Apr 21, 2022 at 06:50:48PM +0200, Dirk Müller wrote:
> > Change --help output for aio option to only list the aio backend options 
> > that
> > are actually available. io_uring is an optional, linux only backend
> > option so hide it for cases where it isn't there.
> 
> As pointed out by Dan, this commit message is not quite accurate.  It
> hides only one of the two conditional options, but 'native' is also a
> Linux-only optional backend (CONFIG_LINUX_AIO).

Stepping back a bit - we already said that making --help
machine-parseable is a non-goal, and so the intent of this patch is
for human readers.  But adding an #ifdef ladder to show all 4 possible
combinations gets hairy; adding another option turns it into 8
combinations.  Is there a better way?

What if we just document ALL strings possible in at least one build
(no #ifdef ladder in the help text), but add --aio=help as a way to do
a runtime list of which aio modes are understood by THIS build?  It
would match how we have --device=help for qemu proper, and may even be
able to reuse some of that framework code (for parsing out when help
is requested).

Yes, that would be a bigger patch, but it may also be easier to
maintain down the road.  And even though there is an #ifdef ladder at
runtime, it only has O(n) growth rather than O(n^2) for each possible
combination of which options are enabled, and would appear only once
in the runtime rather than duplicated across each place which
documents similar help text across multiple utilities.

> 
> > 
> > Reviewed-by: Eric Blake 
> > Signed-off-by: Dirk Müller 
> > ---
> >  block/file-posix.c | 4 
> >  qemu-nbd.c | 4 
> >  qemu-options.hx| 6 +-
> >  3 files changed, 13 insertions(+), 1 deletion(-)
> > 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org




Re: [PATCH v3 0/3] nbd: MULTI_CONN for shared writable exports

2022-04-22 Thread Eric Blake
Ping. Now that 7.1 is open, I'd like to include this series in my next
NBD pull request.

On Mon, Mar 14, 2022 at 03:38:15PM -0500, Eric Blake wrote:
> v2 was here:
> https://lists.gnu.org/archive/html/qemu-devel/2022-02/msg03314.html
> 
> Since then:
> - expose the knob through 'qemu-nbd -m on|off|auto'
> - reuse QAPI OnOffAuto type instead of rolling my own
> - rewrite the iotest from bash into python (thanks: Nir, Vladimir)
> - split out easy preliminary fixes (patches 1 and 2 are new)
> 
> Given that this is a new feature, it probably needs to be 7.1 material.
> 
> Eric Blake (3):
>   docs: Consistent typography for options of qemu-nbd
>   qemu-nbd: Pass max connections to blockdev layer
>   nbd/server: Allow MULTI_CONN for shared writable exports
> 
>  docs/interop/nbd.txt   |   1 +
>  docs/tools/qemu-nbd.rst|  26 ++--
>  qapi/block-export.json |  19 ++-
>  include/block/nbd.h|   5 +-
>  blockdev-nbd.c |  13 +-
>  nbd/server.c   |  27 +++-
>  qemu-nbd.c |  22 ++-
>  MAINTAINERS|   1 +
>  tests/qemu-iotests/tests/nbd-multiconn | 157 +
>  tests/qemu-iotests/tests/nbd-multiconn.out |   5 +
>  10 files changed, 252 insertions(+), 24 deletions(-)
>  create mode 100755 tests/qemu-iotests/tests/nbd-multiconn
>  create mode 100644 tests/qemu-iotests/tests/nbd-multiconn.out
> 
> -- 
> 2.35.1
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org




Re: [PATCH v2 for-7.1 0/3] qapi: nbd-export: select bitmap by node/name pair

2022-04-22 Thread Eric Blake
On Tue, Mar 15, 2022 at 12:45:04AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 15.03.2022 00:32, Vladimir Sementsov-Ogievskiy wrote:
> > From: Vladimir Sementsov-Ogievskiy 
> > 
> > Hi all!
> > 
> > Here is small improvement for bitmap exporting interface.
> > 
> > v2: Sorry for the noise, me trying to find a email service, that don't
> > consider sending patch series by git-send-email as as spam :/
> 
> Aha, and @mail.ru works a lot better. So, that's a candidate for v2 of my 
> "[PATCH] MAINTAINERS: change Vladimir's email address".
> 
> This series itself is OK for reviewing, but email in s-o-b mark may change in 
> v3.

Vladimir, do I have your permission to alter your S-o-b marks from
@ya.ru to @mail.ru (since we have established that you have sent mail
from both addresses, but selected @mail.ru in MAINTAINERS)?  That's
the only thing that's preventing me from queuing this series as-is.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org




Re: [PATCH v2 2/3] qapi: nbd-export: allow select bitmaps by node/name pair

2022-04-22 Thread Eric Blake
On Fri, Apr 08, 2022 at 11:27:42PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 17.03.2022 00:28, Eric Blake wrote:
> > > +++ b/qapi/block-export.json
> > > @@ -6,6 +6,7 @@
> > >   ##
> > >   { 'include': 'sockets.json' }
> > > +{ 'include': 'block-core.json' }
> > Hmm.  Does this extra inclusion negatively impact qemu-storage-daemon,
> > since that is why we created block-export.json in the first place (to
> > minimize the stuff that qsd pulled in without needing all of
> > block-core.json)?  In other words, would it be better to move
> > BlockDirtyBitmapOrStr to this file?
> 
> Actually, looking at storage-daemon/qapi/qapi-schema.json I see 
> block-cores.json.
> 
> That's block.json which is not mentioned in 
> storage-daemon/qapi/qapi-schema.json.
> 
> So, I think it's OK to keep simple include for now.

We're early enough in the 7.1 cycle that if someone proposes a reason
why this would need to change, then we can adjust it.

So for now, I'm adding

Reviewed-by: Eric Blake 

and queuing this series through my NBD tree.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org




[PATCH] nvme: fix bit buckets

2022-04-22 Thread Keith Busch
We can't just ignore the bit buckets since the data offsets read from
disk need to be accounted for. We could either split into multiple reads
for the actual user data requested and skip the buckets, or we could
have a place holder for bucket data. Splitting is too much over head, so
just allocate a memory region to dump unwanted data.

Signed-off-by: Keith Busch 
---
This came out easier than I thought, so we can ignore my previous
feature removal patch:
  https://lists.nongnu.org/archive/html/qemu-block/2022-04/msg00398.html

 hw/nvme/ctrl.c | 9 +
 hw/nvme/nvme.h | 1 +
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 03760ddeae..711c6fac29 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -845,11 +845,11 @@ static uint16_t nvme_map_sgl_data(NvmeCtrl *n, NvmeSg *sg,
 trans_len = MIN(*len, dlen);
 
 if (type == NVME_SGL_DESCR_TYPE_BIT_BUCKET) {
-goto next;
+addr = n->bitBucket.addr;
+} else {
+addr = le64_to_cpu(segment[i].addr);
 }
 
-addr = le64_to_cpu(segment[i].addr);
-
 if (UINT64_MAX - addr < dlen) {
 return NVME_DATA_SGL_LEN_INVALID | NVME_DNR;
 }
@@ -859,7 +859,6 @@ static uint16_t nvme_map_sgl_data(NvmeCtrl *n, NvmeSg *sg,
 return status;
 }
 
-next:
 *len -= trans_len;
 }
 
@@ -6686,6 +6685,8 @@ static int nvme_init_pci(NvmeCtrl *n, PCIDevice *pci_dev, 
Error **errp)
 nvme_init_pmr(n, pci_dev);
 }
 
+memory_region_init(>bitBucket, OBJECT(n), NULL, 0x10);
+
 return 0;
 }
 
diff --git a/hw/nvme/nvme.h b/hw/nvme/nvme.h
index 739c8b8f79..d59eadc69d 100644
--- a/hw/nvme/nvme.h
+++ b/hw/nvme/nvme.h
@@ -411,6 +411,7 @@ typedef struct NvmeCtrl {
 PCIDeviceparent_obj;
 MemoryRegion bar0;
 MemoryRegion iomem;
+MemoryRegion bitBucket;
 NvmeBar  bar;
 NvmeParams   params;
 NvmeBus  bus;
-- 
2.30.2




[PATCH v5 3/3] util/event-loop-base: Introduce options to set the thread pool size

2022-04-22 Thread Nicolas Saenz Julienne
The thread pool regulates itself: when idle, it kills threads until
empty, when in demand, it creates new threads until full. This behaviour
doesn't play well with latency sensitive workloads where the price of
creating a new thread is too high. For example, when paired with qemu's
'-mlock', or using safety features like SafeStack, creating a new thread
has been measured take multiple milliseconds.

In order to mitigate this let's introduce a new 'EventLoopBase'
property to set the thread pool size. The threads will be created during
the pool's initialization or upon updating the property's value, remain
available during its lifetime regardless of demand, and destroyed upon
freeing it. A properly characterized workload will then be able to
configure the pool to avoid any latency spikes.

Signed-off-by: Nicolas Saenz Julienne 
Reviewed-by: Stefan Hajnoczi 
---

Changes since v4:
 - Remove redundant versioning comments
 - Reword thread-pool-min description

Changes since v3:
 - Rework qom.json to avoid duplication

Changes since v2:
 - Don't wait when decreasing pool size
 - Fix qapi versioning

Changes since v1:
 - Add INT_MAX check
 - Have copy of thread pool sizes in AioContext to properly decouple
   both instances
 - More coherent variable naming
 - Handle case where max_threads decreases
 - Code comments

 event-loop-base.c| 23 +
 include/block/aio.h  | 10 ++
 include/block/thread-pool.h  |  3 ++
 include/sysemu/event-loop-base.h |  4 +++
 iothread.c   |  3 ++
 qapi/qom.json| 10 +-
 util/aio-posix.c |  1 +
 util/async.c | 20 
 util/main-loop.c |  9 ++
 util/thread-pool.c   | 55 +---
 10 files changed, 133 insertions(+), 5 deletions(-)

diff --git a/event-loop-base.c b/event-loop-base.c
index e7f99a6ec8..d5be4dc6fc 100644
--- a/event-loop-base.c
+++ b/event-loop-base.c
@@ -14,6 +14,7 @@
 #include "qemu/osdep.h"
 #include "qom/object_interfaces.h"
 #include "qapi/error.h"
+#include "block/thread-pool.h"
 #include "sysemu/event-loop-base.h"
 
 typedef struct {
@@ -21,9 +22,22 @@ typedef struct {
 ptrdiff_t offset; /* field's byte offset in EventLoopBase struct */
 } EventLoopBaseParamInfo;
 
+static void event_loop_base_instance_init(Object *obj)
+{
+EventLoopBase *base = EVENT_LOOP_BASE(obj);
+
+base->thread_pool_max = THREAD_POOL_MAX_THREADS_DEFAULT;
+}
+
 static EventLoopBaseParamInfo aio_max_batch_info = {
 "aio-max-batch", offsetof(EventLoopBase, aio_max_batch),
 };
+static EventLoopBaseParamInfo thread_pool_min_info = {
+"thread-pool-min", offsetof(EventLoopBase, thread_pool_min),
+};
+static EventLoopBaseParamInfo thread_pool_max_info = {
+"thread-pool-max", offsetof(EventLoopBase, thread_pool_max),
+};
 
 static void event_loop_base_get_param(Object *obj, Visitor *v,
 const char *name, void *opaque, Error **errp)
@@ -95,12 +109,21 @@ static void event_loop_base_class_init(ObjectClass *klass, 
void *class_data)
   event_loop_base_get_param,
   event_loop_base_set_param,
   NULL, _max_batch_info);
+object_class_property_add(klass, "thread-pool-min", "int",
+  event_loop_base_get_param,
+  event_loop_base_set_param,
+  NULL, _pool_min_info);
+object_class_property_add(klass, "thread-pool-max", "int",
+  event_loop_base_get_param,
+  event_loop_base_set_param,
+  NULL, _pool_max_info);
 }
 
 static const TypeInfo event_loop_base_info = {
 .name = TYPE_EVENT_LOOP_BASE,
 .parent = TYPE_OBJECT,
 .instance_size = sizeof(EventLoopBase),
+.instance_init = event_loop_base_instance_init,
 .class_size = sizeof(EventLoopBaseClass),
 .class_init = event_loop_base_class_init,
 .abstract = true,
diff --git a/include/block/aio.h b/include/block/aio.h
index 5634173b12..d128558f1d 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -192,6 +192,8 @@ struct AioContext {
 QSLIST_HEAD(, Coroutine) scheduled_coroutines;
 QEMUBH *co_schedule_bh;
 
+int thread_pool_min;
+int thread_pool_max;
 /* Thread pool for performing work and receiving completion callbacks.
  * Has its own locking.
  */
@@ -769,4 +771,12 @@ void aio_context_set_poll_params(AioContext *ctx, int64_t 
max_ns,
 void aio_context_set_aio_params(AioContext *ctx, int64_t max_batch,
 Error **errp);
 
+/**
+ * aio_context_set_thread_pool_params:
+ * @ctx: the aio context
+ * @min: min number of threads to have readily available in the thread pool
+ * @min: max number of threads the thread pool can contain
+ */
+void aio_context_set_thread_pool_params(AioContext *ctx, int64_t min,
+  

[PATCH v5 2/3] util/main-loop: Introduce the main loop into QOM

2022-04-22 Thread Nicolas Saenz Julienne
'event-loop-base' provides basic property handling for all 'AioContext'
based event loops. So let's define a new 'MainLoopClass' that inherits
from it. This will permit tweaking the main loop's properties through
qapi as well as through the command line using the '-object' keyword[1].
Only one instance of 'MainLoopClass' might be created at any time.

'EventLoopBaseClass' learns a new callback, 'can_be_deleted()' so as to
mark 'MainLoop' as non-deletable.

[1] For example:
  -object main-loop,id=main-loop,aio-max-batch=

Signed-off-by: Nicolas Saenz Julienne 
Reviewed-by: Stefan Hajnoczi 
---

Changes since v4:
 - Move unrelevant qom.json code into patch #1

Changes since v3:
 - Rework qom.json

Changes since v2:
 - Fix mainloop's qapi versioning

Changes since v1:
 - Fix json files to differentiate between iothread and main-loop
 - Use OBJECT_DECLARE_TYPE()
 - Fix build dependencies

 event-loop-base.c| 13 
 include/qemu/main-loop.h | 10 ++
 include/sysemu/event-loop-base.h |  1 +
 meson.build  |  3 +-
 qapi/qom.json| 12 +++
 util/main-loop.c | 56 
 6 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/event-loop-base.c b/event-loop-base.c
index a924c73a7c..e7f99a6ec8 100644
--- a/event-loop-base.c
+++ b/event-loop-base.c
@@ -73,10 +73,23 @@ static void event_loop_base_complete(UserCreatable *uc, 
Error **errp)
 }
 }
 
+static bool event_loop_base_can_be_deleted(UserCreatable *uc)
+{
+EventLoopBaseClass *bc = EVENT_LOOP_BASE_GET_CLASS(uc);
+EventLoopBase *backend = EVENT_LOOP_BASE(uc);
+
+if (bc->can_be_deleted) {
+return bc->can_be_deleted(backend);
+}
+
+return true;
+}
+
 static void event_loop_base_class_init(ObjectClass *klass, void *class_data)
 {
 UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass);
 ucc->complete = event_loop_base_complete;
+ucc->can_be_deleted = event_loop_base_can_be_deleted;
 
 object_class_property_add(klass, "aio-max-batch", "int",
   event_loop_base_get_param,
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index d3750c8e76..20c9387654 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -26,9 +26,19 @@
 #define QEMU_MAIN_LOOP_H
 
 #include "block/aio.h"
+#include "qom/object.h"
+#include "sysemu/event-loop-base.h"
 
 #define SIG_IPI SIGUSR1
 
+#define TYPE_MAIN_LOOP  "main-loop"
+OBJECT_DECLARE_TYPE(MainLoop, MainLoopClass, MAIN_LOOP)
+
+struct MainLoop {
+EventLoopBase parent_obj;
+};
+typedef struct MainLoop MainLoop;
+
 /**
  * qemu_init_main_loop: Set up the process so that it can run the main loop.
  *
diff --git a/include/sysemu/event-loop-base.h b/include/sysemu/event-loop-base.h
index 8e77d8b69f..fced4c9fea 100644
--- a/include/sysemu/event-loop-base.h
+++ b/include/sysemu/event-loop-base.h
@@ -25,6 +25,7 @@ struct EventLoopBaseClass {
 
 void (*init)(EventLoopBase *base, Error **errp);
 void (*update_params)(EventLoopBase *base, Error **errp);
+bool (*can_be_deleted)(EventLoopBase *base);
 };
 
 struct EventLoopBase {
diff --git a/meson.build b/meson.build
index 0a14e88ffc..b3472ae62a 100644
--- a/meson.build
+++ b/meson.build
@@ -2823,7 +2823,8 @@ libqemuutil = static_library('qemuutil',
  sources: util_ss.sources() + stub_ss.sources() + 
genh,
  dependencies: [util_ss.dependencies(), libm, 
threads, glib, socket, malloc, pixman])
 qemuutil = declare_dependency(link_with: libqemuutil,
-  sources: genh + version_res)
+  sources: genh + version_res,
+  dependencies: [event_loop_base])
 
 if have_system or have_user
   decodetree = generator(find_program('scripts/decodetree.py'),
diff --git a/qapi/qom.json b/qapi/qom.json
index a2439533c5..51f3acaad8 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -540,6 +540,16 @@
 '*poll-grow': 'int',
 '*poll-shrink': 'int' } }
 
+##
+# @MainLoopProperties:
+#
+# Properties for the main-loop object.
+#
+##
+{ 'struct': 'MainLoopProperties',
+  'base': 'EventLoopBaseProperties',
+  'data': {} }
+
 ##
 # @MemoryBackendProperties:
 #
@@ -830,6 +840,7 @@
 { 'name': 'input-linux',
   'if': 'CONFIG_LINUX' },
 'iothread',
+'main-loop',
 { 'name': 'memory-backend-epc',
   'if': 'CONFIG_LINUX' },
 'memory-backend-file',
@@ -895,6 +906,7 @@
   'input-linux':{ 'type': 'InputLinuxProperties',
   'if': 'CONFIG_LINUX' },
   'iothread':   'IothreadProperties',
+  'main-loop':  'MainLoopProperties',
   'memory-backend-epc': { 'type': 'MemoryBackendEpcProperties',
   'if': 'CONFIG_LINUX' },
   'memory-backend-file':

[PATCH v5 0/3] util/thread-pool: Expose minimun and maximum size

2022-04-22 Thread Nicolas Saenz Julienne
As discussed on the previous RFC[1] the thread-pool's dynamic thread
management doesn't play well with real-time and latency sensitive
systems. This series introduces a set of controls that'll permit
achieving more deterministic behaviours, for example by fixing the
pool's size.

We first introduce a new common interface to event loop configuration by
moving iothread's already available properties into an abstract class
called 'EventLooopBackend' and have both 'IOThread' and the newly
created 'MainLoop' inherit the properties from that class.

With this new configuration interface in place it's relatively simple to
introduce new options to fix the even loop's thread pool sizes. The
resulting QAPI looks like this:

-object main-loop,id=main-loop,thread-pool-min=1,thread-pool-max=1

Note that all patches are bisect friendly and pass all the tests.

[1] 
https://patchwork.ozlabs.org/project/qemu-devel/patch/20220202175234.656711-1-nsaen...@redhat.com/

@Stefan I kept your Signed-off-by, since the changes trivial/not
thread-pool related
---

Changes since v4:
 - Address Markus' comments WRT qom.json

Changes since v3:
 - Avoid duplication in qom.json by creating EventLoopBaseProperties.
 - Fix failures on first compilation due to race between
   event-loop-base.o and qapi header generation.

Changes since v2:
 - Get rid of wrong locking/waiting
 - Fix qapi versioning
 - Better commit messages

Changes since v1:
 - Address all Stefan's comments
 - Introduce new fix

Nicolas Saenz Julienne (3):
  Introduce event-loop-base abstract class
  util/main-loop: Introduce the main loop into QOM
  util/event-loop-base: Introduce options to set the thread pool size

 event-loop-base.c| 140 +++
 include/block/aio.h  |  10 +++
 include/block/thread-pool.h  |   3 +
 include/qemu/main-loop.h |  10 +++
 include/sysemu/event-loop-base.h |  41 +
 include/sysemu/iothread.h|   6 +-
 iothread.c   |  68 +--
 meson.build  |  26 +++---
 qapi/qom.json|  42 --
 util/aio-posix.c |   1 +
 util/async.c |  20 +
 util/main-loop.c |  65 ++
 util/thread-pool.c   |  55 +++-
 13 files changed, 418 insertions(+), 69 deletions(-)
 create mode 100644 event-loop-base.c
 create mode 100644 include/sysemu/event-loop-base.h

-- 
2.35.1




[PATCH v5 1/3] Introduce event-loop-base abstract class

2022-04-22 Thread Nicolas Saenz Julienne
Introduce the 'event-loop-base' abstract class, it'll hold the
properties common to all event loops and provide the necessary hooks for
their creation and maintenance. Then have iothread inherit from it.

EventLoopBaseClass is defined as user creatable and provides a hook for
its children to attach themselves to the user creatable class 'complete'
function. It also provides an update_params() callback to propagate
property changes onto its children.

The new 'event-loop-base' class will live in the root directory. It is
built on its own using the 'link_whole' option (there are no direct
function dependencies between the class and its children, it all happens
trough 'constructor' magic). And also imposes new compilation
dependencies:

qom <- event-loop-base <- blockdev (iothread.c)

And in subsequent patches:

qom <- event-loop-base <- qemuutil (util/main-loop.c)

All this forced some amount of reordering in meson.build:

 - Moved qom build definition before qemuutil. Doing it the other way
   around (i.e. moving qemuutil after qom) isn't possible as a lot of
   core libraries that live in between the two depend on it.

 - Process the 'hw' subdir earlier, as it introduces files into the
   'qom' source set.

No functional changes intended.

Signed-off-by: Nicolas Saenz Julienne 
Reviewed-by: Stefan Hajnoczi 
---

Changes since v4:
 - Introduce relevant qom.json changes, lived previously in patch #2
 - Rework EventLoopBaseProperties' description

Changes since v3:
 - Fix event-loop-base compilation so it depends on qapi header
   generation.

Changes since v2:
 - reword commit message to better explain compilation dependencies.

Changes since v1:
 - Rename to event-loop-base
 - Move event-loop-base into root directory
 - Build event-loop-base on its own, use link_whole to avoid the problem
   of the object file not being linked due to lacking direct calls from
   dependencies.
 - Move poll parameters into iothread, as main loop can't poll
 - Update Authorship (I took what iothread.c had and added myself, I
   hope that's fine)
 - Introduce update_params() callback

 event-loop-base.c| 104 +++
 include/sysemu/event-loop-base.h |  36 +++
 include/sysemu/iothread.h|   6 +-
 iothread.c   |  65 ++-
 meson.build  |  23 ---
 qapi/qom.json|  22 +--
 6 files changed, 192 insertions(+), 64 deletions(-)
 create mode 100644 event-loop-base.c
 create mode 100644 include/sysemu/event-loop-base.h

diff --git a/event-loop-base.c b/event-loop-base.c
new file mode 100644
index 00..a924c73a7c
--- /dev/null
+++ b/event-loop-base.c
@@ -0,0 +1,104 @@
+/*
+ * QEMU event-loop base
+ *
+ * Copyright (C) 2022 Red Hat Inc
+ *
+ * Authors:
+ *  Stefan Hajnoczi 
+ *  Nicolas Saenz Julienne 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qom/object_interfaces.h"
+#include "qapi/error.h"
+#include "sysemu/event-loop-base.h"
+
+typedef struct {
+const char *name;
+ptrdiff_t offset; /* field's byte offset in EventLoopBase struct */
+} EventLoopBaseParamInfo;
+
+static EventLoopBaseParamInfo aio_max_batch_info = {
+"aio-max-batch", offsetof(EventLoopBase, aio_max_batch),
+};
+
+static void event_loop_base_get_param(Object *obj, Visitor *v,
+const char *name, void *opaque, Error **errp)
+{
+EventLoopBase *event_loop_base = EVENT_LOOP_BASE(obj);
+EventLoopBaseParamInfo *info = opaque;
+int64_t *field = (void *)event_loop_base + info->offset;
+
+visit_type_int64(v, name, field, errp);
+}
+
+static void event_loop_base_set_param(Object *obj, Visitor *v,
+const char *name, void *opaque, Error **errp)
+{
+EventLoopBaseClass *bc = EVENT_LOOP_BASE_GET_CLASS(obj);
+EventLoopBase *base = EVENT_LOOP_BASE(obj);
+EventLoopBaseParamInfo *info = opaque;
+int64_t *field = (void *)base + info->offset;
+int64_t value;
+
+if (!visit_type_int64(v, name, , errp)) {
+return;
+}
+
+if (value < 0) {
+error_setg(errp, "%s value must be in range [0, %" PRId64 "]",
+   info->name, INT64_MAX);
+return;
+}
+
+*field = value;
+
+if (bc->update_params) {
+bc->update_params(base, errp);
+}
+
+return;
+}
+
+static void event_loop_base_complete(UserCreatable *uc, Error **errp)
+{
+EventLoopBaseClass *bc = EVENT_LOOP_BASE_GET_CLASS(uc);
+EventLoopBase *base = EVENT_LOOP_BASE(uc);
+
+if (bc->init) {
+bc->init(base, errp);
+}
+}
+
+static void event_loop_base_class_init(ObjectClass *klass, void *class_data)
+{
+UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass);
+ucc->complete = event_loop_base_complete;
+
+object_class_property_add(klass, "aio-max-batch", "int",
+  

[PATCH] nvme: remove bit bucket support for now

2022-04-22 Thread Keith Busch
THe emulated controller correctly accounts for not including bit buckets
in the controller-to-host data transfer, however it doesn't correctly
account for the holes for the on-disk data offsets.

Signed-off-by: Keith Busch 
---
 hw/nvme/ctrl.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 03760ddeae..5e56191d45 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -6773,8 +6773,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice 
*pci_dev)
 id->vwc = NVME_VWC_NSID_BROADCAST_SUPPORT | NVME_VWC_PRESENT;
 
 id->ocfs = cpu_to_le16(NVME_OCFS_COPY_FORMAT_0 | NVME_OCFS_COPY_FORMAT_1);
-id->sgls = cpu_to_le32(NVME_CTRL_SGLS_SUPPORT_NO_ALIGN |
-   NVME_CTRL_SGLS_BITBUCKET);
+id->sgls = cpu_to_le32(NVME_CTRL_SGLS_SUPPORT_NO_ALIGN);
 
 nvme_init_subnqn(n);
 
-- 
2.30.2




Re: [PATCH v4 3/3] util/event-loop-base: Introduce options to set the thread pool size

2022-04-22 Thread Markus Armbruster
Nicolas Saenz Julienne  writes:

> On Fri, 2022-04-22 at 13:15 +0200, Markus Armbruster wrote:
>> Nicolas Saenz Julienne  writes:
>> 
>> > The thread pool regulates itself: when idle, it kills threads until
>> > empty, when in demand, it creates new threads until full. This behaviour
>> > doesn't play well with latency sensitive workloads where the price of
>> > creating a new thread is too high. For example, when paired with qemu's
>> > '-mlock', or using safety features like SafeStack, creating a new thread
>> > has been measured take multiple milliseconds.
>> > 
>> > In order to mitigate this let's introduce a new 'EventLoopBase'
>> > property to set the thread pool size. The threads will be created during
>> > the pool's initialization or upon updating the property's value, remain
>> > available during its lifetime regardless of demand, and destroyed upon
>> > freeing it. A properly characterized workload will then be able to
>> > configure the pool to avoid any latency spikes.
>> > 
>> > Signed-off-by: Nicolas Saenz Julienne 
>> > Reviewed-by: Stefan Hajnoczi 
>> 
>> [...]
>> 
>> > diff --git a/qapi/qom.json b/qapi/qom.json
>> > index e5f31c4469..06b8c3d10b 100644
>> > --- a/qapi/qom.json
>> > +++ b/qapi/qom.json
>> > @@ -506,9 +506,17 @@
>> >  #
>> >  # @aio-max-batch: maximum number of requests in a batch for the AIO 
>> > engine,
>> >  # 0 means that the engine will use its default.
>> > +#
>> > +# @thread-pool-min: minimum number of threads readily available in the 
>> > thread
>> > +#   pool (default:0, since 7.1)
>> 
>> What do you mean by "readily available in the thread pool"?
>
> How about "minimum number of threads reserved in the thread pool"?

Works for me, thanks!

>> > +#
>> > +# @thread-pool-max: maximum number of threads the thread pool can contain
>> > +#   (default:64, since 7.1)
>> >  ##
>> 
>> If you add "Since: 7.1" in the previous patch, then the "since 7.1" here
>> need to go.
>
> Noted.




Re: [PATCH 09/10] block: move fcntl_setfl()

2022-04-22 Thread Richard Henderson

On 4/22/22 01:36, marcandre.lur...@redhat.com wrote:

+/* Sets a specific flag */
+static int fcntl_setfl(int fd, int flag)
+{
+int flags;
+
+flags = fcntl(fd, F_GETFL);
+if (flags == -1)
+return -errno;
+
+if (fcntl(fd, F_SETFL, flags | flag) == -1)
+return -errno;
+
+return 0;
+}
+
  static int raw_reconfigure_getfd(BlockDriverState *bs, int flags,
   int *open_flags, uint64_t perm, bool 
force_dup,
   Error **errp)
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 543c9944b083..1c231087408f 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -794,21 +794,6 @@ size_t qemu_get_host_physmem(void)
  return 0;
  }
  
-/* Sets a specific flag */

-int fcntl_setfl(int fd, int flag)
-{
-int flags;
-
-flags = fcntl(fd, F_GETFL);
-if (flags == -1) {
-return -errno;
-}
-if (fcntl(fd, F_SETFL, flags | flag) == -1) {
-return -errno;
-}
-return 0;
-}


Lost braces in the move.

r~



Re: [PATCH v4 2/3] util/main-loop: Introduce the main loop into QOM

2022-04-22 Thread Markus Armbruster
Nicolas Saenz Julienne  writes:

> 'event-loop-base' provides basic property handling for all 'AioContext'
> based event loops. So let's define a new 'MainLoopClass' that inherits
> from it. This will permit tweaking the main loop's properties through
> qapi as well as through the command line using the '-object' keyword[1].
> Only one instance of 'MainLoopClass' might be created at any time.
>
> 'EventLoopBaseClass' learns a new callback, 'can_be_deleted()' so as to
> mark 'MainLoop' as non-deletable.
>
> [1] For example:
>   -object main-loop,id=main-loop,aio-max-batch=
>
> Signed-off-by: Nicolas Saenz Julienne 
> Reviewed-by: Stefan Hajnoczi 

[...]

> diff --git a/qapi/qom.json b/qapi/qom.json
> index eeb5395ff3..e5f31c4469 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -499,6 +499,17 @@
>  '*repeat': 'bool',
>  '*grab-toggle': 'GrabToggleKeys' } }
>  
> +##
> +# @EventLoopBaseProperties:
> +#
> +# Common properties for objects derived from EventLoopBase

This makes sense as internal documentation: QAPI type
EventLoopBaseProperties is associated with C type EventLoopBase.  Doc
comments are *external* documentation: they go into the QEMU QMP
Reference Manual.

What about "Common properties for event loops"?

> +#
> +# @aio-max-batch: maximum number of requests in a batch for the AIO engine,
> +# 0 means that the engine will use its default.

Missing:

   # Since: 7.1

Permit me a short digression.  We add these unthinkingly, because
thinking is expensive.  Even when the type isn't really part of the
external interface.  The deeper problem is that we're trying to generate
documentation of the external interface from doc comments that are
written as documentation of the internal QAPI data structures.  Here,
for example, we document EventLoopBaseProperties even though it is a
purely internal thing: whether we factor out common members into a base
type or not is not visible in QMP.

> +##
> +{ 'struct': 'EventLoopBaseProperties',
> +  'data': { '*aio-max-batch': 'int' } }
> +
>  ##
>  # @IothreadProperties:
>  #
> @@ -516,17 +527,26 @@
>  #   algorithm detects it is spending too long polling without
>  #   encountering events. 0 selects a default behaviour (default: 
> 0)
>  #
> -# @aio-max-batch: maximum number of requests in a batch for the AIO engine,
> -# 0 means that the engine will use its default
> -# (default:0, since 6.1)
> +# The @aio-max-batch option is available since 6.1.

This separates the member's "since" information from its defintion.
Can't be helped, because its defined in the base type, but the since
only applies here.  Okay.

>  #
>  # Since: 2.0
>  ##
>  { 'struct': 'IothreadProperties',
> +  'base': 'EventLoopBaseProperties',
>'data': { '*poll-max-ns': 'int',
>  '*poll-grow': 'int',
> -'*poll-shrink': 'int',
> -'*aio-max-batch': 'int' } }
> +'*poll-shrink': 'int' } }
> +
> +##
> +# @MainLoopProperties:
> +#
> +# Properties for the main-loop object.
> +#
> +# Since: 7.1
> +##
> +{ 'struct': 'MainLoopProperties',
> +  'base': 'EventLoopBaseProperties',
> +  'data': {} }

The patch does two things:

1. Factor EventLoopBaseProperties out of IothreadProperties.

2. Create MainLoopProperties.

I'd split it.  This is not a demand.

>  
>  ##
>  # @MemoryBackendProperties:
> @@ -818,6 +838,7 @@
>  { 'name': 'input-linux',
>'if': 'CONFIG_LINUX' },
>  'iothread',
> +'main-loop',
>  { 'name': 'memory-backend-epc',
>'if': 'CONFIG_LINUX' },
>  'memory-backend-file',
> @@ -883,6 +904,7 @@
>'input-linux':{ 'type': 'InputLinuxProperties',
>'if': 'CONFIG_LINUX' },
>'iothread':   'IothreadProperties',
> +  'main-loop':  'MainLoopProperties',
>'memory-backend-epc': { 'type': 'MemoryBackendEpcProperties',
>'if': 'CONFIG_LINUX' },
>'memory-backend-file':'MemoryBackendFileProperties',

[...]




Re: [PATCH v4 2/3] util/main-loop: Introduce the main loop into QOM

2022-04-22 Thread Nicolas Saenz Julienne
On Fri, 2022-04-22 at 13:40 +0200, Nicolas Saenz Julienne wrote:
> > > +##
> > > +{ 'struct': 'EventLoopBaseProperties',
> > > +  'data': { '*aio-max-batch': 'int' } }
> > > +
> > >  ##
> > >  # @IothreadProperties:
> > >  #
> > > @@ -516,17 +527,26 @@
> > >  #   algorithm detects it is spending too long polling without
> > >  #   encountering events. 0 selects a default behaviour 
> > > (default: 0)
> > >  #
> > > -# @aio-max-batch: maximum number of requests in a batch for the AIO 
> > > engine,
> > > -# 0 means that the engine will use its default
> > > -# (default:0, since 6.1)
> > > +# The @aio-max-batch option is available since 6.1.
> > 
> > This separates the member's "since" information from its defintion.
> > Can't be helped, because its defined in the base type, but the since
> > only applies here.  Okay.
> 
> IIUC my compromise of having the 'since version' annotated on each externally
> visible type is good, right? No need to add the info in
> EventLoopBaseProperties.

OK, nevermind this reply. I misunderstood you. I'll:
 - Add a "since 7.1" in EventLoopBaseProperties.
 - Add a special comment in IothreadProperties mentioning aio-max-batch is
   available since 6.1.
 - Remove version info from MainLoopProperties.

Thanks!

-- 
Nicolás Sáenz




Re: [PATCH v4 3/3] util/event-loop-base: Introduce options to set the thread pool size

2022-04-22 Thread Nicolas Saenz Julienne
On Fri, 2022-04-22 at 13:15 +0200, Markus Armbruster wrote:
> Nicolas Saenz Julienne  writes:
> 
> > The thread pool regulates itself: when idle, it kills threads until
> > empty, when in demand, it creates new threads until full. This behaviour
> > doesn't play well with latency sensitive workloads where the price of
> > creating a new thread is too high. For example, when paired with qemu's
> > '-mlock', or using safety features like SafeStack, creating a new thread
> > has been measured take multiple milliseconds.
> > 
> > In order to mitigate this let's introduce a new 'EventLoopBase'
> > property to set the thread pool size. The threads will be created during
> > the pool's initialization or upon updating the property's value, remain
> > available during its lifetime regardless of demand, and destroyed upon
> > freeing it. A properly characterized workload will then be able to
> > configure the pool to avoid any latency spikes.
> > 
> > Signed-off-by: Nicolas Saenz Julienne 
> > Reviewed-by: Stefan Hajnoczi 
> 
> [...]
> 
> > diff --git a/qapi/qom.json b/qapi/qom.json
> > index e5f31c4469..06b8c3d10b 100644
> > --- a/qapi/qom.json
> > +++ b/qapi/qom.json
> > @@ -506,9 +506,17 @@
> >  #
> >  # @aio-max-batch: maximum number of requests in a batch for the AIO engine,
> >  # 0 means that the engine will use its default.
> > +#
> > +# @thread-pool-min: minimum number of threads readily available in the 
> > thread
> > +#   pool (default:0, since 7.1)
> 
> What do you mean by "readily available in the thread pool"?

How about "minimum number of threads reserved in the thread pool"?

> > +#
> > +# @thread-pool-max: maximum number of threads the thread pool can contain
> > +#   (default:64, since 7.1)
> >  ##
> 
> If you add "Since: 7.1" in the previous patch, then the "since 7.1" here
> need to go.

Noted.

-- 
Nicolás Sáenz




Re: [PATCH v4 3/3] util/event-loop-base: Introduce options to set the thread pool size

2022-04-22 Thread Markus Armbruster
Nicolas Saenz Julienne  writes:

> The thread pool regulates itself: when idle, it kills threads until
> empty, when in demand, it creates new threads until full. This behaviour
> doesn't play well with latency sensitive workloads where the price of
> creating a new thread is too high. For example, when paired with qemu's
> '-mlock', or using safety features like SafeStack, creating a new thread
> has been measured take multiple milliseconds.
>
> In order to mitigate this let's introduce a new 'EventLoopBase'
> property to set the thread pool size. The threads will be created during
> the pool's initialization or upon updating the property's value, remain
> available during its lifetime regardless of demand, and destroyed upon
> freeing it. A properly characterized workload will then be able to
> configure the pool to avoid any latency spikes.
>
> Signed-off-by: Nicolas Saenz Julienne 
> Reviewed-by: Stefan Hajnoczi 

[...]

> diff --git a/qapi/qom.json b/qapi/qom.json
> index e5f31c4469..06b8c3d10b 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -506,9 +506,17 @@
>  #
>  # @aio-max-batch: maximum number of requests in a batch for the AIO engine,
>  # 0 means that the engine will use its default.
> +#
> +# @thread-pool-min: minimum number of threads readily available in the thread
> +#   pool (default:0, since 7.1)

What do you mean by "readily available in the thread pool"?

> +#
> +# @thread-pool-max: maximum number of threads the thread pool can contain
> +#   (default:64, since 7.1)
>  ##

If you add "Since: 7.1" in the previous patch, then the "since 7.1" here
need to go.

>  { 'struct': 'EventLoopBaseProperties',
> -  'data': { '*aio-max-batch': 'int' } }
> +  'data': { '*aio-max-batch': 'int',
> +'*thread-pool-min': 'int',
> +'*thread-pool-max': 'int' } }
>  
>  ##
>  # @IothreadProperties:

[...]




Re: [PATCH v4 2/3] util/main-loop: Introduce the main loop into QOM

2022-04-22 Thread Nicolas Saenz Julienne
On Fri, 2022-04-22 at 13:13 +0200, Markus Armbruster wrote:
> Nicolas Saenz Julienne  writes:
> 
> > 'event-loop-base' provides basic property handling for all 'AioContext'
> > based event loops. So let's define a new 'MainLoopClass' that inherits
> > from it. This will permit tweaking the main loop's properties through
> > qapi as well as through the command line using the '-object' keyword[1].
> > Only one instance of 'MainLoopClass' might be created at any time.
> > 
> > 'EventLoopBaseClass' learns a new callback, 'can_be_deleted()' so as to
> > mark 'MainLoop' as non-deletable.
> > 
> > [1] For example:
> >   -object main-loop,id=main-loop,aio-max-batch=
> > 
> > Signed-off-by: Nicolas Saenz Julienne 
> > Reviewed-by: Stefan Hajnoczi 
> 
> [...]
> 
> > diff --git a/qapi/qom.json b/qapi/qom.json
> > index eeb5395ff3..e5f31c4469 100644
> > --- a/qapi/qom.json
> > +++ b/qapi/qom.json
> > @@ -499,6 +499,17 @@
> >  '*repeat': 'bool',
> >  '*grab-toggle': 'GrabToggleKeys' } }
> >  
> > +##
> > +# @EventLoopBaseProperties:
> > +#
> > +# Common properties for objects derived from EventLoopBase
> 
> This makes sense as internal documentation: QAPI type
> EventLoopBaseProperties is associated with C type EventLoopBase.  Doc
> comments are *external* documentation: they go into the QEMU QMP
> Reference Manual.
> 
> What about "Common properties for event loops"?

Sounds better, yes. I'll change it.

> > +#
> > +# @aio-max-batch: maximum number of requests in a batch for the AIO engine,
> > +# 0 means that the engine will use its default.
> 
> Missing:
> 
># Since: 7.1
> 
> Permit me a short digression.  We add these unthinkingly, because
> thinking is expensive.  Even when the type isn't really part of the
> external interface.  The deeper problem is that we're trying to generate
> documentation of the external interface from doc comments that are
> written as documentation of the internal QAPI data structures.  Here,
> for example, we document EventLoopBaseProperties even though it is a
> purely internal thing: whether we factor out common members into a base
> type or not is not visible in QMP.

Thanks for the explanation.

> > +##
> > +{ 'struct': 'EventLoopBaseProperties',
> > +  'data': { '*aio-max-batch': 'int' } }
> > +
> >  ##
> >  # @IothreadProperties:
> >  #
> > @@ -516,17 +527,26 @@
> >  #   algorithm detects it is spending too long polling without
> >  #   encountering events. 0 selects a default behaviour 
> > (default: 0)
> >  #
> > -# @aio-max-batch: maximum number of requests in a batch for the AIO engine,
> > -# 0 means that the engine will use its default
> > -# (default:0, since 6.1)
> > +# The @aio-max-batch option is available since 6.1.
> 
> This separates the member's "since" information from its defintion.
> Can't be helped, because its defined in the base type, but the since
> only applies here.  Okay.

IIUC my compromise of having the 'since version' annotated on each externally
visible type is good, right? No need to add the info in
EventLoopBaseProperties.

> >  #
> >  # Since: 2.0
> >  ##
> >  { 'struct': 'IothreadProperties',
> > +  'base': 'EventLoopBaseProperties',
> >'data': { '*poll-max-ns': 'int',
> >  '*poll-grow': 'int',
> > -'*poll-shrink': 'int',
> > -'*aio-max-batch': 'int' } }
> > +'*poll-shrink': 'int' } }
> > +
> > +##
> > +# @MainLoopProperties:
> > +#
> > +# Properties for the main-loop object.
> > +#
> > +# Since: 7.1
> > +##
> > +{ 'struct': 'MainLoopProperties',
> > +  'base': 'EventLoopBaseProperties',
> > +  'data': {} }
> 
> The patch does two things:
> 
> 1. Factor EventLoopBaseProperties out of IothreadProperties.
> 
> 2. Create MainLoopProperties.
> 
> I'd split it.  This is not a demand.

Since I'm preparing a v5 of the series, I agree it makes sense to move 1. to
the fist patch.

Thanks!

-- 
Nicolás Sáenz




Re: [PATCH 06/10] Replace qemu_pipe() with g_unix_open_pipe()

2022-04-22 Thread Marc-André Lureau
Hi

On Fri, Apr 22, 2022 at 1:00 PM Daniel P. Berrangé  wrote:
>
> On Fri, Apr 22, 2022 at 12:36:35PM +0400, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau 
> >
> > GLib g_unix_open_pipe() is essentially like qemu_pipe(), available since
> > 2.30.
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  include/qemu/osdep.h|  4 
> >  qemu-nbd.c  |  5 +++--
> >  util/event_notifier-posix.c |  2 +-
> >  util/oslib-posix.c  | 22 --
> >  4 files changed, 4 insertions(+), 29 deletions(-)
>
> There are a bunch of places still using 'pipe'instead of 'qemu_pipe'
> that should be switched also.
>

Ok, that would be a different patch though. Can you ack this one for now?

>
> With regards,
> Daniel
> --
> |: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
>




Re: [PATCH 06/10] Replace qemu_pipe() with g_unix_open_pipe()

2022-04-22 Thread Daniel P . Berrangé
On Fri, Apr 22, 2022 at 12:36:35PM +0400, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> GLib g_unix_open_pipe() is essentially like qemu_pipe(), available since
> 2.30.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  include/qemu/osdep.h|  4 
>  qemu-nbd.c  |  5 +++--
>  util/event_notifier-posix.c |  2 +-
>  util/oslib-posix.c  | 22 --
>  4 files changed, 4 insertions(+), 29 deletions(-)

There are a bunch of places still using 'pipe'instead of 'qemu_pipe'
that should be switched also.


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|




[PATCH 06/10] Replace qemu_pipe() with g_unix_open_pipe()

2022-04-22 Thread marcandre . lureau
From: Marc-André Lureau 

GLib g_unix_open_pipe() is essentially like qemu_pipe(), available since
2.30.

Signed-off-by: Marc-André Lureau 
---
 include/qemu/osdep.h|  4 
 qemu-nbd.c  |  5 +++--
 util/event_notifier-posix.c |  2 +-
 util/oslib-posix.c  | 22 --
 4 files changed, 4 insertions(+), 29 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 220b44f710e5..241d249d7e5b 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -547,10 +547,6 @@ static inline void qemu_timersub(const struct timeval 
*val1,
 ssize_t qemu_write_full(int fd, const void *buf, size_t count)
 G_GNUC_WARN_UNUSED_RESULT;
 
-#ifndef _WIN32
-int qemu_pipe(int pipefd[2]);
-#endif
-
 void qemu_set_cloexec(int fd);
 
 void fips_set_state(bool requested);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 397ffa64d768..a184c6b9992f 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -902,13 +902,14 @@ int main(int argc, char **argv)
 
 if ((device && !verbose) || fork_process) {
 #ifndef WIN32
+g_autoptr(GError) err = NULL;
 int stderr_fd[2];
 pid_t pid;
 int ret;
 
-if (qemu_pipe(stderr_fd) < 0) {
+if (!g_unix_open_pipe(stderr_fd, FD_CLOEXEC, )) {
 error_report("Error setting up communication pipe: %s",
- strerror(errno));
+ err->message);
 exit(EXIT_FAILURE);
 }
 
diff --git a/util/event_notifier-posix.c b/util/event_notifier-posix.c
index 8dc30c51414d..df21c2583e1f 100644
--- a/util/event_notifier-posix.c
+++ b/util/event_notifier-posix.c
@@ -49,7 +49,7 @@ int event_notifier_init(EventNotifier *e, int active)
 if (errno != ENOSYS) {
 return -errno;
 }
-if (qemu_pipe(fds) < 0) {
+if (!g_unix_open_pipe(fds, FD_CLOEXEC, NULL)) {
 return -errno;
 }
 ret = fcntl_setfl(fds[0], O_NONBLOCK);
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 03d97741562c..543c9944b083 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -274,28 +274,6 @@ void qemu_set_cloexec(int fd)
 assert(f != -1);
 }
 
-/*
- * Creates a pipe with FD_CLOEXEC set on both file descriptors
- */
-int qemu_pipe(int pipefd[2])
-{
-int ret;
-
-#ifdef CONFIG_PIPE2
-ret = pipe2(pipefd, O_CLOEXEC);
-if (ret != -1 || errno != ENOSYS) {
-return ret;
-}
-#endif
-ret = pipe(pipefd);
-if (ret == 0) {
-qemu_set_cloexec(pipefd[0]);
-qemu_set_cloexec(pipefd[1]);
-}
-
-return ret;
-}
-
 char *
 qemu_get_local_state_dir(void)
 {
-- 
2.36.0




[PATCH 09/10] block: move fcntl_setfl()

2022-04-22 Thread marcandre . lureau
From: Marc-André Lureau 

It is only used by block/file-posix.c, move it there.

Signed-off-by: Marc-André Lureau 
---
 include/sysemu/os-posix.h |  2 --
 block/file-posix.c| 15 +++
 util/oslib-posix.c| 15 ---
 3 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index adbe19d3e468..58de7c994d85 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -96,8 +96,6 @@ static inline void qemu_funlockfile(FILE *f)
 funlockfile(f);
 }
 
-int fcntl_setfl(int fd, int flag);
-
 #ifdef __cplusplus
 }
 #endif
diff --git a/block/file-posix.c b/block/file-posix.c
index bfd9b243..1164ca9e1c6d 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1022,6 +1022,21 @@ static int raw_handle_perm_lock(BlockDriverState *bs,
 return ret;
 }
 
+/* Sets a specific flag */
+static int fcntl_setfl(int fd, int flag)
+{
+int flags;
+
+flags = fcntl(fd, F_GETFL);
+if (flags == -1)
+return -errno;
+
+if (fcntl(fd, F_SETFL, flags | flag) == -1)
+return -errno;
+
+return 0;
+}
+
 static int raw_reconfigure_getfd(BlockDriverState *bs, int flags,
  int *open_flags, uint64_t perm, bool 
force_dup,
  Error **errp)
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 543c9944b083..1c231087408f 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -794,21 +794,6 @@ size_t qemu_get_host_physmem(void)
 return 0;
 }
 
-/* Sets a specific flag */
-int fcntl_setfl(int fd, int flag)
-{
-int flags;
-
-flags = fcntl(fd, F_GETFL);
-if (flags == -1) {
-return -errno;
-}
-if (fcntl(fd, F_SETFL, flags | flag) == -1) {
-return -errno;
-}
-return 0;
-}
-
 int qemu_msync(void *addr, size_t length, int fd)
 {
 size_t align_mask = ~(qemu_real_host_page_size() - 1);
-- 
2.36.0




[PATCH 03/10] include: move qemu_*_exec_dir() to cutils

2022-04-22 Thread marcandre . lureau
From: Marc-André Lureau 

The function is required by get_relocated_path() (already in cutils),
and used by qemu-ga and may be generally useful.

Signed-off-by: Marc-André Lureau 
---
 include/qemu/cutils.h|   7 ++
 include/qemu/osdep.h |   8 --
 qemu-io.c|   1 +
 storage-daemon/qemu-storage-daemon.c |   1 +
 tests/qtest/fuzz/fuzz.c  |   1 +
 util/cutils.c| 108 +++
 util/oslib-posix.c   |  81 
 util/oslib-win32.c   |  36 -
 8 files changed, 118 insertions(+), 125 deletions(-)

diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index 5c6572d44422..40e10e19a7ed 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -193,6 +193,13 @@ int uleb128_decode_small(const uint8_t *in, uint32_t *n);
  */
 int qemu_pstrcmp0(const char **str1, const char **str2);
 
+/* Find program directory, and save it for later usage with
+ * qemu_get_exec_dir().
+ * Try OS specific API first, if not working, parse from argv0. */
+void qemu_init_exec_dir(const char *argv0);
+
+/* Get the saved exec dir.  */
+const char *qemu_get_exec_dir(void);
 
 /**
  * get_relocated_path:
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index baaa23c1568d..220b44f710e5 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -564,14 +564,6 @@ bool fips_get_state(void);
  */
 char *qemu_get_local_state_dir(void);
 
-/* Find program directory, and save it for later usage with
- * qemu_get_exec_dir().
- * Try OS specific API first, if not working, parse from argv0. */
-void qemu_init_exec_dir(const char *argv0);
-
-/* Get the saved exec dir.  */
-const char *qemu_get_exec_dir(void);
-
 /**
  * qemu_getauxval:
  * @type: the auxiliary vector key to lookup
diff --git a/qemu-io.c b/qemu-io.c
index d70d3dd4fde5..2bd7bfb65073 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -16,6 +16,7 @@
 #endif
 
 #include "qemu/help-texts.h"
+#include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "qemu-io.h"
 #include "qemu/error-report.h"
diff --git a/storage-daemon/qemu-storage-daemon.c 
b/storage-daemon/qemu-storage-daemon.c
index 9b8b17f52e48..c104817cdddc 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -44,6 +44,7 @@
 
 #include "qemu/help-texts.h"
 #include "qemu-version.h"
+#include "qemu/cutils.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #include "qemu/help_option.h"
diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
index 5f77c849837f..d3afd294db24 100644
--- a/tests/qtest/fuzz/fuzz.c
+++ b/tests/qtest/fuzz/fuzz.c
@@ -15,6 +15,7 @@
 
 #include 
 
+#include "qemu/cutils.h"
 #include "qemu/datadir.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/qtest.h"
diff --git a/util/cutils.c b/util/cutils.c
index b2777210e7da..6cc7cc8cde99 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -931,6 +931,114 @@ static inline const char *next_component(const char *dir, 
int *p_len)
 return dir;
 }
 
+static const char *exec_dir;
+
+void qemu_init_exec_dir(const char *argv0)
+{
+#ifdef G_OS_WIN32
+char *p;
+char buf[MAX_PATH];
+DWORD len;
+
+if (exec_dir) {
+return;
+}
+
+len = GetModuleFileName(NULL, buf, sizeof(buf) - 1);
+if (len == 0) {
+return;
+}
+
+buf[len] = 0;
+p = buf + len - 1;
+while (p != buf && *p != '\\') {
+p--;
+}
+*p = 0;
+if (access(buf, R_OK) == 0) {
+exec_dir = g_strdup(buf);
+} else {
+exec_dir = CONFIG_BINDIR;
+}
+#else
+char *p = NULL;
+char buf[PATH_MAX];
+
+if (exec_dir) {
+return;
+}
+
+#if defined(__linux__)
+{
+int len;
+len = readlink("/proc/self/exe", buf, sizeof(buf) - 1);
+if (len > 0) {
+buf[len] = 0;
+p = buf;
+}
+}
+#elif defined(__FreeBSD__) \
+  || (defined(__NetBSD__) && defined(KERN_PROC_PATHNAME))
+{
+#if defined(__FreeBSD__)
+static int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, -1};
+#else
+static int mib[4] = {CTL_KERN, KERN_PROC_ARGS, -1, KERN_PROC_PATHNAME};
+#endif
+size_t len = sizeof(buf) - 1;
+
+*buf = '\0';
+if (!sysctl(mib, ARRAY_SIZE(mib), buf, , NULL, 0) &&
+*buf) {
+buf[sizeof(buf) - 1] = '\0';
+p = buf;
+}
+}
+#elif defined(__APPLE__)
+{
+char fpath[PATH_MAX];
+uint32_t len = sizeof(fpath);
+if (_NSGetExecutablePath(fpath, ) == 0) {
+p = realpath(fpath, buf);
+if (!p) {
+return;
+}
+}
+}
+#elif defined(__HAIKU__)
+{
+image_info ii;
+int32_t c = 0;
+
+*buf = '\0';
+while (get_next_image_info(0, , ) == B_OK) {
+if (ii.type == B_APP_IMAGE) {
+strncpy(buf, ii.name, sizeof(buf));
+   

[PATCH 04/10] tests: move libqtest.h back under qtest/

2022-04-22 Thread marcandre . lureau
From: Marc-André Lureau 

Since commit a2ce7dbd917 ("meson: convert tests/qtest to meson"),
libqtest.h is under libqos/ directory, while libqtest.c is still in
qtest/. Move back to its original location to avoid mixing with libqos/.

Suggested-by: Thomas Huth 
Signed-off-by: Marc-André Lureau 
---
 docs/devel/qtest.rst | 2 +-
 tests/qtest/acpi-utils.h | 2 +-
 tests/qtest/boot-sector.h| 2 +-
 tests/qtest/fuzz/fuzz.h  | 2 +-
 tests/qtest/libqos/fw_cfg.h  | 2 +-
 tests/qtest/libqos/i2c.h | 2 +-
 tests/qtest/libqos/libqos.h  | 2 +-
 tests/qtest/libqos/malloc.h  | 2 +-
 tests/qtest/libqos/pci.h | 2 +-
 tests/qtest/libqos/sdhci-cmd.h   | 2 +-
 tests/qtest/libqtest-single.h| 2 +-
 tests/qtest/{libqos => }/libqtest.h  | 0
 tests/qtest/migration-helpers.h  | 2 +-
 tests/qtest/tpm-emu.h| 2 +-
 tests/qtest/ac97-test.c  | 2 +-
 tests/qtest/ahci-test.c  | 2 +-
 tests/qtest/am53c974-test.c  | 2 +-
 tests/qtest/arm-cpu-features.c   | 2 +-
 tests/qtest/aspeed_hace-test.c   | 2 +-
 tests/qtest/boot-order-test.c| 2 +-
 tests/qtest/boot-sector.c| 2 +-
 tests/qtest/boot-serial-test.c   | 2 +-
 tests/qtest/cdrom-test.c | 2 +-
 tests/qtest/dbus-display-test.c  | 2 +-
 tests/qtest/dbus-vmstate-test.c  | 2 +-
 tests/qtest/device-introspect-test.c | 2 +-
 tests/qtest/device-plug-test.c   | 2 +-
 tests/qtest/drive_del-test.c | 2 +-
 tests/qtest/ds1338-test.c| 2 +-
 tests/qtest/e1000-test.c | 2 +-
 tests/qtest/eepro100-test.c  | 2 +-
 tests/qtest/endianness-test.c| 2 +-
 tests/qtest/erst-test.c  | 2 +-
 tests/qtest/es1370-test.c| 2 +-
 tests/qtest/fuzz-e1000e-test.c   | 2 +-
 tests/qtest/fuzz-lsi53c895a-test.c   | 2 +-
 tests/qtest/fuzz-megasas-test.c  | 2 +-
 tests/qtest/fuzz-sb16-test.c | 2 +-
 tests/qtest/fuzz-sdcard-test.c   | 2 +-
 tests/qtest/fuzz-virtio-scsi-test.c  | 2 +-
 tests/qtest/fuzz-xlnx-dp-test.c  | 2 +-
 tests/qtest/fuzz/fuzz.c  | 2 +-
 tests/qtest/fuzz/generic_fuzz.c  | 2 +-
 tests/qtest/fuzz/i440fx_fuzz.c   | 2 +-
 tests/qtest/fuzz/qos_fuzz.c  | 2 +-
 tests/qtest/fuzz/virtio_blk_fuzz.c   | 2 +-
 tests/qtest/fuzz/virtio_net_fuzz.c   | 2 +-
 tests/qtest/fuzz/virtio_scsi_fuzz.c  | 2 +-
 tests/qtest/fw_cfg-test.c| 2 +-
 tests/qtest/hd-geo-test.c| 2 +-
 tests/qtest/hexloader-test.c | 2 +-
 tests/qtest/ide-test.c   | 2 +-
 tests/qtest/ipoctal232-test.c| 2 +-
 tests/qtest/ivshmem-test.c   | 2 +-
 tests/qtest/libqos/aarch64-xlnx-zcu102-machine.c | 2 +-
 tests/qtest/libqos/ahci.c| 2 +-
 tests/qtest/libqos/arm-imx25-pdk-machine.c   | 2 +-
 tests/qtest/libqos/arm-n800-machine.c| 2 +-
 tests/qtest/libqos/arm-raspi2-machine.c  | 2 +-
 tests/qtest/libqos/arm-sabrelite-machine.c   | 2 +-
 tests/qtest/libqos/arm-smdkc210-machine.c| 2 +-
 tests/qtest/libqos/arm-virt-machine.c| 2 +-
 tests/qtest/libqos/arm-xilinx-zynq-a9-machine.c  | 2 +-
 tests/qtest/libqos/e1000e.c  | 2 +-
 tests/qtest/libqos/fw_cfg.c  | 2 +-
 tests/qtest/libqos/i2c-imx.c | 2 +-
 tests/qtest/libqos/i2c-omap.c| 2 +-
 tests/qtest/libqos/i2c.c | 2 +-
 tests/qtest/libqos/libqos.c  | 2 +-
 tests/qtest/libqos/pci-pc.c  | 2 +-
 tests/qtest/libqos/pci-spapr.c   | 2 +-
 tests/qtest/libqos/ppc64_pseries-machine.c   | 2 +-
 tests/qtest/libqos/qgraph.c  | 2 +-
 tests/qtest/libqos/qos_external.c| 2 +-
 tests/qtest/libqos/rtas.c| 2 +-
 tests/qtest/libqos/sdhci-cmd.c   | 2 +-
 tests/qtest/libqos/sdhci.c   | 2 +-
 tests/qtest/libqos/tpci200.c | 2 +-
 tests/qtest/libqos/usb.c | 2 +-
 tests/qtest/libqos/vhost-user-blk.c  | 2 +-
 tests/qtest/libqos/virtio-9p.c   | 2 +-
 tests/qtest/libqos/virtio-balloon.c  | 2 +-
 

[PATCH 02/10] Use QEMU_SANITIZE_ADDRESS

2022-04-22 Thread marcandre . lureau
From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
---
 tests/qtest/fdc-test.c| 2 +-
 util/coroutine-ucontext.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c
index 4aa72f36431f..0b3c2c0d523f 100644
--- a/tests/qtest/fdc-test.c
+++ b/tests/qtest/fdc-test.c
@@ -550,7 +550,7 @@ static void fuzz_registers(void)
 
 static bool qtest_check_clang_sanitizer(void)
 {
-#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)
+#ifdef QEMU_SANITIZE_ADDRESS
 return true;
 #else
 g_test_skip("QEMU not configured using --enable-sanitizers");
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 904b375192ca..ed368e1a3ec3 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -30,7 +30,7 @@
 #include 
 #endif
 
-#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)
+#ifdef QEMU_SANITIZE_ADDRESS
 #ifdef CONFIG_ASAN_IFACE_FIBER
 #define CONFIG_ASAN 1
 #include 
-- 
2.36.0