Re: [PATCH v5 19/20] job.c: enable job lock/unlock and remove Aiocontext locks

2022-03-08 Thread Stefan Hajnoczi
On Tue, Feb 08, 2022 at 09:35:12AM -0500, Emanuele Giuseppe Esposito wrote:
> diff --git a/include/qemu/job.h b/include/qemu/job.h
> index ca46e46f5b..574110a1f2 100644
> --- a/include/qemu/job.h
> +++ b/include/qemu/job.h
> @@ -75,11 +75,14 @@ typedef struct Job {
>  ProgressMeter progress;
>  
>  
> +/** Protected by job_mutex */
> +
>  /**
>   * AioContext to run the job coroutine in.
> - * This field can be read when holding either the BQL (so we are in
> - * the main loop) or the job_mutex.
> - * Instead, it can be only written when we hold *both* BQL and job_mutex.
> + * The job Aiocontext can be read when holding *either*
> + * the BQL (so we are in the main loop) or the job_mutex.
> + * Instead, it can only be written when we hold *both* BQL

s/Instead,//

> @@ -456,7 +440,10 @@ void job_unref_locked(Job *job)
>  
>  if (job->driver->free) {
>  job_unlock();
> +/* FIXME: aiocontext lock is required because cb calls blk_unref 
> */
> +aio_context_acquire(job->aio_context);

We break the locking rules for job->aio_context here because the job is
already being destroyed and there can be no races? Can we avoid the
special case:

  AioContext *aio_context = job->aio_context;

  job_unlock();
  /* FIXME: aiocontext lock is required because cb calls blk_unref */
  aio_context_acquire(aio_context);
  job->driver->free(job);
  aio_context_release(aio_context);
  job_lock();

?


signature.asc
Description: PGP signature


Re: [PATCH v5 16/20] commit and mirror: create new nodes using bdrv_get_aio_context, and not the job aiocontext

2022-03-08 Thread Stefan Hajnoczi
On Tue, Feb 08, 2022 at 09:35:09AM -0500, Emanuele Giuseppe Esposito wrote:
> We are always using the given bs AioContext, so there is no need
> to take the job ones (which is identical anyways).
> This also reduces the point we need to check when protecting
> job.aio_context field.
> 
> Signed-off-by: Emanuele Giuseppe Esposito 
> ---
>  block/commit.c | 4 ++--
>  block/mirror.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature


Re: [PATCH v5 17/20] job: detect change of aiocontext within job coroutine

2022-03-08 Thread Stefan Hajnoczi
On Tue, Feb 08, 2022 at 09:35:10AM -0500, Emanuele Giuseppe Esposito wrote:
> From: Paolo Bonzini 
> 
> We want to make sure access of job->aio_context is always done
> under either BQL or job_mutex. The problem is that using
> aio_co_enter(job->aiocontext, job->co) in job_start and job_enter_cond
> makes the coroutine immediately resume, so we can't hold the job lock.
> And caching it is not safe either, as it might change.
> 
> job_start is under BQL, so it can freely read job->aiocontext, but
> job_enter_cond is not. In order to fix this, use aio_co_wake():
> the advantage is that it won't use job->aiocontext, but the
> main disadvantage is that it won't be able to detect a change of
> job AioContext.
> 
> Calling bdrv_try_set_aio_context() will issue the following calls
> (simplified):
> * in terms of  bdrv callbacks:
>   .drained_begin -> .set_aio_context -> .drained_end
> * in terms of child_job functions:
>   child_job_drained_begin -> child_job_set_aio_context -> 
> child_job_drained_end
> * in terms of job functions:
>   job_pause_locked -> job_set_aio_context -> job_resume_locked
> 
> We can see that after setting the new aio_context, job_resume_locked
> calls again job_enter_cond, which then invokes aio_co_wake(). But
> while job->aiocontext has been set in job_set_aio_context,
> job->co->ctx has not changed, so the coroutine would be entering in
> the wrong aiocontext.
> 
> Using aio_co_schedule in job_resume_locked() might seem as a valid
> alternative, but the problem is that the bh resuming the coroutine
> is not scheduled immediately, and if in the meanwhile another
> bdrv_try_set_aio_context() is run (see test_propagate_mirror() in
> test-block-iothread.c), we would have the first schedule in the
> wrong aiocontext, and the second set of drains won't even manage
> to schedule the coroutine, as job->busy would still be true from
> the previous job_resume_locked().
> 
> The solution is to stick with aio_co_wake(), but then detect every time
> the coroutine resumes back from yielding if job->aio_context
> has changed. If so, we can reschedule it to the new context.
> 
> Check for the aiocontext change in job_do_yield_locked because:
> 1) aio_co_reschedule_self requires to be in the running coroutine
> 2) since child_job_set_aio_context allows changing the aiocontext only
>while the job is paused, this is the exact place where the coroutine
>resumes, before running JobDriver's code.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  job.c | 24 +---
>  1 file changed, 21 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature


Re: [PATCH v5 20/20] block_job_query: remove atomic read

2022-03-08 Thread Stefan Hajnoczi
On Tue, Feb 08, 2022 at 09:35:13AM -0500, Emanuele Giuseppe Esposito wrote:
> Not sure what the atomic here was supposed to do, since job.busy
> is protected by the job lock. Since the whole function
> is called under job_mutex, just remove the atomic.
> 
> Signed-off-by: Emanuele Giuseppe Esposito 
> ---
>  blockjob.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature


Re: [PATCH v3 3/5] iotests: Remove explicit checks for qemu_img() == 0

2022-03-08 Thread Eric Blake
On Mon, Mar 07, 2022 at 08:57:26PM -0500, John Snow wrote:
> qemu_img() returning zero ought to be the rule, not the
> exception. Remove all explicit checks against the condition in
> preparation for making non-zero returns an Exception.
> 
> Signed-off-by: John Snow 
> ---

Reviewed-by: Eric Blake 

As this is a testsuite improvement rather than a new feature, I think
it's fine for the series to go in during soft freeze.

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




Re: [PATCH v5 18/20] jobs: protect job.aio_context with BQL and job_mutex

2022-03-08 Thread Stefan Hajnoczi
On Tue, Feb 08, 2022 at 09:35:11AM -0500, Emanuele Giuseppe Esposito wrote:
>  static AioContext *child_job_get_parent_aio_context(BdrvChild *c)
>  {
>  BlockJob *job = c->opaque;
> +assert(qemu_in_main_thread());
>  
>  return job->job.aio_context;
>  }

It's not clear to me that .get_parent_aio_context() should only be
called from the main thread. The API is read-only so someone might try
to call from I/O code in the future expecting it to work like other
read-only graph APIs that are available from I/O code.

Currently the assertion is true because the only user is
bdrv_attach_child_*() but please document this invariant for
bdrv_child_get_parent_aio_context() and the callback. Maybe move the
assertion into a higher-level function like
bdrv_child_get_parent_aio_context() (if that still covers all cases).

> diff --git a/include/qemu/job.h b/include/qemu/job.h
> index dfbf2ea501..ca46e46f5b 100644
> --- a/include/qemu/job.h
> +++ b/include/qemu/job.h
> @@ -75,7 +75,12 @@ typedef struct Job {
>  ProgressMeter progress;
>  
>  
> -/** AioContext to run the job coroutine in */
> +/**
> + * AioContext to run the job coroutine in.
> + * This field can be read when holding either the BQL (so we are in
> + * the main loop) or the job_mutex.
> + * Instead, it can be only written when we hold *both* BQL and job_mutex.

s/Instead,//

(It sounds weird because "instead" means "replacement" or "substitution"
We're comparing "read" and "write" here, not substituting them.
Something like "on the other hand" or "conversely" works.)

> + */
>  AioContext *aio_context;
>  
>  /** Reference count of the block job */
> @@ -706,4 +711,16 @@ void job_dismiss_locked(Job **job, Error **errp);
>  int job_finish_sync_locked(Job *job, void (*finish)(Job *, Error **errp),
> Error **errp);
>  
> +/**
> + * Sets the @job->aio_context.
> + * Called with job_mutex *not* held.
> + *
> + * This function must run in the main thread to protect against
> + * concurrent read in job_finish_sync_locked(),
> + * takes the job_mutex lock to protect against the read in
> + * job_do_yield_locked(), and must be called when the coroutine
> + * is quiescent.
> + */
> +void job_set_aio_context(Job *job, AioContext *ctx);
> +
>  #endif
> diff --git a/job.c b/job.c
> index f05850a337..7a07d25ec3 100644
> --- a/job.c
> +++ b/job.c
> @@ -354,6 +354,17 @@ Job *job_get_locked(const char *id)
>  return NULL;
>  }
>  
> +void job_set_aio_context(Job *job, AioContext *ctx)
> +{
> +/* protect against read in job_finish_sync_locked and job_start */
> +assert(qemu_in_main_thread());
> +/* protect against read in job_do_yield_locked */
> +JOB_LOCK_GUARD();
> +/* ensure the coroutine is quiescent while the AioContext is changed */
> +assert(job->pause_count > 0);
> +job->aio_context = ctx;
> +}
> +
>  /* Called with job_mutex *not* held. */
>  static void job_sleep_timer_cb(void *opaque)
>  {
> @@ -1256,6 +1267,7 @@ int job_finish_sync_locked(Job *job, void (*finish)(Job 
> *, Error **errp),
>  {
>  Error *local_err = NULL;
>  int ret;
> +assert(qemu_in_main_thread());
>  
>  job_ref_locked(job);
>  
> -- 
> 2.31.1
> 


signature.asc
Description: PGP signature


Re: [PATCH v3 3/5] iotests: Remove explicit checks for qemu_img() == 0

2022-03-08 Thread John Snow
On Tue, Mar 8, 2022, 10:16 AM Eric Blake  wrote:

> On Mon, Mar 07, 2022 at 08:57:26PM -0500, John Snow wrote:
> > qemu_img() returning zero ought to be the rule, not the
> > exception. Remove all explicit checks against the condition in
> > preparation for making non-zero returns an Exception.
> >
> > Signed-off-by: John Snow 
> > ---
>
> Reviewed-by: Eric Blake 
>
> As this is a testsuite improvement rather than a new feature, I think
> it's fine for the series to go in during soft freeze.
>

Yup, I agree. I'd like to move this in sooner rather than later to guard
against rot, and to have the better failure messages during testing season.

I have followup patches that finish the audit of qemu-img calls. It's less
clear if those should also go in during soft freeze, but I suppose I can
send them and we can see how confident we feel about it.

(Also note, I am giving the same treatment to qemu-io in another branch,
too. That branch has revealed actual logical errors in our testing in
several places. That series isn't 100% ready yet, but it might also qualify
for freeze because it fixes real test defects.)


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


[PATCH 13/14] iotests: make qemu_img_log() check log level

2022-03-08 Thread John Snow
Improve qemu_img_log() to actually check if logging is turned on. If it
isn't, revert to the behavior of qemu_img(). This is done so that there
really is no way to avoid scrutinizing qemu-ing subprocess calls by
accident.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index f8d966cd73..6af503058f 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -318,7 +318,19 @@ def qemu_img_map(*args: str) -> Any:
 return qemu_img_json('map', "--output", "json", *args)
 
 def qemu_img_log(*args: str) -> subprocess.CompletedProcess[str]:
-result = qemu_img(*args, check=False)
+"""
+Logged, unchecked variant of qemu_img() that allows non-zero exit codes.
+
+If logging is perceived to be disabled, this function will fall back
+to prohibiting non-zero return codes.
+
+:raise VerboseProcessError:
+When the return code is negative, or when logging is disabled
+on any non-zero return code.
+
+:return: a CompletedProcess instance with returncode and console output.
+"""
+result = qemu_img(*args, check=not logging_enabled())
 log(result.stdout, filters=[filter_testfiles])
 return result
 
@@ -1640,6 +1652,11 @@ def activate_logging():
 test_logger.setLevel(logging.INFO)
 test_logger.propagate = False
 
+def logging_enabled() -> bool:
+"""Return True if iotest logging is active."""
+return (test_logger.hasHandlers()
+and test_logger.getEffectiveLevel() >= logging.INFO)
+
 # This is called from script-style iotests without a single point of entry
 def script_initialize(*args, **kwargs):
 """Initialize script-style tests without running any tests."""
-- 
2.34.1




[PATCH 06/14] iotests: change supports_quorum to use qemu_img

2022-03-08 Thread John Snow
Similar to other recent changes: use the qemu_img() invocation that
supports throwing loud, nasty exceptions when it fails for surprising
reasons.

(Why would "--help" ever fail? I don't know, but eliminating *all* calls
to qemu-img that do not go through qemu_img() is my goal, so
qemu_img_pipe() has to be removed.)

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 3896440238..698d5a7fdc 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1433,8 +1433,8 @@ def _verify_imgopts(unsupported: Sequence[str] = ()) -> 
None:
 notrun(f'not suitable for this imgopts: {imgopts}')
 
 
-def supports_quorum():
-return 'quorum' in qemu_img_pipe('--help')
+def supports_quorum() -> bool:
+return 'quorum' in qemu_img('--help').stdout
 
 def verify_quorum():
 '''Skip test suite if quorum support is not available'''
-- 
2.34.1




[PATCH 00/14] iotests: ensure all qemu-img calls are either checked or logged

2022-03-08 Thread John Snow
Based-On: 20220308015728.1269649-1-js...@redhat.com

Hi, this series ensures all calls to qemu-img ultimately go through
qemu_img(). After the previous series, qemu_img() is a function that
defaults to raising a VerboseProcessError exception when qemu-img
returns a non-zero exit code.

After this series, all qemu-img invocations from the iotests suite are
guaranteed to go through either qemu_img_log(), which allows non-zero
exit codes (but logs its output), or qemu_img().

Some callsites manually disable the error checking with check=False,
which is a good visual reminder directly at the callsite that additional
verification must be performed. The callsites utilizing this are very
few and far between.

The VerboseProcessError exception will print the command line, return
code, and all console output to the terminal if it goes unhandled. In
effect, all non-logging calls to qemu-img are now guaranteed to print
detailed failure information to the terminal on any unanticipated
behavior.

(Even logging calls will still raise this exception if the exit code was
negative, so you get all of the same failure output whenever qemu-img
crashes *anywhere* in iotests now.)

As a result of this change, some of the helpers change. Here's a summary
of the changes between the last series and this one:

qemu_img()   => qemu_img().returncode
qemu_img_pipe()  => qemu_img().stdout
qemu_img_pipe_and_status()   => qemu_img()
json.loads(qemu_img_pipe())) => qemu_img_json()
qemu_img_log()   => qemu_img_log()

John Snow (14):
  iotests: add qemu_img_json()
  iotests: use qemu_img_json() when applicable
  iotests: add qemu_img_info()
  iotests/remove-bitmap-from-backing: use qemu_img_info()
  iotests: add qemu_img_map() function
  iotests: change supports_quorum to use qemu_img
  iotests: replace unchecked calls to qemu_img_pipe()
  iotests/149: Remove qemu_img_pipe() call
  iotests: remove remaining calls to qemu_img_pipe()
  iotests: use qemu_img() in has_working_luks()
  iotests: replace qemu_img_log('create', ...) calls
  iotests: remove qemu_img_pipe_and_status()
  iotests: make qemu_img_log() check log level
  iotests: make img_info_log() call qemu_img_log()

 tests/qemu-iotests/041|   5 +-
 tests/qemu-iotests/065|   7 +-
 tests/qemu-iotests/149|   7 +-
 tests/qemu-iotests/149.out|  21 ---
 tests/qemu-iotests/194|   4 +-
 tests/qemu-iotests/202|   4 +-
 tests/qemu-iotests/203|   4 +-
 tests/qemu-iotests/211|   6 +-
 tests/qemu-iotests/234|   4 +-
 tests/qemu-iotests/237|   3 +-
 tests/qemu-iotests/237.out|   3 -
 tests/qemu-iotests/242|   5 +-
 tests/qemu-iotests/255|   8 +-
 tests/qemu-iotests/255.out|   4 -
 tests/qemu-iotests/262|   2 +-
 tests/qemu-iotests/274|  17 ++-
 tests/qemu-iotests/274.out|  29 
 tests/qemu-iotests/280|   2 +-
 tests/qemu-iotests/280.out|   1 -
 tests/qemu-iotests/296|  12 +-
 tests/qemu-iotests/303|   2 +-
 tests/qemu-iotests/iotests.py | 134 +-
 tests/qemu-iotests/tests/block-status-cache   |  11 +-
 .../qemu-iotests/tests/parallels-read-bitmap  |   6 +-
 .../tests/remove-bitmap-from-backing  |   6 +-
 25 files changed, 150 insertions(+), 157 deletions(-)

-- 
2.34.1





[PATCH 01/14] iotests: add qemu_img_json()

2022-03-08 Thread John Snow
qemu_img_json() is a new helper built on top of qemu_img() that tries to
pull a valid JSON document out of the stdout stream.

In the event that the return code is negative (the program crashed), or
the code is greater than zero and did not produce valid JSON output, the
VerboseProcessError raised by qemu_img() is re-raised.

In the event that the return code is zero but we can't parse valid JSON,
allow the JSON deserialization error to be raised.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 38 +++
 1 file changed, 38 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 7057db0686..546b142a6c 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -276,6 +276,44 @@ def ordered_qmp(qmsg, conv_keys=True):
 def qemu_img_create(*args: str) -> subprocess.CompletedProcess[str]:
 return qemu_img('create', *args)
 
+def qemu_img_json(*args: str) -> Any:
+"""
+Run qemu-img and return its output as deserialized JSON.
+
+:raise CalledProcessError:
+When qemu-img crashes, or returns a non-zero exit code without
+producing a valid JSON document to stdout.
+:raise JSONDecoderError:
+When qemu-img returns 0, but failed to produce a valid JSON document.
+
+:return: A deserialized JSON object; probably a dict[str, Any].
+"""
+json_data = ...  # json.loads can legitimately return 'None'.
+
+try:
+res = qemu_img(*args, combine_stdio=False)
+except subprocess.CalledProcessError as exc:
+# Terminated due to signal. Don't bother.
+if exc.returncode < 0:
+raise
+
+# Commands like 'check' can return failure (exit codes 2 and 3)
+# to indicate command completion, but with errors found. For
+# multi-command flexibility, ignore the exact error codes and
+# *try* to load JSON.
+try:
+json_data = json.loads(exc.stdout)
+except json.JSONDecodeError:
+# Nope. This thing is toast. Raise the process error.
+pass
+
+if json_data is ...:
+raise
+
+if json_data is ...:
+json_data = json.loads(res.stdout)
+return json_data
+
 def qemu_img_measure(*args):
 return json.loads(qemu_img_pipe("measure", "--output", "json", *args))
 
-- 
2.34.1




[PATCH 04/14] iotests/remove-bitmap-from-backing: use qemu_img_info()

2022-03-08 Thread John Snow
This removes two more usages of qemu_img_pipe() and replaces them with
calls to qemu_img(), which provides better diagnostic information on
failure.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/tests/remove-bitmap-from-backing | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/tests/remove-bitmap-from-backing 
b/tests/qemu-iotests/tests/remove-bitmap-from-backing
index fee3141340..15be32dcb9 100755
--- a/tests/qemu-iotests/tests/remove-bitmap-from-backing
+++ b/tests/qemu-iotests/tests/remove-bitmap-from-backing
@@ -19,7 +19,7 @@
 #
 
 import iotests
-from iotests import log, qemu_img_create, qemu_img, qemu_img_pipe
+from iotests import log, qemu_img_create, qemu_img, qemu_img_info
 
 iotests.script_initialize(supported_fmts=['qcow2'],
   unsupported_imgopts=['compat'])
@@ -33,7 +33,7 @@ qemu_img_create('-f', iotests.imgfmt, '-b', base,
 
 qemu_img('bitmap', '--add', base, 'bitmap0')
 # Just assert that our method of checking bitmaps in the image works.
-assert 'bitmaps' in qemu_img_pipe('info', base)
+assert 'bitmaps' in qemu_img_info(base)['format-specific']['data']
 
 vm = iotests.VM().add_drive(top, 'backing.node-name=base')
 vm.launch()
@@ -68,5 +68,5 @@ if result != {'return': {}}:
 
 vm.shutdown()
 
-if 'bitmaps' in qemu_img_pipe('info', base):
+if 'bitmaps' in qemu_img_info(base)['format-specific']['data']:
 log('ERROR: Bitmap is still in the base image')
-- 
2.34.1




[PATCH 02/14] iotests: use qemu_img_json() when applicable

2022-03-08 Thread John Snow
qemu_img_json() gives better diagnostic information on failure.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 546b142a6c..7b37938d45 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -314,11 +314,11 @@ def qemu_img_json(*args: str) -> Any:
 json_data = json.loads(res.stdout)
 return json_data
 
-def qemu_img_measure(*args):
-return json.loads(qemu_img_pipe("measure", "--output", "json", *args))
+def qemu_img_measure(*args: str) -> Any:
+return qemu_img_json("measure", "--output", "json", *args)
 
-def qemu_img_check(*args):
-return json.loads(qemu_img_pipe("check", "--output", "json", *args))
+def qemu_img_check(*args: str) -> Any:
+return qemu_img_json("check", "--output", "json", *args)
 
 def qemu_img_pipe(*args: str) -> str:
 '''Run qemu-img and return its output'''
-- 
2.34.1




[PATCH 12/14] iotests: remove qemu_img_pipe_and_status()

2022-03-08 Thread John Snow
With the exceptional 'create' calls removed in the prior commit, change
qemu_img_log() and img_info_log() to call qemu_img() directly
instead.

In keeping with the spirit of diff-based tests, allow these calls to
qemu_img() to return an unchecked non-zero status code -- because any
error we'd see from the output is going into the log anyway.

Every last call to qemu-img is now either checked for a return code of
zero or has its output logged. It should be very hard to accidentally
ignore the return code *or* output from qemu-img now; intentional malice
remains unhandled.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 26 +++---
 1 file changed, 7 insertions(+), 19 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 1b6e28ba64..f8d966cd73 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -207,15 +207,6 @@ def qemu_img_create_prepare_args(args: List[str]) -> 
List[str]:
 
 return result
 
-def qemu_img_pipe_and_status(*args: str) -> Tuple[str, int]:
-"""
-Run qemu-img and return both its output and its exit code
-"""
-is_create = bool(args and args[0] == 'create')
-full_args = qemu_img_args + qemu_img_create_prepare_args(list(args))
-return qemu_tool_pipe_and_status('qemu-img', full_args,
- drop_successful_output=is_create)
-
 def qemu_img(*args: str, check: bool = True, combine_stdio: bool = True
  ) -> subprocess.CompletedProcess[str]:
 """
@@ -326,17 +317,14 @@ def qemu_img_info(*args: str) -> Any:
 def qemu_img_map(*args: str) -> Any:
 return qemu_img_json('map', "--output", "json", *args)
 
-def qemu_img_pipe(*args: str) -> str:
-'''Run qemu-img and return its output'''
-return qemu_img_pipe_and_status(*args)[0]
-
-def qemu_img_log(*args):
-result = qemu_img_pipe(*args)
-log(result, filters=[filter_testfiles])
+def qemu_img_log(*args: str) -> subprocess.CompletedProcess[str]:
+result = qemu_img(*args, check=False)
+log(result.stdout, filters=[filter_testfiles])
 return result
 
-def img_info_log(filename, filter_path=None, use_image_opts=False,
- extra_args=()):
+def img_info_log(filename: str, filter_path: Optional[str] = None,
+ use_image_opts: bool = False, extra_args: Sequence[str] = (),
+ ) -> None:
 args = ['info']
 if use_image_opts:
 args.append('--image-opts')
@@ -345,7 +333,7 @@ def img_info_log(filename, filter_path=None, 
use_image_opts=False,
 args += extra_args
 args.append(filename)
 
-output = qemu_img_pipe(*args)
+output = qemu_img(*args, check=False).stdout
 if not filter_path:
 filter_path = filename
 log(filter_img_info(output, filter_path))
-- 
2.34.1




[PATCH 11/14] iotests: replace qemu_img_log('create', ...) calls

2022-03-08 Thread John Snow
qemu_img_log() calls into qemu_img_pipe(), which always removes output
for 'create' commands on success anyway. Replace all of these calls to
the simpler qemu_img_create(...) which doesn't log, but raises a
detailed exception object on failure instead.

Blank lines are removed from output files where appropriate.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/255 |  8 
 tests/qemu-iotests/255.out |  4 
 tests/qemu-iotests/274 | 17 -
 tests/qemu-iotests/274.out | 29 -
 tests/qemu-iotests/280 |  2 +-
 tests/qemu-iotests/280.out |  1 -
 6 files changed, 13 insertions(+), 48 deletions(-)

diff --git a/tests/qemu-iotests/255 b/tests/qemu-iotests/255
index 3d6d0e80cb..f86fa851b6 100755
--- a/tests/qemu-iotests/255
+++ b/tests/qemu-iotests/255
@@ -42,8 +42,8 @@ with iotests.FilePath('t.qcow2') as disk_path, \
 size_str = str(size)
 
 iotests.create_image(base_path, size)
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, mid_path, size_str)
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, disk_path, size_str)
+iotests.qemu_img_create('-f', iotests.imgfmt, mid_path, size_str)
+iotests.qemu_img_create('-f', iotests.imgfmt, disk_path, size_str)
 
 # Create a backing chain like this:
 # base <- [throttled: bps-read=4096] <- mid <- overlay
@@ -92,8 +92,8 @@ with iotests.FilePath('src.qcow2') as src_path, \
 size = 128 * 1024 * 1024
 size_str = str(size)
 
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, src_path, size_str)
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, dst_path, size_str)
+iotests.qemu_img_create('-f', iotests.imgfmt, src_path, size_str)
+iotests.qemu_img_create('-f', iotests.imgfmt, dst_path, size_str)
 
 iotests.log(iotests.qemu_io('-f', iotests.imgfmt, '-c', 'write 0 1M',
 src_path),
diff --git a/tests/qemu-iotests/255.out b/tests/qemu-iotests/255.out
index 11a05a5213..2e837cbb5f 100644
--- a/tests/qemu-iotests/255.out
+++ b/tests/qemu-iotests/255.out
@@ -3,8 +3,6 @@ Finishing a commit job with background reads
 
 === Create backing chain and start VM ===
 
-
-
 === Start background read requests ===
 
 === Run a commit job ===
@@ -21,8 +19,6 @@ Closing the VM while a job is being cancelled
 
 === Create images and start VM ===
 
-
-
 wrote 1048576/1048576 bytes at offset 0
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
diff --git a/tests/qemu-iotests/274 b/tests/qemu-iotests/274
index 080a90f10f..2495e051a2 100755
--- a/tests/qemu-iotests/274
+++ b/tests/qemu-iotests/274
@@ -31,12 +31,11 @@ size_long = 2 * 1024 * 1024
 size_diff = size_long - size_short
 
 def create_chain() -> None:
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, base,
- str(size_long))
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, '-b', base,
- '-F', iotests.imgfmt, mid, str(size_short))
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, '-b', mid,
- '-F', iotests.imgfmt, top, str(size_long))
+iotests.qemu_img_create('-f', iotests.imgfmt, base, str(size_long))
+iotests.qemu_img_create('-f', iotests.imgfmt, '-b', base,
+'-F', iotests.imgfmt, mid, str(size_short))
+iotests.qemu_img_create('-f', iotests.imgfmt, '-b', mid,
+'-F', iotests.imgfmt, top, str(size_long))
 
 iotests.qemu_io_log('-c', 'write -P 1 0 %d' % size_long, base)
 
@@ -160,9 +159,9 @@ with iotests.FilePath('base') as base, \
 ('off',  '512k', '256k', '500k', '436k')]:
 
 iotests.log('=== preallocation=%s ===' % prealloc)
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, base, base_size)
-iotests.qemu_img_log('create', '-f', iotests.imgfmt, '-b', base,
- '-F', iotests.imgfmt, top, top_size_old)
+iotests.qemu_img_create('-f', iotests.imgfmt, base, base_size)
+iotests.qemu_img_create('-f', iotests.imgfmt, '-b', base,
+'-F', iotests.imgfmt, top, top_size_old)
 iotests.qemu_io_log('-c', 'write -P 1 %s 64k' % off, base)
 
 # After this, top_size_old to base_size should be allocated/zeroed.
diff --git a/tests/qemu-iotests/274.out b/tests/qemu-iotests/274.out
index 1ce40d839a..acd8b166a6 100644
--- a/tests/qemu-iotests/274.out
+++ b/tests/qemu-iotests/274.out
@@ -1,7 +1,4 @@
 == Commit tests ==
-
-
-
 wrote 2097152/2097152 bytes at offset 0
 2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
@@ -63,9 +60,6 @@ read 1048576/1048576 bytes at offset 1048576
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 === Testing HMP commit (top -> mid) ===
-
-
-
 wrote 2097152/2097152 bytes at offset 0
 2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
@@ -92,9 +86,6 @@ read 1048576/1048576 bytes at offset 1048576
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 

[PATCH 08/14] iotests/149: Remove qemu_img_pipe() call

2022-03-08 Thread John Snow
qemu_img_pipe calls blank their output when the command being run is a
'create' call and the command succeeds. Thus, the normative output for
this command in iotest 149 is to print a blank line. We can remove the
logging from this invocation and use a checked invocation, but we still
need to inspect the actual output to see if we want to retroactively
skip the test due to missing cipher support.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/149 |  7 +--
 tests/qemu-iotests/149.out | 21 -
 2 files changed, 5 insertions(+), 23 deletions(-)

diff --git a/tests/qemu-iotests/149 b/tests/qemu-iotests/149
index d49646ca60..9bb96d6a1d 100755
--- a/tests/qemu-iotests/149
+++ b/tests/qemu-iotests/149
@@ -265,8 +265,11 @@ def qemu_img_create(config, size_mb):
 "%dM" % size_mb]
 
 iotests.log("qemu-img " + " ".join(args), 
filters=[iotests.filter_test_dir])
-iotests.log(check_cipher_support(config, iotests.qemu_img_pipe(*args)),
-filters=[iotests.filter_test_dir])
+try:
+iotests.qemu_img(*args)
+except subprocess.CalledProcessError as exc:
+check_cipher_support(config, exc.output)
+raise
 
 def qemu_io_image_args(config, dev=False):
 """Get the args for access an image or device with qemu-io"""
diff --git a/tests/qemu-iotests/149.out b/tests/qemu-iotests/149.out
index ab879596ce..2cc5b82f7c 100644
--- a/tests/qemu-iotests/149.out
+++ b/tests/qemu-iotests/149.out
@@ -61,7 +61,6 @@ unlink TEST_DIR/luks-aes-256-xts-plain64-sha1.img
 # = qemu-img aes-256-xts-plain64-sha1 =
 # Create image
 qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,iter-time=10,cipher-alg=aes-256,cipher-mode=xts,ivgen-alg=plain64,hash-alg=sha1
 TEST_DIR/luks-aes-256-xts-plain64-sha1.img 4194304M
-
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-xts-plain64-sha1.img 
qiotest-145-aes-256-xts-plain64-sha1
 # Write test pattern 0xa7
@@ -180,7 +179,6 @@ unlink TEST_DIR/luks-twofish-256-xts-plain64-sha1.img
 # = qemu-img twofish-256-xts-plain64-sha1 =
 # Create image
 qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,iter-time=10,cipher-alg=twofish-256,cipher-mode=xts,ivgen-alg=plain64,hash-alg=sha1
 TEST_DIR/luks-twofish-256-xts-plain64-sha1.img 4194304M
-
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-twofish-256-xts-plain64-sha1.img 
qiotest-145-twofish-256-xts-plain64-sha1
 # Write test pattern 0xa7
@@ -299,7 +297,6 @@ unlink TEST_DIR/luks-serpent-256-xts-plain64-sha1.img
 # = qemu-img serpent-256-xts-plain64-sha1 =
 # Create image
 qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,iter-time=10,cipher-alg=serpent-256,cipher-mode=xts,ivgen-alg=plain64,hash-alg=sha1
 TEST_DIR/luks-serpent-256-xts-plain64-sha1.img 4194304M
-
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-serpent-256-xts-plain64-sha1.img 
qiotest-145-serpent-256-xts-plain64-sha1
 # Write test pattern 0xa7
@@ -418,7 +415,6 @@ unlink TEST_DIR/luks-cast5-128-cbc-plain64-sha1.img
 # = qemu-img cast5-128-cbc-plain64-sha1 =
 # Create image
 qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,iter-time=10,cipher-alg=cast5-128,cipher-mode=cbc,ivgen-alg=plain64,hash-alg=sha1
 TEST_DIR/luks-cast5-128-cbc-plain64-sha1.img 4194304M
-
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-cast5-128-cbc-plain64-sha1.img 
qiotest-145-cast5-128-cbc-plain64-sha1
 # Write test pattern 0xa7
@@ -538,7 +534,6 @@ unlink TEST_DIR/luks-aes-256-cbc-plain-sha1.img
 # = qemu-img aes-256-cbc-plain-sha1 =
 # Create image
 qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,iter-time=10,cipher-alg=aes-256,cipher-mode=cbc,ivgen-alg=plain,hash-alg=sha1
 TEST_DIR/luks-aes-256-cbc-plain-sha1.img 4194304M
-
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-cbc-plain-sha1.img 
qiotest-145-aes-256-cbc-plain-sha1
 # Write test pattern 0xa7
@@ -657,7 +652,6 @@ unlink TEST_DIR/luks-aes-256-cbc-plain64-sha1.img
 # = qemu-img aes-256-cbc-plain64-sha1 =
 # Create image
 qemu-img create -f luks --object secret,id=sec0,data=MTIzNDU2,format=base64 -o 
key-secret=sec0,iter-time=10,cipher-alg=aes-256,cipher-mode=cbc,ivgen-alg=plain64,hash-alg=sha1
 TEST_DIR/luks-aes-256-cbc-plain64-sha1.img 4194304M
-
 # Open dev
 sudo cryptsetup -q -v luksOpen TEST_DIR/luks-aes-256-cbc-plain64-sha1.img 
qiotest-145-aes-256-cbc-plain64-sha1
 # Write test pattern 0xa7
@@ -776,7 +770,6 @@ unlink TEST_DIR/luks-aes-256-cbc-essiv-sha256-sha1.img
 # = qemu-img aes-256-cbc-essiv-sha256-sha1 =
 # Create image
 qemu-img create -f luks --object 

[PATCH 07/14] iotests: replace unchecked calls to qemu_img_pipe()

2022-03-08 Thread John Snow
qemu_img_pipe() discards the return code from qemu-img in favor of
returning just its output. Some tests using this function don't save,
log, or check the output either, though, which is unsafe.

Replace all of these calls with a checked version.

Tests affected are 194, 202, 203, 234, 262, and 303.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/194 | 4 ++--
 tests/qemu-iotests/202 | 4 ++--
 tests/qemu-iotests/203 | 4 ++--
 tests/qemu-iotests/234 | 4 ++--
 tests/qemu-iotests/262 | 2 +-
 tests/qemu-iotests/303 | 2 +-
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/194 b/tests/qemu-iotests/194
index e44b8df728..68894371f5 100755
--- a/tests/qemu-iotests/194
+++ b/tests/qemu-iotests/194
@@ -33,8 +33,8 @@ with iotests.FilePath('source.img') as source_img_path, \
  iotests.VM('dest') as dest_vm:
 
 img_size = '1G'
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, source_img_path, 
img_size)
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, dest_img_path, 
img_size)
+iotests.qemu_img_create('-f', iotests.imgfmt, source_img_path, img_size)
+iotests.qemu_img_create('-f', iotests.imgfmt, dest_img_path, img_size)
 
 iotests.log('Launching VMs...')
 (source_vm.add_drive(source_img_path)
diff --git a/tests/qemu-iotests/202 b/tests/qemu-iotests/202
index 8eb5f32d15..b784dcd791 100755
--- a/tests/qemu-iotests/202
+++ b/tests/qemu-iotests/202
@@ -35,8 +35,8 @@ with iotests.FilePath('disk0.img') as disk0_img_path, \
  iotests.VM() as vm:
 
 img_size = '10M'
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk0_img_path, 
img_size)
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk1_img_path, 
img_size)
+iotests.qemu_img_create('-f', iotests.imgfmt, disk0_img_path, img_size)
+iotests.qemu_img_create('-f', iotests.imgfmt, disk1_img_path, img_size)
 
 iotests.log('Launching VM...')
 vm.launch()
diff --git a/tests/qemu-iotests/203 b/tests/qemu-iotests/203
index ea30e50497..ab80fd0e44 100755
--- a/tests/qemu-iotests/203
+++ b/tests/qemu-iotests/203
@@ -33,8 +33,8 @@ with iotests.FilePath('disk0.img') as disk0_img_path, \
  iotests.VM() as vm:
 
 img_size = '10M'
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk0_img_path, 
img_size)
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, disk1_img_path, 
img_size)
+iotests.qemu_img_create('-f', iotests.imgfmt, disk0_img_path, img_size)
+iotests.qemu_img_create('-f', iotests.imgfmt, disk1_img_path, img_size)
 
 iotests.log('Launching VM...')
 (vm.add_object('iothread,id=iothread0')
diff --git a/tests/qemu-iotests/234 b/tests/qemu-iotests/234
index cb5f1753e0..a9f764bb2c 100755
--- a/tests/qemu-iotests/234
+++ b/tests/qemu-iotests/234
@@ -34,8 +34,8 @@ with iotests.FilePath('img') as img_path, \
  iotests.VM(path_suffix='a') as vm_a, \
  iotests.VM(path_suffix='b') as vm_b:
 
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, backing_path, '64M')
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, img_path, '64M')
+iotests.qemu_img_create('-f', iotests.imgfmt, backing_path, '64M')
+iotests.qemu_img_create('-f', iotests.imgfmt, img_path, '64M')
 
 os.mkfifo(fifo_a)
 os.mkfifo(fifo_b)
diff --git a/tests/qemu-iotests/262 b/tests/qemu-iotests/262
index 32d69988ef..2294fd5ecb 100755
--- a/tests/qemu-iotests/262
+++ b/tests/qemu-iotests/262
@@ -51,7 +51,7 @@ with iotests.FilePath('img') as img_path, \
 
 vm.add_device('virtio-blk,drive=%s,iothread=iothread0' % root)
 
-iotests.qemu_img_pipe('create', '-f', iotests.imgfmt, img_path, '64M')
+iotests.qemu_img_create('-f', iotests.imgfmt, img_path, '64M')
 
 os.mkfifo(fifo)
 
diff --git a/tests/qemu-iotests/303 b/tests/qemu-iotests/303
index 16c2e10827..93aa5ce9b7 100755
--- a/tests/qemu-iotests/303
+++ b/tests/qemu-iotests/303
@@ -38,7 +38,7 @@ def create_bitmap(bitmap_number, disabled):
 if disabled:
 args.append('--disable')
 
-iotests.qemu_img_pipe(*args)
+iotests.qemu_img(*args)
 
 
 def write_to_disk(offset, size):
-- 
2.34.1




[PATCH 05/14] iotests: add qemu_img_map() function

2022-03-08 Thread John Snow
Add a qemu_img_map() function by analogy with qemu_img_measure(),
qemu_img_check(), and qemu_img_info() that all return JSON information.

Replace calls to qemu_img_pipe('map', '--output=json', ...) with this
new function, which provides better diagnostic information on failure.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/041 |  5 ++---
 tests/qemu-iotests/211 |  6 +++---
 tests/qemu-iotests/iotests.py  |  3 +++
 tests/qemu-iotests/tests/block-status-cache| 11 ---
 tests/qemu-iotests/tests/parallels-read-bitmap |  6 ++
 5 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index db9f5dc540..3e16acee56 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -24,7 +24,7 @@ import os
 import re
 import json
 import iotests
-from iotests import qemu_img, qemu_img_pipe, qemu_io
+from iotests import qemu_img, qemu_img_map, qemu_io
 
 backing_img = os.path.join(iotests.test_dir, 'backing.img')
 target_backing_img = os.path.join(iotests.test_dir, 'target-backing.img')
@@ -1360,8 +1360,7 @@ class TestFilters(iotests.QMPTestCase):
 
 self.vm.qmp('blockdev-del', node_name='target')
 
-target_map = qemu_img_pipe('map', '--output=json', target_img)
-target_map = json.loads(target_map)
+target_map = qemu_img_map(target_img)
 
 assert target_map[0]['start'] == 0
 assert target_map[0]['length'] == 512 * 1024
diff --git a/tests/qemu-iotests/211 b/tests/qemu-iotests/211
index f52cadade1..1a3b4596c8 100755
--- a/tests/qemu-iotests/211
+++ b/tests/qemu-iotests/211
@@ -59,7 +59,7 @@ with iotests.FilePath('t.vdi') as disk_path, \
 vm.shutdown()
 
 iotests.img_info_log(disk_path)
-iotests.log(iotests.qemu_img_pipe('map', '--output=json', disk_path))
+iotests.log(iotests.qemu_img_map(disk_path))
 
 #
 # Successful image creation (explicit defaults)
@@ -83,7 +83,7 @@ with iotests.FilePath('t.vdi') as disk_path, \
 vm.shutdown()
 
 iotests.img_info_log(disk_path)
-iotests.log(iotests.qemu_img_pipe('map', '--output=json', disk_path))
+iotests.log(iotests.qemu_img_map(disk_path))
 
 #
 # Successful image creation (with non-default options)
@@ -107,7 +107,7 @@ with iotests.FilePath('t.vdi') as disk_path, \
 vm.shutdown()
 
 iotests.img_info_log(disk_path)
-iotests.log(iotests.qemu_img_pipe('map', '--output=json', disk_path))
+iotests.log(iotests.qemu_img_map(disk_path))
 
 #
 # Invalid BlockdevRef
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 62f82281a9..3896440238 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -323,6 +323,9 @@ def qemu_img_check(*args: str) -> Any:
 def qemu_img_info(*args: str) -> Any:
 return qemu_img_json('info', "--output", "json", *args)
 
+def qemu_img_map(*args: str) -> Any:
+return qemu_img_json('map', "--output", "json", *args)
+
 def qemu_img_pipe(*args: str) -> str:
 '''Run qemu-img and return its output'''
 return qemu_img_pipe_and_status(*args)[0]
diff --git a/tests/qemu-iotests/tests/block-status-cache 
b/tests/qemu-iotests/tests/block-status-cache
index 40e648e251..5a7bc2c149 100755
--- a/tests/qemu-iotests/tests/block-status-cache
+++ b/tests/qemu-iotests/tests/block-status-cache
@@ -22,7 +22,7 @@
 import os
 import signal
 import iotests
-from iotests import qemu_img_create, qemu_img_pipe, qemu_nbd
+from iotests import qemu_img_create, qemu_img_map, qemu_nbd
 
 
 image_size = 1 * 1024 * 1024
@@ -76,8 +76,7 @@ class TestBscWithNbd(iotests.QMPTestCase):
 # to allocate the first sector to facilitate alignment probing), and
 # then the rest to be zero.  The BSC will thus contain (if anything)
 # one range covering the first sector.
-map_pre = qemu_img_pipe('map', '--output=json', '--image-opts',
-nbd_img_opts)
+map_pre = qemu_img_map('--image-opts', nbd_img_opts)
 
 # qemu:allocation-depth maps for want_zero=false.
 # want_zero=false should (with the file driver, which the server is
@@ -111,14 +110,12 @@ class TestBscWithNbd(iotests.QMPTestCase):
 # never loop too many times here.
 for _ in range(2):
 # (Ignore the result, this is just to contaminate the cache)
-qemu_img_pipe('map', '--output=json', '--image-opts',
-  nbd_img_opts_alloc_depth)
+qemu_img_map('--image-opts', nbd_img_opts_alloc_depth)
 
 # Now let's see whether the cache reports everything as data, or
 # whether we get correct information (i.e. the same as we got on our
 # first attempt).
-map_post = qemu_img_pipe('map', '--output=json', '--image-opts',
- nbd_img_opts)
+map_post = qemu_img_map('--image-opts', nbd_img_opts)
 
 if map_pre != 

[PATCH 03/14] iotests: add qemu_img_info()

2022-03-08 Thread John Snow
Add qemu_img_info() by analogy with qemu_img_measure() and
qemu_img_check(). Modify image_size() to use this function instead to
take advantage of the better diagnostic information on failure provided
(ultimately) by qemu_img().

Signed-off-by: John Snow 
---
 tests/qemu-iotests/065|  5 ++---
 tests/qemu-iotests/242|  5 ++---
 tests/qemu-iotests/iotests.py | 15 +++
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065
index f7c1b68dad..9466ce7df4 100755
--- a/tests/qemu-iotests/065
+++ b/tests/qemu-iotests/065
@@ -24,7 +24,7 @@ import os
 import re
 import json
 import iotests
-from iotests import qemu_img, qemu_img_pipe
+from iotests import qemu_img, qemu_img_info, qemu_img_pipe
 import unittest
 
 test_img = os.path.join(iotests.test_dir, 'test.img')
@@ -49,8 +49,7 @@ class TestQemuImgInfo(TestImageInfoSpecific):
 human_compare = None
 
 def test_json(self):
-data = json.loads(qemu_img_pipe('info', '--output=json', test_img))
-data = data['format-specific']
+data = qemu_img_info(test_img)['format-specific']
 self.assertEqual(data['type'], iotests.imgfmt)
 self.assertEqual(data['data'], self.json_compare)
 
diff --git a/tests/qemu-iotests/242 b/tests/qemu-iotests/242
index 96a30152b0..547bf382e3 100755
--- a/tests/qemu-iotests/242
+++ b/tests/qemu-iotests/242
@@ -22,7 +22,7 @@
 import iotests
 import json
 import struct
-from iotests import qemu_img_create, qemu_io, qemu_img_pipe, \
+from iotests import qemu_img_create, qemu_io, qemu_img_info, \
 file_path, img_info_log, log, filter_qemu_io
 
 iotests.script_initialize(supported_fmts=['qcow2'],
@@ -39,8 +39,7 @@ flag_offset = 0x5000f
 def print_bitmap(extra_args):
 log('qemu-img info dump:\n')
 img_info_log(disk, extra_args=extra_args)
-result = json.loads(qemu_img_pipe('info', '--force-share',
-  '--output=json', disk))
+result = qemu_img_info('--force-share', disk)
 if 'bitmaps' in result['format-specific']['data']:
 bitmaps = result['format-specific']['data']['bitmaps']
 log('The same bitmaps in JSON format:')
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 7b37938d45..62f82281a9 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -320,6 +320,9 @@ def qemu_img_measure(*args: str) -> Any:
 def qemu_img_check(*args: str) -> Any:
 return qemu_img_json("check", "--output", "json", *args)
 
+def qemu_img_info(*args: str) -> Any:
+return qemu_img_json('info', "--output", "json", *args)
+
 def qemu_img_pipe(*args: str) -> str:
 '''Run qemu-img and return its output'''
 return qemu_img_pipe_and_status(*args)[0]
@@ -570,10 +573,14 @@ def create_image(name, size):
 file.write(sector)
 i = i + 512
 
-def image_size(img):
-'''Return image's virtual size'''
-r = qemu_img_pipe('info', '--output=json', '-f', imgfmt, img)
-return json.loads(r)['virtual-size']
+def image_size(img: str) -> int:
+"""Return image's virtual size"""
+value = qemu_img_info('-f', imgfmt, img)['virtual-size']
+if not isinstance(value, int):
+type_name = type(value).__name__
+raise TypeError("Expected 'int' for 'virtual-size', "
+f"got '{value}' of type '{type_name}'")
+return value
 
 def is_str(val):
 return isinstance(val, str)
-- 
2.34.1




[PATCH 10/14] iotests: use qemu_img() in has_working_luks()

2022-03-08 Thread John Snow
Admittedly a mostly lateral move, but qemu_img() is essentially the
replacement for qemu_img_pipe_and_status(). It will give slightly better
diagnostics on crash.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 698d5a7fdc..1b6e28ba64 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1450,20 +1450,20 @@ def has_working_luks() -> Tuple[bool, str]:
 """
 
 img_file = f'{test_dir}/luks-test.luks'
-(output, status) = \
-qemu_img_pipe_and_status('create', '-f', 'luks',
- '--object', luks_default_secret_object,
- '-o', luks_default_key_secret_opt,
- '-o', 'iter-time=10',
- img_file, '1G')
+res = qemu_img('create', '-f', 'luks',
+   '--object', luks_default_secret_object,
+   '-o', luks_default_key_secret_opt,
+   '-o', 'iter-time=10',
+   img_file, '1G',
+   check=False)
 try:
 os.remove(img_file)
 except OSError:
 pass
 
-if status != 0:
-reason = output
-for line in output.splitlines():
+if res.returncode:
+reason = res.stdout
+for line in res.stdout.splitlines():
 if img_file + ':' in line:
 reason = line.split(img_file + ':', 1)[1].strip()
 break
-- 
2.34.1




[PATCH 09/14] iotests: remove remaining calls to qemu_img_pipe()

2022-03-08 Thread John Snow
As part of moving all python iotest invocations of qemu-img onto a
single qemu_img() implementation, remove a few lingering uses of
qemu_img_pipe() from outside of iotests.py itself.

Several cases here rely on the knowledge that qemu_img_pipe() suppresses
*all* output on a successful case when the command being issued is
'create'.

065: This call's output is inspected, but it appears as if it's expected
 to succeed. Replace this call with the checked qemu_img() variant
 instead to get better diagnostics if/when qemu-img itself fails.

237: "create" call output isn't actually logged. Use qemu_img_create()
 instead, which checks the return code. Remove the empty lines from
 the test output.

296: Two calls;
 -create: Expected to succeed. Like other create calls, the output
  isn't actually logged.  Switch to a checked variant
  (qemu_img_create) instead. The output for this test is
  a mixture of both test styles, so actually replace the
  blank line for readability.
 -amend:  This is expected to fail. Log the output.

After this patch, the only uses of qemu_img_pipe are internal to
iotests.py and will be removed in subsequent patches.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/065 |  4 ++--
 tests/qemu-iotests/237 |  3 +--
 tests/qemu-iotests/237.out |  3 ---
 tests/qemu-iotests/296 | 12 ++--
 4 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065
index 9466ce7df4..ba94e19349 100755
--- a/tests/qemu-iotests/065
+++ b/tests/qemu-iotests/065
@@ -24,7 +24,7 @@ import os
 import re
 import json
 import iotests
-from iotests import qemu_img, qemu_img_info, qemu_img_pipe
+from iotests import qemu_img, qemu_img_info
 import unittest
 
 test_img = os.path.join(iotests.test_dir, 'test.img')
@@ -54,7 +54,7 @@ class TestQemuImgInfo(TestImageInfoSpecific):
 self.assertEqual(data['data'], self.json_compare)
 
 def test_human(self):
-data = qemu_img_pipe('info', '--output=human', test_img).split('\n')
+data = qemu_img('info', '--output=human', test_img).stdout.split('\n')
 data = data[(data.index('Format specific information:') + 1)
 :data.index('')]
 for field in data:
diff --git a/tests/qemu-iotests/237 b/tests/qemu-iotests/237
index 43dfd3bd40..5ea13eb01f 100755
--- a/tests/qemu-iotests/237
+++ b/tests/qemu-iotests/237
@@ -165,8 +165,7 @@ with iotests.FilePath('t.vmdk') as disk_path, \
 iotests.log("")
 
 for path in [ extent1_path, extent2_path, extent3_path ]:
-msg = iotests.qemu_img_pipe('create', '-f', imgfmt, path, '0')
-iotests.log(msg, [iotests.filter_testfiles])
+iotests.qemu_img_create('-f', imgfmt, path, '0')
 
 vm.add_blockdev('driver=file,filename=%s,node-name=ext1' % (extent1_path))
 vm.add_blockdev('driver=file,filename=%s,node-name=ext2' % (extent2_path))
diff --git a/tests/qemu-iotests/237.out b/tests/qemu-iotests/237.out
index aeb9724492..62b8865677 100644
--- a/tests/qemu-iotests/237.out
+++ b/tests/qemu-iotests/237.out
@@ -129,9 +129,6 @@ Job failed: Cannot find device='this doesn't exist' nor 
node-name='this doesn't
 
 === Other subformats ===
 
-
-
-
 == Missing extent ==
 
 {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": 
{"driver": "vmdk", "file": "node0", "size": 33554432, "subformat": 
"monolithicFlat"}}}
diff --git a/tests/qemu-iotests/296 b/tests/qemu-iotests/296
index f80ef3434a..0d21b740a7 100755
--- a/tests/qemu-iotests/296
+++ b/tests/qemu-iotests/296
@@ -76,7 +76,7 @@ class EncryptionSetupTestCase(iotests.QMPTestCase):
 # create the encrypted block device using qemu-img
 def createImg(self, file, secret):
 
-output = iotests.qemu_img_pipe(
+iotests.qemu_img(
 'create',
 '--object', *secret.to_cmdline_object(),
 '-f', iotests.imgfmt,
@@ -84,8 +84,7 @@ class EncryptionSetupTestCase(iotests.QMPTestCase):
 '-o', 'iter-time=10',
 file,
 '1M')
-
-iotests.log(output, filters=[iotests.filter_test_dir])
+iotests.log('')
 
 # attempts to add a key using qemu-img
 def addKey(self, file, secret, new_secret):
@@ -99,7 +98,7 @@ class EncryptionSetupTestCase(iotests.QMPTestCase):
 }
 }
 
-output = iotests.qemu_img_pipe(
+output = iotests.qemu_img(
 'amend',
 '--object', *secret.to_cmdline_object(),
 '--object', *new_secret.to_cmdline_object(),
@@ -108,8 +107,9 @@ class EncryptionSetupTestCase(iotests.QMPTestCase):
 '-o', 'new-secret=' + new_secret.id(),
 '-o', 'iter-time=10',
 
-"json:" + json.dumps(image_options)
-)
+"json:" + json.dumps(image_options),
+check=False  # Expected to fail. Log output.
+).stdout
 
 iotests.log(output, 

[PATCH 14/14] iotests: make img_info_log() call qemu_img_log()

2022-03-08 Thread John Snow
Add configurable filters to qemu_img_log(), and re-write img_info_log()
to call into qemu_img_log() with a custom filter instead.

After this patch, every last call to qemu_img() is now guaranteed to
either have its return code checked for zero, OR have its output
actually visibly logged somewhere.

Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 6af503058f..0c69327c00 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -317,10 +317,14 @@ def qemu_img_info(*args: str) -> Any:
 def qemu_img_map(*args: str) -> Any:
 return qemu_img_json('map', "--output", "json", *args)
 
-def qemu_img_log(*args: str) -> subprocess.CompletedProcess[str]:
+def qemu_img_log(
+*args: str,
+filters: Iterable[Callable[[str], str]] = (),
+) -> subprocess.CompletedProcess[str]:
 """
 Logged, unchecked variant of qemu_img() that allows non-zero exit codes.
 
+By default, output will be filtered through filter_testfiles().
 If logging is perceived to be disabled, this function will fall back
 to prohibiting non-zero return codes.
 
@@ -331,7 +335,7 @@ def qemu_img_log(*args: str) -> 
subprocess.CompletedProcess[str]:
 :return: a CompletedProcess instance with returncode and console output.
 """
 result = qemu_img(*args, check=not logging_enabled())
-log(result.stdout, filters=[filter_testfiles])
+log(result.stdout, filters=filters or [filter_testfiles])
 return result
 
 def img_info_log(filename: str, filter_path: Optional[str] = None,
@@ -345,10 +349,11 @@ def img_info_log(filename: str, filter_path: 
Optional[str] = None,
 args += extra_args
 args.append(filename)
 
-output = qemu_img(*args, check=False).stdout
 if not filter_path:
 filter_path = filename
-log(filter_img_info(output, filter_path))
+qemu_img_log(
+*args,
+filters=[lambda output: filter_img_info(output, filter_path)])
 
 def qemu_io_wrap_args(args: Sequence[str]) -> List[str]:
 if '-f' in args or '--image-opts' in args:
-- 
2.34.1




Re: [PULL 00/11] Python patches

2022-03-08 Thread Peter Maydell
On Mon, 7 Mar 2022 at 22:15, John Snow  wrote:
>
> The following changes since commit b49872aa8fc0f3f5a3036cc37aa2cb5c92866f33:
>
>   Merge remote-tracking branch 
> 'remotes/hreitz-gitlab/tags/pull-block-2022-03-07' into staging (2022-03-07 
> 17:14:09 +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/jsnow/qemu.git tags/python-pull-request
>
> for you to fetch changes up to 7cba010e821bf227e5fa016d0df06f2a33a0c318:
>
>   scripts/qmp-shell-wrap: Fix import path (2022-03-07 14:36:47 -0500)
>
> 
> Python patches
>
> Hopefully, fixes the race conditions witnessed through the NetBSD vm tests.
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/7.0
for any user-visible changes.

-- PMM



[PATCH v2] block/nbd.c: Fixed IO request coroutine not being wakeup when kill NBD server

2022-03-08 Thread Rao Lei
During the IO stress test, the IO request coroutine has a probability that is
can't be awakened when the NBD server is killed.

The GDB stack is as follows:
(gdb) bt
0  0x7f2ff990cbf6 in __ppoll (fds=0x55575de85000, nfds=1, 
timeout=, sigmask=0x0) at ../sysdeps/unix/sysv/linux/ppoll.c:44
1  0x55575c302e7c in qemu_poll_ns (fds=0x55575de85000, nfds=1, 
timeout=59603140) at ../util/qemu-timer.c:348
2  0x55575c2d3c34 in fdmon_poll_wait (ctx=0x55575dc480f0, 
ready_list=0x7ffd9dd1dae0, timeout=59603140) at ../util/fdmon-poll.c:80
3  0x55575c2d350d in aio_poll (ctx=0x55575dc480f0, blocking=true) at 
../util/aio-posix.c:655
4  0x55575c16eabd in bdrv_do_drained_begin(bs=0x55575dee7fe0, 
recursive=false, parent=0x0, ignore_bds_parents=false, poll=true)at 
../block/io.c:474
5  0x55575c16eba6 in bdrv_drained_begin (bs=0x55575dee7fe0) at 
../block/io.c:480
6  0x55575c1aff33 in quorum_del_child (bs=0x55575dee7fe0, 
child=0x55575dcea690, errp=0x7ffd9dd1dd08) at ../block/quorum.c:1130
7  0x55575c14239b in bdrv_del_child (parent_bs=0x55575dee7fe0, 
child=0x55575dcea690, errp=0x7ffd9dd1dd08) at ../block.c:7705
8  0x55575c12da28 in qmp_x_blockdev_change(parent=0x55575df404c0 
"colo-disk0", has_child=true, child=0x55575de867f0 "children.1", 
has_node=false, no   de=0x0, errp=0x7ffd9dd1dd08) at ../blockdev.c:3676
9  0x55575c258435 in qmp_marshal_x_blockdev_change (args=0x7f2fec008190, 
ret=0x7f2ff7b0bd98, errp=0x7f2ff7b0bd90) at qapi/qapi-commands-block-core.c   
:1675
10 0x55575c2c6201 in do_qmp_dispatch_bh (opaque=0x7f2ff7b0be30) at 
../qapi/qmp-dispatch.c:129
11 0x55575c2ebb1c in aio_bh_call (bh=0x55575dc429c0) at ../util/async.c:141
12 0x55575c2ebc2a in aio_bh_poll (ctx=0x55575dc480f0) at ../util/async.c:169
13 0x55575c2d2d96 in aio_dispatch (ctx=0x55575dc480f0) at 
../util/aio-posix.c:415
14 0x55575c2ec07f in aio_ctx_dispatch (source=0x55575dc480f0, callback=0x0, 
user_data=0x0) at ../util/async.c:311
15 0x7f2ff9e7cfbd in g_main_context_dispatch () at 
/lib/x86_64-linux-gnu/libglib-2.0.so.0
16 0x55575c2fd581 in glib_pollfds_poll () at ../util/main-loop.c:232
17 0x55575c2fd5ff in os_host_main_loop_wait (timeout=0) at 
../util/main-loop.c:255
18 0x55575c2fd710 in main_loop_wait (nonblocking=0) at 
../util/main-loop.c:531
19 0x55575bfa7588 in qemu_main_loop () at ../softmmu/runstate.c:726
20 0x55575bbee57a in main (argc=60, argv=0x7ffd9dd1e0e8, 
envp=0x7ffd9dd1e2d0) at ../softmmu/main.c:50

(gdb) qemu coroutine 0x55575e16aac0
0  0x55575c2ee7dc in qemu_coroutine_switch (from_=0x55575e16aac0, 
to_=0x7f2ff830fba0, action=COROUTINE_YIELD) at ../util/coroutine-ucontext.c:302
1  0x55575c2fe2a9 in qemu_coroutine_yield () at ../util/qemu-coroutine.c:195
2  0x55575c2fe93c in qemu_co_queue_wait_impl (queue=0x55575dc46170, 
lock=0x7f2b32ad9850) at ../util/qemu-coroutine-lock.c:56
3  0x55575c17ddfb in nbd_co_send_request (bs=0x55575ebfaf20, 
request=0x7f2b32ad9920, qiov=0x55575dfc15d8) at ../block/nbd.c:478
4  0x55575c17f931 in nbd_co_request (bs=0x55575ebfaf20, 
request=0x7f2b32ad9920, write_qiov=0x55575dfc15d8) at ../block/nbd.c:1182
5  0x55575c17fe14 in nbd_client_co_pwritev (bs=0x55575ebfaf20, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, flags=0) at 
../block/nbd.c:1284
6  0x55575c170d25 in bdrv_driver_pwritev (bs=0x55575ebfaf20, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
at ../block/io.c:1264
7  0x55575c1733b4 in bdrv_aligned_pwritev
(child=0x55575dff6890, req=0x7f2b32ad9ad0, offset=403487858688, 
bytes=4538368, align=1, qiov=0x55575dfc15d8, qiov_offset=0, flags=0) at 
../block/io.c:2126
8  0x55575c173c67 in bdrv_co_pwritev_part (child=0x55575dff6890, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
at ../block/io.c:2314
9  0x55575c17391b in bdrv_co_pwritev (child=0x55575dff6890, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, flags=0) at 
../block/io.c:2233
10 0x55575c1ee506 in replication_co_writev (bs=0x55575e9824f0, 
sector_num=788062224, remaining_sectors=8864, qiov=0x55575dfc15d8, flags=0)
at ../block/replication.c:270
11 0x55575c170eed in bdrv_driver_pwritev (bs=0x55575e9824f0, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
at ../block/io.c:1297
12 0x55575c1733b4 in bdrv_aligned_pwritev
(child=0x55575dcea690, req=0x7f2b32ad9e00, offset=403487858688, 
bytes=4538368, align=512, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
at ../block/io.c:2126
13 0x55575c173c67 in bdrv_co_pwritev_part (child=0x55575dcea690, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
at ../block/io.c:2314
14 0x55575c17391b in bdrv_co_pwritev (child=0x55575dcea690, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, flags=0) at 
../block/io.c:2233
15 0x55575c1aeffa in write_quorum_entry 

Re: [PATCH] block/nbd.c: Fixed IO request coroutine not being wakeup when kill NBD server.

2022-03-08 Thread Rao, Lei




On 3/3/2022 10:05 PM, Rao, Lei wrote:



On 3/3/2022 5:25 PM, Vladimir Sementsov-Ogievskiy wrote:

03.03.2022 05:21, Rao Lei wrote:

During the stress test, the IO request coroutine has a probability that it
can't be awakened when the NBD server is killed.

The GDB statck is as follows:
(gdb) bt
0  0x7f2ff990cbf6 in __ppoll (fds=0x55575de85000, nfds=1, timeout=, sigmask=0x0) at ../sysdeps/unix/sysv/linux/ppoll.c:44
1  0x55575c302e7c in qemu_poll_ns (fds=0x55575de85000, nfds=1, 
timeout=59603140) at ../util/qemu-timer.c:348
2  0x55575c2d3c34 in fdmon_poll_wait (ctx=0x55575dc480f0, 
ready_list=0x7ffd9dd1dae0, timeout=59603140) at ../util/fdmon-poll.c:80
3  0x55575c2d350d in aio_poll (ctx=0x55575dc480f0, blocking=true) at 
../util/aio-posix.c:655
4  0x55575c16eabd in bdrv_do_drained_begin (bs=0x55575dee7fe0, 
recursive=false, parent=0x0, ignore_bds_parents=false, poll=true) at 
../block/io.c:474
5  0x55575c16eba6 in bdrv_drained_begin (bs=0x55575dee7fe0) at 
../block/io.c:480
6  0x55575c1aff33 in quorum_del_child (bs=0x55575dee7fe0, 
child=0x55575dcea690, errp=0x7ffd9dd1dd08) at ../block/quorum.c:1130
7  0x55575c14239b in bdrv_del_child (parent_bs=0x55575dee7fe0, 
child=0x55575dcea690, errp=0x7ffd9dd1dd08) at ../block.c:7705
8  0x55575c12da28 in qmp_x_blockdev_change
 (parent=0x55575df404c0 "colo-disk0", has_child=true, child=0x55575de867f0 
"children.1", has_node=false, node=0x0, errp=0x7ffd9dd1dd08)
 at ../blockdev.c:3676
9  0x55575c258435 in qmp_marshal_x_blockdev_change (args=0x7f2fec008190, 
ret=0x7f2ff7b0bd98, errp=0x7f2ff7b0bd90) at qapi/qapi-commands-block-core.c:1675
10 0x55575c2c6201 in do_qmp_dispatch_bh (opaque=0x7f2ff7b0be30) at 
../qapi/qmp-dispatch.c:129
11 0x55575c2ebb1c in aio_bh_call (bh=0x55575dc429c0) at ../util/async.c:141
12 0x55575c2ebc2a in aio_bh_poll (ctx=0x55575dc480f0) at ../util/async.c:169
13 0x55575c2d2d96 in aio_dispatch (ctx=0x55575dc480f0) at 
../util/aio-posix.c:415
14 0x55575c2ec07f in aio_ctx_dispatch (source=0x55575dc480f0, callback=0x0, 
user_data=0x0) at ../util/async.c:311
15 0x7f2ff9e7cfbd in g_main_context_dispatch () at 
/lib/x86_64-linux-gnu/libglib-2.0.so.0
16 0x55575c2fd581 in glib_pollfds_poll () at ../util/main-loop.c:232
17 0x55575c2fd5ff in os_host_main_loop_wait (timeout=0) at 
../util/main-loop.c:255
18 0x55575c2fd710 in main_loop_wait (nonblocking=0) at 
../util/main-loop.c:531
19 0x55575bfa7588 in qemu_main_loop () at ../softmmu/runstate.c:726
20 0x55575bbee57a in main (argc=60, argv=0x7ffd9dd1e0e8, 
envp=0x7ffd9dd1e2d0) at ../softmmu/main.c:50

(gdb) qemu coroutine 0x55575e16aac0
0  0x55575c2ee7dc in qemu_coroutine_switch (from_=0x55575e16aac0, 
to_=0x7f2ff830fba0, action=COROUTINE_YIELD) at ../util/coroutine-ucontext.c:302
1  0x55575c2fe2a9 in qemu_coroutine_yield () at ../util/qemu-coroutine.c:195
2  0x55575c2fe93c in qemu_co_queue_wait_impl (queue=0x55575dc46170, 
lock=0x7f2b32ad9850) at ../util/qemu-coroutine-lock.c:56
3  0x55575c17ddfb in nbd_co_send_request (bs=0x55575ebfaf20, 
request=0x7f2b32ad9920, qiov=0x55575dfc15d8) at ../block/nbd.c:478
4  0x55575c17f931 in nbd_co_request (bs=0x55575ebfaf20, 
request=0x7f2b32ad9920, write_qiov=0x55575dfc15d8) at ../block/nbd.c:1182
5  0x55575c17fe14 in nbd_client_co_pwritev (bs=0x55575ebfaf20, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, flags=0) at 
../block/nbd.c:1284
6  0x55575c170d25 in bdrv_driver_pwritev (bs=0x55575ebfaf20, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
 at ../block/io.c:1264
7  0x55575c1733b4 in bdrv_aligned_pwritev
 (child=0x55575dff6890, req=0x7f2b32ad9ad0, offset=403487858688, 
bytes=4538368, align=1, qiov=0x55575dfc15d8, qiov_offset=0, flags=0) at 
../block/io.c:2126
8  0x55575c173c67 in bdrv_co_pwritev_part (child=0x55575dff6890, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
 at ../block/io.c:2314
9  0x55575c17391b in bdrv_co_pwritev (child=0x55575dff6890, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, flags=0) at 
../block/io.c:2233
10 0x55575c1ee506 in replication_co_writev (bs=0x55575e9824f0, 
sector_num=788062224, remaining_sectors=8864, qiov=0x55575dfc15d8, flags=0)
 at ../block/replication.c:270
11 0x55575c170eed in bdrv_driver_pwritev (bs=0x55575e9824f0, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
 at ../block/io.c:1297
12 0x55575c1733b4 in bdrv_aligned_pwritev
 (child=0x55575dcea690, req=0x7f2b32ad9e00, offset=403487858688, 
bytes=4538368, align=512, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
 at ../block/io.c:2126
13 0x55575c173c67 in bdrv_co_pwritev_part (child=0x55575dcea690, 
offset=403487858688, bytes=4538368, qiov=0x55575dfc15d8, qiov_offset=0, flags=0)
 at ../block/io.c:2314
14 0x55575c17391b in bdrv_co_pwritev (child=0x55575dcea690, 

Re: [PATCH] hw/block: m25p80: Add support for w25q01jvq

2022-03-08 Thread Cédric Le Goater

On 3/4/22 19:09, Patrick Williams wrote:

The w25q01jvq is a 128MB part.  Support is being added to the kernel[1]
and the two have been tested together.

1. https://lore.kernel.org/lkml/2022022209.23108-1-potin@quantatw.com/

Signed-off-by: Patrick Williams 
Cc: Potin Lai 
---
  hw/block/m25p80.c | 1 +
  1 file changed, 1 insertion(+)


If that's ok with the maintainers, I am going to take this patch through
the aspeed machine queue since the new 'bletchley-bmc' machine depends
on it :

  
http://patchwork.ozlabs.org/project/qemu-devel/patch/20220305000656.1944589-2-patr...@stwcx.xyz/

I should send the PR today.

Thanks,

C.


diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index c6bf3c6bfa..7d3d8b12e0 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -340,6 +340,7 @@ static const FlashPartInfo known_devices[] = {
  { INFO("w25q80bl",0xef4014,  0,  64 << 10,  16, ER_4K) },
  { INFO("w25q256", 0xef4019,  0,  64 << 10, 512, ER_4K) },
  { INFO("w25q512jv",   0xef4020,  0,  64 << 10, 1024, ER_4K) },
+{ INFO("w25q01jvq",   0xef4021,  0,  64 << 10, 2048, ER_4K) },
  };
  
  typedef enum {