[PULL 09/11] ui: honour the actual guest display dimensions without rounding

[Gerd Hoffmann]

From: Daniel P. Berrangé 

A long time ago the VNC server code had some memory corruption
fixes done in:

  commit bea60dd7679364493a0d7f5b54316c767cf894ef
  Author: Peter Lieven 
  Date:   Mon Jun 30 10:57:51 2014 +0200

ui/vnc: fix potential memory corruption issues

One of the implications of the fix was that the VNC server would have a
thin black bad down the right hand side if the guest desktop width was
not a multiple of 16. In practice this was a non-issue since the VNC
server was always honouring a guest specified resolution and guests
essentially always pick from a small set of sane resolutions likely in
real world hardware.

We recently introduced support for the extended desktop resize extension
and as a result the VNC client has ability to specify an arbitrary
desktop size and the guest OS may well honour it exactly. As a result we
no longer have any guarantee that the width will be a multiple of 16,
and so when resizing the desktop we have a 93% chance of getting the
black bar on the right hand size.

The VNC server maintains three different desktop dimensions

 1. The guest surface
 2. The server surface
 3. The client desktop

The requirement for the width to be a multiple of 16 only applies to
item 2, the server surface, for the purpose of doing dirty bitmap
tracking.

Normally we will set the client desktop size to always match the server
surface size, but that's not a strict requirement. In order to cope with
clients that don't support the desktop size encoding, we already allow
for the client desktop to be a different size that the server surface.

Thus we can trivially eliminate the black bar, but setting the client
desktop size to be the un-rounded server surface size - the so called
"true width".

Signed-off-by: Daniel P. Berrangé 
Reviewed-by: Marc-André Lureau 
Message-Id: <20210311182957.486939-5-berra...@redhat.com>
Signed-off-by: Gerd Hoffmann 
---
 ui/vnc.h|  1 +
 ui/vnc.c| 23 +++
 ui/trace-events |  2 ++
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/ui/vnc.h b/ui/vnc.h
index 116463d5f099..d4f3e1555809 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -164,6 +164,7 @@ struct VncDisplay
 
 struct VncSurface guest;   /* guest visible surface (aka ds->surface) */
 pixman_image_t *server;/* vnc server surface */
+int true_width; /* server surface width before rounding up */
 
 const char *id;
 QTAILQ_ENTRY(VncDisplay) next;
diff --git a/ui/vnc.c b/ui/vnc.c
index 8c9890b3cdc4..9c004a11f495 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -608,6 +608,11 @@ static int vnc_width(VncDisplay *vd)
VNC_DIRTY_PIXELS_PER_BIT));
 }
 
+static int vnc_true_width(VncDisplay *vd)
+{
+return MIN(VNC_MAX_WIDTH, surface_width(vd->ds));
+}
+
 static int vnc_height(VncDisplay *vd)
 {
 return MIN(VNC_MAX_HEIGHT, surface_height(vd->ds));
@@ -691,16 +696,16 @@ static void vnc_desktop_resize(VncState *vs)
 !vnc_has_feature(vs, VNC_FEATURE_RESIZE_EXT))) {
 return;
 }
-if (vs->client_width == pixman_image_get_width(vs->vd->server) &&
+if (vs->client_width == vs->vd->true_width &&
 vs->client_height == pixman_image_get_height(vs->vd->server)) {
 return;
 }
 
-assert(pixman_image_get_width(vs->vd->server) < 65536 &&
-   pixman_image_get_width(vs->vd->server) >= 0);
+assert(vs->vd->true_width < 65536 &&
+   vs->vd->true_width >= 0);
 assert(pixman_image_get_height(vs->vd->server) < 65536 &&
pixman_image_get_height(vs->vd->server) >= 0);
-vs->client_width = pixman_image_get_width(vs->vd->server);
+vs->client_width = vs->vd->true_width;
 vs->client_height = pixman_image_get_height(vs->vd->server);
 
 if (vnc_has_feature(vs, VNC_FEATURE_RESIZE_EXT)) {
@@ -774,6 +779,7 @@ static void vnc_update_server_surface(VncDisplay *vd)
 
 width = vnc_width(vd);
 height = vnc_height(vd);
+vd->true_width = vnc_true_width(vd);
 vd->server = pixman_image_create_bits(VNC_SERVER_FB_FORMAT,
   width, height,
   NULL, 0);
@@ -809,13 +815,22 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl,
 vd->guest.fb = pixman_image_ref(surface->image);
 vd->guest.format = surface->format;
 
+
 if (pageflip) {
+trace_vnc_server_dpy_pageflip(vd,
+  surface_width(surface),
+  surface_height(surface),
+  surface_format(surface));
 vnc_set_area_dirty(vd->guest.dirty, vd, 0, 0,
surface_width(surface),
surface_height(surface));
 return;
 }
 
+trace_vnc_server_dpy_recreate(vd,
+  surface_width(surface),
+  surface_height(surface),
+   

[PULL 07/11] ui: avoid sending framebuffer updates outside client desktop bounds

[Gerd Hoffmann]

From: Daniel P. Berrangé 

We plan framebuffer update rects based on the VNC server surface. If the
client doesn't support desktop resize, then the client bounds may differ
from the server surface bounds. VNC clients may become upset if we then
send an update message outside the bounds of the client desktop.

This takes the approach of clamping the rectangles from the worker
thread immediately before sending them. This may sometimes results in
sending a framebuffer update message with zero rectangles.

Signed-off-by: Daniel P. Berrangé 
Reviewed-by: Marc-André Lureau 
Message-Id: <20210311182957.486939-3-berra...@redhat.com>
Signed-off-by: Gerd Hoffmann 
---
 ui/vnc-jobs.c   | 44 
 ui/trace-events |  5 +
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
index dbbfbefe5619..4562bf89288e 100644
--- a/ui/vnc-jobs.c
+++ b/ui/vnc-jobs.c
@@ -32,6 +32,7 @@
 #include "qemu/sockets.h"
 #include "qemu/main-loop.h"
 #include "block/aio.h"
+#include "trace.h"
 
 /*
  * Locking:
@@ -94,6 +95,8 @@ int vnc_job_add_rect(VncJob *job, int x, int y, int w, int h)
 {
 VncRectEntry *entry = g_new0(VncRectEntry, 1);
 
+trace_vnc_job_add_rect(job->vs, job, x, y, w, h);
+
 entry->rect.x = x;
 entry->rect.y = y;
 entry->rect.w = w;
@@ -190,6 +193,8 @@ static void vnc_async_encoding_start(VncState *orig, 
VncState *local)
 local->zlib = orig->zlib;
 local->hextile = orig->hextile;
 local->zrle = orig->zrle;
+local->client_width = orig->client_width;
+local->client_height = orig->client_height;
 }
 
 static void vnc_async_encoding_end(VncState *orig, VncState *local)
@@ -202,6 +207,34 @@ static void vnc_async_encoding_end(VncState *orig, 
VncState *local)
 orig->lossy_rect = local->lossy_rect;
 }
 
+static bool vnc_worker_clamp_rect(VncState *vs, VncJob *job, VncRect *rect)
+{
+trace_vnc_job_clamp_rect(vs, job, rect->x, rect->y, rect->w, rect->h);
+
+if (rect->x >= vs->client_width) {
+goto discard;
+}
+rect->w = MIN(vs->client_width - rect->x, rect->w);
+if (rect->w == 0) {
+goto discard;
+}
+
+if (rect->y >= vs->client_height) {
+goto discard;
+}
+rect->h = MIN(vs->client_height - rect->y, rect->h);
+if (rect->h == 0) {
+goto discard;
+}
+
+trace_vnc_job_clamped_rect(vs, job, rect->x, rect->y, rect->w, rect->h);
+return true;
+
+ discard:
+trace_vnc_job_discard_rect(vs, job, rect->x, rect->y, rect->w, rect->h);
+return false;
+}
+
 static int vnc_worker_thread_loop(VncJobQueue *queue)
 {
 VncJob *job;
@@ -260,14 +293,17 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
 goto disconnected;
 }
 
-n = vnc_send_framebuffer_update(&vs, entry->rect.x, entry->rect.y,
-entry->rect.w, entry->rect.h);
+if (vnc_worker_clamp_rect(&vs, job, &entry->rect)) {
+n = vnc_send_framebuffer_update(&vs, entry->rect.x, entry->rect.y,
+entry->rect.w, entry->rect.h);
 
-if (n >= 0) {
-n_rectangles += n;
+if (n >= 0) {
+n_rectangles += n;
+}
 }
 g_free(entry);
 }
+trace_vnc_job_nrects(&vs, job, n_rectangles);
 vnc_unlock_display(job->vs->vd);
 
 /* Put n_rectangles at the beginning of the message */
diff --git a/ui/trace-events b/ui/trace-events
index bd8f8a9d186a..3838ae2d849a 100644
--- a/ui/trace-events
+++ b/ui/trace-events
@@ -59,6 +59,11 @@ vnc_client_throttle_audio(void *state, void *ioc, size_t 
offset) "VNC client thr
 vnc_client_unthrottle_forced(void *state, void *ioc) "VNC client unthrottle 
forced offset state=%p ioc=%p"
 vnc_client_unthrottle_incremental(void *state, void *ioc, size_t offset) "VNC 
client unthrottle incremental state=%p ioc=%p offset=%zu"
 vnc_client_output_limit(void *state, void *ioc, size_t offset, size_t 
threshold) "VNC client output limit state=%p ioc=%p offset=%zu threshold=%zu"
+vnc_job_add_rect(void *state, void *job, int x, int y, int w, int h) "VNC add 
rect state=%p job=%p offset=%d,%d size=%dx%d"
+vnc_job_discard_rect(void *state, void *job, int x, int y, int w, int h) "VNC 
job discard rect state=%p job=%p offset=%d,%d size=%dx%d"
+vnc_job_clamp_rect(void *state, void *job, int x, int y, int w, int h) "VNC 
job clamp rect state=%p job=%p offset=%d,%d size=%dx%d"
+vnc_job_clamped_rect(void *state, void *job, int x, int y, int w, int h) "VNC 
job clamp rect state=%p job=%p offset=%d,%d size=%dx%d"
+vnc_job_nrects(void *state, void *job, int nrects) "VNC job state=%p job=%p 
nrects=%d"
 vnc_auth_init(void *display, int websock, int auth, int subauth) "VNC auth 
init state=%p websock=%d auth=%d subauth=%d"
 vnc_auth_start(void *state, int method) "VNC client auth start state=%p 
method=%d"
 vnc_auth_pass(void *state, int method) "VNC client auth passed state=%p

[PULL 08/11] ui: use client width/height in WMVi message

[Gerd Hoffmann]

From: Daniel P. Berrangé 

The WMVi message is supposed to provide the same width/height
information as the regular desktop resize and extended desktop
resize messages. There can be times where the client width and
height are different from the pixman surface dimensions.

Signed-off-by: Daniel P. Berrangé 
Reviewed-by: Marc-André Lureau 
Message-Id: <20210311182957.486939-4-berra...@redhat.com>
Signed-off-by: Gerd Hoffmann 
---
 ui/vnc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 7291429c04cf..8c9890b3cdc4 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2319,8 +2319,8 @@ static void vnc_colordepth(VncState *vs)
 vnc_write_u8(vs, 0);
 vnc_write_u16(vs, 1); /* number of rects */
 vnc_framebuffer_update(vs, 0, 0,
-   pixman_image_get_width(vs->vd->server),
-   pixman_image_get_height(vs->vd->server),
+   vs->client_width,
+   vs->client_height,
VNC_ENCODING_WMVi);
 pixel_format_message(vs);
 vnc_unlock_output(vs);
-- 
2.29.2

[PULL 06/11] ui: add more trace points for VNC client/server messages

[Gerd Hoffmann]

From: Daniel P. Berrangé 

This adds trace points for desktop size and audio related messages.

Signed-off-by: Daniel P. Berrangé 
Reviewed-by: Marc-André Lureau 
Message-Id: <20210311182957.486939-2-berra...@redhat.com>
Signed-off-by: Gerd Hoffmann 
---
 ui/vnc.c| 21 +++--
 ui/trace-events |  9 +
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index e8e3426a65c4..7291429c04cf 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -659,6 +659,9 @@ void vnc_framebuffer_update(VncState *vs, int x, int y, int 
w, int h,
 
 static void vnc_desktop_resize_ext(VncState *vs, int reject_reason)
 {
+trace_vnc_msg_server_ext_desktop_resize(
+vs, vs->ioc, vs->client_width, vs->client_height, reject_reason);
+
 vnc_lock_output(vs);
 vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
 vnc_write_u8(vs, 0);
@@ -705,6 +708,9 @@ static void vnc_desktop_resize(VncState *vs)
 return;
 }
 
+trace_vnc_msg_server_desktop_resize(
+vs, vs->ioc, vs->client_width, vs->client_height);
+
 vnc_lock_output(vs);
 vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
 vnc_write_u8(vs, 0);
@@ -1182,6 +1188,7 @@ static void audio_capture_notify(void *opaque, 
audcnotification_e cmd)
 assert(vs->magic == VNC_MAGIC);
 switch (cmd) {
 case AUD_CNOTIFY_DISABLE:
+trace_vnc_msg_server_audio_end(vs, vs->ioc);
 vnc_lock_output(vs);
 vnc_write_u8(vs, VNC_MSG_SERVER_QEMU);
 vnc_write_u8(vs, VNC_MSG_SERVER_QEMU_AUDIO);
@@ -1191,6 +1198,7 @@ static void audio_capture_notify(void *opaque, 
audcnotification_e cmd)
 break;
 
 case AUD_CNOTIFY_ENABLE:
+trace_vnc_msg_server_audio_begin(vs, vs->ioc);
 vnc_lock_output(vs);
 vnc_write_u8(vs, VNC_MSG_SERVER_QEMU);
 vnc_write_u8(vs, VNC_MSG_SERVER_QEMU_AUDIO);
@@ -1210,6 +1218,7 @@ static void audio_capture(void *opaque, const void *buf, 
int size)
 VncState *vs = opaque;
 
 assert(vs->magic == VNC_MAGIC);
+trace_vnc_msg_server_audio_data(vs, vs->ioc, buf, size);
 vnc_lock_output(vs);
 if (vs->output.offset < vs->throttle_output_offset) {
 vnc_write_u8(vs, VNC_MSG_SERVER_QEMU);
@@ -2454,9 +2463,11 @@ static int protocol_client_msg(VncState *vs, uint8_t 
*data, size_t len)
 
 switch (read_u16 (data, 2)) {
 case VNC_MSG_CLIENT_QEMU_AUDIO_ENABLE:
+trace_vnc_msg_client_audio_enable(vs, vs->ioc);
 audio_add(vs);
 break;
 case VNC_MSG_CLIENT_QEMU_AUDIO_DISABLE:
+trace_vnc_msg_client_audio_disable(vs, vs->ioc);
 audio_del(vs);
 break;
 case VNC_MSG_CLIENT_QEMU_AUDIO_SET_FORMAT:
@@ -2492,6 +2503,8 @@ static int protocol_client_msg(VncState *vs, uint8_t 
*data, size_t len)
 break;
 }
 vs->as.freq = freq;
+trace_vnc_msg_client_audio_format(
+vs, vs->ioc, vs->as.fmt, vs->as.nchannels, vs->as.freq);
 break;
 default:
 VNC_DEBUG("Invalid audio message %d\n", read_u8(data, 4));
@@ -2510,6 +2523,7 @@ static int protocol_client_msg(VncState *vs, uint8_t 
*data, size_t len)
 {
 size_t size;
 uint8_t screens;
+int w, h;
 
 if (len < 8) {
 return 8;
@@ -2520,12 +2534,15 @@ static int protocol_client_msg(VncState *vs, uint8_t 
*data, size_t len)
 if (len < size) {
 return size;
 }
+w = read_u16(data, 2);
+h = read_u16(data, 4);
 
+trace_vnc_msg_client_set_desktop_size(vs, vs->ioc, w, h, screens);
 if (dpy_ui_info_supported(vs->vd->dcl.con)) {
 QemuUIInfo info;
 memset(&info, 0, sizeof(info));
-info.width = read_u16(data, 2);
-info.height = read_u16(data, 4);
+info.width = w;
+info.height = h;
 dpy_set_ui_info(vs->vd->dcl.con, &info);
 vnc_desktop_resize_ext(vs, 4 /* Request forwarded */);
 } else {
diff --git a/ui/trace-events b/ui/trace-events
index 0ffcdb4408a6..bd8f8a9d186a 100644
--- a/ui/trace-events
+++ b/ui/trace-events
@@ -37,6 +37,15 @@ vnc_key_event_ext(bool down, int sym, int keycode, const 
char *name) "down %d, s
 vnc_key_event_map(bool down, int sym, int keycode, const char *name) "down %d, 
sym 0x%x -> keycode 0x%x [%s]"
 vnc_key_sync_numlock(bool on) "%d"
 vnc_key_sync_capslock(bool on) "%d"
+vnc_msg_server_audio_begin(void *state, void *ioc) "VNC server msg audio begin 
state=%p ioc=%p"
+vnc_msg_server_audio_end(void *state, void *ioc) "VNC server msg audio end 
state=%p ioc=%p"
+vnc_msg_server_audio_data(void *state, void *ioc, const void *buf, size_t len) 
"VNC server msg audio data state=%p ioc=%p buf=%p len=%zd"
+vnc_msg_server_desktop_resize(void *state, void *ioc, int width, int height) 
"VNC server msg

[PULL 02/11] ui: introduce "password-secret" option for SPICE server

[Gerd Hoffmann]

From: Daniel P. Berrangé 

Currently when using SPICE the "password" option provides the password
in plain text on the command line. This is insecure as it is visible
to all processes on the host. As an alternative, the password can be
provided separately via the monitor.

This introduces a "password-secret" option which lets the password be
provided up front.

  $QEMU --object secret,id=vncsec0,file=passwd.txt \
--spice port=5901,password-secret=vncsec0

Signed-off-by: Daniel P. Berrangé 
Message-Id: <2021034343.439820-3-berra...@redhat.com>
Signed-off-by: Gerd Hoffmann 
---
 ui/spice-core.c | 30 --
 qemu-options.hx |  9 +++--
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index cadec766fe3a..b9d8aced91df 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -34,6 +34,7 @@
 #include "qapi/qapi-events-ui.h"
 #include "qemu/notify.h"
 #include "qemu/option.h"
+#include "crypto/secret_common.h"
 #include "migration/misc.h"
 #include "hw/pci/pci_bus.h"
 #include "ui/spice-display.h"
@@ -415,6 +416,9 @@ static QemuOptsList qemu_spice_opts = {
 },{
 .name = "password",
 .type = QEMU_OPT_STRING,
+},{
+.name = "password-secret",
+.type = QEMU_OPT_STRING,
 },{
 .name = "disable-ticketing",
 .type = QEMU_OPT_BOOL,
@@ -636,7 +640,9 @@ void qemu_spice_display_init_done(void)
 static void qemu_spice_init(void)
 {
 QemuOpts *opts = QTAILQ_FIRST(&qemu_spice_opts.head);
-const char *password, *str, *x509_dir, *addr,
+char *password = NULL;
+const char *passwordSecret;
+const char *str, *x509_dir, *addr,
 *x509_key_password = NULL,
 *x509_dh_file = NULL,
 *tls_ciphers = NULL;
@@ -663,7 +669,26 @@ static void qemu_spice_init(void)
 error_report("spice tls-port is out of range");
 exit(1);
 }
-password = qemu_opt_get(opts, "password");
+passwordSecret = qemu_opt_get(opts, "password-secret");
+if (passwordSecret) {
+Error *local_err = NULL;
+if (qemu_opt_get(opts, "password")) {
+error_report("'password' option is mutually exclusive with "
+ "'password-secret'");
+exit(1);
+}
+password = qcrypto_secret_lookup_as_utf8(passwordSecret,
+ &local_err);
+if (!password) {
+error_report_err(local_err);
+exit(1);
+}
+} else {
+str = qemu_opt_get(opts, "password");
+if (str) {
+password = g_strdup(str);
+}
+}
 
 if (tls_port) {
 x509_dir = qemu_opt_get(opts, "x509-dir");
@@ -809,6 +834,7 @@ static void qemu_spice_init(void)
 g_free(x509_key_file);
 g_free(x509_cert_file);
 g_free(x509_cacert_file);
+g_free(password);
 
 #ifdef HAVE_SPICE_GL
 if (qemu_opt_get_bool(opts, "gl", 0)) {
diff --git a/qemu-options.hx b/qemu-options.hx
index 357fc4596ecc..a98f8e84a29d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1899,7 +1899,8 @@ DEF("spice", HAS_ARG, QEMU_OPTION_spice,
 "   [,tls-ciphers=]\n"
 "   [,tls-channel=[main|display|cursor|inputs|record|playback]]\n"
 "   
[,plaintext-channel=[main|display|cursor|inputs|record|playback]]\n"
-"   [,sasl=on|off][,password=][,disable-ticketing=on|off]\n"
+"   [,sasl=on|off][,disable-ticketing=on|off]\n"
+"   [,password=][,password-secret=]\n"
 "   [,image-compression=[auto_glz|auto_lz|quic|glz|lz|off]]\n"
 "   [,jpeg-wan-compression=[auto|never|always]]\n"
 "   [,zlib-glz-wan-compression=[auto|never|always]]\n"
@@ -1924,9 +1925,13 @@ SRST
 ``ipv4=on|off``; \ ``ipv6=on|off``; \ ``unix=on|off``
 Force using the specified IP version.
 
-``password=``
+``password=``
 Set the password you need to authenticate.
 
+``password-secret=``
+Set the ID of the ``secret`` object containing the password
+you need to authenticate.
+
 ``sasl=on|off``
 Require that the client use SASL to authenticate with the spice.
 The exact choice of authentication method used is controlled
-- 
2.29.2

[PULL 11/11] ui/cocoa: Comment about modifier key input quirks

[Gerd Hoffmann]

From: Akihiko Odaki 

Based-on: <20210310042348.21931-1-akihiko.od...@gmail.com>
Signed-off-by: Akihiko Odaki 
Message-Id: <20210312133212.3131-1-akihiko.od...@gmail.com>
Signed-off-by: Gerd Hoffmann 
---
 ui/cocoa.m | 38 +-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 9da0e884b712..37e1fb52eb4d 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -690,7 +690,43 @@ QemuCocoaView *cocoaView;
 NSPoint p = [self screenLocationOfEvent:event];
 NSUInteger modifiers = [event modifierFlags];
 
-// emulate caps lock keydown and keyup
+/*
+ * Check -[NSEvent modifierFlags] here.
+ *
+ * There is a NSEventType for an event notifying the change of
+ * -[NSEvent modifierFlags], NSEventTypeFlagsChanged but these operations
+ * are performed for any events because a modifier state may change while
+ * the application is inactive (i.e. no events fire) and we don't want to
+ * wait for another modifier state change to detect such a change.
+ *
+ * NSEventModifierFlagCapsLock requires a special treatment. The other 
flags
+ * are handled in similar manners.
+ *
+ * NSEventModifierFlagCapsLock
+ * ---
+ *
+ * If CapsLock state is changed, "up" and "down" events will be fired in
+ * sequence, effectively updates CapsLock state on the guest.
+ *
+ * The other flags
+ * ---
+ *
+ * If a flag is not set, fire "up" events for all keys which correspond to
+ * the flag. Note that "down" events are not fired here because the flags
+ * checked here do not tell what exact keys are down.
+ *
+ * If one of the keys corresponding to a flag is down, we rely on
+ * -[NSEvent keyCode] of an event whose -[NSEvent type] is
+ * NSEventTypeFlagsChanged to know the exact key which is down, which has
+ * the following two downsides:
+ * - It does not work when the application is inactive as described above.
+ * - It malfactions *after* the modifier state is changed while the
+ *   application is inactive. It is because -[NSEvent keyCode] does not 
tell
+ *   if the key is up or down, and requires to infer the current state from
+ *   the previous state. It is still possible to fix such a malfanction by
+ *   completely leaving your hands from the keyboard, which hopefully makes
+ *   this implementation usable enough.
+ */
 if (!!(modifiers & NSEventModifierFlagCapsLock) !=
 qkbd_state_modifier_get(kbd, QKBD_MOD_CAPSLOCK)) {
 qkbd_state_key_event(kbd, Q_KEY_CODE_CAPS_LOCK, true);
-- 
2.29.2

[PULL 10/11] ui: fold qemu_alloc_display in only caller

[Gerd Hoffmann]

From: Marc-André Lureau 

A minor code simplification.

Signed-off-by: Marc-André Lureau 
Message-Id: <20210312100108.2706195-2-marcandre.lur...@redhat.com>
Signed-off-by: Gerd Hoffmann 
---
 ui/console.c | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/ui/console.c b/ui/console.c
index c2fdf975b6b3..2de5f4105b59 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1386,26 +1386,18 @@ static QemuConsole *new_console(DisplayState *ds, 
console_type_t console_type,
 return s;
 }
 
-static void qemu_alloc_display(DisplaySurface *surface, int width, int height)
+DisplaySurface *qemu_create_displaysurface(int width, int height)
 {
-qemu_pixman_image_unref(surface->image);
-surface->image = NULL;
+DisplaySurface *surface = g_new0(DisplaySurface, 1);
 
+trace_displaysurface_create(surface, width, height);
 surface->format = PIXMAN_x8r8g8b8;
 surface->image = pixman_image_create_bits(surface->format,
   width, height,
   NULL, width * 4);
 assert(surface->image != NULL);
-
 surface->flags = QEMU_ALLOCATED_FLAG;
-}
 
-DisplaySurface *qemu_create_displaysurface(int width, int height)
-{
-DisplaySurface *surface = g_new0(DisplaySurface, 1);
-
-trace_displaysurface_create(surface, width, height);
-qemu_alloc_display(surface, width, height);
 return surface;
 }
 
-- 
2.29.2

[PULL 03/11] ui: deprecate "password" option for SPICE server

[Gerd Hoffmann]

From: Daniel P. Berrangé 

With the new "password-secret" option, there is no reason to use the old
inecure "password" option with -spice, so it can be deprecated.

Signed-off-by: Daniel P. Berrangé 
Message-Id: <2021034343.439820-4-berra...@redhat.com>
Signed-off-by: Gerd Hoffmann 
---
 ui/spice-core.c| 2 ++
 docs/system/deprecated.rst | 8 
 qemu-options.hx| 4 
 3 files changed, 14 insertions(+)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index b9d8aced91df..272d19b0c152 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -686,6 +686,8 @@ static void qemu_spice_init(void)
 } else {
 str = qemu_opt_get(opts, "password");
 if (str) {
+warn_report("'password' option is deprecated and insecure, "
+"use 'password-secret' instead");
 password = g_strdup(str);
 }
 }
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 5e3a31c12361..8cba672a7b4a 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -174,6 +174,14 @@ Input parameters that take a size value should only use a 
size suffix
 the value is hexadecimal.  That is, '0x20M' is deprecated, and should
 be written either as '32M' or as '0x200'.
 
+``-spice password=string`` (since 6.0)
+''
+
+This option is insecure because the SPICE password remains visible in
+the process listing. This is replaced by the new ``password-secret``
+option which lets the password be securely provided on the command
+line using a ``secret`` object instance.
+
 QEMU Machine Protocol (QMP) commands
 
 
diff --git a/qemu-options.hx b/qemu-options.hx
index a98f8e84a29d..4da3f4f48c03 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1928,6 +1928,10 @@ SRST
 ``password=``
 Set the password you need to authenticate.
 
+This option is deprecated and insecure because it leaves the
+password visible in the process listing. Use ``password-secret``
+instead.
+
 ``password-secret=``
 Set the ID of the ``secret`` object containing the password
 you need to authenticate.
-- 
2.29.2

[PULL 05/11] ui/cocoa: Do not exit immediately after shutdown

[Gerd Hoffmann]

From: Akihiko Odaki 

ui/cocoa used to call exit immediately after calling
qemu_system_shutdown_request, which prevents QEMU from actually
perfoming system shutdown. Just sleep forever, and wait QEMU to call
exit and kill the Cocoa thread.

Signed-off-by: Akihiko Odaki 
Message-Id: <20210219111652.20623-1-akihiko.od...@gmail.com>
Signed-off-by: Gerd Hoffmann 
---
 ui/cocoa.m | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index a7848ae0a30e..9da0e884b712 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1115,7 +1115,13 @@ QemuCocoaView *cocoaView;
 COCOA_DEBUG("QemuCocoaAppController: applicationWillTerminate\n");
 
 qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_UI);
-exit(0);
+
+/*
+ * Sleep here, because returning will cause OSX to kill us
+ * immediately; the QEMU main loop will handle the shutdown
+ * request and terminate the process.
+ */
+[NSThread sleepForTimeInterval:INFINITY];
 }
 
 - (BOOL)applicationShouldTerminateAfterLastWindowClosed:(NSApplication 
*)theApplication
-- 
2.29.2

[PULL 00/11] Ui 20210316 patches

[Gerd Hoffmann]

The following changes since commit 6157b0e19721aadb4c7fdcfe57b2924af6144b14:

  Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-6.0-pull-=
request' into staging (2021-03-14 17:47:49 +)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/ui-20210316-pull-request

for you to fetch changes up to ad7f2f8ee9fbded410fbf77158b0065f8e2f08e3:

  ui/cocoa: Comment about modifier key input quirks (2021-03-16 06:36:45 +010=
0)


vnc+spice: password-secret option.
bugfixes for cocoa, vnc, opengl.



Akihiko Odaki (3):
  opengl: Do not convert format with glTexImage2D on OpenGL ES
  ui/cocoa: Do not exit immediately after shutdown
  ui/cocoa: Comment about modifier key input quirks

Daniel P. Berrang=C3=A9 (7):
  ui: introduce "password-secret" option for VNC servers
  ui: introduce "password-secret" option for SPICE server
  ui: deprecate "password" option for SPICE server
  ui: add more trace points for VNC client/server messages
  ui: avoid sending framebuffer updates outside client desktop bounds
  ui: use client width/height in WMVi message
  ui: honour the actual guest display dimensions without rounding

Marc-Andr=C3=A9 Lureau (1):
  ui: fold qemu_alloc_display in only caller

 ui/vnc.h   |  1 +
 ui/console-gl.c| 19 +++---
 ui/console.c   | 14 ++--
 ui/spice-core.c| 32 +++--
 ui/vnc-jobs.c  | 44 ---
 ui/vnc.c   | 71 +-
 docs/system/deprecated.rst |  8 +
 qemu-options.hx| 18 --
 ui/cocoa.m | 46 ++--
 ui/trace-events| 16 +
 10 files changed, 234 insertions(+), 35 deletions(-)

--=20
2.29.2

[PULL 01/11] ui: introduce "password-secret" option for VNC servers

[Gerd Hoffmann]

From: Daniel P. Berrangé 

Currently when using VNC the "password" flag turns on password based
authentication. The actual password has to be provided separately via
the monitor.

This introduces a "password-secret" option which lets the password be
provided up front.

  $QEMU --object secret,id=vncsec0,file=passwd.txt \
--vnc localhost:0,password-secret=vncsec0

Signed-off-by: Daniel P. Berrangé 
Message-Id: <2021034343.439820-2-berra...@redhat.com>
Signed-off-by: Gerd Hoffmann 
---
 ui/vnc.c| 23 ++-
 qemu-options.hx |  5 +
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 310abc937812..e8e3426a65c4 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -48,6 +48,7 @@
 #include "crypto/tlscredsanon.h"
 #include "crypto/tlscredsx509.h"
 #include "crypto/random.h"
+#include "crypto/secret_common.h"
 #include "qom/object_interfaces.h"
 #include "qemu/cutils.h"
 #include "qemu/help_option.h"
@@ -3459,6 +3460,9 @@ static QemuOptsList qemu_vnc_opts = {
 },{
 .name = "password",
 .type = QEMU_OPT_BOOL,
+},{
+.name = "password-secret",
+.type = QEMU_OPT_STRING,
 },{
 .name = "reverse",
 .type = QEMU_OPT_BOOL,
@@ -3931,6 +3935,7 @@ void vnc_display_open(const char *id, Error **errp)
 int lock_key_sync = 1;
 int key_delay_ms;
 const char *audiodev;
+const char *passwordSecret;
 
 if (!vd) {
 error_setg(errp, "VNC display not active");
@@ -3948,7 +3953,23 @@ void vnc_display_open(const char *id, Error **errp)
 goto fail;
 }
 
-password = qemu_opt_get_bool(opts, "password", false);
+
+passwordSecret = qemu_opt_get(opts, "password-secret");
+if (passwordSecret) {
+if (qemu_opt_get(opts, "password")) {
+error_setg(errp,
+   "'password' flag is redundant with 'password-secret'");
+goto fail;
+}
+vd->password = qcrypto_secret_lookup_as_utf8(passwordSecret,
+ errp);
+if (!vd->password) {
+goto fail;
+}
+password = true;
+} else {
+password = qemu_opt_get_bool(opts, "password", false);
+}
 if (password) {
 if (fips_get_state()) {
 error_setg(errp,
diff --git a/qemu-options.hx b/qemu-options.hx
index 622d3bfa5a7d..357fc4596ecc 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2165,6 +2165,11 @@ SRST
 time to allow  password to expire immediately or never
 expire.
 
+``password-secret=``
+Require that password based authentication is used for client
+connections, using the password provided by the ``secret``
+object identified by ``secret-id``.
+
 ``tls-creds=ID``
 Provides the ID of a set of TLS credentials to use to secure the
 VNC server. They will apply to both the normal VNC server socket
-- 
2.29.2

[PULL 04/11] opengl: Do not convert format with glTexImage2D on OpenGL ES

[Gerd Hoffmann]

From: Akihiko Odaki 

OpenGL ES does not support conversion from the given data format
to the internal format with glTexImage2D.

Use the given data format as the internal format, and ignore
the given alpha channels with GL_TEXTURE_SWIZZLE_A in case the
format contains alpha channels.

Signed-off-by: Akihiko Odaki 
Message-Id: <20210219094803.90860-1-akihiko.od...@gmail.com>
Signed-off-by: Gerd Hoffmann 
---
 ui/console-gl.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/ui/console-gl.c b/ui/console-gl.c
index 0a6478161fed..7c9894a51d99 100644
--- a/ui/console-gl.c
+++ b/ui/console-gl.c
@@ -73,11 +73,20 @@ void surface_gl_create_texture(QemuGLShader *gls,
 glBindTexture(GL_TEXTURE_2D, surface->texture);
 glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
   surface_stride(surface) / surface_bytes_per_pixel(surface));
-glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB,
- surface_width(surface),
- surface_height(surface),
- 0, surface->glformat, surface->gltype,
- surface_data(surface));
+if (epoxy_is_desktop_gl()) {
+glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB,
+ surface_width(surface),
+ surface_height(surface),
+ 0, surface->glformat, surface->gltype,
+ surface_data(surface));
+} else {
+glTexImage2D(GL_TEXTURE_2D, 0, surface->glformat,
+ surface_width(surface),
+ surface_height(surface),
+ 0, surface->glformat, surface->gltype,
+ surface_data(surface));
+glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_SWIZZLE_A, GL_ONE);
+}
 
 glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
 glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
-- 
2.29.2

Re: [PATCH 2/2] lib: Fix calling of virNetworkUpdate() driver callback

[Ján Tomko]

On a Tuesday in 2021, Michal Privoznik wrote:

Let me take you ~8 years back. Back then, virNetworkUpdate() API
was introduced and the public implementation is nothing special -
it calls the networkUpdate() callback of the network driver.
Except, a small "typo" slipped through - while the public API
takes @command argument first followed by @section argument,
these are passed to the callback in swapped order. This wasn't
visible, until split daemons and sub-driver URIs became a thing.
Simply because the client was talking to the network driver via
our remote driver.

On client side, when the public API was called the RPC layer
swapped the order (because it was called with swapped arguments
already). 


So it did not swap it, then? :)


Then, on the server side, after deserialization the
public API was called again (still with swapped arguments) and it
subsequently called network driver callback (where the arguments
were in the right order again).

Since both arguments are of the same type (unsigned int) XDR was
happy too.

The problem arose with split daemons and sub-driver URIs. Imagine
a user running split daemons. When they connect to
network:///system, they talk to virnetworkd "directly" (as in no
proxy daemon sits in between). Both sides still use remote driver
though, so the order is fixed by RPC layer, just like in libvirtd
case. Connecting to qemu:///system is where things get
interesting because virtqemud serves as a proxy to virtnetworkd
(the former talks to the later on users behalf and forwards API
calls).

What happens is, virtqemud sees incoming RPC packet (with swapped
arguments), it decodes it and calls remoteDispatchNetworkUpdate()
(valued are still swapped in remote_network_update_args struct).
Subsequently, virNetworkUpdate() is called and since virtqemud
has no network driver, it calls remote driver again. But this
time, the API callback gets the arguments in correct order (just
like network driver would in case of libvirtd). But this means,
that virnetworkd will see them in wrong order, because it swaps
them again.

After fixing the call of the callback, the API works again in
both cases, if both sides run with the fix.

And to make things work for newer clients talking to older
servers, we have to swap the order on RPC layer too. If both
sides run with the change, they both encode and decode packet
properly. But if newer client talks to older server, it will
encode packet just how the older server expects it.

Fixes: 574b9bc66b6b10cc4cf50f299c3f0ff55f2cbefb
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1870552
Signed-off-by: Michal Privoznik 
---
src/libvirt-network.c| 2 +-
src/remote/remote_protocol.x | 7 ++-
src/remote_protocol-structs  | 2 +-
3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/libvirt-network.c b/src/libvirt-network.c
index b84389f762..310da44a4e 100644
--- a/src/libvirt-network.c
+++ b/src/libvirt-network.c
@@ -543,7 +543,7 @@ virNetworkUpdate(virNetworkPtr network,

if (conn->networkDriver && conn->networkDriver->networkUpdate) {
int ret;
-ret = conn->networkDriver->networkUpdate(network, section, command,
+ret = conn->networkDriver->networkUpdate(network, command, section,
 parentIndex, xml, flags);


Especially in the migration code, we use VIR_DRV_SUPPORTS_FEATURE to
figure out whether the remote side has some functionality.

With something like VIR_DRV_FEATURE_NETWORK_UPDATE_HAS_CORRECT_ORDER,
we could pass the correct order if it's safe to do so and fix the
virtqemud use case for newer daemons. (Since it never worked, I think
it's nicer to not fix it for older virtqemud than to break client
compatibility with older libvirt)


if (ret < 0)
goto error;
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index d3724bc305..464c6b4af1 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -1542,10 +1542,15 @@ struct remote_network_undefine_args {
remote_nonnull_network net;
};

+/* The @section and @command members are intentionally inverted compared to the
+ * virNetworkUpdate() API. The reason is that since it's introduction until the
+ * 7.2.0 release the driver callback was given arguments in inverted order.
+ * After it was fixed, the XDR has to be swapped to keep compatibility with
+ * older daemons. */


This does not actually affect RPC at all, merely the names used in the
autogenerated stubs. If it did, I'm afraid it would just move the bug
from the driver to the RPC layer.

Jano


struct remote_network_update_args {
remote_nonnull_network net;
-unsigned int command;
unsigned int section;
+unsigned int command;
int parentIndex;
remote_nonnull_string xml;
unsigned int flags;
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs
index c0c034ac6a..800678c92e 100644
--- a/src/remote_protocol-structs
+++ b/src/remote_protocol-structs
@@

Re: [PATCH 1/2] lib: Debug print all arguments of virNetworkUpdate()

[Ján Tomko]

On a Tuesday in 2021, Michal Privoznik wrote:

Somehow, command argument was not printed into debug logs. It is
imperative that all arguments are logged.

Signed-off-by: Michal Privoznik 
---
src/libvirt-network.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [PATCH] qemu_process: Use accessor for def->mem.total_memory

[Ján Tomko]

On a Monday in 2021, Michal Privoznik wrote:

When connecting to the monitor, a timeout is calculated that is
bigger the more memory guest has (because QEMU has to allocate
and possibly zero out the memory and what not, empirically
deducted). However, when computing the timeout the @total_memory
mmember is accessed directly even though
virDomainDefGetMemoryTotal() should have been used.

Signed-off-by: Michal Privoznik 
---
src/qemu/qemu_process.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

Re: [PATCH] coding-style: Don't encourage virXXXPtr typedefs

[Ján Tomko]

On a Monday in 2021, Michal Privoznik wrote:

We don't like virXXXPtr typedefs really and they are going away
shortly, possibly. Do not encourage new code to put in the
typedefs.

Signed-off-by: Michal Privoznik 
---
docs/coding-style.rst | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: PGP signature

[PATCH] coding-style: Don't encourage virXXXPtr typedefs

[Michal Privoznik]

We don't like virXXXPtr typedefs really and they are going away
shortly, possibly. Do not encourage new code to put in the
typedefs.

Signed-off-by: Michal Privoznik 
---
 docs/coding-style.rst | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/docs/coding-style.rst b/docs/coding-style.rst
index 22de86a657..470c61860f 100644
--- a/docs/coding-style.rst
+++ b/docs/coding-style.rst
@@ -53,13 +53,10 @@ Struct type names
All structs should have a 'vir' prefix in their typedef name,
and each following word should have its first letter in
uppercase. The struct name should be the same as the typedef
-   name with a leading underscore. A second typedef should be
-   given for a pointer to the struct with a 'Ptr' suffix.
-
+   name with a leading underscore.
::
 
  typedef struct _virHashTable virHashTable;
- typedef virHashTable *virHashTablePtr;
  struct _virHashTable {
  ...
  };
@@ -426,11 +423,11 @@ Conditional expressions
 
 For readability reasons new code should avoid shortening
 comparisons to 0 for numeric types. Boolean and pointer
-comparisions may be shortened. All long forms are okay:
+comparisons may be shortened. All long forms are okay:
 
 ::
 
-  virFooPtr foos = NULL;
+  virFoo *foos = NULL;
   size nfoos = 0;
   bool hasFoos = false;
 
-- 
2.26.2

Re: [PATCH] meson: Don't check for addr2line

[Pavel Hrdina]

On Mon, Mar 15, 2021 at 06:30:42PM +0100, Michal Privoznik wrote:
> In the past, we used to have this oomtrace.pl script that
> attempted to print the stack trace of where an OOM error
> occurred and it used addr2line for that. But since v5.8.0-rc1~189
> we don't really care about OOM anymore and the script is long
> gone so there's no need to check for addr2line program either.
> 
> Fixes: 2c52ecd96086b4643b99b4570b5823d40ce2787b
> Signed-off-by: Michal Privoznik 
> ---
>  meson.build | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Pavel Hrdina 


signature.asc
Description: PGP signature

Re: [PATCH v2 10/13] block: remove 'encryption_key_missing' flag from QAPI

[Eric Blake]

On 3/15/21 12:45 PM, Daniel P. Berrangé wrote:
> This has been hardcoded to "false" since 2.10.0, since secrets required
> to unlock block devices are now always provided upfront instead of using

up front

> interactive prompts.
> 
> Reviewed-by: Thomas Huth 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  block/qapi.c |  1 -
>  docs/system/deprecated.rst   | 10 ---
>  docs/system/removed-features.rst | 10 +++
>  qapi/block-core.json |  8 --
>  tests/qemu-iotests/184.out   |  6 ++--
>  tests/qemu-iotests/191.out   | 48 +++-
>  tests/qemu-iotests/273.out   | 15 --
>  7 files changed, 33 insertions(+), 65 deletions(-)

Reviewed-by: Eric Blake 

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

Re: [PATCH v2 03/13] monitor: remove 'query-events' QMP command

[Eric Blake]

On 3/15/21 12:45 PM, Daniel P. Berrangé wrote:
> The code comment suggests removing QAPIEvent_(str|lookup) symbols too,
> however, these are both auto-generated as standard for any enum in
> QAPI. As such it they'll exist whether we use them or not.
> 
> Reviewed-by: Thomas Huth 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  docs/system/deprecated.rst   |  6 -
>  docs/system/removed-features.rst |  6 +
>  monitor/qmp-cmds-control.c   | 24 -
>  qapi/control.json| 45 
>  4 files changed, 6 insertions(+), 75 deletions(-)

Reviewed-by: Eric Blake 

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

[PULL 11/13] usb/storage move usb-storage device to separate source file

[Gerd Hoffmann]

Pure code motion, no functional change.

Signed-off-by: Gerd Hoffmann 
Message-Id: <20210312090425.772900-4-kra...@redhat.com>
---
 hw/usb/dev-storage-classic.c | 156 +++
 hw/usb/dev-storage.c | 135 --
 hw/usb/meson.build   |   1 +
 3 files changed, 157 insertions(+), 135 deletions(-)
 create mode 100644 hw/usb/dev-storage-classic.c

diff --git a/hw/usb/dev-storage-classic.c b/hw/usb/dev-storage-classic.c
new file mode 100644
index ..00cb34b22f02
--- /dev/null
+++ b/hw/usb/dev-storage-classic.c
@@ -0,0 +1,156 @@
+/*
+ * USB Mass Storage Device emulation
+ *
+ * Copyright (c) 2006 CodeSourcery.
+ * Written by Paul Brook
+ *
+ * This code is licensed under the LGPL.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/typedefs.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
+#include "hw/usb.h"
+#include "hw/usb/desc.h"
+#include "hw/usb/msd.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/block-backend.h"
+
+static const struct SCSIBusInfo usb_msd_scsi_info_storage = {
+.tcq = false,
+.max_target = 0,
+.max_lun = 0,
+
+.transfer_data = usb_msd_transfer_data,
+.complete = usb_msd_command_complete,
+.cancel = usb_msd_request_cancelled,
+.load_request = usb_msd_load_request,
+};
+
+static void usb_msd_storage_realize(USBDevice *dev, Error **errp)
+{
+MSDState *s = USB_STORAGE_DEV(dev);
+BlockBackend *blk = s->conf.blk;
+SCSIDevice *scsi_dev;
+
+if (!blk) {
+error_setg(errp, "drive property not set");
+return;
+}
+
+if (!blkconf_blocksizes(&s->conf, errp)) {
+return;
+}
+
+if (!blkconf_apply_backend_options(&s->conf, !blk_supports_write_perm(blk),
+   true, errp)) {
+return;
+}
+
+/*
+ * Hack alert: this pretends to be a block device, but it's really
+ * a SCSI bus that can serve only a single device, which it
+ * creates automatically.  But first it needs to detach from its
+ * blockdev, or else scsi_bus_legacy_add_drive() dies when it
+ * attaches again. We also need to take another reference so that
+ * blk_detach_dev() doesn't free blk while we still need it.
+ *
+ * The hack is probably a bad idea.
+ */
+blk_ref(blk);
+blk_detach_dev(blk, DEVICE(s));
+s->conf.blk = NULL;
+
+usb_desc_create_serial(dev);
+usb_desc_init(dev);
+scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(dev),
+ &usb_msd_scsi_info_storage, NULL);
+scsi_dev = scsi_bus_legacy_add_drive(&s->bus, blk, 0, !!s->removable,
+ s->conf.bootindex, s->conf.share_rw,
+ s->conf.rerror, s->conf.werror,
+ dev->serial,
+ errp);
+blk_unref(blk);
+if (!scsi_dev) {
+return;
+}
+usb_msd_handle_reset(dev);
+s->scsi_dev = scsi_dev;
+}
+
+static Property msd_properties[] = {
+DEFINE_BLOCK_PROPERTIES(MSDState, conf),
+DEFINE_BLOCK_ERROR_PROPERTIES(MSDState, conf),
+DEFINE_PROP_BOOL("removable", MSDState, removable, false),
+DEFINE_PROP_BOOL("commandlog", MSDState, commandlog, false),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void usb_msd_class_storage_initfn(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
+
+uc->realize = usb_msd_storage_realize;
+device_class_set_props(dc, msd_properties);
+}
+
+static void usb_msd_get_bootindex(Object *obj, Visitor *v, const char *name,
+  void *opaque, Error **errp)
+{
+USBDevice *dev = USB_DEVICE(obj);
+MSDState *s = USB_STORAGE_DEV(dev);
+
+visit_type_int32(v, name, &s->conf.bootindex, errp);
+}
+
+static void usb_msd_set_bootindex(Object *obj, Visitor *v, const char *name,
+  void *opaque, Error **errp)
+{
+USBDevice *dev = USB_DEVICE(obj);
+MSDState *s = USB_STORAGE_DEV(dev);
+int32_t boot_index;
+Error *local_err = NULL;
+
+if (!visit_type_int32(v, name, &boot_index, errp)) {
+return;
+}
+/* check whether bootindex is present in fw_boot_order list  */
+check_boot_index(boot_index, &local_err);
+if (local_err) {
+goto out;
+}
+/* change bootindex to a new one */
+s->conf.bootindex = boot_index;
+
+if (s->scsi_dev) {
+object_property_set_int(OBJECT(s->scsi_dev), "bootindex", boot_index,
+&error_abort);
+}
+
+out:
+error_propagate(errp, local_err);
+}
+
+static void usb_msd_instance_init(Object *obj)
+{
+object_property_add(obj, "bootindex", "int32",
+usb_msd_get_bootindex,
+usb_msd_set_bootindex, NULL, NULL);
+object_property_set_int(obj, "bootindex", -1, NULL);
+}
+
+static const TypeInfo ms

[PULL 05/13] usb: Document the missing -usbdevice options

[Gerd Hoffmann]

From: Thomas Huth 

There are some more -usbdevice options that have never been mentioned
in the documentation. Now that we removed -usbdevice from the list
of deprecated features again, we should document them properly.

While we're at it, also sort them alphabetically.

Signed-off-by: Thomas Huth 
Message-Id: <20210310173323.1422754-5-th...@redhat.com>
Signed-off-by: Gerd Hoffmann 
---
 qemu-options.hx | 35 +--
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 622d3bfa5a7d..fe83ea09b25e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1705,7 +1705,7 @@ ERST
 
 DEFHEADING()
 
-DEFHEADING(USB options:)
+DEFHEADING(USB convenience options:)
 
 DEF("usb", 0, QEMU_OPTION_usb,
 "-usbenable on-board USB host controller (if not enabled by 
default)\n",
@@ -1723,9 +1723,31 @@ DEF("usbdevice", HAS_ARG, QEMU_OPTION_usbdevice,
 QEMU_ARCH_ALL)
 SRST
 ``-usbdevice devname``
-Add the USB device devname. Note that this option is deprecated,
-please use ``-device usb-...`` instead. See the chapter about
+Add the USB device devname, and enable an on-board USB controller
+if possible and necessary (just like it can be done via
+``-machine usb=on``). Note that this option is mainly intended for
+the user's convenience only. More fine-grained control can be
+achieved by selecting a USB host controller (if necessary) and the
+desired USB device via the ``-device`` option instead. For example,
+instead of using ``-usbdevice mouse`` it is possible to use
+``-device qemu-xhci -device usb-mouse`` to connect the USB mouse
+to a USB 3.0 controller instead (at least on machines that support
+PCI and do not have an USB controller enabled by default yet).
+For more details, see the chapter about
 :ref:`Connecting USB devices` in the System Emulation Users Guide.
+Possible devices for devname are:
+
+``braille``
+Braille device. This will use BrlAPI to display the braille
+output on a real or fake device (i.e. it also creates a
+corresponding ``braille`` chardev automatically beside the
+``usb-braille`` USB device).
+
+``ccid``
+Smartcard reader device
+
+``keyboard``
+Standard USB keyboard. Will override the PS/2 keyboard (if present).
 
 ``mouse``
 Virtual Mouse. This will override the PS/2 mouse emulation when
@@ -1737,9 +1759,10 @@ SRST
 position without having to grab the mouse. Also overrides the
 PS/2 mouse emulation when activated.
 
-``braille``
-Braille device. This will use BrlAPI to display the braille
-output on a real or fake device.
+``wacom-tablet``
+Wacom PenPartner USB tablet.
+
+
 ERST
 
 DEFHEADING()
-- 
2.29.2

[PULL 10/13] usb/storage: move usb-bot device to separate source file

[Gerd Hoffmann]

Pure code motion, no functional change.

Signed-off-by: Gerd Hoffmann 
Message-Id: <20210312090425.772900-3-kra...@redhat.com>
---
 hw/usb/dev-storage-bot.c | 63 
 hw/usb/dev-storage.c | 42 ---
 hw/usb/meson.build   |  1 +
 3 files changed, 64 insertions(+), 42 deletions(-)
 create mode 100644 hw/usb/dev-storage-bot.c

diff --git a/hw/usb/dev-storage-bot.c b/hw/usb/dev-storage-bot.c
new file mode 100644
index ..6aad026d1133
--- /dev/null
+++ b/hw/usb/dev-storage-bot.c
@@ -0,0 +1,63 @@
+/*
+ * USB Mass Storage Device emulation
+ *
+ * Copyright (c) 2006 CodeSourcery.
+ * Written by Paul Brook
+ *
+ * This code is licensed under the LGPL.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/typedefs.h"
+#include "qapi/error.h"
+#include "hw/usb.h"
+#include "hw/usb/desc.h"
+#include "hw/usb/msd.h"
+
+static const struct SCSIBusInfo usb_msd_scsi_info_bot = {
+.tcq = false,
+.max_target = 0,
+.max_lun = 15,
+
+.transfer_data = usb_msd_transfer_data,
+.complete = usb_msd_command_complete,
+.cancel = usb_msd_request_cancelled,
+.load_request = usb_msd_load_request,
+};
+
+static void usb_msd_bot_realize(USBDevice *dev, Error **errp)
+{
+MSDState *s = USB_STORAGE_DEV(dev);
+DeviceState *d = DEVICE(dev);
+
+usb_desc_create_serial(dev);
+usb_desc_init(dev);
+if (d->hotplugged) {
+s->dev.auto_attach = 0;
+}
+
+scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(dev),
+ &usb_msd_scsi_info_bot, NULL);
+usb_msd_handle_reset(dev);
+}
+
+static void usb_msd_class_bot_initfn(ObjectClass *klass, void *data)
+{
+USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
+
+uc->realize = usb_msd_bot_realize;
+uc->attached_settable = true;
+}
+
+static const TypeInfo bot_info = {
+.name  = "usb-bot",
+.parent= TYPE_USB_STORAGE,
+.class_init= usb_msd_class_bot_initfn,
+};
+
+static void register_types(void)
+{
+type_register_static(&bot_info);
+}
+
+type_init(register_types)
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 027e29dda3f5..3e613ecc886b 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -551,17 +551,6 @@ static const struct SCSIBusInfo usb_msd_scsi_info_storage 
= {
 .load_request = usb_msd_load_request,
 };
 
-static const struct SCSIBusInfo usb_msd_scsi_info_bot = {
-.tcq = false,
-.max_target = 0,
-.max_lun = 15,
-
-.transfer_data = usb_msd_transfer_data,
-.complete = usb_msd_command_complete,
-.cancel = usb_msd_request_cancelled,
-.load_request = usb_msd_load_request,
-};
-
 static void usb_msd_storage_realize(USBDevice *dev, Error **errp)
 {
 MSDState *s = USB_STORAGE_DEV(dev);
@@ -613,22 +602,6 @@ static void usb_msd_storage_realize(USBDevice *dev, Error 
**errp)
 s->scsi_dev = scsi_dev;
 }
 
-static void usb_msd_bot_realize(USBDevice *dev, Error **errp)
-{
-MSDState *s = USB_STORAGE_DEV(dev);
-DeviceState *d = DEVICE(dev);
-
-usb_desc_create_serial(dev);
-usb_desc_init(dev);
-if (d->hotplugged) {
-s->dev.auto_attach = 0;
-}
-
-scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(dev),
- &usb_msd_scsi_info_bot, NULL);
-usb_msd_handle_reset(dev);
-}
-
 static const VMStateDescription vmstate_usb_msd = {
 .name = "usb-storage",
 .version_id = 1,
@@ -734,14 +707,6 @@ static void usb_msd_instance_init(Object *obj)
 object_property_set_int(obj, "bootindex", -1, NULL);
 }
 
-static void usb_msd_class_bot_initfn(ObjectClass *klass, void *data)
-{
-USBDeviceClass *uc = USB_DEVICE_CLASS(klass);
-
-uc->realize = usb_msd_bot_realize;
-uc->attached_settable = true;
-}
-
 static const TypeInfo msd_info = {
 .name  = "usb-storage",
 .parent= TYPE_USB_STORAGE,
@@ -749,17 +714,10 @@ static const TypeInfo msd_info = {
 .instance_init = usb_msd_instance_init,
 };
 
-static const TypeInfo bot_info = {
-.name  = "usb-bot",
-.parent= TYPE_USB_STORAGE,
-.class_init= usb_msd_class_bot_initfn,
-};
-
 static void usb_msd_register_types(void)
 {
 type_register_static(&usb_storage_dev_type_info);
 type_register_static(&msd_info);
-type_register_static(&bot_info);
 }
 
 type_init(usb_msd_register_types)
diff --git a/hw/usb/meson.build b/hw/usb/meson.build
index 6e3159798e93..518cd1fbbbea 100644
--- a/hw/usb/meson.build
+++ b/hw/usb/meson.build
@@ -41,6 +41,7 @@ softmmu_ss.add(when: 'CONFIG_USB', if_true: 
files('dev-hub.c'))
 softmmu_ss.add(when: 'CONFIG_USB', if_true: files('dev-hid.c'))
 softmmu_ss.add(when: 'CONFIG_USB_TABLET_WACOM', if_true: files('dev-wacom.c'))
 softmmu_ss.add(when: 'CONFIG_USB_STORAGE_BOT', if_true: files('dev-storage.c'))
+softmmu_ss.add(when: 'CONFIG_USB_STORAGE_BOT', if_true: 
files('dev-storage-bot.c'))
 softmmu_ss.add(when: 'CONFIG_USB_STORAGE_UAS', if_true: files('dev-uas.c'))
 softmmu_ss.add(when: 'CONFIG_USB_A

[PULL 12/13] usb/storage: add kconfig symbols

[Gerd Hoffmann]

Add new kconfig symbols so usb-storage and usb-bot can
be enabled or disabled individually at build time.

Signed-off-by: Gerd Hoffmann 
Message-Id: <20210312090425.772900-5-kra...@redhat.com>
---
 hw/usb/Kconfig | 13 -
 hw/usb/meson.build |  4 ++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index 40093d7ea6bf..53f8283ffdc1 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -66,11 +66,22 @@ config USB_TABLET_WACOM
 default y
 depends on USB
 
+config USB_STORAGE_CORE
+bool
+depends on USB
+select SCSI
+
+config USB_STORAGE_CLASSIC
+bool
+default y
+depends on USB
+select USB_STORAGE_CORE
+
 config USB_STORAGE_BOT
 bool
 default y
 depends on USB
-select SCSI
+select USB_STORAGE_CORE
 
 config USB_STORAGE_UAS
 bool
diff --git a/hw/usb/meson.build b/hw/usb/meson.build
index 9e4da68e3b6c..fb7a74e73ae8 100644
--- a/hw/usb/meson.build
+++ b/hw/usb/meson.build
@@ -40,9 +40,9 @@ specific_ss.add(when: 'CONFIG_XLNX_USB_SUBSYS', if_true: 
files('xlnx-usb-subsyst
 softmmu_ss.add(when: 'CONFIG_USB', if_true: files('dev-hub.c'))
 softmmu_ss.add(when: 'CONFIG_USB', if_true: files('dev-hid.c'))
 softmmu_ss.add(when: 'CONFIG_USB_TABLET_WACOM', if_true: files('dev-wacom.c'))
-softmmu_ss.add(when: 'CONFIG_USB_STORAGE_BOT', if_true: files('dev-storage.c'))
+softmmu_ss.add(when: 'CONFIG_USB_STORAGE_CORE', if_true: 
files('dev-storage.c'))
 softmmu_ss.add(when: 'CONFIG_USB_STORAGE_BOT', if_true: 
files('dev-storage-bot.c'))
-softmmu_ss.add(when: 'CONFIG_USB_STORAGE_BOT', if_true: 
files('dev-storage-classic.c'))
+softmmu_ss.add(when: 'CONFIG_USB_STORAGE_CLASSIC', if_true: 
files('dev-storage-classic.c'))
 softmmu_ss.add(when: 'CONFIG_USB_STORAGE_UAS', if_true: files('dev-uas.c'))
 softmmu_ss.add(when: 'CONFIG_USB_AUDIO', if_true: files('dev-audio.c'))
 softmmu_ss.add(when: 'CONFIG_USB_SERIAL', if_true: files('dev-serial.c'))
-- 
2.29.2

[PULL 09/13] usb/storage: move declarations to usb/msd.h header

[Gerd Hoffmann]

In preparation for splitting the usb-storage.c file move
declarations to the new usb/msd.h header file.

Signed-off-by: Gerd Hoffmann 
Message-Id: <20210312090425.772900-2-kra...@redhat.com>
---
 include/hw/usb/msd.h | 54 
 hw/usb/dev-storage.c | 48 +--
 2 files changed, 60 insertions(+), 42 deletions(-)
 create mode 100644 include/hw/usb/msd.h

diff --git a/include/hw/usb/msd.h b/include/hw/usb/msd.h
new file mode 100644
index ..7538c54569bf
--- /dev/null
+++ b/include/hw/usb/msd.h
@@ -0,0 +1,54 @@
+/*
+ * USB Mass Storage Device emulation
+ *
+ * Copyright (c) 2006 CodeSourcery.
+ * Written by Paul Brook
+ *
+ * This code is licensed under the LGPL.
+ */
+
+#include "hw/usb.h"
+#include "hw/scsi/scsi.h"
+
+enum USBMSDMode {
+USB_MSDM_CBW, /* Command Block.  */
+USB_MSDM_DATAOUT, /* Transfer data to device.  */
+USB_MSDM_DATAIN, /* Transfer data from device.  */
+USB_MSDM_CSW /* Command Status.  */
+};
+
+struct usb_msd_csw {
+uint32_t sig;
+uint32_t tag;
+uint32_t residue;
+uint8_t status;
+};
+
+struct MSDState {
+USBDevice dev;
+enum USBMSDMode mode;
+uint32_t scsi_off;
+uint32_t scsi_len;
+uint32_t data_len;
+struct usb_msd_csw csw;
+SCSIRequest *req;
+SCSIBus bus;
+/* For async completion.  */
+USBPacket *packet;
+/* usb-storage only */
+BlockConf conf;
+bool removable;
+bool commandlog;
+SCSIDevice *scsi_dev;
+};
+
+typedef struct MSDState MSDState;
+#define TYPE_USB_STORAGE "usb-storage-dev"
+DECLARE_INSTANCE_CHECKER(MSDState, USB_STORAGE_DEV,
+ TYPE_USB_STORAGE)
+
+void usb_msd_transfer_data(SCSIRequest *req, uint32_t len);
+void usb_msd_command_complete(SCSIRequest *req, size_t resid);
+void usb_msd_request_cancelled(SCSIRequest *req);
+void *usb_msd_load_request(QEMUFile *f, SCSIRequest *req);
+void usb_msd_handle_reset(USBDevice *dev);
diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index a5f76fc00120..027e29dda3f5 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -14,6 +14,7 @@
 #include "qemu/option.h"
 #include "qemu/config-file.h"
 #include "hw/usb.h"
+#include "hw/usb/msd.h"
 #include "desc.h"
 #include "hw/qdev-properties.h"
 #include "hw/scsi/scsi.h"
@@ -29,43 +30,6 @@
 #define MassStorageReset  0xff
 #define GetMaxLun 0xfe
 
-enum USBMSDMode {
-USB_MSDM_CBW, /* Command Block.  */
-USB_MSDM_DATAOUT, /* Transfer data to device.  */
-USB_MSDM_DATAIN, /* Transfer data from device.  */
-USB_MSDM_CSW /* Command Status.  */
-};
-
-struct usb_msd_csw {
-uint32_t sig;
-uint32_t tag;
-uint32_t residue;
-uint8_t status;
-};
-
-struct MSDState {
-USBDevice dev;
-enum USBMSDMode mode;
-uint32_t scsi_off;
-uint32_t scsi_len;
-uint32_t data_len;
-struct usb_msd_csw csw;
-SCSIRequest *req;
-SCSIBus bus;
-/* For async completion.  */
-USBPacket *packet;
-/* usb-storage only */
-BlockConf conf;
-bool removable;
-bool commandlog;
-SCSIDevice *scsi_dev;
-};
-typedef struct MSDState MSDState;
-
-#define TYPE_USB_STORAGE "usb-storage-dev"
-DECLARE_INSTANCE_CHECKER(MSDState, USB_STORAGE_DEV,
- TYPE_USB_STORAGE)
-
 struct usb_msd_cbw {
 uint32_t sig;
 uint32_t tag;
@@ -259,7 +223,7 @@ static void usb_msd_packet_complete(MSDState *s)
 usb_packet_complete(&s->dev, p);
 }
 
-static void usb_msd_transfer_data(SCSIRequest *req, uint32_t len)
+void usb_msd_transfer_data(SCSIRequest *req, uint32_t len)
 {
 MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
 USBPacket *p = s->packet;
@@ -277,7 +241,7 @@ static void usb_msd_transfer_data(SCSIRequest *req, 
uint32_t len)
 }
 }
 
-static void usb_msd_command_complete(SCSIRequest *req, size_t resid)
+void usb_msd_command_complete(SCSIRequest *req, size_t resid)
 {
 MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
 USBPacket *p = s->packet;
@@ -320,7 +284,7 @@ static void usb_msd_command_complete(SCSIRequest *req, 
size_t resid)
 s->req = NULL;
 }
 
-static void usb_msd_request_cancelled(SCSIRequest *req)
+void usb_msd_request_cancelled(SCSIRequest *req)
 {
 MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
 
@@ -337,7 +301,7 @@ static void usb_msd_request_cancelled(SCSIRequest *req)
 }
 }
 
-static void usb_msd_handle_reset(USBDevice *dev)
+void usb_msd_handle_reset(USBDevice *dev)
 {
 MSDState *s = (MSDState *)dev;
 
@@ -565,7 +529,7 @@ static void usb_msd_handle_data(USBDevice *dev, USBPacket 
*p)
 }
 }
 
-static void *usb_msd_load_request(QEMUFile *f, SCSIRequest *req)
+void *usb_msd_load_request(QEMUFile *f, SCSIRequest *req)
 {
 MSDState *s = DO_UPCAST(MSDState, dev.qdev, req->bus->qbus.parent);
 
-- 
2.29.2

[PULL 13/13] usb/storage: clear csw on reset

[Gerd Hoffmann]

Stale data in csw (specifically residue) can confuse the state machine
and allows the guest trigger an assert().  So clear csw on reset to
avoid this happening in case the guest resets the device in the middle
of a request.

Buglink: https://bugs.launchpad.net/qemu/+bug/1523811
Signed-off-by: Gerd Hoffmann 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20210312094954.796799-1-kra...@redhat.com>
---
 hw/usb/dev-storage.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c
index 7b587ad051ff..dca62d544fe9 100644
--- a/hw/usb/dev-storage.c
+++ b/hw/usb/dev-storage.c
@@ -313,6 +313,7 @@ void usb_msd_handle_reset(USBDevice *dev)
 usb_msd_packet_complete(s);
 }
 
+memset(&s->csw, 0, sizeof(s->csw));
 s->mode = USB_MSDM_CBW;
 }
 
-- 
2.29.2

[PULL 06/13] hw/southbridge: Add missing Kconfig dependency VT82C686 on USB_UHCI

[Gerd Hoffmann]

From: Philippe Mathieu-Daudé 

The VT82C686 south bridge provides a USB UHCI bus via a PCI function.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: BALATON Zoltan 
Message-Id: <20210309190802.830969-2-f4...@amsat.org>
Signed-off-by: Gerd Hoffmann 
---
 hw/isa/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/isa/Kconfig b/hw/isa/Kconfig
index c7f07854f7e7..2691eae2f0c6 100644
--- a/hw/isa/Kconfig
+++ b/hw/isa/Kconfig
@@ -47,6 +47,7 @@ config VT82C686
 select ACPI_SMBUS
 select SERIAL_ISA
 select FDC
+select USB_UHCI
 
 config SMC37C669
 bool
-- 
2.29.2

[PULL 07/13] hw/usb/hcd-uhci: Expose generic prototypes to local header

[Gerd Hoffmann]

From: Philippe Mathieu-Daudé 

Extract generic UHCI prototypes into a new "hcd-uhci.h" local
header so we can reuse them in other units.

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20210309190802.830969-3-f4...@amsat.org>
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/hcd-uhci.h | 93 +++
 hw/usb/hcd-uhci.c | 60 ++
 2 files changed, 96 insertions(+), 57 deletions(-)
 create mode 100644 hw/usb/hcd-uhci.h

diff --git a/hw/usb/hcd-uhci.h b/hw/usb/hcd-uhci.h
new file mode 100644
index ..e61d8fcb1921
--- /dev/null
+++ b/hw/usb/hcd-uhci.h
@@ -0,0 +1,93 @@
+/*
+ * USB UHCI controller emulation
+ *
+ * Copyright (c) 2005 Fabrice Bellard
+ *
+ * Copyright (c) 2008 Max Krasnyansky
+ * Magor rewrite of the UHCI data structures parser and frame processor
+ * Support for fully async operation and multiple outstanding transactions
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#ifndef HW_USB_HCD_UHCI_H
+#define HW_USB_HCD_UHCI_H
+
+#include "exec/memory.h"
+#include "qemu/timer.h"
+#include "hw/pci/pci.h"
+#include "hw/usb.h"
+
+typedef struct UHCIQueue UHCIQueue;
+
+#define NB_PORTS 2
+
+typedef struct UHCIPort {
+USBPort port;
+uint16_t ctrl;
+} UHCIPort;
+
+typedef struct UHCIState {
+PCIDevice dev;
+MemoryRegion io_bar;
+USBBus bus; /* Note unused when we're a companion controller */
+uint16_t cmd; /* cmd register */
+uint16_t status;
+uint16_t intr; /* interrupt enable register */
+uint16_t frnum; /* frame number */
+uint32_t fl_base_addr; /* frame list base address */
+uint8_t sof_timing;
+uint8_t status2; /* bit 0 and 1 are used to generate UHCI_STS_USBINT */
+int64_t expire_time;
+QEMUTimer *frame_timer;
+QEMUBH *bh;
+uint32_t frame_bytes;
+uint32_t frame_bandwidth;
+bool completions_only;
+UHCIPort ports[NB_PORTS];
+
+/* Interrupts that should be raised at the end of the current frame.  */
+uint32_t pending_int_mask;
+
+/* Active packets */
+QTAILQ_HEAD(, UHCIQueue) queues;
+uint8_t num_ports_vmstate;
+
+/* Properties */
+char *masterbus;
+uint32_t firstport;
+uint32_t maxframes;
+} UHCIState;
+
+#define TYPE_UHCI "pci-uhci-usb"
+DECLARE_INSTANCE_CHECKER(UHCIState, UHCI, TYPE_UHCI)
+
+typedef struct UHCIInfo {
+const char *name;
+uint16_t   vendor_id;
+uint16_t   device_id;
+uint8_trevision;
+uint8_tirq_pin;
+void   (*realize)(PCIDevice *dev, Error **errp);
+bool   unplug;
+} UHCIInfo;
+
+void uhci_data_class_init(ObjectClass *klass, void *data);
+void usb_uhci_common_realize(PCIDevice *dev, Error **errp);
+
+#endif
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 5969eb86b31b..d6338c33d863 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -40,6 +40,7 @@
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "qom/object.h"
+#include "hcd-uhci.h"
 
 #define FRAME_TIMER_FREQ 1000
 
@@ -50,8 +51,6 @@
 
 #define MAX_FRAMES_PER_TICK(QH_VALID / 2)
 
-#define NB_PORTS 2
-
 enum {
 TD_RESULT_STOP_FRAME = 10,
 TD_RESULT_COMPLETE,
@@ -62,20 +61,8 @@ enum {
 
 typedef struct UHCIState UHCIState;
 typedef struct UHCIAsync UHCIAsync;
-typedef struct UHCIQueue UHCIQueue;
-typedef struct UHCIInfo UHCIInfo;
 typedef struct UHCIPCIDeviceClass UHCIPCIDeviceClass;
 
-struct UHCIInfo {
-const char *name;
-uint16_t   vendor_id;
-uint16_t   device_id;
-uint8_trevision;
-uint8_tirq_pin;
-void   (*realize)(PCIDevice *dev, Error **errp);
-bool   unplug;
-};
-
 struct UHCIPCIDeviceClass {
 PCIDeviceClass parent_class;
 UHCIInfo   info;
@@ -107,43 +94,6 @@ struct UHCIQueue {
 int8_tvalid;
 };
 
-typedef struct UHCIPort {
-USBPort port;
-uint16_t ctrl;
-} UHCIPort;
-
-struct UHCIState {
-PCIDevice dev;
-MemoryRegion io_bar;
-USBBus bus; /*

[PULL 04/13] usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets removed)

[Gerd Hoffmann]

From: Thomas Huth 

When trying to remove the -usbdevice option, there were complaints that
"-usbdevice braille" is still a very useful shortcut for some people.
Thus we never remove this option. Since it's not such a big burden to
keep it around, and it's also convenient in the sense that you don't
have to worry to enable a host controller explicitly with this option,
we should remove it from he deprecation list again.

However, there is one exception: "-usbdevice audio" should go away, since
audio devices without "audiodev=..." parameter are also on the deprecation
list and you cannot use "-usbdevice audio" with "audiodev".

Signed-off-by: Thomas Huth 
Message-Id: <20210310173323.1422754-4-th...@redhat.com>
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/dev-audio.c   | 1 -
 softmmu/vl.c | 2 --
 docs/system/deprecated.rst   | 9 -
 docs/system/removed-features.rst | 8 
 4 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index e1486f81e06b..f5cb2467929a 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -1024,7 +1024,6 @@ static const TypeInfo usb_audio_info = {
 static void usb_audio_register_types(void)
 {
 type_register_static(&usb_audio_info);
-usb_legacy_register(TYPE_USB_AUDIO, "audio", NULL);
 }
 
 type_init(usb_audio_register_types)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index b7673b96134d..a750dae6b1ef 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -3180,8 +3180,6 @@ void qemu_init(int argc, char **argv, char **envp)
 qemu_opts_parse_noisily(olist, "usb=on", false);
 break;
 case QEMU_OPTION_usbdevice:
-error_report("'-usbdevice' is deprecated, please use "
- "'-device usb-...' instead");
 olist = qemu_find_opts("machine");
 qemu_opts_parse_noisily(olist, "usb=on", false);
 add_device_config(DEV_USB, optarg);
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 5e3a31c12361..62a64a8c41e1 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -21,15 +21,6 @@ deprecated.
 System emulator command line arguments
 --
 
-``-usbdevice`` (since 2.10.0)
-'
-
-The ``-usbdevice DEV`` argument is now a synonym for setting
-the ``-device usb-DEV`` argument instead. The deprecated syntax
-would automatically enable USB support on the machine type.
-If using the new syntax, USB support must be explicitly
-enabled via the ``-machine usb=on`` argument.
-
 ``-drive file=json:{...{'driver':'file'}}`` (since 3.0)
 '''
 
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 83148dcfda6a..82e7fcc51715 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -38,6 +38,14 @@ or ``-display default,show-cursor=on`` instead.
 QEMU 5.0 introduced an alternative syntax to specify the size of the 
translation
 block cache, ``-accel tcg,tb-size=``.
 
+``-usbdevice audio`` (removed in 6.0)
+'
+
+This option lacked the possibility to specify an audio backend device.
+Use ``-device usb-audio`` now instead (and specify a corresponding USB
+host controller or ``-usb`` if necessary).
+
+
 QEMU Machine Protocol (QMP) commands
 
 
-- 
2.29.2

[PULL 01/13] hw/usb/bus: Remove the "full-path" property

[Gerd Hoffmann]

From: Thomas Huth 

This property was only required for the pc-1.0 and earlier machine
types. Since these have been removed now, we can delete the property
as well.

Signed-off-by: Thomas Huth 
Message-Id: <20210302120152.118042-1-th...@redhat.com>
Signed-off-by: Gerd Hoffmann 
---
 include/hw/usb.h | 1 -
 hw/usb/bus.c | 7 +--
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/include/hw/usb.h b/include/hw/usb.h
index abfbfc5284c2..9f42394efaea 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -216,7 +216,6 @@ struct USBEndpoint {
 };
 
 enum USBDeviceFlags {
-USB_DEV_FLAG_FULL_PATH,
 USB_DEV_FLAG_IS_HOST,
 USB_DEV_FLAG_MSOS_DESC_ENABLE,
 USB_DEV_FLAG_MSOS_DESC_IN_USE,
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 064f94e9c3cc..df7411fea8e4 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -19,8 +19,6 @@ static void usb_qdev_unrealize(DeviceState *qdev);
 static Property usb_props[] = {
 DEFINE_PROP_STRING("port", USBDevice, port_path),
 DEFINE_PROP_STRING("serial", USBDevice, serial),
-DEFINE_PROP_BIT("full-path", USBDevice, flags,
-USB_DEV_FLAG_FULL_PATH, true),
 DEFINE_PROP_BIT("msos-desc", USBDevice, flags,
 USB_DEV_FLAG_MSOS_DESC_ENABLE, true),
 DEFINE_PROP_STRING("pcap", USBDevice, pcap_filename),
@@ -596,11 +594,8 @@ static char *usb_get_dev_path(DeviceState *qdev)
 {
 USBDevice *dev = USB_DEVICE(qdev);
 DeviceState *hcd = qdev->parent_bus->parent;
-char *id = NULL;
+char *id = qdev_get_dev_path(hcd);
 
-if (dev->flags & (1 << USB_DEV_FLAG_FULL_PATH)) {
-id = qdev_get_dev_path(hcd);
-}
 if (id) {
 char *ret = g_strdup_printf("%s/%s", id, dev->port->path);
 g_free(id);
-- 
2.29.2

[PULL 03/13] usb: remove '-usbdevice u2f-key'

[Gerd Hoffmann]

From: Paolo Bonzini 

It never worked.

Signed-off-by: Paolo Bonzini 
Cc: Gerd Hoffmann 
Signed-off-by: Thomas Huth 
Message-Id: <20210310173323.1422754-3-th...@redhat.com>
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/u2f.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/usb/u2f.c b/hw/usb/u2f.c
index bc09191f063e..56001249a449 100644
--- a/hw/usb/u2f.c
+++ b/hw/usb/u2f.c
@@ -346,7 +346,6 @@ static const TypeInfo u2f_key_info = {
 static void u2f_key_register_types(void)
 {
 type_register_static(&u2f_key_info);
-usb_legacy_register(TYPE_U2F_KEY, "u2f-key", NULL);
 }
 
 type_init(u2f_key_register_types)
-- 
2.29.2

[PULL 08/13] hw/usb: Extract VT82C686 UHCI PCI function into a new unit

[Gerd Hoffmann]

From: Philippe Mathieu-Daudé 

Extract the VT82C686 PCI UHCI function into a new unit so
it is only build when the VT82C686 south bridge is selected.

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20210309190802.830969-4-f4...@amsat.org>
Signed-off-by: Gerd Hoffmann 
---
 hw/usb/hcd-uhci.c  | 23 
 hw/usb/vt82c686-uhci-pci.c | 43 ++
 MAINTAINERS|  1 +
 hw/usb/meson.build |  1 +
 4 files changed, 45 insertions(+), 23 deletions(-)
 create mode 100644 hw/usb/vt82c686-uhci-pci.c

diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index d6338c33d863..0cb02a643214 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -1207,21 +1207,6 @@ void usb_uhci_common_realize(PCIDevice *dev, Error 
**errp)
 pci_register_bar(&s->dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &s->io_bar);
 }
 
-static void usb_uhci_vt82c686b_realize(PCIDevice *dev, Error **errp)
-{
-UHCIState *s = UHCI(dev);
-uint8_t *pci_conf = s->dev.config;
-
-/* USB misc control 1/2 */
-pci_set_long(pci_conf + 0x40,0x1000);
-/* PM capability */
-pci_set_long(pci_conf + 0x80,0x00020001);
-/* USB legacy support  */
-pci_set_long(pci_conf + 0xc0,0x2000);
-
-usb_uhci_common_realize(dev, errp);
-}
-
 static void usb_uhci_exit(PCIDevice *dev)
 {
 UHCIState *s = UHCI(dev);
@@ -1318,14 +1303,6 @@ static UHCIInfo uhci_info[] = {
 .revision  = 0x01,
 .irq_pin   = 3,
 .unplug= true,
-},{
-.name  = "vt82c686b-usb-uhci",
-.vendor_id = PCI_VENDOR_ID_VIA,
-.device_id = PCI_DEVICE_ID_VIA_UHCI,
-.revision  = 0x01,
-.irq_pin   = 3,
-.realize   = usb_uhci_vt82c686b_realize,
-.unplug= true,
 },{
 .name  = "ich9-usb-uhci1", /* 00:1d.0 */
 .vendor_id = PCI_VENDOR_ID_INTEL,
diff --git a/hw/usb/vt82c686-uhci-pci.c b/hw/usb/vt82c686-uhci-pci.c
new file mode 100644
index ..b109c2160335
--- /dev/null
+++ b/hw/usb/vt82c686-uhci-pci.c
@@ -0,0 +1,43 @@
+#include "qemu/osdep.h"
+#include "hcd-uhci.h"
+
+static void usb_uhci_vt82c686b_realize(PCIDevice *dev, Error **errp)
+{
+UHCIState *s = UHCI(dev);
+uint8_t *pci_conf = s->dev.config;
+
+/* USB misc control 1/2 */
+pci_set_long(pci_conf + 0x40, 0x1000);
+/* PM capability */
+pci_set_long(pci_conf + 0x80, 0x00020001);
+/* USB legacy support  */
+pci_set_long(pci_conf + 0xc0, 0x2000);
+
+usb_uhci_common_realize(dev, errp);
+}
+
+static UHCIInfo uhci_info[] = {
+{
+.name  = "vt82c686b-usb-uhci",
+.vendor_id = PCI_VENDOR_ID_VIA,
+.device_id = PCI_DEVICE_ID_VIA_UHCI,
+.revision  = 0x01,
+.irq_pin   = 3,
+.realize   = usb_uhci_vt82c686b_realize,
+.unplug= true,
+}
+};
+
+static const TypeInfo vt82c686b_usb_uhci_type_info = {
+.parent = TYPE_UHCI,
+.name   = "vt82c686b-usb-uhci",
+.class_init = uhci_data_class_init,
+.class_data = uhci_info,
+};
+
+static void vt82c686b_usb_uhci_register_types(void)
+{
+type_register_static(&vt82c686b_usb_uhci_type_info);
+}
+
+type_init(vt82c686b_usb_uhci_register_types)
diff --git a/MAINTAINERS b/MAINTAINERS
index 5ca3c9f851f9..b6ab3d25a751 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1180,6 +1180,7 @@ S: Odd Fixes
 F: hw/mips/fuloong2e.c
 F: hw/isa/vt82c686.c
 F: hw/pci-host/bonito.c
+F: hw/usb/vt82c686-uhci-pci.c
 F: include/hw/isa/vt82c686.h
 
 Loongson-3 virtual platforms
diff --git a/hw/usb/meson.build b/hw/usb/meson.build
index 653192cff6fa..6e3159798e93 100644
--- a/hw/usb/meson.build
+++ b/hw/usb/meson.build
@@ -32,6 +32,7 @@ softmmu_ss.add(when: 'CONFIG_USB_DWC3', if_true: 
files('hcd-dwc3.c'))
 softmmu_ss.add(when: 'CONFIG_TUSB6010', if_true: files('tusb6010.c'))
 softmmu_ss.add(when: 'CONFIG_IMX', if_true: files('chipidea.c'))
 softmmu_ss.add(when: 'CONFIG_IMX_USBPHY', if_true: files('imx-usb-phy.c'))
+softmmu_ss.add(when: 'CONFIG_VT82C686', if_true: files('vt82c686-uhci-pci.c'))
 specific_ss.add(when: 'CONFIG_XLNX_VERSAL', if_true: 
files('xlnx-versal-usb2-ctrl-regs.c'))
 specific_ss.add(when: 'CONFIG_XLNX_USB_SUBSYS', if_true: 
files('xlnx-usb-subsystem.c'))
 
-- 
2.29.2

[PULL 02/13] usb: remove support for -usbdevice parameters

[Gerd Hoffmann]

From: Paolo Bonzini 

No device needs them anymore and in fact they're undocumented.
Remove the code.  The only change in behavior is that "-usbdevice
braille:hello" now reports an error, which is a bugfix.

Signed-off-by: Paolo Bonzini 
Cc: Gerd Hoffmann 
Signed-off-by: Thomas Huth 
Message-Id: <20210310173323.1422754-2-th...@redhat.com>
Signed-off-by: Gerd Hoffmann 
---
 include/hw/usb.h|  2 +-
 hw/usb/bus.c| 32 +++-
 hw/usb/dev-serial.c |  2 +-
 3 files changed, 9 insertions(+), 27 deletions(-)

diff --git a/include/hw/usb.h b/include/hw/usb.h
index 9f42394efaea..436e07b30404 100644
--- a/include/hw/usb.h
+++ b/include/hw/usb.h
@@ -499,7 +499,7 @@ void usb_bus_new(USBBus *bus, size_t bus_size,
 void usb_bus_release(USBBus *bus);
 USBBus *usb_bus_find(int busnr);
 void usb_legacy_register(const char *typename, const char *usbdevice_name,
- USBDevice *(*usbdevice_init)(const char *params));
+ USBDevice *(*usbdevice_init)(void));
 USBDevice *usb_new(const char *name);
 bool usb_realize_and_unref(USBDevice *dev, USBBus *bus, Error **errp);
 USBDevice *usb_create_simple(USBBus *bus, const char *name);
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index df7411fea8e4..07083349f51b 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -310,13 +310,13 @@ typedef struct LegacyUSBFactory
 {
 const char *name;
 const char *usbdevice_name;
-USBDevice *(*usbdevice_init)(const char *params);
+USBDevice *(*usbdevice_init)(void);
 } LegacyUSBFactory;
 
 static GSList *legacy_usb_factory;
 
 void usb_legacy_register(const char *typename, const char *usbdevice_name,
- USBDevice *(*usbdevice_init)(const char *params))
+ USBDevice *(*usbdevice_init)(void))
 {
 if (usbdevice_name) {
 LegacyUSBFactory *f = g_malloc0(sizeof(*f));
@@ -658,27 +658,17 @@ void hmp_info_usb(Monitor *mon, const QDict *qdict)
 }
 
 /* handle legacy -usbdevice cmd line option */
-USBDevice *usbdevice_create(const char *cmdline)
+USBDevice *usbdevice_create(const char *driver)
 {
 USBBus *bus = usb_bus_find(-1 /* any */);
 LegacyUSBFactory *f = NULL;
 Error *err = NULL;
 GSList *i;
-char driver[32];
-const char *params;
-int len;
 USBDevice *dev;
 
-params = strchr(cmdline,':');
-if (params) {
-params++;
-len = params - cmdline;
-if (len > sizeof(driver))
-len = sizeof(driver);
-pstrcpy(driver, len, cmdline);
-} else {
-params = "";
-pstrcpy(driver, sizeof(driver), cmdline);
+if (strchr(driver, ':')) {
+error_report("usbdevice parameters are not supported anymore");
+return NULL;
 }
 
 for (i = legacy_usb_factory; i; i = i->next) {
@@ -702,15 +692,7 @@ USBDevice *usbdevice_create(const char *cmdline)
 return NULL;
 }
 
-if (f->usbdevice_init) {
-dev = f->usbdevice_init(params);
-} else {
-if (*params) {
-error_report("usbdevice %s accepts no params", driver);
-return NULL;
-}
-dev = usb_new(f->name);
-}
+dev = f->usbdevice_init ? f->usbdevice_init() : usb_new(f->name);
 if (!dev) {
 error_report("Failed to create USB device '%s'", f->name);
 return NULL;
diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index b58c4eb90822..63047d79cfd1 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -614,7 +614,7 @@ static void usb_serial_realize(USBDevice *dev, Error **errp)
 s->intr = usb_ep_get(dev, USB_TOKEN_IN, 1);
 }
 
-static USBDevice *usb_braille_init(const char *unused)
+static USBDevice *usb_braille_init(void)
 {
 USBDevice *dev;
 Chardev *cdrv;
-- 
2.29.2

[PULL 00/13] Usb 20210315 patches

[Gerd Hoffmann]

The following changes since commit 6157b0e19721aadb4c7fdcfe57b2924af6144b14:

  Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-6.0-pull-=
request' into staging (2021-03-14 17:47:49 +)

are available in the Git repository at:

  git://git.kraxel.org/qemu tags/usb-20210315-pull-request

for you to fetch changes up to 39912c14da07a2dbc73854addcfa0a42596340ac:

  usb/storage: clear csw on reset (2021-03-15 17:01:17 +0100)


usb: -usbdevice cleanup and un-deprecation.
usb: split usb-storage.
usb: misc fixes and cleanups.



Gerd Hoffmann (5):
  usb/storage: move declarations to usb/msd.h header
  usb/storage: move usb-bot device to separate source file
  usb/storage move usb-storage device to separate source file
  usb/storage: add kconfig symbols
  usb/storage: clear csw on reset

Paolo Bonzini (2):
  usb: remove support for -usbdevice parameters
  usb: remove '-usbdevice u2f-key'

Philippe Mathieu-Daud=C3=A9 (3):
  hw/southbridge: Add missing Kconfig dependency VT82C686 on USB_UHCI
  hw/usb/hcd-uhci: Expose generic prototypes to local header
  hw/usb: Extract VT82C686 UHCI PCI function into a new unit

Thomas Huth (3):
  hw/usb/bus: Remove the "full-path" property
  usb: Un-deprecate -usbdevice (except for -usbdevice audio which gets
removed)
  usb: Document the missing -usbdevice options

 hw/usb/hcd-uhci.h|  93 +
 include/hw/usb.h |   3 +-
 include/hw/usb/msd.h |  54 
 hw/usb/bus.c |  39 ++
 hw/usb/dev-audio.c   |   1 -
 hw/usb/dev-serial.c  |   2 +-
 hw/usb/dev-storage-bot.c |  63 +
 hw/usb/dev-storage-classic.c | 156 +
 hw/usb/dev-storage.c | 226 +--
 hw/usb/hcd-uhci.c|  83 +---
 hw/usb/u2f.c |   1 -
 hw/usb/vt82c686-uhci-pci.c   |  43 ++
 softmmu/vl.c |   2 -
 MAINTAINERS  |   1 +
 docs/system/deprecated.rst   |   9 --
 docs/system/removed-features.rst |   8 ++
 hw/isa/Kconfig   |   1 +
 hw/usb/Kconfig   |  13 +-
 hw/usb/meson.build   |   5 +-
 qemu-options.hx  |  35 -
 20 files changed, 484 insertions(+), 354 deletions(-)
 create mode 100644 hw/usb/hcd-uhci.h
 create mode 100644 include/hw/usb/msd.h
 create mode 100644 hw/usb/dev-storage-bot.c
 create mode 100644 hw/usb/dev-storage-classic.c
 create mode 100644 hw/usb/vt82c686-uhci-pci.c

--=20
2.29.2

Re: [libvirt PATCH v3 06/10] ci: Implement 'shell' helper action

[Andrea Bolognani]

On Mon, 2021-03-15 at 14:24 +0100, Erik Skultety wrote:
> On Fri, Mar 12, 2021 at 06:28:18PM +0100, Andrea Bolognani wrote:
> > +containerparser.add_argument(
> > +"target",
> > +help="build on target OS",
> 
> With the 'shell' action, not sure if "build" is the right word in that case,
> how about "perform action on target OS" -- since the "target" argument is
> shared among ['shell', 'build', 'test'].

Good catch! The word "build" was simply a leftover from development,
when I implemented (and documented) the "build" action first :)

I've made all the changes you suggested and pushed the series. Thanks
for your reviews!

-- 
Andrea Bolognani / Red Hat / Virtualization

[PATCH v2 13/13] block: remove support for using "file" driver with block/char devices

[Daniel P . Berrangé]

The 'host_device' and 'host_cdrom' drivers must be used instead.

Reviewed-by: Eric Blake 
Signed-off-by: Daniel P. Berrangé 
---
 block/file-posix.c   | 17 ++---
 docs/system/deprecated.rst   |  7 ---
 docs/system/removed-features.rst |  7 +++
 tests/qemu-iotests/226.out   | 10 +-
 4 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index 05079b40ca..20e14f8e96 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -719,15 +719,9 @@ static int raw_open_common(BlockDriverState *bs, QDict 
*options,
 }
 
 if (!device) {
-if (S_ISBLK(st.st_mode)) {
-warn_report("Opening a block device as a file using the '%s' "
-"driver is deprecated", bs->drv->format_name);
-} else if (S_ISCHR(st.st_mode)) {
-warn_report("Opening a character device as a file using the '%s' "
-"driver is deprecated", bs->drv->format_name);
-} else if (!S_ISREG(st.st_mode)) {
-error_setg(errp, "A regular file was expected by the '%s' driver, "
-   "but something else was given", bs->drv->format_name);
+if (!S_ISREG(st.st_mode)) {
+error_setg(errp, "'%s' driver requires '%s' to be a regular file",
+   bs->drv->format_name, bs->filename);
 ret = -EINVAL;
 goto fail;
 } else {
@@ -736,8 +730,9 @@ static int raw_open_common(BlockDriverState *bs, QDict 
*options,
 }
 } else {
 if (!(S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode))) {
-error_setg(errp, "'%s' driver expects either "
-   "a character or block device", bs->drv->format_name);
+error_setg(errp, "'%s' driver requires '%s' to be either "
+   "a character or block device",
+   bs->drv->format_name, bs->filename);
 ret = -EINVAL;
 goto fail;
 }
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index f6d0b16579..176b837a42 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -30,13 +30,6 @@ would automatically enable USB support on the machine type.
 If using the new syntax, USB support must be explicitly
 enabled via the ``-machine usb=on`` argument.
 
-``-drive file=json:{...{'driver':'file'}}`` (since 3.0)
-'''
-
-The 'file' driver for drives is no longer appropriate for character or host
-devices and will only accept regular files (S_IFREG). The correct driver
-for these file types is 'host_cdrom' or 'host_device' as appropriate.
-
 ``QEMU_AUDIO_`` environment variables and ``-audio-help`` (since 4.0)
 '
 
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index eb4b71750c..7b7bf7214e 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -50,6 +50,13 @@ by the ``tls-authz`` and ``sasl-authz`` options.
 The ``pretty=on|off`` switch has no effect for HMP monitors and
 its use is rejected.
 
+``-drive file=json:{...{'driver':'file'}}`` (removed 6.0)
+'
+
+The 'file' driver for drives is no longer appropriate for character or host
+devices and will only accept regular files (S_IFREG). The correct driver
+for these file types is 'host_cdrom' or 'host_device' as appropriate.
+
 QEMU Machine Protocol (QMP) commands
 
 
diff --git a/tests/qemu-iotests/226.out b/tests/qemu-iotests/226.out
index 42be973ff2..55504d29c4 100644
--- a/tests/qemu-iotests/226.out
+++ b/tests/qemu-iotests/226.out
@@ -3,23 +3,23 @@ QA output created by 226
 === Testing with driver:file ===
 
 == Testing RO ==
-qemu-io: can't open: A regular file was expected by the 'file' driver, but 
something else was given
-qemu-io: warning: Opening a character device as a file using the 'file' driver 
is deprecated
+qemu-io: can't open: 'file' driver requires 'TEST_DIR/t.IMGFMT' to be a 
regular file
+qemu-io: can't open: 'file' driver requires '/dev/null' to be a regular file
 == Testing RW ==
 qemu-io: can't open: Could not open 'TEST_DIR/t.IMGFMT': Is a directory
-qemu-io: warning: Opening a character device as a file using the 'file' driver 
is deprecated
+qemu-io: can't open: 'file' driver requires '/dev/null' to be a regular file
 
 === Testing with driver:host_device ===
 
 == Testing RO ==
-qemu-io: can't open: 'host_device' driver expects either a character or block 
device
+qemu-io: can't open: 'host_device' driver requires 'TEST_DIR/t.IMGFMT' to be 
either a character or block device
 == Testing RW ==
 qemu-io: can't open: Could not open 'TEST_DIR/t.IMGFMT': Is a directory
 
 === Testing with driver:host_cdrom ===
 
 == Testing RO ==
-qemu-io: can't open: 'host_cdrom' driver 

[PATCH v2 12/13] block: remove 'dirty-bitmaps' field from 'BlockInfo' struct

[Daniel P . Berrangé]

The same data is available in the 'BlockDeviceInfo' struct.

Reviewed-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Daniel P. Berrangé 
---
 block/qapi.c|  5 -
 docs/system/deprecated.rst  | 13 -
 docs/system/removed-features.rst|  9 +
 qapi/block-core.json| 11 +--
 tests/qemu-iotests/194  |  4 ++--
 tests/qemu-iotests/236  |  2 +-
 tests/qemu-iotests/246  |  3 ++-
 tests/qemu-iotests/254  |  2 +-
 tests/qemu-iotests/260  |  5 +++--
 .../tests/migrate-bitmaps-postcopy-test |  6 --
 10 files changed, 23 insertions(+), 37 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 3acc118c44..943e7b15ad 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -383,11 +383,6 @@ static void bdrv_query_info(BlockBackend *blk, BlockInfo 
**p_info,
 info->io_status = blk_iostatus(blk);
 }
 
-if (bs && !QLIST_EMPTY(&bs->dirty_bitmaps)) {
-info->has_dirty_bitmaps = true;
-info->dirty_bitmaps = bdrv_query_dirty_bitmaps(bs);
-}
-
 if (bs && bs->drv) {
 info->has_inserted = true;
 info->inserted = bdrv_block_device_info(blk, bs, false, errp);
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index b1b487256a..f6d0b16579 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -199,19 +199,6 @@ Use arguments ``base-node`` and ``top-node`` instead.
 
 Specify the properties for the object as top-level arguments instead.
 
-``query-block`` result field ``dirty-bitmaps`` (Since 4.2)
-''
-
-The ``dirty-bitmaps`` field of the ``BlockInfo`` structure, returned by
-the query-block command is itself now deprecated. The ``dirty-bitmaps``
-field of the ``BlockDeviceInfo`` struct should be used instead, which is the
-type of the ``inserted`` field in query-block replies, as well as the
-type of array items in query-named-block-nodes.
-
-Since the ``dirty-bitmaps`` field is optionally present in both the old and
-new locations, clients must use introspection to learn where to anticipate
-the field if/when it does appear in command output.
-
 ``nbd-server-add`` and ``nbd-server-remove`` (since 5.2)
 
 
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 67526f1dd9..eb4b71750c 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -120,6 +120,15 @@ The ``status`` field of the ``BlockDirtyInfo`` structure, 
returned by
 these commands is removed. Two new boolean fields, ``recording`` and
 ``busy`` effectively replace it.
 
+``query-block`` result field ``dirty-bitmaps`` (removed in 6.0)
+'''
+
+The ``dirty-bitmaps`` field of the ``BlockInfo`` structure, returned by
+the query-block command is itself now removed. The ``dirty-bitmaps``
+field of the ``BlockDeviceInfo`` struct should be used instead, which is the
+type of the ``inserted`` field in query-block replies, as well as the
+type of array items in query-named-block-nodes.
+
 Human Monitor Protocol (HMP) commands
 -
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 2a0c345c2c..0399449e13 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -539,9 +539,6 @@
 # @tray_open: True if the device's tray is open
 # (only present if it has a tray)
 #
-# @dirty-bitmaps: dirty bitmaps information (only present if the
-# driver has one or more dirty bitmaps) (Since 2.0)
-#
 # @io-status: @BlockDeviceIoStatus. Only present if the device
 # supports it and the VM is configured to stop on errors
 # (supported device models: virtio-blk, IDE, SCSI except
@@ -550,18 +547,12 @@
 # @inserted: @BlockDeviceInfo describing the device if media is
 #present
 #
-# Features:
-# @deprecated: Member @dirty-bitmaps is deprecated.  Use @inserted
-#  member @dirty-bitmaps instead.
-#
 # Since:  0.14
 ##
 { 'struct': 'BlockInfo',
   'data': {'device': 'str', '*qdev': 'str', 'type': 'str', 'removable': 'bool',
'locked': 'bool', '*inserted': 'BlockDeviceInfo',
-   '*tray_open': 'bool', '*io-status': 'BlockDeviceIoStatus',
-   '*dirty-bitmaps': { 'type': ['BlockDirtyInfo'],
-   'features': [ 'deprecated' ] } } }
+   '*tray_open': 'bool', '*io-status': 'BlockDeviceIoStatus' } }
 
 ##
 # @BlockMeasureInfo:
diff --git a/tests/qemu-iotests/194 b/tests/qemu-iotests/194
index 3889266afa..e44b8df728 100755
--- a/tests/qemu-iotests/194
+++ b/tests/qemu-iotests/194
@@ -95,7 +95,7 @@ with iotests.FilePath

[PATCH v2 11/13] block: remove dirty bitmaps 'status' field

[Daniel P . Berrangé]

The same information is available via the 'recording' and 'busy' fields.

Reviewed-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Daniel P. Berrangé 
---
 block/dirty-bitmap.c |  38 
 docs/system/deprecated.rst   |   7 -
 docs/system/removed-features.rst |   7 +
 include/block/dirty-bitmap.h |   1 -
 qapi/block-core.json |  45 
 tests/qemu-iotests/124   |   4 -
 tests/qemu-iotests/194.out   |   4 +-
 tests/qemu-iotests/236.out   |  42 ++--
 tests/qemu-iotests/246.out   |  66 ++
 tests/qemu-iotests/254.out   |   9 +-
 tests/qemu-iotests/257.out   | 378 +++
 11 files changed, 174 insertions(+), 427 deletions(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index a0eaa28785..68d295d6e3 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -166,43 +166,6 @@ bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
 return !bitmap->disabled;
 }
 
-/**
- * bdrv_dirty_bitmap_status: This API is now deprecated.
- * Called with BQL taken.
- *
- * A BdrvDirtyBitmap can be in four possible user-visible states:
- * (1) Active:   successor is NULL, and disabled is false: full r/w mode
- * (2) Disabled: successor is NULL, and disabled is true: qualified r/w mode,
- *   guest writes are dropped, but monitor writes are possible,
- *   through commands like merge and clear.
- * (3) Frozen:   successor is not NULL.
- *   A frozen bitmap cannot be renamed, deleted, cleared, set,
- *   enabled, merged to, etc. A frozen bitmap can only abdicate()
- *   or reclaim().
- *   In this state, the anonymous successor bitmap may be either
- *   Active and recording writes from the guest (e.g. backup jobs),
- *   or it can be Disabled and not recording writes.
- * (4) Locked:   Whether Active or Disabled, the user cannot modify this bitmap
- *   in any way from the monitor.
- * (5) Inconsistent: This is a persistent bitmap whose "in use" bit is set, and
- *   is unusable by QEMU. It can be deleted to remove it from
- *   the qcow2.
- */
-DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap)
-{
-if (bdrv_dirty_bitmap_inconsistent(bitmap)) {
-return DIRTY_BITMAP_STATUS_INCONSISTENT;
-} else if (bdrv_dirty_bitmap_has_successor(bitmap)) {
-return DIRTY_BITMAP_STATUS_FROZEN;
-} else if (bdrv_dirty_bitmap_busy(bitmap)) {
-return DIRTY_BITMAP_STATUS_LOCKED;
-} else if (!bdrv_dirty_bitmap_enabled(bitmap)) {
-return DIRTY_BITMAP_STATUS_DISABLED;
-} else {
-return DIRTY_BITMAP_STATUS_ACTIVE;
-}
-}
-
 /* Called with BQL taken.  */
 static bool bdrv_dirty_bitmap_recording(BdrvDirtyBitmap *bitmap)
 {
@@ -582,7 +545,6 @@ BlockDirtyInfoList 
*bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 info->granularity = bdrv_dirty_bitmap_granularity(bm);
 info->has_name = !!bm->name;
 info->name = g_strdup(bm->name);
-info->status = bdrv_dirty_bitmap_status(bm);
 info->recording = bdrv_dirty_bitmap_recording(bm);
 info->busy = bdrv_dirty_bitmap_busy(bm);
 info->persistent = bm->persistent;
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 5e776fb4b0..b1b487256a 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -199,13 +199,6 @@ Use arguments ``base-node`` and ``top-node`` instead.
 
 Specify the properties for the object as top-level arguments instead.
 
-``query-named-block-nodes`` and ``query-block`` result dirty-bitmaps[i].status 
(since 4.0)
-''
-
-The ``status`` field of the ``BlockDirtyInfo`` structure, returned by
-these commands is deprecated. Two new boolean fields, ``recording`` and
-``busy`` effectively replace it.
-
 ``query-block`` result field ``dirty-bitmaps`` (Since 4.2)
 ''
 
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 06d6540ad2..67526f1dd9 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -113,6 +113,13 @@ Removed with no replacement.
 
 Removed with no replacement.
 
+``query-named-block-nodes`` and ``query-block`` result dirty-bitmaps[i].status 
(removed in 6.0)
+'''
+
+The ``status`` field of the ``BlockDirtyInfo`` structure, returned by
+these commands is removed. Two new boolean fields, ``recording`` and
+``busy`` effectively replace it.
+
 Human Monitor Protocol (HMP) commands
 -
 
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index f581cf9fd7..40950ae3d5 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty

[PATCH v2 10/13] block: remove 'encryption_key_missing' flag from QAPI

[Daniel P . Berrangé]

This has been hardcoded to "false" since 2.10.0, since secrets required
to unlock block devices are now always provided upfront instead of using
interactive prompts.

Reviewed-by: Thomas Huth 
Signed-off-by: Daniel P. Berrangé 
---
 block/qapi.c |  1 -
 docs/system/deprecated.rst   | 10 ---
 docs/system/removed-features.rst | 10 +++
 qapi/block-core.json |  8 --
 tests/qemu-iotests/184.out   |  6 ++--
 tests/qemu-iotests/191.out   | 48 +++-
 tests/qemu-iotests/273.out   | 15 --
 7 files changed, 33 insertions(+), 65 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index 84a0aadc09..3acc118c44 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -62,7 +62,6 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
 info->ro = bs->read_only;
 info->drv= g_strdup(bs->drv->format_name);
 info->encrypted  = bs->encrypted;
-info->encryption_key_missing = false;
 
 info->cache = g_new(BlockdevCacheInfo, 1);
 *info->cache = (BlockdevCacheInfo) {
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 9c8c62da44..5e776fb4b0 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -184,16 +184,6 @@ Use argument ``id`` instead.
 
 Use argument ``id`` instead.
 
-``query-named-block-nodes`` result ``encryption_key_missing`` (since 2.10.0)
-
-
-Always false.
-
-``query-block`` result ``inserted.encryption_key_missing`` (since 2.10.0)
-'
-
-Always false.
-
 ``blockdev-add`` empty string argument ``backing`` (since 2.10.0)
 '
 
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 8ca51e1f7c..06d6540ad2 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -103,6 +103,16 @@ chardev client socket with ``wait`` option (removed in 6.0)
 Character devices creating sockets in client mode should not specify
 the 'wait' field, which is only applicable to sockets in server mode
 
+``query-named-block-nodes`` result ``encryption_key_missing`` (removed in 6.0)
+''
+
+Removed with no replacement.
+
+``query-block`` result ``inserted.encryption_key_missing`` (removed in 6.0)
+'''
+
+Removed with no replacement.
+
 Human Monitor Protocol (HMP) commands
 -
 
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9f555d5c1d..d256b7b776 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -319,8 +319,6 @@
 #
 # @encrypted: true if the backing device is encrypted
 #
-# @encryption_key_missing: always false
-#
 # @detect_zeroes: detect and optimize zero writes (Since 2.1)
 #
 # @bps: total throughput limit in bytes per second is specified
@@ -385,10 +383,6 @@
 # @dirty-bitmaps: dirty bitmaps information (only present if node
 # has one or more dirty bitmaps) (Since 4.2)
 #
-# Features:
-# @deprecated: Member @encryption_key_missing is deprecated.  It is
-#  always false.
-#
 # Since: 0.14
 #
 ##
@@ -396,8 +390,6 @@
   'data': { 'file': 'str', '*node-name': 'str', 'ro': 'bool', 'drv': 'str',
 '*backing_file': 'str', 'backing_file_depth': 'int',
 'encrypted': 'bool',
-'encryption_key_missing': { 'type': 'bool',
-'features': [ 'deprecated' ] },
 'detect_zeroes': 'BlockdevDetectZeroesOptions',
 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int',
diff --git a/tests/qemu-iotests/184.out b/tests/qemu-iotests/184.out
index 87c73070e3..77e5489d65 100644
--- a/tests/qemu-iotests/184.out
+++ b/tests/qemu-iotests/184.out
@@ -54,8 +54,7 @@ Testing:
 "direct": false,
 "writeback": true
 },
-"file": "json:{\"throttle-group\": \"group0\", \"driver\": 
\"throttle\", \"file\": {\"driver\": \"null-co\"}}",
-"encryption_key_missing": false
+"file": "json:{\"throttle-group\": \"group0\", \"driver\": 
\"throttle\", \"file\": {\"driver\": \"null-co\"}}"
 },
 {
 "iops_rd": 0,
@@ -82,8 +81,7 @@ Testing:
 "direct": false,
 "writeback": true
 },
-"file": "null-co://",
-"encryption_key_missing": false
+"file": "null-co://"
 }
 ]
 }
diff --git a/tests/qemu-iotests/191.out b/tests/qemu-iotests/191.out
index 022021efab..ea88777374 100644
--- a/tests/qemu-iotests/191.out
+++ b/tests/qemu-iotests/191.out
@@

[PATCH v2 09/13] hw/scsi: remove 'scsi-disk' device

[Daniel P . Berrangé]

The 'scsi-hd' and 'scsi-cd' devices provide suitable alternatives.

Reviewed-by: Thomas Huth 
Signed-off-by: Daniel P. Berrangé 
---
 docs/system/deprecated.rst   |  9 -
 docs/system/removed-features.rst |  6 
 hw/i386/pc.c |  1 -
 hw/scsi/scsi-disk.c  | 62 
 scripts/device-crash-test|  1 -
 tests/qemu-iotests/051   |  2 --
 tests/qemu-iotests/051.pc.out| 10 --
 7 files changed, 6 insertions(+), 85 deletions(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 93f0d01ee3..9c8c62da44 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -276,15 +276,6 @@ The ``I7200`` guest CPU relies on the nanoMIPS ISA, which 
is deprecated
 (the ISA has never been upstreamed to a compiler toolchain). Therefore
 this CPU is also deprecated.
 
-System emulator devices

-
-``scsi-disk`` (since 4.2)
-'
-
-The 'scsi-disk' device is deprecated. Users should use 'scsi-hd' or
-'scsi-cd' as appropriate to get a SCSI hard disk or CD-ROM as needed.
-
 System emulator machines
 
 
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 5707b463cd..8ca51e1f7c 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -226,6 +226,12 @@ System emulator devices
 The 'ide-drive' device has been removed. Users should use 'ide-hd' or
 'ide-cd' as appropriate to get an IDE hard disk or CD-ROM as needed.
 
+``scsi-disk`` (removed in 6.0)
+''
+
+The 'scsi-disk' device has been removed. Users should use 'scsi-hd' or
+'scsi-cd' as appropriate to get a SCSI hard disk or CD-ROM as needed.
+
 Related binaries
 
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 804913bb7e..35e1770950 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -338,7 +338,6 @@ GlobalProperty pc_compat_1_4[] = {
 PC_CPU_MODEL_IDS("1.4.0")
 { "scsi-hd", "discard_granularity", "0" },
 { "scsi-cd", "discard_granularity", "0" },
-{ "scsi-disk", "discard_granularity", "0" },
 { "ide-hd", "discard_granularity", "0" },
 { "ide-cd", "discard_granularity", "0" },
 { "virtio-blk-pci", "discard_granularity", "0" },
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 2eaea7e637..3580e7ee61 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2476,28 +2476,6 @@ static void scsi_cd_realize(SCSIDevice *dev, Error 
**errp)
 aio_context_release(ctx);
 }
 
-static void scsi_disk_realize(SCSIDevice *dev, Error **errp)
-{
-DriveInfo *dinfo;
-Error *local_err = NULL;
-
-warn_report("'scsi-disk' is deprecated, "
-"please use 'scsi-hd' or 'scsi-cd' instead");
-
-if (!dev->conf.blk) {
-scsi_realize(dev, &local_err);
-assert(local_err);
-error_propagate(errp, local_err);
-return;
-}
-
-dinfo = blk_legacy_dinfo(dev->conf.blk);
-if (dinfo && dinfo->media_cd) {
-scsi_cd_realize(dev, errp);
-} else {
-scsi_hd_realize(dev, errp);
-}
-}
 
 static const SCSIReqOps scsi_disk_emulate_reqops = {
 .size = sizeof(SCSIDiskReq),
@@ -3161,45 +3139,6 @@ static const TypeInfo scsi_block_info = {
 };
 #endif
 
-static Property scsi_disk_properties[] = {
-DEFINE_SCSI_DISK_PROPERTIES(),
-DEFINE_PROP_BIT("removable", SCSIDiskState, features,
-SCSI_DISK_F_REMOVABLE, false),
-DEFINE_PROP_BIT("dpofua", SCSIDiskState, features,
-SCSI_DISK_F_DPOFUA, false),
-DEFINE_PROP_UINT64("wwn", SCSIDiskState, qdev.wwn, 0),
-DEFINE_PROP_UINT64("port_wwn", SCSIDiskState, qdev.port_wwn, 0),
-DEFINE_PROP_UINT16("port_index", SCSIDiskState, port_index, 0),
-DEFINE_PROP_UINT64("max_unmap_size", SCSIDiskState, max_unmap_size,
-   DEFAULT_MAX_UNMAP_SIZE),
-DEFINE_PROP_UINT64("max_io_size", SCSIDiskState, max_io_size,
-   DEFAULT_MAX_IO_SIZE),
-DEFINE_PROP_INT32("scsi_version", SCSIDiskState, qdev.default_scsi_version,
-  5),
-DEFINE_PROP_END_OF_LIST(),
-};
-
-static void scsi_disk_class_initfn(ObjectClass *klass, void *data)
-{
-DeviceClass *dc = DEVICE_CLASS(klass);
-SCSIDeviceClass *sc = SCSI_DEVICE_CLASS(klass);
-
-sc->realize  = scsi_disk_realize;
-sc->alloc_req= scsi_new_request;
-sc->unit_attention_reported = scsi_disk_unit_attention_reported;
-dc->fw_name = "disk";
-dc->desc = "virtual SCSI disk or CD-ROM (legacy)";
-dc->reset = scsi_disk_reset;
-device_class_set_props(dc, scsi_disk_properties);
-dc->vmsd  = &vmstate_scsi_disk_state;
-}
-
-static const TypeInfo scsi_disk_info = {
-.name  = "scsi-disk",
-.parent= TYPE_SCSI_DISK_BASE,
-.class_init= scsi_disk_class_initfn,
-};
-
 static void scsi_disk_register_types(void)
 {
 type_register_static(

[PATCH v2 08/13] hw/ide: remove 'ide-drive' device

[Daniel P . Berrangé]

The 'ide-hd' and 'ide-cd' devices provide suitable alternatives.

Reviewed-by: Thomas Huth 
Signed-off-by: Daniel P. Berrangé 
---
 docs/qdev-device-use.txt |  2 +-
 docs/system/deprecated.rst   |  6 -
 docs/system/removed-features.rst |  8 +++
 hw/i386/pc.c |  1 -
 hw/ide/qdev.c| 38 
 hw/ppc/mac_newworld.c| 13 ---
 hw/ppc/mac_oldworld.c| 13 ---
 hw/sparc64/sun4u.c   | 15 -
 scripts/device-crash-test|  1 -
 softmmu/vl.c |  1 -
 tests/qemu-iotests/051   |  2 --
 tests/qemu-iotests/051.pc.out| 10 -
 12 files changed, 9 insertions(+), 101 deletions(-)

diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
index 245cdf29c7..2408889334 100644
--- a/docs/qdev-device-use.txt
+++ b/docs/qdev-device-use.txt
@@ -388,7 +388,7 @@ type.
 some DEVNAMEs:
 
 default device  suppressing DEVNAMEs
-CD-ROM  ide-cd, ide-drive, ide-hd, scsi-cd, scsi-hd
+CD-ROM  ide-cd, ide-hd, scsi-cd, scsi-hd
 floppy  floppy, isa-fdc
 parallelisa-parallel
 serial  isa-serial
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 9afff39e5e..93f0d01ee3 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -279,12 +279,6 @@ this CPU is also deprecated.
 System emulator devices
 ---
 
-``ide-drive`` (since 4.2)
-'
-
-The 'ide-drive' device is deprecated. Users should use 'ide-hd' or
-'ide-cd' as appropriate to get an IDE hard disk or CD-ROM as needed.
-
 ``scsi-disk`` (since 4.2)
 '
 
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 9f3697afec..5707b463cd 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -217,6 +217,14 @@ the upstream Linux kernel in 2018, and it has also been 
dropped from glibc, so
 there is no new Linux development taking place with this architecture. For
 running the old binaries, you can use older versions of QEMU.
 
+System emulator devices
+---
+
+``ide-drive`` (removed in 6.0)
+''
+
+The 'ide-drive' device has been removed. Users should use 'ide-hd' or
+'ide-cd' as appropriate to get an IDE hard disk or CD-ROM as needed.
 
 Related binaries
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 410db9ef96..804913bb7e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -341,7 +341,6 @@ GlobalProperty pc_compat_1_4[] = {
 { "scsi-disk", "discard_granularity", "0" },
 { "ide-hd", "discard_granularity", "0" },
 { "ide-cd", "discard_granularity", "0" },
-{ "ide-drive", "discard_granularity", "0" },
 { "virtio-blk-pci", "discard_granularity", "0" },
 /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string: */
 { "virtio-serial-pci", "vectors", "0x" },
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 8cd19fa5e9..e70ebc83a0 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -283,20 +283,6 @@ static void ide_cd_realize(IDEDevice *dev, Error **errp)
 ide_dev_initfn(dev, IDE_CD, errp);
 }
 
-static void ide_drive_realize(IDEDevice *dev, Error **errp)
-{
-DriveInfo *dinfo = NULL;
-
-warn_report("'ide-drive' is deprecated, "
-"please use 'ide-hd' or 'ide-cd' instead");
-
-if (dev->conf.blk) {
-dinfo = blk_legacy_dinfo(dev->conf.blk);
-}
-
-ide_dev_initfn(dev, dinfo && dinfo->media_cd ? IDE_CD : IDE_HD, errp);
-}
-
 #define DEFINE_IDE_DEV_PROPERTIES() \
 DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf),\
 DEFINE_BLOCK_ERROR_PROPERTIES(IDEDrive, dev.conf),  \
@@ -355,29 +341,6 @@ static const TypeInfo ide_cd_info = {
 .class_init= ide_cd_class_init,
 };
 
-static Property ide_drive_properties[] = {
-DEFINE_IDE_DEV_PROPERTIES(),
-DEFINE_PROP_END_OF_LIST(),
-};
-
-static void ide_drive_class_init(ObjectClass *klass, void *data)
-{
-DeviceClass *dc = DEVICE_CLASS(klass);
-IDEDeviceClass *k = IDE_DEVICE_CLASS(klass);
-
-k->realize  = ide_drive_realize;
-dc->fw_name = "drive";
-dc->desc= "virtual IDE disk or CD-ROM (legacy)";
-device_class_set_props(dc, ide_drive_properties);
-}
-
-static const TypeInfo ide_drive_info = {
-.name  = "ide-drive",
-.parent= TYPE_IDE_DEVICE,
-.instance_size = sizeof(IDEDrive),
-.class_init= ide_drive_class_init,
-};
-
 static void ide_device_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *k = DEVICE_CLASS(klass);
@@ -402,7 +365,6 @@ static void ide_register_types(void)
 type_register_static(&ide_bus_info);
 type_register_static(&ide_hd_info);
 type_register_static(&ide_cd_info);
-type_register_static(&ide_drive_info);
 type_register_static(&ide_device_type_info

[PATCH v2 07/13] chardev: reject use of 'wait' flag for socket client chardevs

[Daniel P . Berrangé]

This only makes sense conceptually when used with listener chardevs.

Reviewed-by: Marc-André Lureau 
Signed-off-by: Daniel P. Berrangé 
---
 chardev/char-socket.c| 12 
 docs/system/deprecated.rst   |  6 --
 docs/system/removed-features.rst |  6 ++
 3 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index c8bced76b7..f618bdec28 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1339,14 +1339,10 @@ static bool qmp_chardev_validate_socket(ChardevSocket 
*sock,
 return false;
 }
 if (sock->has_wait) {
-warn_report("'wait' option is deprecated with "
-"socket in client connect mode");
-if (sock->wait) {
-error_setg(errp, "%s",
-   "'wait' option is incompatible with "
-   "socket in client connect mode");
-return false;
-}
+error_setg(errp, "%s",
+   "'wait' option is incompatible with "
+   "socket in client connect mode");
+return false;
 }
 }
 
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index f196bbb9ec..9afff39e5e 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -229,12 +229,6 @@ Since the ``dirty-bitmaps`` field is optionally present in 
both the old and
 new locations, clients must use introspection to learn where to anticipate
 the field if/when it does appear in command output.
 
-chardev client socket with ``wait`` option (since 4.0)
-''
-
-Character devices creating sockets in client mode should not specify
-the 'wait' field, which is only applicable to sockets in server mode
-
 ``nbd-server-add`` and ``nbd-server-remove`` (since 5.2)
 
 
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 51f0e308d9..9f3697afec 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -97,6 +97,12 @@ The ``query-cpus`` command is replaced by the 
``query-cpus-fast`` command.
 The ``arch`` output member of the ``query-cpus-fast`` command is
 replaced by the ``target`` output member.
 
+chardev client socket with ``wait`` option (removed in 6.0)
+'''
+
+Character devices creating sockets in client mode should not specify
+the 'wait' field, which is only applicable to sockets in server mode
+
 Human Monitor Protocol (HMP) commands
 -
 
-- 
2.30.2

[PATCH v2 04/13] migrate: remove QMP/HMP commands for speed, downtime and cache size

[Daniel P . Berrangé]

The generic 'migrate_set_parameters' command handle all types of param.

Only the QMP commands were documented in the deprecations page, but the
rationale for deprecating applies equally to HMP, and the replacements
exist. Furthermore the HMP commands are just shims to the QMP commands,
so removing the latter breaks the former unless they get re-implemented.

Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Daniel P. Berrangé 
---
 docs/devel/migration.rst|  2 +-
 docs/rdma.txt   |  2 +-
 docs/system/deprecated.rst  | 10 ---
 docs/system/removed-features.rst| 20 ++
 docs/xbzrle.txt |  5 --
 hmp-commands-info.hx| 13 
 hmp-commands.hx | 45 -
 include/monitor/hmp.h   |  4 --
 migration/migration.c   | 45 -
 migration/ram.c |  2 +-
 monitor/hmp-cmds.c  | 34 --
 qapi/migration.json | 98 -
 tests/migration/guestperf/engine.py | 16 ++---
 tests/qemu-iotests/181  |  2 +-
 tests/qtest/migration-test.c| 48 --
 tests/qtest/test-hmp.c  |  6 +-
 tests/qtest/vhost-user-test.c   |  8 +--
 17 files changed, 40 insertions(+), 320 deletions(-)

diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst
index ad381b89b2..19c3d4f3ea 100644
--- a/docs/devel/migration.rst
+++ b/docs/devel/migration.rst
@@ -641,7 +641,7 @@ time per vCPU.
 
 .. note::
   During the postcopy phase, the bandwidth limits set using
-  ``migrate_set_speed`` is ignored (to avoid delaying requested pages that
+  ``migrate_set_parameter`` is ignored (to avoid delaying requested pages that
   the destination is waiting for).
 
 Postcopy device transfer
diff --git a/docs/rdma.txt b/docs/rdma.txt
index 49dc9f8bca..2b4cdea1d8 100644
--- a/docs/rdma.txt
+++ b/docs/rdma.txt
@@ -89,7 +89,7 @@ RUNNING:
 First, set the migration speed to match your hardware's capabilities:
 
 QEMU Monitor Command:
-$ migrate_set_speed 40g # or whatever is the MAX of your RDMA device
+$ migrate_set_parameter max_bandwidth 40g # or whatever is the MAX of your 
RDMA device
 
 Next, on the destination machine, add the following to the QEMU command line:
 
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 9fb347724c..e68f80d0c8 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -184,11 +184,6 @@ Use argument ``id`` instead.
 
 Use argument ``id`` instead.
 
-``migrate_set_downtime`` and ``migrate_set_speed`` (since 2.8.0)
-
-
-Use ``migrate-set-parameters`` instead.
-
 ``query-named-block-nodes`` result ``encryption_key_missing`` (since 2.10.0)
 
 
@@ -204,11 +199,6 @@ Always false.
 
 Use argument value ``null`` instead.
 
-``migrate-set-cache-size`` and ``query-migrate-cache-size`` (since 2.11.0)
-''
-
-Use ``migrate-set-parameters`` and ``query-migrate-parameters`` instead.
-
 ``block-commit`` arguments ``base`` and ``top`` (since 3.1.0)
 '
 
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 408cc340d2..6e653bfc4e 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -76,6 +76,16 @@ Use ``blockdev-change-medium`` or ``change-vnc-password`` 
instead.
 The ``query-events`` command has been superseded by the more powerful
 and accurate ``query-qmp-schema`` command.
 
+``migrate_set_cache_size`` and ``query-migrate-cache-size`` (removed in 6.0)
+
+
+Use ``migrate_set_parameter`` and ``info migrate_parameters`` instead.
+
+``migrate_set_downtime`` and ``migrate_set_speed`` (removed in 6.0)
+'''
+
+Use ``migrate_set_parameter`` instead.
+
 Human Monitor Protocol (HMP) commands
 -
 
@@ -104,6 +114,16 @@ The ``acl_show``, ``acl_reset``, ``acl_policy``, 
``acl_add``, and
 ``acl_remove`` commands were removed with no replacement. Authorization
 for VNC should be performed using the pluggable QAuthZ objects.
 
+``migrate-set-cache-size`` and ``info migrate-cache-size`` (removed in 6.0)
+'''
+
+Use ``migrate-set-parameters`` and ``info migrate-parameters`` instead.
+
+``migrate_set_downtime`` and ``migrate_set_speed`` (removed in 6.0)
+'''
+
+Use ``migrate-set-parameters`` instead.
+
 Guest Emulator ISAs
 ---
 
diff --git a/docs/xbzrle.txt b/docs/xbzrle.txt
index 6bd1828f34..bcb3f0c901 10064

[PATCH v2 06/13] machine: remove 'arch' field from 'query-cpus-fast' QMP command

[Daniel P . Berrangé]

Reviewed-by: Thomas Huth 
Signed-off-by: Daniel P. Berrangé 
---
 docs/system/deprecated.rst   |  6 -
 docs/system/removed-features.rst |  6 +
 hw/core/machine-qmp-cmds.c   | 41 
 qapi/machine.json| 22 -
 4 files changed, 6 insertions(+), 69 deletions(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index fd377353f6..f196bbb9ec 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -229,12 +229,6 @@ Since the ``dirty-bitmaps`` field is optionally present in 
both the old and
 new locations, clients must use introspection to learn where to anticipate
 the field if/when it does appear in command output.
 
-``query-cpus-fast`` ``arch`` output member (since 3.0.0)
-
-
-The ``arch`` output member of the ``query-cpus-fast`` command is
-replaced by the ``target`` output member.
-
 chardev client socket with ``wait`` option (since 4.0)
 ''
 
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 37946e2401..51f0e308d9 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -91,6 +91,12 @@ Use ``migrate_set_parameter`` instead.
 
 The ``query-cpus`` command is replaced by the ``query-cpus-fast`` command.
 
+``query-cpus-fast`` ``arch`` output member (removed in 6.0)
+'''
+
+The ``arch`` output member of the ``query-cpus-fast`` command is
+replaced by the ``target`` output member.
+
 Human Monitor Protocol (HMP) commands
 -
 
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index af60cd969d..68a942595a 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -24,46 +24,6 @@
 #include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
 
-static CpuInfoArch sysemu_target_to_cpuinfo_arch(SysEmuTarget target)
-{
-/*
- * The @SysEmuTarget -> @CpuInfoArch mapping below is based on the
- * TARGET_ARCH -> TARGET_BASE_ARCH mapping in the "configure" script.
- */
-switch (target) {
-case SYS_EMU_TARGET_I386:
-case SYS_EMU_TARGET_X86_64:
-return CPU_INFO_ARCH_X86;
-
-case SYS_EMU_TARGET_PPC:
-case SYS_EMU_TARGET_PPC64:
-return CPU_INFO_ARCH_PPC;
-
-case SYS_EMU_TARGET_SPARC:
-case SYS_EMU_TARGET_SPARC64:
-return CPU_INFO_ARCH_SPARC;
-
-case SYS_EMU_TARGET_MIPS:
-case SYS_EMU_TARGET_MIPSEL:
-case SYS_EMU_TARGET_MIPS64:
-case SYS_EMU_TARGET_MIPS64EL:
-return CPU_INFO_ARCH_MIPS;
-
-case SYS_EMU_TARGET_TRICORE:
-return CPU_INFO_ARCH_TRICORE;
-
-case SYS_EMU_TARGET_S390X:
-return CPU_INFO_ARCH_S390;
-
-case SYS_EMU_TARGET_RISCV32:
-case SYS_EMU_TARGET_RISCV64:
-return CPU_INFO_ARCH_RISCV;
-
-default:
-return CPU_INFO_ARCH_OTHER;
-}
-}
-
 static void cpustate_to_cpuinfo_s390(CpuInfoS390 *info, const CPUState *cpu)
 {
 #ifdef TARGET_S390X
@@ -104,7 +64,6 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
 value->props = props;
 }
 
-value->arch = sysemu_target_to_cpuinfo_arch(target);
 value->target = target;
 if (target == SYS_EMU_TARGET_S390X) {
 cpustate_to_cpuinfo_s390(&value->u.s390x, cpu);
diff --git a/qapi/machine.json b/qapi/machine.json
index 9811927504..c0c52aef10 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -34,21 +34,6 @@
  'sh4eb', 'sparc', 'sparc64', 'tricore', 'unicore32',
  'x86_64', 'xtensa', 'xtensaeb' ] }
 
-##
-# @CpuInfoArch:
-#
-# An enumeration of cpu types that enable additional information during
-# @query-cpus-fast.
-#
-# @s390: since 2.12
-#
-# @riscv: since 2.12
-#
-# Since: 2.6
-##
-{ 'enum': 'CpuInfoArch',
-  'data': ['x86', 'sparc', 'ppc', 'mips', 'tricore', 's390', 'riscv', 'other' 
] }
-
 ##
 # @CpuS390State:
 #
@@ -86,14 +71,9 @@
 # @props: properties describing to which node/socket/core/thread
 # virtual CPU belongs to, provided if supported by board
 #
-# @arch: base architecture of the cpu
-#
 # @target: the QEMU system emulation target, which determines which
 #  additional fields will be listed (since 3.0)
 #
-# Features:
-# @deprecated: Member @arch is deprecated.  Use @target instead.
-#
 # Since: 2.12
 #
 ##
@@ -102,8 +82,6 @@
   'qom-path' : 'str',
   'thread-id': 'int',
   '*props'   : 'CpuInstanceProperties',
-  'arch' : { 'type': 'CpuInfoArch',
- 'features': [ 'deprecated' ] },
   'target'   : 'SysEmuTarget' },
   'discriminator' : 'target',
   'data'  : { 's390x': 'CpuInfoS390' } }
-- 
2.30.2

[PATCH v2 05/13] machine: remove 'query-cpus' QMP command

[Daniel P . Berrangé]

The newer 'query-cpus-fast' command avoids side effects on the guest
execution. Note that some of the field names are different in the
'query-cpus-fast' command.

Reviewed-by: Wainer dos Santos Moschetta 
Tested-by: Wainer dos Santos Moschetta 
Signed-off-by: Daniel P. Berrangé 
---
 docs/system/deprecated.rst |   5 -
 docs/system/removed-features.rst   |   5 +
 hw/core/machine-hmp-cmds.c |   8 +-
 hw/core/machine-qmp-cmds.c |  79 --
 qapi/machine.json  | 161 +
 tests/acceptance/pc_cpu_hotplug_props.py   |   2 +-
 tests/acceptance/x86_cpu_model_versions.py |   2 +-
 tests/migration/guestperf/engine.py|   2 +-
 tests/qtest/numa-test.c|   6 +-
 tests/qtest/qmp-test.c |   6 +-
 tests/qtest/test-x86-cpuid-compat.c|   4 +-
 11 files changed, 22 insertions(+), 258 deletions(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index e68f80d0c8..fd377353f6 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -229,11 +229,6 @@ Since the ``dirty-bitmaps`` field is optionally present in 
both the old and
 new locations, clients must use introspection to learn where to anticipate
 the field if/when it does appear in command output.
 
-``query-cpus`` (since 2.12.0)
-'
-
-The ``query-cpus`` command is replaced by the ``query-cpus-fast`` command.
-
 ``query-cpus-fast`` ``arch`` output member (since 3.0.0)
 
 
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 6e653bfc4e..37946e2401 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -86,6 +86,11 @@ Use ``migrate_set_parameter`` and ``info 
migrate_parameters`` instead.
 
 Use ``migrate_set_parameter`` instead.
 
+``query-cpus`` (removed in 6.0)
+'''
+
+The ``query-cpus`` command is replaced by the ``query-cpus-fast`` command.
+
 Human Monitor Protocol (HMP) commands
 -
 
diff --git a/hw/core/machine-hmp-cmds.c b/hw/core/machine-hmp-cmds.c
index 6357be9c6b..58248cffa3 100644
--- a/hw/core/machine-hmp-cmds.c
+++ b/hw/core/machine-hmp-cmds.c
@@ -130,7 +130,7 @@ void hmp_info_numa(Monitor *mon, const QDict *qdict)
 {
 int i, nb_numa_nodes;
 NumaNodeMem *node_mem;
-CpuInfoList *cpu_list, *cpu;
+CpuInfoFastList *cpu_list, *cpu;
 MachineState *ms = MACHINE(qdev_get_machine());
 
 nb_numa_nodes = ms->numa_state ? ms->numa_state->num_nodes : 0;
@@ -139,7 +139,7 @@ void hmp_info_numa(Monitor *mon, const QDict *qdict)
 return;
 }
 
-cpu_list = qmp_query_cpus(&error_abort);
+cpu_list = qmp_query_cpus_fast(&error_abort);
 node_mem = g_new0(NumaNodeMem, nb_numa_nodes);
 
 query_numa_node_mem(node_mem, ms);
@@ -148,7 +148,7 @@ void hmp_info_numa(Monitor *mon, const QDict *qdict)
 for (cpu = cpu_list; cpu; cpu = cpu->next) {
 if (cpu->value->has_props && cpu->value->props->has_node_id &&
 cpu->value->props->node_id == i) {
-monitor_printf(mon, " %" PRIi64, cpu->value->CPU);
+monitor_printf(mon, " %" PRIi64, cpu->value->cpu_index);
 }
 }
 monitor_printf(mon, "\n");
@@ -157,6 +157,6 @@ void hmp_info_numa(Monitor *mon, const QDict *qdict)
 monitor_printf(mon, "node %d plugged: %" PRId64 " MB\n", i,
node_mem[i].node_plugged_mem >> 20);
 }
-qapi_free_CpuInfoList(cpu_list);
+qapi_free_CpuInfoFastList(cpu_list);
 g_free(node_mem);
 }
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 44e979e503..af60cd969d 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -24,85 +24,6 @@
 #include "sysemu/runstate.h"
 #include "sysemu/sysemu.h"
 
-CpuInfoList *qmp_query_cpus(Error **errp)
-{
-MachineState *ms = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(ms);
-CpuInfoList *head = NULL, **tail = &head;
-CPUState *cpu;
-
-CPU_FOREACH(cpu) {
-CpuInfo *value;
-#if defined(TARGET_I386)
-X86CPU *x86_cpu = X86_CPU(cpu);
-CPUX86State *env = &x86_cpu->env;
-#elif defined(TARGET_PPC)
-PowerPCCPU *ppc_cpu = POWERPC_CPU(cpu);
-CPUPPCState *env = &ppc_cpu->env;
-#elif defined(TARGET_SPARC)
-SPARCCPU *sparc_cpu = SPARC_CPU(cpu);
-CPUSPARCState *env = &sparc_cpu->env;
-#elif defined(TARGET_RISCV)
-RISCVCPU *riscv_cpu = RISCV_CPU(cpu);
-CPURISCVState *env = &riscv_cpu->env;
-#elif defined(TARGET_MIPS)
-MIPSCPU *mips_cpu = MIPS_CPU(cpu);
-CPUMIPSState *env = &mips_cpu->env;
-#elif defined(TARGET_TRICORE)
-TriCoreCPU *tricore_cpu = TRICORE_CPU(cpu);
-CPUTriCoreState *env = &tricore_cpu->env;
-#elif defined(TARGET_S390X)
-  

[PATCH v2 03/13] monitor: remove 'query-events' QMP command

[Daniel P . Berrangé]

The code comment suggests removing QAPIEvent_(str|lookup) symbols too,
however, these are both auto-generated as standard for any enum in
QAPI. As such it they'll exist whether we use them or not.

Reviewed-by: Thomas Huth 
Signed-off-by: Daniel P. Berrangé 
---
 docs/system/deprecated.rst   |  6 -
 docs/system/removed-features.rst |  6 +
 monitor/qmp-cmds-control.c   | 24 -
 qapi/control.json| 45 
 4 files changed, 6 insertions(+), 75 deletions(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 269fe2bcac..9fb347724c 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -250,12 +250,6 @@ The ``query-cpus`` command is replaced by the 
``query-cpus-fast`` command.
 The ``arch`` output member of the ``query-cpus-fast`` command is
 replaced by the ``target`` output member.
 
-``query-events`` (since 4.0)
-
-
-The ``query-events`` command has been superseded by the more powerful
-and accurate ``query-qmp-schema`` command.
-
 chardev client socket with ``wait`` option (since 4.0)
 ''
 
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 5fbec3b963..408cc340d2 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -70,6 +70,12 @@ documentation of ``query-hotpluggable-cpus`` for additional 
details.
 
 Use ``blockdev-change-medium`` or ``change-vnc-password`` instead.
 
+``query-events`` (removed in 6.0)
+'
+
+The ``query-events`` command has been superseded by the more powerful
+and accurate ``query-qmp-schema`` command.
+
 Human Monitor Protocol (HMP) commands
 -
 
diff --git a/monitor/qmp-cmds-control.c b/monitor/qmp-cmds-control.c
index 509ae870bd..513b547233 100644
--- a/monitor/qmp-cmds-control.c
+++ b/monitor/qmp-cmds-control.c
@@ -130,30 +130,6 @@ CommandInfoList *qmp_query_commands(Error **errp)
 return list;
 }
 
-EventInfoList *qmp_query_events(Error **errp)
-{
-/*
- * TODO This deprecated command is the only user of
- * QAPIEvent_str() and QAPIEvent_lookup[].  When the command goes,
- * they should go, too.
- */
-EventInfoList *ev_list = NULL;
-QAPIEvent e;
-
-for (e = 0 ; e < QAPI_EVENT__MAX ; e++) {
-const char *event_name = QAPIEvent_str(e);
-EventInfo *info;
-
-assert(event_name != NULL);
-info = g_malloc0(sizeof(*info));
-info->name = g_strdup(event_name);
-
-QAPI_LIST_PREPEND(ev_list, info);
-}
-
-return ev_list;
-}
-
 /*
  * Minor hack: generated marshalling suppressed for this command
  * ('gen': false in the schema) so we can parse the JSON string
diff --git a/qapi/control.json b/qapi/control.json
index 2615d5170b..71a838d49e 100644
--- a/qapi/control.json
+++ b/qapi/control.json
@@ -159,51 +159,6 @@
 { 'command': 'query-commands', 'returns': ['CommandInfo'],
   'allow-preconfig': true }
 
-##
-# @EventInfo:
-#
-# Information about a QMP event
-#
-# @name: The event name
-#
-# Since: 1.2
-##
-{ 'struct': 'EventInfo', 'data': {'name': 'str'} }
-
-##
-# @query-events:
-#
-# Return information on QMP events.
-#
-# Features:
-# @deprecated: This command is deprecated, because its output doesn't
-#  reflect compile-time configuration.  Use 'query-qmp-schema'
-#  instead.
-#
-# Returns: A list of @EventInfo.
-#
-# Since: 1.2
-#
-# Example:
-#
-# -> { "execute": "query-events" }
-# <- {
-#  "return": [
-#  {
-# "name":"SHUTDOWN"
-#  },
-#  {
-# "name":"RESET"
-#  }
-#   ]
-#}
-#
-# Note: This example has been shortened as the real response is too long.
-#
-##
-{ 'command': 'query-events', 'returns': ['EventInfo'],
-  'features': [ 'deprecated' ] }
-
 ##
 # @quit:
 #
-- 
2.30.2

[PATCH v2 02/13] monitor: raise error when 'pretty' option is used with HMP

[Daniel P . Berrangé]

This is only semantically useful for QMP.

Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Daniel P. Berrangé 
---
 docs/system/deprecated.rst   | 7 ---
 docs/system/removed-features.rst | 6 ++
 monitor/monitor.c| 4 ++--
 qemu-options.hx  | 5 +++--
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 32bda5f4ef..269fe2bcac 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -62,13 +62,6 @@ needs two devices (``-device intel-hda -device hda-duplex``) 
and
 ``pcspk`` which can be activated using ``-machine
 pcspk-audiodev=``.
 
-``-mon ...,control=readline,pretty=on|off`` (since 4.1)
-'''
-
-The ``pretty=on|off`` switch has no effect for HMP monitors, but is
-silently ignored. Using the switch with HMP monitors will become an
-error in the future.
-
 RISC-V ``-bios`` (since 5.1)
 
 
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 93384746a1..5fbec3b963 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -44,6 +44,12 @@ block cache, ``-accel tcg,tb-size=``.
 The ``acl`` option to the ``-vnc`` argument has been replaced
 by the ``tls-authz`` and ``sasl-authz`` options.
 
+``-mon ...,control=readline,pretty=on|off`` (removed in 6.0)
+
+
+The ``pretty=on|off`` switch has no effect for HMP monitors and
+its use is rejected.
+
 QEMU Machine Protocol (QMP) commands
 
 
diff --git a/monitor/monitor.c b/monitor/monitor.c
index e94f532cf5..515efb015e 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -720,8 +720,8 @@ int monitor_init(MonitorOptions *opts, bool allow_hmp, 
Error **errp)
 return -1;
 }
 if (opts->pretty) {
-warn_report("'pretty' is deprecated for HMP monitors, it has no "
-"effect and will be removed in future versions");
+error_setg(errp, "'pretty' is not compatible with HMP monitors");
+return -1;
 }
 monitor_init_hmp(chr, true, &local_err);
 break;
diff --git a/qemu-options.hx b/qemu-options.hx
index 622d3bfa5a..17dc791d5d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3703,8 +3703,9 @@ DEF("mon", HAS_ARG, QEMU_OPTION_mon, \
 "-mon [chardev=]name[,mode=readline|control][,pretty[=on|off]]\n", 
QEMU_ARCH_ALL)
 SRST
 ``-mon [chardev=]name[,mode=readline|control][,pretty[=on|off]]``
-Setup monitor on chardev name. ``pretty`` turns on JSON pretty
-printing easing human reading and debugging.
+Setup monitor on chardev name. ``pretty`` is only valid when
+``mode=control``, turning on JSON pretty printing to ease
+human reading and debugging.
 ERST
 
 DEF("debugcon", HAS_ARG, QEMU_OPTION_debugcon, \
-- 
2.30.2

[PATCH v2 01/13] ui, monitor: remove deprecated VNC ACL option and HMP commands

[Daniel P . Berrangé]

The VNC ACL concept has been replaced by the pluggable "authz" framework
which does not use monitor commands.

Reviewed-by: Thomas Huth 
Reviewed-by: Dr. David Alan Gilbert 
Signed-off-by: Daniel P. Berrangé 
---
 docs/system/deprecated.rst   |  16 ---
 docs/system/removed-features.rst |  13 +++
 hmp-commands.hx  |  76 -
 monitor/misc.c   | 187 ---
 ui/vnc.c |  38 ---
 5 files changed, 13 insertions(+), 317 deletions(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 5e3a31c123..32bda5f4ef 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -37,12 +37,6 @@ The 'file' driver for drives is no longer appropriate for 
character or host
 devices and will only accept regular files (S_IFREG). The correct driver
 for these file types is 'host_cdrom' or 'host_device' as appropriate.
 
-``-vnc acl`` (since 4.0.0)
-''
-
-The ``acl`` option to the ``-vnc`` argument has been replaced
-by the ``tls-authz`` and ``sasl-authz`` options.
-
 ``QEMU_AUDIO_`` environment variables and ``-audio-help`` (since 4.0)
 '
 
@@ -282,16 +276,6 @@ Use the more generic commands ``block-export-add`` and 
``block-export-del``
 instead.  As part of this deprecation, where ``nbd-server-add`` used a
 single ``bitmap``, the new ``block-export-add`` uses a list of ``bitmaps``.
 
-Human Monitor Protocol (HMP) commands
--
-
-``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, ``acl_remove`` 
(since 4.0.0)
-''
-
-The ``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, and
-``acl_remove`` commands are deprecated with no replacement. Authorization
-for VNC should be performed using the pluggable QAuthZ objects.
-
 System emulator CPUS
 
 
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 83148dcfda..93384746a1 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -38,6 +38,12 @@ or ``-display default,show-cursor=on`` instead.
 QEMU 5.0 introduced an alternative syntax to specify the size of the 
translation
 block cache, ``-accel tcg,tb-size=``.
 
+``-vnc acl`` (removed in 6.0)
+'
+
+The ``acl`` option to the ``-vnc`` argument has been replaced
+by the ``tls-authz`` and ``sasl-authz`` options.
+
 QEMU Machine Protocol (QMP) commands
 
 
@@ -79,6 +85,13 @@ documentation of ``query-hotpluggable-cpus`` for additional 
details.
 No replacement.  The ``change vnc password`` and ``change DEVICE MEDIUM``
 commands are not affected.
 
+``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, ``acl_remove`` 
(removed in 6.0)
+'
+
+The ``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, and
+``acl_remove`` commands were removed with no replacement. Authorization
+for VNC should be performed using the pluggable QAuthZ objects.
+
 Guest Emulator ISAs
 ---
 
diff --git a/hmp-commands.hx b/hmp-commands.hx
index d4001f9c5d..b500b8526d 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1433,82 +1433,6 @@ SRST
   Change watchdog action.
 ERST
 
-{
-.name   = "acl_show",
-.args_type  = "aclname:s",
-.params = "aclname",
-.help   = "list rules in the access control list",
-.cmd= hmp_acl_show,
-},
-
-SRST
-``acl_show`` *aclname*
-  List all the matching rules in the access control list, and the default
-  policy. There are currently two named access control lists,
-  *vnc.x509dname* and *vnc.username* matching on the x509 client
-  certificate distinguished name, and SASL username respectively.
-ERST
-
-{
-.name   = "acl_policy",
-.args_type  = "aclname:s,policy:s",
-.params = "aclname allow|deny",
-.help   = "set default access control list policy",
-.cmd= hmp_acl_policy,
-},
-
-SRST
-``acl_policy`` *aclname* ``allow|deny``
-  Set the default access control list policy, used in the event that
-  none of the explicit rules match. The default policy at startup is
-  always ``deny``.
-ERST
-
-{
-.name   = "acl_add",
-.args_type  = "aclname:s,match:s,policy:s,index:i?",
-.params = "aclname match allow|deny [index]",
-.help   = "add a match rule to the access control list",
-.cmd= hmp_acl_add,
-},
-
-SRST
-``acl_add`` *aclname* *match* ``allow|deny`` [*index*]
-  Add a match rule to the access control list, allowing or denying access.
-  The match will normally be an exact username or x509 distinguished name,
-  but can optionally incl

[PATCH v2 00/13] deprecations: remove many old deprecations

[Daniel P . Berrangé]

This is an update  to

   https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg07558.html

The following features have been deprecated for well over the 2
release cycle we promise

  ``-drive file=3Djson:{...{'driver':'file'}}`` (since 3.0)
  ``-vnc acl`` (since 4.0.0)
  ``-mon ...,control=3Dreadline,pretty=3Don|off`` (since 4.1)
  ``migrate_set_downtime`` and ``migrate_set_speed`` (since 2.8.0)
  ``query-named-block-nodes`` result ``encryption_key_missing`` (since 2.10.0)
  ``query-block`` result ``inserted.encryption_key_missing`` (since 2.10.0)
  ``migrate-set-cache-size`` and ``query-migrate-cache-size`` (since 2.11.0)
  ``query-named-block-nodes`` and ``query-block`` result dirty-bitmaps[i].sta=
tus (since 4.0)
  ``query-cpus`` (since 2.12.0)
  ``query-cpus-fast`` ``arch`` output member (since 3.0.0)
  ``query-events`` (since 4.0)
  chardev client socket with ``wait`` option (since 4.0)
  ``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, ``acl_remove`` (s=
ince 4.0.0)
  ``ide-drive`` (since 4.2)
  ``scsi-disk`` (since 4.2)

AFAICT, libvirt has ceased to use all of these too.

These patches all have reviews now, and I'll send a pull request tomorrow
for soft freeze unless I hear from subsystem maintainers with any
negative feedback.

In v2:

 - Dropped -usbdevice removal, as that's spun off into separate debate
 - Fixed misplaced hunk in scsi-disk patch
 - Fixed non-default iotest jobs in dirty-bitmap code

Daniel P. Berrang=C3=A9 (13):
  ui, monitor: remove deprecated VNC ACL option and HMP commands
  monitor: raise error when 'pretty' option is used with HMP
  monitor: remove 'query-events' QMP command
  migrate: remove QMP/HMP commands for speed, downtime and cache size
  machine: remove 'query-cpus' QMP command
  machine: remove 'arch' field from 'query-cpus-fast' QMP command
  chardev: reject use of 'wait' flag for socket client chardevs
  hw/ide: remove 'ide-drive' device
  hw/scsi: remove 'scsi-disk' device
  block: remove 'encryption_key_missing' flag from QAPI
  block: remove dirty bitmaps 'status' field
  block: remove 'dirty-bitmaps' field from 'BlockInfo' struct
  block: remove support for using "file" driver with block/char devices

 block/dirty-bitmap.c  |  38 --
 block/file-posix.c|  17 +-
 block/qapi.c  |   6 -
 chardev/char-socket.c |  12 +-
 docs/devel/migration.rst  |   2 +-
 docs/qdev-device-use.txt  |   2 +-
 docs/rdma.txt |   2 +-
 docs/system/deprecated.rst| 108 -
 docs/system/removed-features.rst  | 109 +
 docs/xbzrle.txt   |   5 -
 hmp-commands-info.hx  |  13 -
 hmp-commands.hx   | 121 --
 hw/core/machine-hmp-cmds.c|   8 +-
 hw/core/machine-qmp-cmds.c| 120 --
 hw/i386/pc.c  |   2 -
 hw/ide/qdev.c |  38 --
 hw/ppc/mac_newworld.c |  13 -
 hw/ppc/mac_oldworld.c |  13 -
 hw/scsi/scsi-disk.c   |  62 ---
 hw/sparc64/sun4u.c|  15 -
 include/block/dirty-bitmap.h  |   1 -
 include/monitor/hmp.h |   4 -
 migration/migration.c |  45 ---
 migration/ram.c   |   2 +-
 monitor/hmp-cmds.c|  34 --
 monitor/misc.c| 187 -
 monitor/monitor.c |   4 +-
 monitor/qmp-cmds-control.c|  24 --
 qapi/block-core.json  |  64 +--
 qapi/control.json |  45 ---
 qapi/machine.json | 181 +
 qapi/migration.json   |  98 -
 qemu-options.hx   |   5 +-
 scripts/device-crash-test |   2 -
 softmmu/vl.c  |   1 -
 tests/acceptance/pc_cpu_hotplug_props.py  |   2 +-
 tests/acceptance/x86_cpu_model_versions.py|   2 +-
 tests/migration/guestperf/engine.py   |  18 +-
 tests/qemu-iotests/051|   4 -
 tests/qemu-iotests/051.pc.out |  20 -
 tests/qemu-iotests/124|   4 -
 tests/qemu-iotests/181|   2 +-
 tests/qemu-iotests/184.out|   6 +-
 tests/qemu-iotests/191.out|  48 +--
 tests/qemu-iotests/194|   4 +-
 tests/qemu-iotests/194.out|   4 +-
 tests/qemu-iotests/226.out|  10 +-
 tests/qemu-iotests/236|   2 +-
 tests/qemu-iotests/236.out|  42 +-
 tests/qemu-iotests/246  

[libvirt PATCH v2 3/3] docs: kbase: Fix a broken formatdomain reference in locking-sanlock

[Erik Skultety]

Signed-off-by: Erik Skultety 
---
 docs/kbase/locking-sanlock.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/kbase/locking-sanlock.rst b/docs/kbase/locking-sanlock.rst
index bd85c2e36e..24895b42f4 100644
--- a/docs/kbase/locking-sanlock.rst
+++ b/docs/kbase/locking-sanlock.rst
@@ -173,7 +173,7 @@ Domain configuration
 
 In case sanlock loses access to disk locks for some reason, it will kill
 all domains that lost their locks. This default behavior may be changed
-using `on_lockfailure element `__ in
+using `on_lockfailure element <../formatdomain.html#elementsEvents>`__ in
 domain XML. When this element is present, sanlock will call
 ``sanlock_helper`` (provided by libvirt) with the specified action. This
 helper binary will connect to libvirtd and thus it may need to
-- 
2.29.2

[libvirt PATCH v2 1/3] docs: html.in: Drop the architecture page

[Erik Skultety]

The page isn't linked from anywhere and the contents is dated.
Images related to the page are also dropped.

Signed-off-by: Erik Skultety 
---
 docs/architecture.gif | Bin 5571 -> 0 bytes
 docs/architecture.html.in |  82 -
 docs/architecture.svg | 239 --
 docs/meson.build  |   2 -
 4 files changed, 323 deletions(-)
 delete mode 100644 docs/architecture.gif
 delete mode 100644 docs/architecture.html.in
 delete mode 100644 docs/architecture.svg

diff --git a/docs/architecture.gif b/docs/architecture.gif
deleted file mode 100644
index 
9b820eef1878da18981e133d23c645a88df35dd6..
GIT binary patch
literal 0
HcmV?d1

literal 5571
zcmV;!6+G%kNk%w1VL<|t0e}Di0002y=H?;!1OWg50RSuj%0+Im$0{)DTsmtvT
zqnxzbi?iOm`wxcVNR|`;nCi;5?hD8AOxN~}=lag~{tpZahs2`sh)gP%%%<}RjY_A~
zs`ZM^YPa03_X`e-$KG%A;{|^`_I7nD%c!;>ylE~QT
z_y`#pSRz?zd5QT*nSv+7x!LI{f)X0@336Kcl8UM-`oZeD61zbwYhqh_3zJ*XyBqR*
z0?a!~yxJjr?4cadysR4x&GOt(J$-S_ppES--Hpq^4K6XBU{3ykuCSiIE$%+j9xxw|
zP;Y>5PY=Hj%k1x8tANS+5sYT=U6|=XiW;y
zy?HEIE+I+?8&{@0>C!>Sm@G-sl=d!P*W8)ukep%=3gytTQQv
zIacFVu2!v`My(MyXJ=qeh0dC}Zt$b5bWf(2g7akD`ILS3;hXW};en|mXUEBSbK=>P
z-+t*OI&93fftQw9-1|LJ+P5>m&ijk_siLhnw*LP5d-kgS-G7fC-fQ%kC)-wp?Z=;H
z@GY3&df;_2-%kXV7n@)RHs_mq+iln$fDj5OVTE1Ic3_4Ma@Zey9o8fti09Skm`jlP
zf!%J%b?2RDHhRcKgfC7wqJbeA@mvo++BaQ{D=GFvkvtr!Bub;fh$M8qxme|rSxPx%
z5zX|0Av!yf0A+!snMq}oJ5;lyL`9ZKA$dWbsiqFsteD#@Sl0Q`lt9MmWj5J$rsq-8
z$+%@nOj-%jm|=3)CLe|7bEuYlaVP7SB;=1rXd@|c3uwn&s6zcVD@Y`ENd&GK>WcoIuD!mwr>Vi3M5nVv3XAN5l=6zF
zJk0_s9JhNVYA3YEzSR96RC0X-Uz9$Cjl#`yJ<=rXp6BD`>nUt
zU3!C)Yi`T#v%}O1ZVm>A`>(C}QX8$nK|Jg*!uqzmaK#Bv>uSyaYy-wSEut~S`);i8$sosU@X9!o-0{sU7n^d=HS65+&;j#HG|xsy+%Tq5XO!ua
z1>5^;rCh?SP}K)J?WE5(oBSZzt1%(-u8wL8HP;9EnRC!cq^;T7=+fr)EHrkuEZsc*
zTr{i9#M|`LF@qiU(Q*SZFQG&+%JkLTq5e`_zlPV~cfHKE9r>F7r503^mxFR5=#LBG
zHN-RsPKn}Y7kitVp+oNS3<%18`o#dFjydaOx?VQp>D8+&f6LCUY2NY-AEs)`MT)%c
z5?k(x?`(^_yPeFp*Lb#>v)Fm>$+k)^sMT_w{OGp#ZhhA3J~@2fmm>5POnu8U;CB#sKI}=4fu;zc0O^A-=)}%i`bm-Z7)L?L
z)Qy7;njZ)8M!Wx|a21h(l>|dbI&&Qlg0E8G;gmOu|3Ht1C;Z;xl*T*T0m6kStexb}
z$2||4$c8smp(&O~KNB90iK@e4{xO_JGb;YjNPAn4jXY(%x?wG4FdUrqaz!tCjR}o~
zvmy&y1II`J?{7eRoyP!2nK82DP6QMQ-0HZ+Jkl+L>%!yGjOYftseq7&99!J%7z#y}
zgOBeU7!VIr#_;ITG>&v*L^Ao6?8QNnt*ay^5xF%`_GpuLn_eM1nKV?gfs}iMr7ZoZ
zN^qo34YcGW(r($OTy|rZY3OAug^7(n#`2P*WMv<5!Z2V;&6RX8rfqD=%p^H6m7dh3
zm?$~2W&W~}hMa>nO*u|Wmh+U`oTW3-8Ax)*Q=Qp-!#UN4&2_%4o|Zyp6(-TBdWT!O#&CtRuhVzvTB`7~@5yy-=Vv+-$r$al6C~7{lNnL#5Hoz%L
zgI-ioDHZA5&PYU;TC}1neJD+d#72`&bfvgFC^?7;QZMDShzh!C@Bm3iaR$|=)jN(B
z->FlZcFmvA0IE@8x3@+=FR?Ir~io1$wW{IIpiY^tj?=tOZ&w5&5pmnlaa1KgE>s8iH6}N*$nKN!1
z+g+qqo^4ectDOFt2joubdWGFneCa#Odo#b*tD0maSLvbZz5
zqjqyUTH8K$kK(azN3Dz9ny%Kl+?B2v)h4emu5mg1)sS*aTS~=p^}O_bF8J6xJ5ZJD
zQPu5~A`7sCI>__1zNC@|@Sa7(2gtCR1MVp)dXCJ+EP}
zH|p)q4m)^Be|poW9`%~Dc5Yxld)e1M_Pmey)Vt1lA%9%$!^eZnYkR@p1(xhlH!|1F
zK2(Wky!dxmJM-Hd2pITf^gP#1UZ=M?zx
z3-$tmB#RMI5qwjYBz6
z=z0hzcT|XeDgi852t_LLb=DGmuP1{;$Tv8mRZciv5?F**Cx`vVU>vwQBM2L2*mD`S
zVl~1yZPAih;`nT
zZI07JWMnJ#@PIP(hibSPQG|km;)U+^g-4_^j<`OdI6k$Qh^BZMa&cT>bQ+NdXUWHg
zbm)eJ@_cG(E9s1!sqLLRD8N0*qSdiv2@~*heO7
z1&&Ebc;k4DO#~N32ZUIqiT~$~phrRu_Jq4wj?LGF_IM%b$U~i|aO;!^T{n+X$cfN1
zKGjk}az>7%IB_ZnItnRg^n+Noc#zOIip_+86Ieii7z_W{K_{h;5IKGP=s1uK1MH7MhagQ(d8XvTe68VxX7K|dyx9OjfBXqox9h;o1xE+>o$)|o*0mCo}ym>5v>C?$zmnxw{<
zR+yQ6$$L~8c%S)Q+!$#an0>T~id{CQ-gph1@U-}$BCRL
zDOu}Dl-zlis`!cciIDT@IrZs`J13k@*_adNmd3M}#{!o@7=6H5WWs5d7|NgGiACbq
zpkbDrY4}p$8KR@9K^kgl8tFMGT3;dQOC?I6LPVkwX`);dqiLC<=IJmNiVL@y88dpH
zH;SM%I+NYENs4(`JQ_n9X$$bkqNCT7HM*ig%6>X}HA;G(B2|zZhc@vUcTd`nU@DMf
z23czeS?MXF%4wZ`g`?rgp{NsmLNugg+NEY%J<21dC<&WYN|_fEr*~SSK1o+tnnS@k
ze@khMxml-nT1Ca#fq&`*4FNAeN;!y{C(>8`o^ME~gQ=)fx~V&AsW0k@85*jiDr0(+
zr?3dAEJTuQx~PoGs@sREBuA^Y3P*!#tGTMHkkE*`>Z`w6a*_0@d%70uCmO~oT$EaO
zuG)byhpbQWtf{7~qbH;mN~NYre;-+W*=mzt0jqX68_>x!4tK4l^HZw|i{VPFBvP(t
z;H5qKt-m#`!_=;YR
z3mS=53O^GOu?{h{sMWR6Ig6_(tEAeq3@foh>rQBkXSnjQj~cbH`8HphrsgEKD;YIv
ziym5Aw=ny!c-vHds;GXunQwcng6pn(3wJpxvv3Qqgj;`StFD4LX}I>Tja#;i3te@K
zxZVi3Xu6+a<+#w=x8!QIfs0U7y0sCpxfi#(!WXtcma-t5x1o!v=W4hR#d9Ici}O{v
zleDsPcz`==yY$JrsuHa}D@__o2(x>+g-g7`YP^DAu2TkqO>4c^i@n*az1wREjY?p>
zJA2$KzT->2Sn8_yODakYe1mmaKThlmS(%00jt2GNWCGf
zJ+RBA`b)z8W5B|ze*YwTri#KTd|0=%zEpR+|Mj3*sFJ`K!*)x;8T_;|Jh+dej!X%l
zJSJ2^{J`Y3!F}S7UB-df(!@&aS!~O~tK`BI*^FN1VoFKHXrRF!+`k_CZ6GO%N_Li8
zsa8}B#47s5;sRv>`JV0}u5il6SF5LQOlK?S;28Z
z%CC!@sXTM5%*v+x7_Xek>s5HToW;C+$F1zjz^u5ryeYm68kj7|pIjEgY@aI(vJ5Od
z$}Ggv9L!i6#+Yk|%q+p$9KzIm7u@{0GK0(k>&-uG%Y$*w;jES5T&C=-!0DVAgnG?m
zOTj(7l+Y}?Q|xy5InR&#%=?Vb%WTh8i;7QM&QNgB=B&^AY`W__(0=vM-g?IkP0tj3
zxDkD>6YaU3xPJldr<$89=lr06TEa@)##qZ<3!Ty@`ngo=vj6(UsCmkki@O_mE@J$O
zwCsJ6T*%x0Sh`^;vly+@R>RX*th3SFl=CGV7;Aw{hG^($g8NuKByo}op5i8-`Tz2GhEdD4d7th-2qPE*;n5M
z9^VM=y@{>h`Ptw^O~?-};U4)7=EfIZ(Hb75Zpz`6`r&(6;UP}qJ5$f{i{dG+
z;w#SLD_-K9_~J3{PBMO|G;ZU#d*eB-;~cx=J>J4Tj?nB42eLv1#ZloZbbY`v9dP^n2enD(jov?8dIT
z&>p)sD(P;%mcCB1*gogtv+ZRL%IupJuldx}2#&Fm?r=E0<38!+o|*zJI36Y`Ma+<1
zdG971?uRZ*!!FU)C7`Ux?xN>lx#y@4-t4;l%`WYqI!xBuebey%-tq3-v+0mzoj|;z
z9|A

[libvirt PATCH v2 2/3] docs: html.in: Convert auth to rst

[Erik Skultety]

There was one external reference to this page's section which was fixed
manually.

Signed-off-by: Erik Skultety 
---
 docs/auth.html.in  | 368 -
 docs/auth.rst  | 343 ++
 docs/kbase/locking-sanlock.rst |   2 +-
 docs/meson.build   |   2 +-
 4 files changed, 345 insertions(+), 370 deletions(-)
 delete mode 100644 docs/auth.html.in
 create mode 100644 docs/auth.rst

diff --git a/docs/auth.html.in b/docs/auth.html.in
deleted file mode 100644
index 9b940a8598..00
--- a/docs/auth.html.in
+++ /dev/null
@@ -1,368 +0,0 @@
-
-
-http://www.w3.org/1999/xhtml";>
-  
-Connection authentication
-
-  When connecting to libvirt, some connections may require client
-  authentication before allowing use of the APIs. The set of possible
-  authentication mechanisms is administrator controlled, independent
-  of applications using libvirt. Once authenticated, libvirt can apply
-  fine grained access control to the operations
-  performed by a client.
-
-
-
-
-Client configuration
-
-
-  When connecting to a remote hypervisor which requires authentication,
-most libvirt applications will prompt the user for the credentials. It is
-also possible to provide a client configuration file containing all the
-authentication credentials, avoiding any interaction. Libvirt will look
-for the authentication file using the following sequence:
-
-
-  The file path specified by the $LIBVIRT_AUTH_FILE environment
-variable.
-  The file path specified by the "authfile=/some/file" URI
-query parameter
-  The file $XDG_CONFIG_HOME/libvirt/auth.conf
-  The file /etc/libvirt/auth.conf
-
-
-
-  The auth configuration file uses the traditional ".ini"
-  style syntax. There are two types of groups that can be present in
-  the config. First there are one or more credential
-  sets, which provide the actual authentication credentials. The keys
-  within the group may be:
-
-
-
-  username: the user login name to act as. This
-is relevant for ESX, Xen, HyperV and SSH, but probably not
-the one you want to libvirtd with SASL.
-  authname: the name to authorize as. This is
-what is commonly required for libvirtd with SASL.
-  password: the secret password
-  realm: the domain realm for SASL, mostly
-unused
-
-
-
-  Each set of credentials has a name, which is part of the group
-  entry name. Overall the syntax is
-
-
-
-[credentials-$NAME]
-credname1=value1
-credname2=value2
-
-
-  For example, to define two sets of credentials used for production
-  and test machines, using libvirtd, and a further ESX server for dev:
-
-
-[credentials-test]
-authname=fred
-password=123456
-
-[credentials-prod]
-authname=bar
-password=letmein
-
-[credentials-dev]
-username=joe
-password=hello
-
-[credentials-defgrp]
-username=defuser
-password=defpw
-
-
-  The second set of groups provide mappings of credentials to
-  specific machine services. The config file group names compromise
-  the service type and host:
-
-
-
-[auth-$SERVICE-$HOSTNAME]
-credentials=$CREDENTIALS
-
-
-  For example, following the previous example, here is how to
-  map some machines. For convenience libvirt supports a default
-  mapping of credentials to machines:
-
-
-
-[auth-libvirt-test1.example.com]
-credentials=test
-
-[auth-libvirt-test2.example.com]
-credentials=test
-
-[auth-libvirt-demo3.example.com]
-credentials=test
-
-[auth-libvirt-prod1.example.com]
-credentials=prod
-
-[auth-libvirt-default]
-credentials=defgrp
-
-[auth-esx-dev1.example.com]
-credentials=dev
-
-[auth-esx-default]
-credentials=defgrp
-
-
-
-  The following service types are known to libvirt:
-
-
-
-  esx - used for connections to an ESX or
-VirtualCenter server
-  hyperv - used for connections to an HyperV
-server
-  libvirt - used for connections to a libvirtd
-server, which is configured with SASL auth
-  ssh - used for connections to a remote QEMU driver
-over SSH
-
-
-
-  Applications using libvirt are free to use this same configuration
-  file for storing other credentials. For example, it can be used
-  to storage VNC or SPICE login credentials
-
-
-Server configuration
-
-The libvirt daemon allows the administrator to choose the authentication
-mechanisms used for client connections on each network socket independently.
-This is primarily controlled via the libvirt daemon master config file in
-/etc/libvirt/libvirtd.conf. Each of the libvirt sockets can
-have its authentication mechanism configured independently. There is
-currently a choice of none, polkit, and 
sasl.
-The SASL scheme can be further configured to choose between a large
-number of di

[libvirt PATCH v2 0/3] docs: Convert html.in to rst (volume 1)

[Erik Skultety]

since v1:
- fixed a broken heading reference from a kbase article caused by RST convers=
ion
- dropped architecture.html.in along with images

v1: https://listman.redhat.com/archives/libvir-list/2021-March/msg00584.html

Erik Skultety (3):
  docs: html.in: Drop the architecture page
  docs: html.in: Convert auth to rst
  docs: kbase: Fix a broken formatdomain reference in locking-sanlock

 docs/architecture.gif  | Bin 5571 -> 0 bytes
 docs/architecture.html.in  |  82 
 docs/architecture.svg  | 239 -
 docs/auth.html.in  | 368 -
 docs/auth.rst  | 343 ++
 docs/kbase/locking-sanlock.rst |   4 +-
 docs/meson.build   |   4 +-
 7 files changed, 346 insertions(+), 694 deletions(-)
 delete mode 100644 docs/architecture.gif
 delete mode 100644 docs/architecture.html.in
 delete mode 100644 docs/architecture.svg
 delete mode 100644 docs/auth.html.in
 create mode 100644 docs/auth.rst

--=20
2.29.2

Re: [libvirt PATCH 0/2] Coverity diaries

[Michal Privoznik]

On 3/15/21 4:46 PM, Ján Tomko wrote:

two minor bug fixes

Ján Tomko (2):
   cmdList: mark title as autofree
   nss: findLease: do not leak path

  tools/nss/libvirt_nss.c  | 4 +++-
  tools/virsh-domain-monitor.c | 4 ++--
  2 files changed, 5 insertions(+), 3 deletions(-)



Reviewed-by: Michal Privoznik 

Michal

[PATCH] meson: Don't check for addr2line

[Michal Privoznik]

In the past, we used to have this oomtrace.pl script that
attempted to print the stack trace of where an OOM error
occurred and it used addr2line for that. But since v5.8.0-rc1~189
we don't really care about OOM anymore and the script is long
gone so there's no need to check for addr2line program either.

Fixes: 2c52ecd96086b4643b99b4570b5823d40ce2787b
Signed-off-by: Michal Privoznik 
---
 meson.build | 1 -
 1 file changed, 1 deletion(-)

diff --git a/meson.build b/meson.build
index c81c6ab205..69a7b4c88e 100644
--- a/meson.build
+++ b/meson.build
@@ -925,7 +925,6 @@ endforeach
 # optional programs
 
 optional_programs = [
-  'addr2line',
   'augparse',
   'dmidecode',
   'dnsmasq',
-- 
2.26.2

[PATCH] qemu_process: Use accessor for def->mem.total_memory

[Michal Privoznik]

When connecting to the monitor, a timeout is calculated that is
bigger the more memory guest has (because QEMU has to allocate
and possibly zero out the memory and what not, empirically
deducted). However, when computing the timeout the @total_memory
mmember is accessed directly even though
virDomainDefGetMemoryTotal() should have been used.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 800b7b197d..f6da5afb48 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2077,7 +2077,7 @@ qemuConnectMonitor(virQEMUDriverPtr driver, 
virDomainObjPtr vm, int asyncJob,
  * handing them over to qemu. This can be very time
  * consuming. Therefore, add a second to timeout for each
  * 1GiB of guest RAM. */
-timeout = vm->def->mem.total_memory / (1024 * 1024);
+timeout = virDomainDefGetMemoryTotal(vm->def) / (1024 * 1024);
 
 ignore_value(virTimeMillisNow(&priv->monStart));
 
-- 
2.26.2

Re: [libvirt PATCH 0/9] docs: Convert html.in to rst (volume 1)

[Erik Skultety]

On Fri, Mar 12, 2021 at 01:57:30PM +0100, Peter Krempa wrote:
> On Fri, Mar 12, 2021 at 12:43:03 +0100, Erik Skultety wrote:
> > '#itsfriday'
> 
> For docs changes a good help for reviewers is to push to your private
> gitlab clone and provide a link to the artifacts of the 'webpage' job so
> that the output can be viewed easily.

Okay, I'll push patches 1-4 and 6,8,9 since the patches are independent and
resolve the review comments for the rest in v2.

Thanks,
Erik

Re: [libvirt PATCH v2 1/4] util: Try to get limits from /proc

[Andrea Bolognani]

On Mon, 2021-03-15 at 14:28 +0100, Michal Privoznik wrote:
> On 3/15/21 12:22 PM, Andrea Bolognani wrote:
> >   if (virFileReadAllQuiet(procfile, 2048, &buf) < 0)
> > -return -1;
> > +goto error;
> 
> virFileReadAllQuiet() sets errno, this would just overwrite it with 
> something less specific.

Good point. I've changed this to

  if (virFileReadAllQuiet(procfile, 2048, &buf) < 0) {
  /* This function already sets errno, so don't overwrite that
   * and return immediately instead */
  return -1;
  }

so that some explanation for the choice is retained.

> >   if (!(lines = g_strsplit(buf, "\n", 0)))
> > -return -1;
> > +goto error;
> 
> I think this check can be dropped. g_strsplit() doesn't ever return NULL 
> really, does it?

You're right, it doesn't. I've changed it.

> The rest looks good.

Thanks for the review and for the very good suggestions!

I'll see whether I can get some feedback from KubeVirt developers
regarding whether or not this actually improves things for them
before pushing.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add

[Kevin Wolf]

Am 15.03.2021 um 16:26 hat Markus Armbruster geschrieben:
> Kevin Wolf  writes:
> 
> > Am 13.03.2021 um 14:40 hat Markus Armbruster geschrieben:
> >> Markus Armbruster  writes:
> >> 
> >> > Paolo Bonzini  writes:
> >> >
> >> >> On 11/03/21 15:08, Markus Armbruster wrote:
> >>  I would rather keep the OptsVisitor here.  Do the same check for JSON
> >>  syntax that you have in qobject_input_visitor_new_str, and whenever
> >>  you need to walk all -object arguments, use something like this:
> >> 
> >>   typedef struct ObjectArgument {
> >>   const char *id;
> >>   QDict *json;/* or NULL for QemuOpts */
> >>   QSIMPLEQ_ENTRY(ObjectArgument) next;
> >>   }
> >> 
> >>  I already had patches in my queue to store -object in a GSList of
> >>  dictionaries, changing it to use the above is easy enough.
> >> >>> 
> >> >>> I think I'd prefer following -display's precedence.  See my reply to
> >> >>> Kevin for details.
> >> >>
> >> >> Yeah, I got independently to the same conclusion and posted patches
> >> >> for that.  I was scared that visit_type_ObjectOptions was too much for 
> >> >> OptsVisitor but it seems to work...
> >> >
> >> > We have reason to be scared.  I'll try to cover this in my review.
> >> 
> >> The opts visitor has serious limitations.  From its header:
> >> 
> >>  * The Opts input visitor does not implement support for visiting QAPI
> >>  * alternates, numbers (other than integers), null, or arbitrary
> >>  * QTypes.  It also requires a non-null list argument to
> >>  * visit_start_list().
> >> 
> >> This is retro-documentation for hairy code.  I don't trust it.  Commit
> >> eb7ee2cbeb "qapi: introduce OptsVisitor" hints at additional
> >> restrictions:
> >> 
> >> The type tree in the schema, corresponding to an option with a
> >> discriminator, must have the following structure:
> >> 
> >>   struct
> >> scalar member for non-discriminated optarg 1 [*]
> >> list for repeating non-discriminated optarg 2 [*]
> >>   wrapper struct
> >> single scalar member
> >> union
> >>   struct for discriminator case 1
> >> scalar member for optarg 3 [*]
> >> list for repeating optarg 4 [*]
> >>   wrapper struct
> >> single scalar member
> >> scalar member for optarg 5 [*]
> >>   struct for discriminator case 2
> >> ...
> >
> > Is this a long-winded way of saying that it has to be flat, except that
> > it allows lists, i.e. there must be no nested objects on the "wire"?
> 
> I think so.
> 
> > The difference between structs and unions, and different branches inside
> > the union isn't visible for the visitor anyway.
> 
> Yes, only the code using the visitor deals with that.
> 
> >> The "type" optarg name is fixed for the discriminator role. Its schema
> >> representation is "union of structures", and each discriminator value 
> >> must
> >> correspond to a member name in the union.
> >> 
> >> If the option takes no "type" descriminator, then the type subtree 
> >> rooted
> >> at the union must be absent from the schema (including the union 
> >> itself).
> >> 
> >> Optarg values can be of scalar types str / bool / integers / size.
> >> 
> >> Unsupported visits are treated as programming error.  Which is a nice
> >> way to say "they crash".
> >
> > The OptsVisitor never seems to crash explicitly by calling something
> > like abort().
> >
> > It may crash because of missing callbacks that are called without a NULL
> > check, like v->type_null.
> 
> Correct.
> 
> >   This case should probably be fixed in
> > qapi/qapi-visit-core.c to do the check and simply return an error.
> 
> I retro-documented what I inherited: qapi-visit-core.c code expects the
> visitors to implement the full visitor-impl.h interface, but some
> visitors don't.  So I documented "method must be set to visit FOOs" in
> visitor-impl.h, and for the visitors that don't, I documented "can't
> visit FOOs".
> 
> If the crashing behavior we've always had gets in the way, there are two
> ways to change it:
> 
> 1. Complicate qapi-visit-core.c slightly to cope with incomplete visitor
>implementations.
> 
> 2. Complete the visitor implementations: add dummy callbacks that fail.
> 
> I prefer 2., because I feel it keeps the visitor-impl.h interface
> simpler, and puts the extra complications where they belong.

I suggested making the callbacks optional because I expected that there
might be more than one visitor that doesn't support a callback and I
wouldn't like duplicating dummy callbacks in multiple places. But if
it's only the OptsVisitor, then we wouldn't get any duplication either
way and it becomes a matter of taste.

Kevin

[libvirt PATCH 2/2] nss: findLease: do not leak path

[Ján Tomko]

We still check for realloc's return value in the nss module.

Free the path in case it fails before jumping to cleanup.

Signed-off-by: Ján Tomko 
---
 tools/nss/libvirt_nss.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c
index b021efc3c9..87a731da79 100644
--- a/tools/nss/libvirt_nss.c
+++ b/tools/nss/libvirt_nss.c
@@ -138,8 +138,10 @@ findLease(const char *name,
 goto cleanup;
 
 tmpLease = realloc(leaseFiles, sizeof(char *) * (nleaseFiles + 1));
-if (!tmpLease)
+if (!tmpLease) {
+free(path);
 goto cleanup;
+}
 leaseFiles = tmpLease;
 leaseFiles[nleaseFiles++] = path;
 #if defined(LIBVIRT_NSS_GUEST)
-- 
2.29.2

[libvirt PATCH 1/2] cmdList: mark title as autofree

[Ján Tomko]

Fixes the (im)possible memory leak on vshTableRowAppend failure.

Signed-off-by: Ján Tomko 
---
 tools/virsh-domain-monitor.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 897339b6f9..fcbc2fe9f0 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1961,7 +1961,6 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
 bool optName = vshCommandOptBool(cmd, "name");
 bool optID = vshCommandOptBool(cmd, "id");
 size_t i;
-char *title;
 char uuid[VIR_UUID_STRING_BUFLEN];
 int state;
 bool ret = false;
@@ -2044,6 +2043,8 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
 state = -2;
 
 if (optTitle) {
+g_autofree char *title = NULL;
+
 if (!(title = virshGetDomainDescription(ctl, dom, true, 0)))
 goto cleanup;
 if (vshTableRowAppend(table, id_buf,
@@ -2052,7 +2053,6 @@ cmdList(vshControl *ctl, const vshCmd *cmd)
   : virshDomainStateToString(state),
   title, NULL) < 0)
 goto cleanup;
-VIR_FREE(title);
 } else {
 if (vshTableRowAppend(table, id_buf,
   virDomainGetName(dom),
-- 
2.29.2

[libvirt PATCH 0/2] Coverity diaries

[Ján Tomko]

two minor bug fixes

Ján Tomko (2):
  cmdList: mark title as autofree
  nss: findLease: do not leak path

 tools/nss/libvirt_nss.c  | 4 +++-
 tools/virsh-domain-monitor.c | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

-- 
2.29.2

Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add

[Markus Armbruster]

Kevin Wolf  writes:

> Am 13.03.2021 um 14:40 hat Markus Armbruster geschrieben:
>> Markus Armbruster  writes:
>> 
>> > Paolo Bonzini  writes:
>> >
>> >> On 11/03/21 15:08, Markus Armbruster wrote:
>>  I would rather keep the OptsVisitor here.  Do the same check for JSON
>>  syntax that you have in qobject_input_visitor_new_str, and whenever
>>  you need to walk all -object arguments, use something like this:
>> 
>>   typedef struct ObjectArgument {
>>   const char *id;
>>   QDict *json;/* or NULL for QemuOpts */
>>   QSIMPLEQ_ENTRY(ObjectArgument) next;
>>   }
>> 
>>  I already had patches in my queue to store -object in a GSList of
>>  dictionaries, changing it to use the above is easy enough.
>> >>> 
>> >>> I think I'd prefer following -display's precedence.  See my reply to
>> >>> Kevin for details.
>> >>
>> >> Yeah, I got independently to the same conclusion and posted patches
>> >> for that.  I was scared that visit_type_ObjectOptions was too much for 
>> >> OptsVisitor but it seems to work...
>> >
>> > We have reason to be scared.  I'll try to cover this in my review.
>> 
>> The opts visitor has serious limitations.  From its header:
>> 
>>  * The Opts input visitor does not implement support for visiting QAPI
>>  * alternates, numbers (other than integers), null, or arbitrary
>>  * QTypes.  It also requires a non-null list argument to
>>  * visit_start_list().
>> 
>> This is retro-documentation for hairy code.  I don't trust it.  Commit
>> eb7ee2cbeb "qapi: introduce OptsVisitor" hints at additional
>> restrictions:
>> 
>> The type tree in the schema, corresponding to an option with a
>> discriminator, must have the following structure:
>> 
>>   struct
>> scalar member for non-discriminated optarg 1 [*]
>> list for repeating non-discriminated optarg 2 [*]
>>   wrapper struct
>> single scalar member
>> union
>>   struct for discriminator case 1
>> scalar member for optarg 3 [*]
>> list for repeating optarg 4 [*]
>>   wrapper struct
>> single scalar member
>> scalar member for optarg 5 [*]
>>   struct for discriminator case 2
>> ...
>
> Is this a long-winded way of saying that it has to be flat, except that
> it allows lists, i.e. there must be no nested objects on the "wire"?

I think so.

> The difference between structs and unions, and different branches inside
> the union isn't visible for the visitor anyway.

Yes, only the code using the visitor deals with that.

>> The "type" optarg name is fixed for the discriminator role. Its schema
>> representation is "union of structures", and each discriminator value 
>> must
>> correspond to a member name in the union.
>> 
>> If the option takes no "type" descriminator, then the type subtree rooted
>> at the union must be absent from the schema (including the union itself).
>> 
>> Optarg values can be of scalar types str / bool / integers / size.
>> 
>> Unsupported visits are treated as programming error.  Which is a nice
>> way to say "they crash".
>
> The OptsVisitor never seems to crash explicitly by calling something
> like abort().
>
> It may crash because of missing callbacks that are called without a NULL
> check, like v->type_null.

Correct.

>   This case should probably be fixed in
> qapi/qapi-visit-core.c to do the check and simply return an error.

I retro-documented what I inherited: qapi-visit-core.c code expects the
visitors to implement the full visitor-impl.h interface, but some
visitors don't.  So I documented "method must be set to visit FOOs" in
visitor-impl.h, and for the visitors that don't, I documented "can't
visit FOOs".

If the crashing behavior we've always had gets in the way, there are two
ways to change it:

1. Complicate qapi-visit-core.c slightly to cope with incomplete visitor
   implementations.

2. Complete the visitor implementations: add dummy callbacks that fail.

I prefer 2., because I feel it keeps the visitor-impl.h interface
simpler, and puts the extra complications where they belong.

> Any other cases?

I don't think so.

>> Before this series, we use it for -object as follows.
>> 
>> user_creatable_add_opts() massages the QemuOpts into a QDict containing
>> just the properties, then calls user_creatable_add_type() with the opts
>> visitor wrapped around the QemuOpts, and the QDict.
>> 
>> user_creatable_add_type() performs a virtual visit.  The outermost
>> object it visits itself.  Then it visits members one by one by calling
>> object_property_set().  It uses the QDict as a list of members to visit.
>> 
>> As long as the object_property_set() only visit scalars other than
>> floating-point numbers, we safely stay with the opts visitors'
>> limitations.
>
> Minor addition: This visits inside object_property_set() are
> non

Re: [PATCH v3 26/30] qemu-img: Use user_creatable_process_cmdline() for --object

[Kevin Wolf]

Am 15.03.2021 um 15:15 hat Markus Armbruster geschrieben:
> Kevin Wolf  writes:
> 
> > Am 13.03.2021 um 13:30 hat Markus Armbruster geschrieben:
> >> Paolo Bonzini  writes:
> >> 
> >> > On 13/03/21 08:40, Markus Armbruster wrote:
> >> >>> +if (!user_creatable_add_from_str(optarg, &local_err)) 
> >> >>> {
> >> >>> +if (local_err) {
> >> >>> +error_report_err(local_err);
> >> >>> +exit(2);
> >> >>> +} else {
> >> >>> +/* Help was printed */
> >> >>> +exit(EXIT_SUCCESS);
> >> >>> +}
> >> >>> +}
> >> >>> +break;
> >> >>>   }
> >> >>> -}   break;
> >> >>>   case OPTION_IMAGE_OPTS:
> >> >>>   image_opts = true;
> >> >>>   break;
> >> >> Why is this one different?  The others all call
> >> >> user_creatable_process_cmdline().
> >> >> 
> >> >> 
> >> >
> >> > It's to exit with status code 2 instead of 1.
> >> 
> >> I see.  Worth a comment?
> >
> > There is a comment at the start of the function (which is just a few
> > lines above) that explains the exit codes:
> >
> >  * Compares two images. Exit codes:
> >  *
> >  * 0 - Images are identical or the requested help was printed
> >  * 1 - Images differ
> >  * >1 - Error occurred
> 
> I had in mind a comment that helps me over the "why is this not using
> user_creatable_process_cmdline()" hump.  Like so:
> 
> case OPTION_OBJECT:
> {
> /*
>  * Can't use user_creatable_process_cmdline(), because
>  * we need to exit(2) on error.
>  */
> ... open-coded variation of
> user_creatable_process_cmdline() ...
> }
> 
> Entirely up to you.

I see. This patch is already part of a pull request, but I wouldn't mind
a follow-up patch to add this comment if you want to send one.

Kevin

Re: [PATCH v3 26/30] qemu-img: Use user_creatable_process_cmdline() for --object

[Markus Armbruster]

Kevin Wolf  writes:

> Am 13.03.2021 um 13:30 hat Markus Armbruster geschrieben:
>> Paolo Bonzini  writes:
>> 
>> > On 13/03/21 08:40, Markus Armbruster wrote:
>> >>> +if (!user_creatable_add_from_str(optarg, &local_err)) {
>> >>> +if (local_err) {
>> >>> +error_report_err(local_err);
>> >>> +exit(2);
>> >>> +} else {
>> >>> +/* Help was printed */
>> >>> +exit(EXIT_SUCCESS);
>> >>> +}
>> >>> +}
>> >>> +break;
>> >>>   }
>> >>> -}   break;
>> >>>   case OPTION_IMAGE_OPTS:
>> >>>   image_opts = true;
>> >>>   break;
>> >> Why is this one different?  The others all call
>> >> user_creatable_process_cmdline().
>> >> 
>> >> 
>> >
>> > It's to exit with status code 2 instead of 1.
>> 
>> I see.  Worth a comment?
>
> There is a comment at the start of the function (which is just a few
> lines above) that explains the exit codes:
>
>  * Compares two images. Exit codes:
>  *
>  * 0 - Images are identical or the requested help was printed
>  * 1 - Images differ
>  * >1 - Error occurred

I had in mind a comment that helps me over the "why is this not using
user_creatable_process_cmdline()" hump.  Like so:

case OPTION_OBJECT:
{
/*
 * Can't use user_creatable_process_cmdline(), because
 * we need to exit(2) on error.
 */
... open-coded variation of
user_creatable_process_cmdline() ...
}

Entirely up to you.

Re: [libvirt PATCH] qemu_driver: fix setting vcpu_quota if not all vCPUs are online

[Erik Skultety]

On Mon, Mar 15, 2021 at 02:13:50PM +0100, Pavel Hrdina wrote:
> When switching to g_autoptr this was incorrectly changed from
> 'continue;' into 'return -1;' resulting into an error when user tries
> to set vcpu_quota of running VM:
> 
> error: An error occurred, but the cause is unknown
> 
> Fixes: e4a8bbfaf2b4cdd741bb441873bb730f9134b714
> Signed-off-by: Pavel Hrdina 
> ---
Reviewed-by: Erik Skultety 

Re: [libvirt PATCH v3 00/10] ci: Add helper script

[Erik Skultety]

On Fri, Mar 12, 2021 at 06:28:12PM +0100, Andrea Bolognani wrote:
> Changes from [v2]:
> 
>   * address review feedback;
> 
>   * wrap more Makefile functionality;
> 
>   * split into smaller, easier to review patches.
> 
> Changes from [v1]:
> 
>   * implement (partial) support for running builds and spawning
> shells inside the container;
> 
>   * make the code more maintainable by using a couple of classes.
> 
> 
> [v2] 
> https://listman.redhat.com/archives/libvir-list/2021-February/msg00922.html
> [v1] 
> https://listman.redhat.com/archives/libvir-list/2021-February/msg00900.html

See my comments on a few of the patches, but overall, I like this, so:
Reviewed-by: Erik Skultety 

Re: [libvirt PATCH v3 07/10] ci: Implement 'build' helper action

[Erik Skultety]

On Fri, Mar 12, 2021 at 06:28:19PM +0100, Andrea Bolognani wrote:
> This simply calls the underlying Makefile target, but allows
> additional arguments to be specified in a more convenient and
> discoverable way.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  ci/helper | 33 -
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/ci/helper b/ci/helper
> index 8eb521ae40..4a552595df 100755
> --- a/ci/helper
> +++ b/ci/helper
> @@ -55,6 +55,20 @@ class Parser:
>  help="use container images with non-default tags",
>  )
>  
> +# Options that are common to all actions that call the
> +# project's build system
> +mesonparser = argparse.ArgumentParser(add_help=False)
> +mesonparser.add_argument(
> +"--meson-args",
> +default="",
> +help="additional arguments passed to meson",

MESON_ARGS are tricky, because --meson-args "-Doption=val" won't work as you'd
expect and that's because argparse cannot handle a dash in the name of the
option value. However, they at least fixed it for the 'key=val' syntax [1], so
I'd suggest giving an example in the help string, how --meson-args should be
specified on the cmdline, otherwise it's confusing why it doesn't work with
the syntax outlined in the help string.

[1] https://github.com/bw2/ConfigArgParse/pull/160

Reviewed-by: Erik Skultety 

> +)
> +mesonparser.add_argument(
> +"--ninja-args",
> +default="",
> +help="additional arguments passed to ninja",
> +)
> +
>  # Options that are common to all actions that use lcitool
>  lcitoolparser = argparse.ArgumentParser(add_help=False)
>  lcitoolparser.add_argument(
> @@ -72,6 +86,14 @@ class Parser:
>  )
>  subparsers.required = True
>  
> +# build action
> +buildparser = subparsers.add_parser(
> +"build",
> +help="run a build in a container",
> +parents=[containerparser, mesonparser],
> +)
> +buildparser.set_defaults(func=Application.action_build)
> +
>  # shell action
>  shellparser = subparsers.add_parser(
>  "shell",
> @@ -115,7 +137,7 @@ class Application:
>  target,
>  ]
>  
> -if self.args.action == "shell":
> +if self.args.action in ["build", "shell"]:
>  args.extend([
>  f"CI_ENGINE={self.args.engine}",
>  f"CI_USER_LOGIN={self.args.login}",
> @@ -123,6 +145,12 @@ class Application:
>  f"CI_IMAGE_TAG={self.args.image_tag}",
>  ])
>  
> +if self.args.action == "build":
> +args.extend([
> +f"CI_MESON_ARGS={self.args.meson_args}",
> +f"CI_NINJA_ARGS={self.args.ninja_args}",
> +])
> +
>  if pty.spawn(["make"] + args) != 0:
>  sys.exit("error: 'make' failed")
>  
> @@ -200,6 +228,9 @@ class Application:
>  print(f"cirrus/{host}")
>  self.generate_vars(host)
>  
> +def action_build(self):
> +self.make_run(f"ci-build@{self.args.target}")
> +
>  def action_shell(self):
>  self.make_run(f"ci-shell@{self.args.target}")
>  
> -- 
> 2.26.2
> 

Re: [libvirt PATCH v2 1/4] util: Try to get limits from /proc

[Michal Privoznik]

On 3/15/21 12:22 PM, Andrea Bolognani wrote:

On Mon, 2021-03-15 at 11:57 +0100, Andrea Bolognani wrote:

The changes you're suggesting are not trivial enough for me to feel
comfortable simply applying them locally and then pushing right away.
Would the diff below look reasonable squashed in?

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 4fa854090d..cae0dabae3 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -799,16 +799,17 @@ virProcessGetLimitFromProc(pid_t pid,
  size_t i;

  if (!(label = virProcessLimitResourceToLabel(resource))) {
+virReportSystemError(EINVAL, "%s", _("Invalid resource"));
  return -1;
  }

  procfile = g_strdup_printf("/proc/%lld/limits", (long long)pid);

  if (virFileReadAllQuiet(procfile, 2048, &buf) < 0)
-return -1;
+goto error;


[...]


+ error:
+virReportSystemError(EIO, "%s", _("Input/output error"));
+return -1;
  }
  # else /* !defined(__linux__) */
  static int
@@ -846,6 +851,7 @@ virProcessGetLimitFromProc(pid_t pid G_GNUC_UNUSED,
 int resource G_GNUC_UNUSED,
 struct rlimit *limit G_GNUC_UNUSED)
  {
+virReportSystemError(ENOSYS, "%s", _("Not supported on this platform"));
  return -1;
  }
  # endif /* !defined(__linux__) */


This probably makes more sense:

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 4fa854090d..b173856b7a 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -799,16 +799,17 @@ virProcessGetLimitFromProc(pid_t pid,
  size_t i;

  if (!(label = virProcessLimitResourceToLabel(resource))) {
+errno = EINVAL;
  return -1;
  }

  procfile = g_strdup_printf("/proc/%lld/limits", (long long)pid);

  if (virFileReadAllQuiet(procfile, 2048, &buf) < 0)
-return -1;
+goto error;


virFileReadAllQuiet() sets errno, this would just overwrite it with 
something less specific.




  if (!(lines = g_strsplit(buf, "\n", 0)))
-return -1;
+goto error;


I think this check can be dropped. g_strsplit() doesn't ever return NULL 
really, does it?




  for (i = 0; lines[i]; i++) {
  g_autofree char *softLimit = NULL;
@@ -820,25 +821,29 @@ virProcessGetLimitFromProc(pid_t pid,
  continue;
  
  if (sscanf(line, "%ms %ms %*s", &softLimit, &hardLimit) < 2)

-return -1;
+goto error;
  
  if (STREQ(softLimit, "unlimited")) {

  limit->rlim_cur = RLIM_INFINITY;
  } else {
  if (virStrToLong_ull(softLimit, NULL, 10, &tmp) < 0)
-return -1;
+goto error;
  limit->rlim_cur = tmp;
  }
  if (STREQ(hardLimit, "unlimited")) {
  limit->rlim_max = RLIM_INFINITY;
  } else {
  if (virStrToLong_ull(hardLimit, NULL, 10, &tmp) < 0)
-return -1;
+goto error;
  limit->rlim_max = tmp;
  }
  }
  
  return 0;

+
+ error:
+errno = EIO;
+return -1;
  }
  # else /* !defined(__linux__) */
  static int
@@ -846,6 +851,7 @@ virProcessGetLimitFromProc(pid_t pid G_GNUC_UNUSED,
 int resource G_GNUC_UNUSED,
 struct rlimit *limit G_GNUC_UNUSED)
  {
+errno = ENOSYS;
  return -1;
  }
  # endif /* !defined(__linux__) */



The rest looks good.

Michal

Re: [libvirt PATCH v3 06/10] ci: Implement 'shell' helper action

[Erik Skultety]

On Fri, Mar 12, 2021 at 06:28:18PM +0100, Andrea Bolognani wrote:
> This simply calls the underlying Makefile target, but allows
> additional arguments to be specified in a more convenient and
> discoverable way.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  ci/helper | 48 
>  1 file changed, 48 insertions(+)
> 
> diff --git a/ci/helper b/ci/helper
> index 420e9b73c2..8eb521ae40 100755
> --- a/ci/helper
> +++ b/ci/helper
> @@ -17,6 +17,7 @@
>  # .
>  
>  import argparse
> +import os
>  import pathlib
>  import pty
>  import shutil
> @@ -26,6 +27,34 @@ import sys
>  
>  class Parser:
>  def __init__(self):
> +# Options that are common to all actions that use containers
> +containerparser = argparse.ArgumentParser(add_help=False)
> +containerparser.add_argument(
> +"target",
> +help="build on target OS",

With the 'shell' action, not sure if "build" is the right word in that case,
how about "perform action on target OS" -- since the "target" argument is
shared among ['shell', 'build', 'test'].

Reviewed-by: Erik Skultety 

Re: [libvirt PATCH v3 04/10] ci: Implement 'refresh' helper action

[Erik Skultety]

On Fri, Mar 12, 2021 at 06:28:16PM +0100, Andrea Bolognani wrote:
> This provides the same functionality as the two refresh scripts
> that are currently in the repository, with the following
> advantages:
> 
>   * all files are refreshed with a single command;
> 
>   * if lcitool is present in the user's $PATH, it will be
> discovered and used automatically;
> 
>   * some output is produced, so the user can follow along with
> the progress of the operation.
> 
> Signed-off-by: Andrea Bolognani 
> ---
...

> +for host in self.lcitool_get_hosts():
> +if host.startswith("freebsd-") or host.startswith("macos-"):
> +continue
> +
> +print(f"containers/{host}")

Not sure whether I like the verbosity ^here, but since I don't have a
compelling argument against, let's keep it. However, in that case I'd expect it
to output the name of the dockerfile verbatim rather than something that
partially looks like a path but really isn't.

> +self.generate_dockerfile(host)
> +
> +if host == "fedora-rawhide":
> +for cross in fedora_cross:
> +print(f"containers/{host} ({cross})")

...same here...

> +self.generate_dockerfile(host, cross)
> +
> +if host.startswith("debian-"):
> +for cross in debian_cross:
> +if host == "debian-sid" and cross == "mips":
> +continue
> +print(f"containers/{host} ({cross})")

...and here...

> +self.generate_dockerfile(host, cross)
> +
> +def refresh_cirrus(self):
> +for host in self.lcitool_get_hosts():
> +if not (host.startswith("freebsd-") or 
> host.startswith("macos-")):
> +continue
> +
> +print(f"cirrus/{host}")
> +self.generate_vars(host)
> +
> +def action_refresh(self):
> +self.refresh_containers()
> +self.refresh_cirrus()
> +
>  def run(self):
>  self.args.func(self)
>  
> -- 
> 2.26.2
> 

Reviewed-by: Erik Skultety 

Re: [libvirt PATCH v3 03/10] ci: Add helper script

[Erik Skultety]

On Fri, Mar 12, 2021 at 06:28:15PM +0100, Andrea Bolognani wrote:
> This is intended to be perform a number of CI-related operations
> that are currently implemented in various different scripts
> written in various different programming languages.
> 
> Eventually, all existing functionality will be reimplemented in
> Python and made available through this single entry point; for
> now, let's start with a very basic skeleton.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  ci/helper | 47 +++
>  1 file changed, 47 insertions(+)
>  create mode 100755 ci/helper
> 
> diff --git a/ci/helper b/ci/helper
> new file mode 100755
> index 00..2a59b8e5ab
> --- /dev/null
> +++ b/ci/helper
> @@ -0,0 +1,47 @@
> +#!/usr/bin/env python3
> +#
> +# Copyright (C) 2021 Red Hat, Inc.
> +#
> +# This library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +#
> +# This library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with this library.  If not, see
> +# .

This is a new file, I think we could adopt the SPDX tag for it. I remember we
discussed this in the past and the conclusion was that it was near to
impossible to adopt SPDX across libvirt main code base due to the need to get
an approval from all contributors, but since the original shell helpers didn't
use any license header, I say we can easily use SPDX here.

Erik

Re: [libvirt PATCH v2 1/1] ci: Add helper script

[Erik Skultety]

On Fri, Mar 12, 2021 at 06:54:15PM +0100, Andrea Bolognani wrote:
> On Tue, 2021-02-23 at 16:17 +0100, Erik Skultety wrote:
> > On Wed, Feb 17, 2021 at 10:24:51AM +0100, Andrea Bolognani wrote:
> > > This is intended to be the perform a number of CI-related
> > > operations that currently are implemented in various different
> > > scripts written in various different programming languages; in
> > > this first iteration it does two things:
> > > 
> > >   * implement the functionality of the existing "refresh"
> > > scripts, which it supersedes;
> > > 
> > >   * provide a nicer front-end for a subset of the
> > > functionality exposed by the ci/Makefile scaffolding, such
> > > as running basic builds.
> > > 
> > > Over time, the plan is to rewrite all CI-related functionality
> > > in Python and move it into this script.
> > 
> > You dived into it more aggressively than what I proposed, but we still need 
> > to
> > approach it gradually, like don't extract the Makefile functionality 
> > partially.
> > Let's lay the foundation first, then replace refresh and then at some point 
> > in
> > the future, drop the Makefile and integrate it into the helper, I don't mind
> > keeping the Makefile for a little longer.
> 
> Sorry for taking a while to get back to you.
> 
> I just posted a [v3] which should address pretty much all of your
> feedback, but I'm not sure we're on the same page when it comes to
> the overall strategy so I'm going to spend a few words on that here.
> 
> Even as a simple wrapper, the new helper script is a massive
> usability win because options work in a standard way and are easily
> discoverable, so I'm very keen on adopting it as the official entry
> point for local CI-related functionality right away.
> 
> Whether the underlying code is implemented or not in Python is not as
> important... That's obviously the end goal, but in the short term I'd
> rather have people call the Makefile through the new script than
> directly.
> 
> And once the new script has been introduced, we can *definitely*
> rewrite functionality to Python in chunks. For example, the code
> underpinning the 'list-images' target is already mostly outsourced to
> a shell script, and your series from last month reimplemented that
> with a Python one: instead of making that script standalone and
> having the Makefile call out to it, we should just implement the
> functionality in the new helper script and drop the corresponding
> Makefile code entirely.
> 
> The rest of the Makefile will have to be rewritten in one go because
> it's all tangled together, but again we can do that whenever we have
> some free cycles, without having to hurry too much since, regardless
> of the underlying implementation, users are already enjoying the
> improved user interface.

That's fine, my point was that I didn't like a partial wrapper of the Makefile
functionality, I just suggested one approach, while you went with a different
one if v3 - which I certainly don't mind.

Erik

[libvirt PATCH] qemu_driver: fix setting vcpu_quota if not all vCPUs are online

[Pavel Hrdina]

When switching to g_autoptr this was incorrectly changed from
'continue;' into 'return -1;' resulting into an error when user tries
to set vcpu_quota of running VM:

error: An error occurred, but the cause is unknown

Fixes: e4a8bbfaf2b4cdd741bb441873bb730f9134b714
Signed-off-by: Pavel Hrdina 
---
 src/qemu/qemu_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b7c89a826a..16c5ccae45 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9204,7 +9204,7 @@ qemuSetVcpusBWLive(virDomainObjPtr vm, virCgroupPtr 
cgroup,
 virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(vm->def, i);
 
 if (!vcpu->online)
-return -1;
+continue;
 
 if (virCgroupNewThread(cgroup, VIR_CGROUP_THREAD_VCPU, i,
false, &cgroup_vcpu) < 0)
-- 
2.30.2

Re: [PATCH 07/14] machine: remove 'arch' field from 'query-cpus-fast' QMP command

[Daniel P . Berrangé]

Ping for anyone willing to review this so I can get this in before freeze.

On Wed, Feb 24, 2021 at 01:11:35PM +, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé 
> ---
>  docs/system/deprecated.rst   |  6 -
>  docs/system/removed-features.rst |  6 +
>  hw/core/machine-qmp-cmds.c   | 41 
>  qapi/machine.json| 22 -
>  4 files changed, 6 insertions(+), 69 deletions(-)
> 
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 484f017119..78474f0845 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -192,12 +192,6 @@ Since the ``dirty-bitmaps`` field is optionally present 
> in both the old and
>  new locations, clients must use introspection to learn where to anticipate
>  the field if/when it does appear in command output.
>  
> -``query-cpus-fast`` ``arch`` output member (since 3.0.0)
> -
> -
> -The ``arch`` output member of the ``query-cpus-fast`` command is
> -replaced by the ``target`` output member.
> -
>  chardev client socket with ``wait`` option (since 4.0)
>  ''
>  
> diff --git a/docs/system/removed-features.rst 
> b/docs/system/removed-features.rst
> index ad146daf9b..7942c2e513 100644
> --- a/docs/system/removed-features.rst
> +++ b/docs/system/removed-features.rst
> @@ -100,6 +100,12 @@ Use ``migrate_set_parameter`` instead.
>  
>  The ``query-cpus`` command is replaced by the ``query-cpus-fast`` command.
>  
> +``query-cpus-fast`` ``arch`` output member (removed in 6.0)
> +'''
> +
> +The ``arch`` output member of the ``query-cpus-fast`` command is
> +replaced by the ``target`` output member.
> +
>  Human Monitor Protocol (HMP) commands
>  -
>  
> diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
> index af60cd969d..68a942595a 100644
> --- a/hw/core/machine-qmp-cmds.c
> +++ b/hw/core/machine-qmp-cmds.c
> @@ -24,46 +24,6 @@
>  #include "sysemu/runstate.h"
>  #include "sysemu/sysemu.h"
>  
> -static CpuInfoArch sysemu_target_to_cpuinfo_arch(SysEmuTarget target)
> -{
> -/*
> - * The @SysEmuTarget -> @CpuInfoArch mapping below is based on the
> - * TARGET_ARCH -> TARGET_BASE_ARCH mapping in the "configure" script.
> - */
> -switch (target) {
> -case SYS_EMU_TARGET_I386:
> -case SYS_EMU_TARGET_X86_64:
> -return CPU_INFO_ARCH_X86;
> -
> -case SYS_EMU_TARGET_PPC:
> -case SYS_EMU_TARGET_PPC64:
> -return CPU_INFO_ARCH_PPC;
> -
> -case SYS_EMU_TARGET_SPARC:
> -case SYS_EMU_TARGET_SPARC64:
> -return CPU_INFO_ARCH_SPARC;
> -
> -case SYS_EMU_TARGET_MIPS:
> -case SYS_EMU_TARGET_MIPSEL:
> -case SYS_EMU_TARGET_MIPS64:
> -case SYS_EMU_TARGET_MIPS64EL:
> -return CPU_INFO_ARCH_MIPS;
> -
> -case SYS_EMU_TARGET_TRICORE:
> -return CPU_INFO_ARCH_TRICORE;
> -
> -case SYS_EMU_TARGET_S390X:
> -return CPU_INFO_ARCH_S390;
> -
> -case SYS_EMU_TARGET_RISCV32:
> -case SYS_EMU_TARGET_RISCV64:
> -return CPU_INFO_ARCH_RISCV;
> -
> -default:
> -return CPU_INFO_ARCH_OTHER;
> -}
> -}
> -
>  static void cpustate_to_cpuinfo_s390(CpuInfoS390 *info, const CPUState *cpu)
>  {
>  #ifdef TARGET_S390X
> @@ -104,7 +64,6 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
>  value->props = props;
>  }
>  
> -value->arch = sysemu_target_to_cpuinfo_arch(target);
>  value->target = target;
>  if (target == SYS_EMU_TARGET_S390X) {
>  cpustate_to_cpuinfo_s390(&value->u.s390x, cpu);
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 9811927504..c0c52aef10 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -34,21 +34,6 @@
>   'sh4eb', 'sparc', 'sparc64', 'tricore', 'unicore32',
>   'x86_64', 'xtensa', 'xtensaeb' ] }
>  
> -##
> -# @CpuInfoArch:
> -#
> -# An enumeration of cpu types that enable additional information during
> -# @query-cpus-fast.
> -#
> -# @s390: since 2.12
> -#
> -# @riscv: since 2.12
> -#
> -# Since: 2.6
> -##
> -{ 'enum': 'CpuInfoArch',
> -  'data': ['x86', 'sparc', 'ppc', 'mips', 'tricore', 's390', 'riscv', 
> 'other' ] }
> -
>  ##
>  # @CpuS390State:
>  #
> @@ -86,14 +71,9 @@
>  # @props: properties describing to which node/socket/core/thread
>  # virtual CPU belongs to, provided if supported by board
>  #
> -# @arch: base architecture of the cpu
> -#
>  # @target: the QEMU system emulation target, which determines which
>  #  additional fields will be listed (since 3.0)
>  #
> -# Features:
> -# @deprecated: Member @arch is deprecated.  Use @target instead.
> -#
>  # Since: 2.12
>  #
>  ##
> @@ -102,8 +82,6 @@
>'qom-path' : 'str',
>'thread-id':

Re: [PATCH v3 27/30] hmp: QAPIfy object_add

[Dr. David Alan Gilbert]

* Kevin Wolf (kw...@redhat.com) wrote:
> Am 15.03.2021 um 10:39 hat Markus Armbruster geschrieben:
> > Paolo Bonzini  writes:
> > 
> > > On 13/03/21 14:28, Markus Armbruster wrote:
> > >> Kevin Wolf  writes:
> > >> 
> > >>> This switches the HMP command object_add from a QemuOpts-based parser to
> > >>> user_creatable_add_from_str() which uses a keyval parser and enforces
> > >>> the QAPI schema.
> > >>>
> > >>> Apart from being a cleanup, this makes non-scalar properties and help
> > >>> accessible. In order for help to be printed to the monitor instead of
> > >>> stdout, the printf() calls in the help functions are changed to
> > >>> qemu_printf().
> > >>>
> > >>> Signed-off-by: Kevin Wolf 
> > >>> Acked-by: Peter Krempa 
> > >>> Reviewed-by: Eric Blake 
> > >>> Reviewed-by: Dr. David Alan Gilbert 
> > >>> ---
> > >>>   monitor/hmp-cmds.c  | 17 ++---
> > >>>   qom/object_interfaces.c | 11 ++-
> > >>>   hmp-commands.hx |  2 +-
> > >>>   3 files changed, 9 insertions(+), 21 deletions(-)
> > >>>
> > >>> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> > >>> index 3c88a4faef..652cf9ff21 100644
> > >>> --- a/monitor/hmp-cmds.c
> > >>> +++ b/monitor/hmp-cmds.c
> > >>> @@ -1670,24 +1670,11 @@ void hmp_netdev_del(Monitor *mon, const QDict 
> > >>> *qdict)
> > >>>   
> > >>>   void hmp_object_add(Monitor *mon, const QDict *qdict)
> > >>>   {
> > >>> +const char *options = qdict_get_str(qdict, "object");
> > >>>   Error *err = NULL;
> > >>> -QemuOpts *opts;
> > >>> -Object *obj = NULL;
> > >>> -
> > >>> -opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
> > >>> -if (err) {
> > >>> -goto end;
> > >>> -}
> > >>>   
> > >>> -obj = user_creatable_add_opts(opts, &err);
> > >>> -qemu_opts_del(opts);
> > >>> -
> > >>> -end:
> > >>> +user_creatable_add_from_str(options, &err);
> > >>>   hmp_handle_error(mon, err);
> > >>> -
> > >>> -if (obj) {
> > >>> -object_unref(obj);
> > >>> -}
> > >>>   }
> > >> 
> > >> Doesn't this break the list-valued properties (Memdev member host-nodes,
> > >> NumaNodeOptions member cpus) exactly the same way that made us keep
> > >> QemuOpts for qemu-system-FOO -object?
> > >
> > > Yes, it does.  I guess it can just be documented, unlike for the command 
> > > line?
> > 
> > Maybe.  Judgement call, not mine to make.
> > 
> > Do people create such objects in HMP?  I figure we don't really know.
> > Educated guess?
> > 
> > If you try, how does it break?  Is it confusing?  Can you show an
> > example?
> 
> (qemu) object_add memory-backend-ram,id=mem,size=4G,policy=bind,host-nodes=0
> Error: Invalid parameter type for 'host-nodes', expected: array
> (qemu) object_add memory-backend-ram,id=mem,size=4G,policy=bind,host-nodes.0=0
> (qemu)
> 
> HMP is not a stable interface, so changing the syntax didn't feel like a
> problem to me. I doubt many people do HMP memory hotplug while setting a
> specific NUMA policy, but it wouldn't change my assessment anyway. I
> should have made this explicit in the commit message, though.

I'm OK for it to change, but yes I'd like to have the before/after
syntax listed somewhere as easy references for people confused.

Dave

> Kevin
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH] bhyve: add support

[Pavel Hrdina]

On Sat, Feb 27, 2021 at 08:34:08AM +0400, Roman Bogorodskiy wrote:
> Implement "" support for bhyve driver.
> As there are not really lot of options, try to find
> "BHYVE_UEFI.fd" firmware which is installed by the
> sysutils/uefi-edk2-bhyve FreeBSD port.
> 
> If not found, just use the first found firmware
> in the firmwares directory (which is configurable via
> config file).
> 
> Signed-off-by: Roman Bogorodskiy 
> ---
> Not extremely happy about the LIBVIRT_BHYVE_FIRMWARE_DIR_OVERRIDE knob,
> but not sure how to test this otherwise.

Agreed, that should not be part of the production code.

You can use tests/bhyvexml2argvmock.c which is already used for that
test where you would provide your custom implementation of opendir()
system call. The implementation should check if it tries to access the
default firmware dir and change it to location in our tests and all
other paths simply pass to the real opendir().

Look into tests/virpcimock.c, but the addition for your use-case should
look something like this:


diff --git a/tests/bhyvexml2argvmock.c b/tests/bhyvexml2argvmock.c
index 25b97f5e04..1c2b1f8876 100644
--- a/tests/bhyvexml2argvmock.c
+++ b/tests/bhyvexml2argvmock.c
@@ -4,10 +4,34 @@
 #include "virstring.h"
 #include "virnetdev.h"
 #include "virnetdevtap.h"
+#include "virmock.h"
 #include "internal.h"

 #define VIR_FROM_THIS VIR_FROM_BHYVE

+#define DEFAULT_FIRMWARE_DIR_TEMPLATE DATADIR "/uefi-firmware"
+#define FAKE_FIRMWARE_DIR_TEMPLATE abs_builddir "/bhyvefakefirmwaredir-XX"
+
+static int (*real_opendir)(const char *name);
+
+static void
+init_syms(void)
+{
+VIR_MOCK_REAL_INIT(opendir);
+}
+
+DIR *
+opendir(const char *path)
+{
+init_syms();
+
+if (STRPREFIX(path, DEFAULT_FIRMWARE_DIR_TEMPLATE)) {
+return real_opendir(FAKE_FIRMWARE_DIR_TEMPLATE);
+} else {
+return real_opendir(path);
+}
+}
+
 void virMacAddrGenerate(const unsigned char prefix[VIR_MAC_PREFIX_BUFLEN],
 virMacAddrPtr addr)
 {


I did not test it :)

Pavel


signature.asc
Description: PGP signature

Re: [PATCH 10/14] hw/scsi: remove 'scsi-disk' device

[Thomas Huth]

On 24/02/2021 14.11, Daniel P. Berrangé wrote:

The 'scsi-hd' and 'scsi-cd' devices provide suitable alternatives.

Signed-off-by: Daniel P. Berrangé 
---
  docs/system/deprecated.rst   |  9 -
  docs/system/removed-features.rst |  6 
  hw/i386/pc.c |  1 -
  hw/scsi/scsi-disk.c  | 62 
  hw/sparc64/sun4u.c   |  1 -
  scripts/device-crash-test|  1 -
  tests/qemu-iotests/051   |  2 --
  tests/qemu-iotests/051.pc.out| 10 --
  8 files changed, 6 insertions(+), 86 deletions(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index f5c82a46dc..cb88fea94f 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -239,15 +239,6 @@ The ``I7200`` guest CPU relies on the nanoMIPS ISA, which 
is deprecated
  (the ISA has never been upstreamed to a compiler toolchain). Therefore
  this CPU is also deprecated.
  
-System emulator devices


-
-``scsi-disk`` (since 4.2)
-'
-
-The 'scsi-disk' device is deprecated. Users should use 'scsi-hd' or
-'scsi-cd' as appropriate to get a SCSI hard disk or CD-ROM as needed.
-
  System emulator machines
  
  
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst

index 8fd3fafb32..bb6bc8dfc8 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -222,6 +222,12 @@ System emulator devices
  The 'ide-drive' device has been removed. Users should use 'ide-hd' or
  'ide-cd' as appropriate to get an IDE hard disk or CD-ROM as needed.
  
+``scsi-disk`` (removed in 6.0)

+''
+
+The 'scsi-disk' device is deprecated. Users should use 'scsi-hd' or
+'scsi-cd' as appropriate to get a SCSI hard disk or CD-ROM as needed.


s/is deprecated/has been removed/


diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
index d7c27144ba..cda7df36e3 100644
--- a/hw/sparc64/sun4u.c
+++ b/hw/sparc64/sun4u.c
@@ -749,7 +749,6 @@ static char *sun4u_fw_dev_path(FWPathProvider *p, BusState 
*bus,
 DeviceState *dev)
  {
  PCIDevice *pci;
-int bus_id;
  
  if (!strcmp(object_get_typename(OBJECT(dev)), "pbm-bridge")) {

  pci = PCI_DEVICE(dev);


Please squash this hunk into the 'ide-drive' patch instead.

With the two nits fixed:
Reviewed-by: Thomas Huth 

Re: [PATCH 14/14] block: remove support for using "file" driver with block/char devices

[Eric Blake]

On 2/24/21 7:11 AM, Daniel P. Berrangé wrote:
> The 'host_device' and 'host_cdrom' drivers must be used instead.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  block/file-posix.c   | 17 ++---
>  docs/system/deprecated.rst   |  7 ---
>  docs/system/removed-features.rst |  7 +++
>  tests/qemu-iotests/226.out   | 10 +-
>  4 files changed, 18 insertions(+), 23 deletions(-)
> 

>  if (!device) {
> -if (S_ISBLK(st.st_mode)) {
> -warn_report("Opening a block device as a file using the '%s' "
> -"driver is deprecated", bs->drv->format_name);
> -} else if (S_ISCHR(st.st_mode)) {
> -warn_report("Opening a character device as a file using the '%s' 
> "
> -"driver is deprecated", bs->drv->format_name);
> -} else if (!S_ISREG(st.st_mode)) {
> -error_setg(errp, "A regular file was expected by the '%s' 
> driver, "
> -   "but something else was given", bs->drv->format_name);
> +if (!S_ISREG(st.st_mode)) {

We're testing with S_ISREG()...


> +++ b/docs/system/deprecated.rst
> @@ -21,13 +21,6 @@ deprecated.
>  System emulator command line arguments
>  --
>  
> -``-drive file=json:{...{'driver':'file'}}`` (since 3.0)
> -'''
> -
> -The 'file' driver for drives is no longer appropriate for character or host
> -devices and will only accept regular files (S_IFREG). The correct driver

but documented with S_IFREG().  Thankfully, the two have semantically
equivalent purposes, so the difference doesn't invalidate the docs.

Reviewed-by: Eric Blake 

but I wouldn't mind if at least one other block maintainer chimes in.

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

Re: [PATCH v3 27/30] hmp: QAPIfy object_add

[Paolo Bonzini]

On 15/03/21 12:38, Dr. David Alan Gilbert wrote:

* Kevin Wolf (kw...@redhat.com) wrote:

Am 15.03.2021 um 10:39 hat Markus Armbruster geschrieben:

Paolo Bonzini  writes:


On 13/03/21 14:28, Markus Armbruster wrote:

Kevin Wolf  writes:


This switches the HMP command object_add from a QemuOpts-based parser to
user_creatable_add_from_str() which uses a keyval parser and enforces
the QAPI schema.

Apart from being a cleanup, this makes non-scalar properties and help
accessible. In order for help to be printed to the monitor instead of
stdout, the printf() calls in the help functions are changed to
qemu_printf().

Signed-off-by: Kevin Wolf 
Acked-by: Peter Krempa 
Reviewed-by: Eric Blake 
Reviewed-by: Dr. David Alan Gilbert 
---
   monitor/hmp-cmds.c  | 17 ++---
   qom/object_interfaces.c | 11 ++-
   hmp-commands.hx |  2 +-
   3 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 3c88a4faef..652cf9ff21 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1670,24 +1670,11 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
   
   void hmp_object_add(Monitor *mon, const QDict *qdict)

   {
+const char *options = qdict_get_str(qdict, "object");
   Error *err = NULL;
-QemuOpts *opts;
-Object *obj = NULL;
-
-opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
-if (err) {
-goto end;
-}
   
-obj = user_creatable_add_opts(opts, &err);

-qemu_opts_del(opts);
-
-end:
+user_creatable_add_from_str(options, &err);
   hmp_handle_error(mon, err);
-
-if (obj) {
-object_unref(obj);
-}
   }


Doesn't this break the list-valued properties (Memdev member host-nodes,
NumaNodeOptions member cpus) exactly the same way that made us keep
QemuOpts for qemu-system-FOO -object?


Yes, it does.  I guess it can just be documented, unlike for the command
line?


Maybe.  Judgement call, not mine to make.

Do people create such objects in HMP?  I figure we don't really know.
Educated guess?

If you try, how does it break?  Is it confusing?  Can you show an
example?


(qemu) object_add memory-backend-ram,id=mem,size=4G,policy=bind,host-nodes=0
Error: Invalid parameter type for 'host-nodes', expected: array
(qemu) object_add memory-backend-ram,id=mem,size=4G,policy=bind,host-nodes.0=0
(qemu)

HMP is not a stable interface, so changing the syntax didn't feel like a
problem to me. I doubt many people do HMP memory hotplug while setting a
specific NUMA policy, but it wouldn't change my assessment anyway. I
should have made this explicit in the commit message, though.


I'm OK for it to change, but yes I'd like to have the before/after
syntax listed somewhere as easy references for people confused.


I think we should try to improve the string-value QObject visitor to 
also allow JSON values in some places, for example to allow


object_add 
memory-backend-ram,id=mem,size=4G,policy=bind,host-nodes=[0,1,2,3]


Paolo

Re: [PATCH 14/14] block: remove support for using "file" driver with block/char devices

[Daniel P . Berrangé]

Ping for anyone, especially block maintainers, willing to review this
before soft freeze.

On Wed, Feb 24, 2021 at 01:11:42PM +, Daniel P. Berrangé wrote:
> The 'host_device' and 'host_cdrom' drivers must be used instead.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  block/file-posix.c   | 17 ++---
>  docs/system/deprecated.rst   |  7 ---
>  docs/system/removed-features.rst |  7 +++
>  tests/qemu-iotests/226.out   | 10 +-
>  4 files changed, 18 insertions(+), 23 deletions(-)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 05079b40ca..20e14f8e96 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -719,15 +719,9 @@ static int raw_open_common(BlockDriverState *bs, QDict 
> *options,
>  }
>  
>  if (!device) {
> -if (S_ISBLK(st.st_mode)) {
> -warn_report("Opening a block device as a file using the '%s' "
> -"driver is deprecated", bs->drv->format_name);
> -} else if (S_ISCHR(st.st_mode)) {
> -warn_report("Opening a character device as a file using the '%s' 
> "
> -"driver is deprecated", bs->drv->format_name);
> -} else if (!S_ISREG(st.st_mode)) {
> -error_setg(errp, "A regular file was expected by the '%s' 
> driver, "
> -   "but something else was given", bs->drv->format_name);
> +if (!S_ISREG(st.st_mode)) {
> +error_setg(errp, "'%s' driver requires '%s' to be a regular 
> file",
> +   bs->drv->format_name, bs->filename);
>  ret = -EINVAL;
>  goto fail;
>  } else {
> @@ -736,8 +730,9 @@ static int raw_open_common(BlockDriverState *bs, QDict 
> *options,
>  }
>  } else {
>  if (!(S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode))) {
> -error_setg(errp, "'%s' driver expects either "
> -   "a character or block device", bs->drv->format_name);
> +error_setg(errp, "'%s' driver requires '%s' to be either "
> +   "a character or block device",
> +   bs->drv->format_name, bs->filename);
>  ret = -EINVAL;
>  goto fail;
>  }
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index dc76584e02..3a86deb450 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -21,13 +21,6 @@ deprecated.
>  System emulator command line arguments
>  --
>  
> -``-drive file=json:{...{'driver':'file'}}`` (since 3.0)
> -'''
> -
> -The 'file' driver for drives is no longer appropriate for character or host
> -devices and will only accept regular files (S_IFREG). The correct driver
> -for these file types is 'host_cdrom' or 'host_device' as appropriate.
> -
>  ``QEMU_AUDIO_`` environment variables and ``-audio-help`` (since 4.0)
>  '
>  
> diff --git a/docs/system/removed-features.rst 
> b/docs/system/removed-features.rst
> index 990bf7e015..1c9e384cb0 100644
> --- a/docs/system/removed-features.rst
> +++ b/docs/system/removed-features.rst
> @@ -59,6 +59,13 @@ would automatically enable USB support on the machine type.
>  When using the new syntax, USB support must be explicitly
>  enabled via the ``-machine usb=on`` argument.
>  
> +``-drive file=json:{...{'driver':'file'}}`` (removed 6.0)
> +'
> +
> +The 'file' driver for drives is no longer appropriate for character or host
> +devices and will only accept regular files (S_IFREG). The correct driver
> +for these file types is 'host_cdrom' or 'host_device' as appropriate.
> +
>  QEMU Machine Protocol (QMP) commands
>  
>  
> diff --git a/tests/qemu-iotests/226.out b/tests/qemu-iotests/226.out
> index 42be973ff2..55504d29c4 100644
> --- a/tests/qemu-iotests/226.out
> +++ b/tests/qemu-iotests/226.out
> @@ -3,23 +3,23 @@ QA output created by 226
>  === Testing with driver:file ===
>  
>  == Testing RO ==
> -qemu-io: can't open: A regular file was expected by the 'file' driver, but 
> something else was given
> -qemu-io: warning: Opening a character device as a file using the 'file' 
> driver is deprecated
> +qemu-io: can't open: 'file' driver requires 'TEST_DIR/t.IMGFMT' to be a 
> regular file
> +qemu-io: can't open: 'file' driver requires '/dev/null' to be a regular file
>  == Testing RW ==
>  qemu-io: can't open: Could not open 'TEST_DIR/t.IMGFMT': Is a directory
> -qemu-io: warning: Opening a character device as a file using the 'file' 
> driver is deprecated
> +qemu-io: can't open: 'file' driver requires '/dev/null' to be a regular file
>  
>  === Testing with driver:host_device ===
>  
>  == Testing RO ==
> -qemu-io: can't open: 'host_device' driver expects either a character or 
> 

Re: [PATCH 07/14] machine: remove 'arch' field from 'query-cpus-fast' QMP command

[Thomas Huth]

On 24/02/2021 14.11, Daniel P. Berrangé wrote:

Signed-off-by: Daniel P. Berrangé 
---
  docs/system/deprecated.rst   |  6 -
  docs/system/removed-features.rst |  6 +
  hw/core/machine-qmp-cmds.c   | 41 
  qapi/machine.json| 22 -
  4 files changed, 6 insertions(+), 69 deletions(-)


Patch looks reasonable to me.

Reviewed-by: Thomas Huth 

Re: [PATCH v3 22/30] qom: Factor out user_creatable_process_cmdline()

[Kevin Wolf]

Am 13.03.2021 um 09:41 hat Markus Armbruster geschrieben:
> Observation, not objection:
> 
> 1. QMP core parses JSON text into QObject, passes to generated
>marshaller.
> 
> 2. Marshaller converts QObject to ObjectOptions with the QObject input
>visitor, passes to qmp_object_add().
> 
> 3. qmp_object_add() wraps around user_creatable_add_qapi().
> 
> 4. user_creatable_add_qapi() converts right back to QObject with the
>QObject output visitor.  It splits the result into qom_type, id and
>the rest, and passes all three to user_creatable_add_type().
> 
> 5. user_creatable_add_type() performs a virtual visit with the QObject
>input visitor.  The outermost object it visits itself, its children
>it visits by calling object_property_set().
> 
> I sure hope we wouldn't write it this way from scratch :)
> 
> I think your patch is a reasonable step towards a QOM that is at peace
> with QAPI.  But there's plenty of work left.

Yes, obviously the conversion back to QObject is not great. There are
two reasons why we currently need it:

1. user_creatable_add_type() wants to iterate over all properties
   without knowing which properties exist. This should be fixed by not
   visiting the top level in user_creatable_add_type(), but moving this
   part to object-specific code (in the final state probably code
   generated from the QAPI schema).

2. We have ObjectOptions, but need to pass a visitor. We don't have a
   general purpose visitor that visits both sides. The clone visitor
   seems somewhat similar to what we need, but I seem to remember it was
   more restricted to its particular use case.

Kevin

Re: [PATCH v3 26/30] qemu-img: Use user_creatable_process_cmdline() for --object

[Kevin Wolf]

Am 13.03.2021 um 13:30 hat Markus Armbruster geschrieben:
> Paolo Bonzini  writes:
> 
> > On 13/03/21 08:40, Markus Armbruster wrote:
> >>> +if (!user_creatable_add_from_str(optarg, &local_err)) {
> >>> +if (local_err) {
> >>> +error_report_err(local_err);
> >>> +exit(2);
> >>> +} else {
> >>> +/* Help was printed */
> >>> +exit(EXIT_SUCCESS);
> >>> +}
> >>> +}
> >>> +break;
> >>>   }
> >>> -}   break;
> >>>   case OPTION_IMAGE_OPTS:
> >>>   image_opts = true;
> >>>   break;
> >> Why is this one different?  The others all call
> >> user_creatable_process_cmdline().
> >> 
> >> 
> >
> > It's to exit with status code 2 instead of 1.
> 
> I see.  Worth a comment?

There is a comment at the start of the function (which is just a few
lines above) that explains the exit codes:

 * Compares two images. Exit codes:
 *
 * 0 - Images are identical or the requested help was printed
 * 1 - Images differ
 * >1 - Error occurred

Kevin

Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add

[Kevin Wolf]

Am 13.03.2021 um 14:40 hat Markus Armbruster geschrieben:
> Markus Armbruster  writes:
> 
> > Paolo Bonzini  writes:
> >
> >> On 11/03/21 15:08, Markus Armbruster wrote:
>  I would rather keep the OptsVisitor here.  Do the same check for JSON
>  syntax that you have in qobject_input_visitor_new_str, and whenever
>  you need to walk all -object arguments, use something like this:
> 
>   typedef struct ObjectArgument {
>   const char *id;
>   QDict *json;/* or NULL for QemuOpts */
>   QSIMPLEQ_ENTRY(ObjectArgument) next;
>   }
> 
>  I already had patches in my queue to store -object in a GSList of
>  dictionaries, changing it to use the above is easy enough.
> >>> 
> >>> I think I'd prefer following -display's precedence.  See my reply to
> >>> Kevin for details.
> >>
> >> Yeah, I got independently to the same conclusion and posted patches
> >> for that.  I was scared that visit_type_ObjectOptions was too much for 
> >> OptsVisitor but it seems to work...
> >
> > We have reason to be scared.  I'll try to cover this in my review.
> 
> The opts visitor has serious limitations.  From its header:
> 
>  * The Opts input visitor does not implement support for visiting QAPI
>  * alternates, numbers (other than integers), null, or arbitrary
>  * QTypes.  It also requires a non-null list argument to
>  * visit_start_list().
> 
> This is retro-documentation for hairy code.  I don't trust it.  Commit
> eb7ee2cbeb "qapi: introduce OptsVisitor" hints at additional
> restrictions:
> 
> The type tree in the schema, corresponding to an option with a
> discriminator, must have the following structure:
> 
>   struct
> scalar member for non-discriminated optarg 1 [*]
> list for repeating non-discriminated optarg 2 [*]
>   wrapper struct
> single scalar member
> union
>   struct for discriminator case 1
> scalar member for optarg 3 [*]
> list for repeating optarg 4 [*]
>   wrapper struct
> single scalar member
> scalar member for optarg 5 [*]
>   struct for discriminator case 2
> ...

Is this a long-winded way of saying that it has to be flat, except that
it allows lists, i.e. there must be no nested objects on the "wire"?

The difference between structs and unions, and different branches inside
the union isn't visible for the visitor anyway.

> The "type" optarg name is fixed for the discriminator role. Its schema
> representation is "union of structures", and each discriminator value must
> correspond to a member name in the union.
> 
> If the option takes no "type" descriminator, then the type subtree rooted
> at the union must be absent from the schema (including the union itself).
> 
> Optarg values can be of scalar types str / bool / integers / size.
> 
> Unsupported visits are treated as programming error.  Which is a nice
> way to say "they crash".

The OptsVisitor never seems to crash explicitly by calling something
like abort().

It may crash because of missing callbacks that are called without a NULL
check, like v->type_null. This case should probably be fixed in
qapi/qapi-visit-core.c to do the check and simply return an error.

Any other cases?

> Before this series, we use it for -object as follows.
> 
> user_creatable_add_opts() massages the QemuOpts into a QDict containing
> just the properties, then calls user_creatable_add_type() with the opts
> visitor wrapped around the QemuOpts, and the QDict.
> 
> user_creatable_add_type() performs a virtual visit.  The outermost
> object it visits itself.  Then it visits members one by one by calling
> object_property_set().  It uses the QDict as a list of members to visit.
> 
> As long as the object_property_set() only visit scalars other than
> floating-point numbers, we safely stay with the opts visitors'
> limitations.

Minor addition: This visits inside object_property_set() are
non-virtual, of course.

> After this series, we use the opts visitor to convert the option
> argument to a ObjectOption.  This is a non-virtual visit.  We then
> convert the ObjectOption to a QDict, and call user_creatable_add_type()
> with the QObject input visitor wrapped around the QDict, and the QDict.
> 
> Here's the difference in opts visitor use: before the patch, we visit
> exactly the members in the optarg that actually name QOM properties (for
> the ones that don't, object_property_set() fails without visiting
> anything).  Afterwards, we visit the members of ObjectOption, i.e.
> all QOM properties, by construction of ObjectOption.
> 
> As long as ObjectOption's construction is correct, the series does not
> add new visits, i.e. we're no worse off than before.
> 
> However, there is now a new way to mess things up: you can change (a
> branch of union) ObjectOption in a way that pushes it beyond the opts
> visitors limi

Re: [libvirt PATCH v2 1/4] util: Try to get limits from /proc

[Andrea Bolognani]

On Mon, 2021-03-15 at 11:57 +0100, Andrea Bolognani wrote:
> The changes you're suggesting are not trivial enough for me to feel
> comfortable simply applying them locally and then pushing right away.
> Would the diff below look reasonable squashed in?
> 
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index 4fa854090d..cae0dabae3 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -799,16 +799,17 @@ virProcessGetLimitFromProc(pid_t pid,
>  size_t i;
> 
>  if (!(label = virProcessLimitResourceToLabel(resource))) {
> +virReportSystemError(EINVAL, "%s", _("Invalid resource"));
>  return -1;
>  }
> 
>  procfile = g_strdup_printf("/proc/%lld/limits", (long long)pid);
> 
>  if (virFileReadAllQuiet(procfile, 2048, &buf) < 0)
> -return -1;
> +goto error;
> 
[...]
> 
> + error:
> +virReportSystemError(EIO, "%s", _("Input/output error"));
> +return -1;
>  }
>  # else /* !defined(__linux__) */
>  static int
> @@ -846,6 +851,7 @@ virProcessGetLimitFromProc(pid_t pid G_GNUC_UNUSED,
> int resource G_GNUC_UNUSED,
> struct rlimit *limit G_GNUC_UNUSED)
>  {
> +virReportSystemError(ENOSYS, "%s", _("Not supported on this platform"));
>  return -1;
>  }
>  # endif /* !defined(__linux__) */

This probably makes more sense:

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 4fa854090d..b173856b7a 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -799,16 +799,17 @@ virProcessGetLimitFromProc(pid_t pid,
 size_t i;

 if (!(label = virProcessLimitResourceToLabel(resource))) {
+errno = EINVAL;
 return -1;
 }

 procfile = g_strdup_printf("/proc/%lld/limits", (long long)pid);

 if (virFileReadAllQuiet(procfile, 2048, &buf) < 0)
-return -1;
+goto error;

 if (!(lines = g_strsplit(buf, "\n", 0)))
-return -1;
+goto error;

 for (i = 0; lines[i]; i++) {
 g_autofree char *softLimit = NULL;
@@ -820,25 +821,29 @@ virProcessGetLimitFromProc(pid_t pid,
 continue;
 
 if (sscanf(line, "%ms %ms %*s", &softLimit, &hardLimit) < 2)
-return -1;
+goto error;
 
 if (STREQ(softLimit, "unlimited")) {
 limit->rlim_cur = RLIM_INFINITY;
 } else {
 if (virStrToLong_ull(softLimit, NULL, 10, &tmp) < 0)
-return -1;
+goto error;
 limit->rlim_cur = tmp;
 }
 if (STREQ(hardLimit, "unlimited")) {
 limit->rlim_max = RLIM_INFINITY;
 } else {
 if (virStrToLong_ull(hardLimit, NULL, 10, &tmp) < 0)
-return -1;
+goto error;
 limit->rlim_max = tmp;
 }
 }
 
 return 0;
+
+ error:
+errno = EIO;
+return -1;
 }
 # else /* !defined(__linux__) */
 static int
@@ -846,6 +851,7 @@ virProcessGetLimitFromProc(pid_t pid G_GNUC_UNUSED,
int resource G_GNUC_UNUSED,
struct rlimit *limit G_GNUC_UNUSED)
 {
+errno = ENOSYS;
 return -1;
 }
 # endif /* !defined(__linux__) */
-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH v3 27/30] hmp: QAPIfy object_add

[Kevin Wolf]

Am 15.03.2021 um 10:39 hat Markus Armbruster geschrieben:
> Paolo Bonzini  writes:
> 
> > On 13/03/21 14:28, Markus Armbruster wrote:
> >> Kevin Wolf  writes:
> >> 
> >>> This switches the HMP command object_add from a QemuOpts-based parser to
> >>> user_creatable_add_from_str() which uses a keyval parser and enforces
> >>> the QAPI schema.
> >>>
> >>> Apart from being a cleanup, this makes non-scalar properties and help
> >>> accessible. In order for help to be printed to the monitor instead of
> >>> stdout, the printf() calls in the help functions are changed to
> >>> qemu_printf().
> >>>
> >>> Signed-off-by: Kevin Wolf 
> >>> Acked-by: Peter Krempa 
> >>> Reviewed-by: Eric Blake 
> >>> Reviewed-by: Dr. David Alan Gilbert 
> >>> ---
> >>>   monitor/hmp-cmds.c  | 17 ++---
> >>>   qom/object_interfaces.c | 11 ++-
> >>>   hmp-commands.hx |  2 +-
> >>>   3 files changed, 9 insertions(+), 21 deletions(-)
> >>>
> >>> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> >>> index 3c88a4faef..652cf9ff21 100644
> >>> --- a/monitor/hmp-cmds.c
> >>> +++ b/monitor/hmp-cmds.c
> >>> @@ -1670,24 +1670,11 @@ void hmp_netdev_del(Monitor *mon, const QDict 
> >>> *qdict)
> >>>   
> >>>   void hmp_object_add(Monitor *mon, const QDict *qdict)
> >>>   {
> >>> +const char *options = qdict_get_str(qdict, "object");
> >>>   Error *err = NULL;
> >>> -QemuOpts *opts;
> >>> -Object *obj = NULL;
> >>> -
> >>> -opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
> >>> -if (err) {
> >>> -goto end;
> >>> -}
> >>>   
> >>> -obj = user_creatable_add_opts(opts, &err);
> >>> -qemu_opts_del(opts);
> >>> -
> >>> -end:
> >>> +user_creatable_add_from_str(options, &err);
> >>>   hmp_handle_error(mon, err);
> >>> -
> >>> -if (obj) {
> >>> -object_unref(obj);
> >>> -}
> >>>   }
> >> 
> >> Doesn't this break the list-valued properties (Memdev member host-nodes,
> >> NumaNodeOptions member cpus) exactly the same way that made us keep
> >> QemuOpts for qemu-system-FOO -object?
> >
> > Yes, it does.  I guess it can just be documented, unlike for the command 
> > line?
> 
> Maybe.  Judgement call, not mine to make.
> 
> Do people create such objects in HMP?  I figure we don't really know.
> Educated guess?
> 
> If you try, how does it break?  Is it confusing?  Can you show an
> example?

(qemu) object_add memory-backend-ram,id=mem,size=4G,policy=bind,host-nodes=0
Error: Invalid parameter type for 'host-nodes', expected: array
(qemu) object_add memory-backend-ram,id=mem,size=4G,policy=bind,host-nodes.0=0
(qemu)

HMP is not a stable interface, so changing the syntax didn't feel like a
problem to me. I doubt many people do HMP memory hotplug while setting a
specific NUMA policy, but it wouldn't change my assessment anyway. I
should have made this explicit in the commit message, though.

Kevin

[PATCH] target/mips: Deprecate Trap-and-Emul KVM support

[Jiaxun Yang]

Upstream kernel had removed both host[1] and guest[2] support.

[1]: 
https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/commit/?id=45c7e8af4a5e3f0bea4ac209eea34118dd57ac64
[2]: 
https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/commit/?id=a1515ec7204edca770c07929df8538fcdb03ad46

Signed-off-by: Jiaxun Yang 
---
 docs/system/deprecated.rst | 8 
 1 file changed, 8 insertions(+)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index cfabe69846..a409c65485 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -496,3 +496,11 @@ nanoMIPS ISA
 
 The ``nanoMIPS`` ISA has never been upstreamed to any compiler toolchain.
 As it is hard to generate binaries for it, declare it deprecated.
+
+KVM features
+---
+
+MIPS Trap-and-Emul KVM support
+
+The MIPS ``Trap-and-Emul`` KVM host and guest support has been removed
+from upstream kernel, declare it deprecated.
-- 
2.30.1

Re: [libvirt PATCH v2 1/4] util: Try to get limits from /proc

[Andrea Bolognani]

On Mon, 2021-03-15 at 10:21 +0100, Michal Privoznik wrote:
> On 3/9/21 2:47 PM, Andrea Bolognani wrote:
> > +static int
> > +virProcessGetLimitFromProc(pid_t pid,
> > +   int resource,
> > +   struct rlimit *limit)
> > +{
> > +g_autofree char *procfile = NULL;
> > +g_autofree char *buf = NULL;
> > +g_auto(GStrv) lines = NULL;
> > +const char *label;
> > +size_t i;
> > +
> > +if (!(label = virProcessLimitResourceToLabel(resource))) {
> > +return -1;
> > +}
> 
> While I am in favor of curly braces, this is incosistent with the rest 
> of the function. However, you'll need to add another line here, probably 
> so in the end you'll need to keep them. See [1].
> 
> > @@ -768,6 +861,15 @@ virProcessGetLimit(pid_t pid,
> >   if (virProcessPrLimit(pid, resource, NULL, old_limit) == 0)
> >   return 0;
> >   
> > +/* For whatever reason, using prlimit() on another process - even
> > + * when it's just to obtain the current limit rather than changing
> > + * it - requires CAP_SYS_RESOURCE, which we might not have in a
> > + * containerized environment; on the other hand, no particular
> > + * permission is needed to poke around /proc, so try that if going
> > + * through the syscall didn't work */
> > +if (virProcessGetLimitFromProc(pid, resource, old_limit) == 0)
> > +return 0;
> 
> There is just a single caller of this virProcessGetLimit() function and 
> it expects errno to be set on failure. But that's not always the case 
> with your patch. Moreover, I'd expect the !Linux stub to set ENOSYS.

Good catch!

The changes you're suggesting are not trivial enough for me to feel
comfortable simply applying them locally and then pushing right away.
Would the diff below look reasonable squashed in?

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 4fa854090d..cae0dabae3 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -799,16 +799,17 @@ virProcessGetLimitFromProc(pid_t pid,
 size_t i;

 if (!(label = virProcessLimitResourceToLabel(resource))) {
+virReportSystemError(EINVAL, "%s", _("Invalid resource"));
 return -1;
 }

 procfile = g_strdup_printf("/proc/%lld/limits", (long long)pid);

 if (virFileReadAllQuiet(procfile, 2048, &buf) < 0)
-return -1;
+goto error;

 if (!(lines = g_strsplit(buf, "\n", 0)))
-return -1;
+goto error;

 for (i = 0; lines[i]; i++) {
 g_autofree char *softLimit = NULL;
@@ -820,25 +821,29 @@ virProcessGetLimitFromProc(pid_t pid,
 continue;

 if (sscanf(line, "%ms %ms %*s", &softLimit, &hardLimit) < 2)
-return -1;
+goto error;

 if (STREQ(softLimit, "unlimited")) {
 limit->rlim_cur = RLIM_INFINITY;
 } else {
 if (virStrToLong_ull(softLimit, NULL, 10, &tmp) < 0)
-return -1;
+goto error;
 limit->rlim_cur = tmp;
 }
 if (STREQ(hardLimit, "unlimited")) {
 limit->rlim_max = RLIM_INFINITY;
 } else {
 if (virStrToLong_ull(hardLimit, NULL, 10, &tmp) < 0)
-return -1;
+goto error;
 limit->rlim_max = tmp;
 }
 }

 return 0;
+
+ error:
+virReportSystemError(EIO, "%s", _("Input/output error"));
+return -1;
 }
 # else /* !defined(__linux__) */
 static int
@@ -846,6 +851,7 @@ virProcessGetLimitFromProc(pid_t pid G_GNUC_UNUSED,
int resource G_GNUC_UNUSED,
struct rlimit *limit G_GNUC_UNUSED)
 {
+virReportSystemError(ENOSYS, "%s", _("Not supported on this platform"));
 return -1;
 }
 # endif /* !defined(__linux__) */
-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH 9/9] docs: html.in: Convert 'compiling' to rst

[Peter Krempa]

On Fri, Mar 12, 2021 at 12:43:12 +0100, Erik Skultety wrote:
> Signed-off-by: Erik Skultety 
> ---
>  docs/compiling.html.in | 115 -
>  docs/compiling.rst |  95 ++
>  docs/meson.build   |   2 +-
>  3 files changed, 96 insertions(+), 116 deletions(-)
>  delete mode 100644 docs/compiling.html.in
>  create mode 100644 docs/compiling.rst

Reviewed-by: Peter Krempa 

Re: [libvirt PATCH 8/9] docs: html.in: Convert bindings to rst

[Peter Krempa]

On Fri, Mar 12, 2021 at 12:43:11 +0100, Erik Skultety wrote:
> Signed-off-by: Erik Skultety 
> ---
>  docs/bindings.html.in | 101 --
>  docs/bindings.rst |  62 ++
>  docs/meson.build  |   2 +-
>  3 files changed, 63 insertions(+), 102 deletions(-)
>  delete mode 100644 docs/bindings.html.in
>  create mode 100644 docs/bindings.rst

Reviewed-by: Peter Krempa 

Re: [libvirt PATCH 6/9] docs: html.in: Convert auditlog to rst

[Peter Krempa]

On Fri, Mar 12, 2021 at 12:43:09 +0100, Erik Skultety wrote:
> Signed-off-by: Erik Skultety 
> ---
>  docs/auditlog.html.in | 375 --
>  docs/auditlog.rst | 321 
>  docs/meson.build  |   2 +-
>  3 files changed, 322 insertions(+), 376 deletions(-)
>  delete mode 100644 docs/auditlog.html.in
>  create mode 100644 docs/auditlog.rst

Reviewed-by: Peter Krempa 

Re: [libvirt PATCH 5/9] docs: html.in: Convert architecture to rst

[Peter Krempa]

On Fri, Mar 12, 2021 at 13:56:06 +0100, Peter Krempa wrote:
> On Fri, Mar 12, 2021 at 12:43:08 +0100, Erik Skultety wrote:
> > Signed-off-by: Erik Skultety 
> > ---
> >  docs/architecture.html.in | 82 --
> >  docs/architecture.rst | 83 +++
> >  docs/meson.build  |  2 +-
> >  3 files changed, 84 insertions(+), 83 deletions(-)
> >  delete mode 100644 docs/architecture.html.in
> >  create mode 100644 docs/architecture.rst
> 
> N.B.: This file doesn't seem to be linked from anywhere on our webpage.
> Search finds it by name, but if you don't know what you are looking for
> it's pretty hidden.

I wanted to suggest to convert it to a kbase article since it's unlinked
but now that I had a look at the contents, the documents seems very
outdated.

As of such I'd probably completely remove it at this point.

Re: [libvirt PATCH 4/9] docs: html.in: Convert apps to rst

[Peter Krempa]

On Fri, Mar 12, 2021 at 12:43:07 +0100, Erik Skultety wrote:
> Signed-off-by: Erik Skultety 
> ---
>  docs/apps.html.in | 488 --
>  docs/apps.rst | 348 +
>  docs/meson.build  |   2 +-
>  3 files changed, 349 insertions(+), 489 deletions(-)
>  delete mode 100644 docs/apps.html.in
>  create mode 100644 docs/apps.rst

Reviewed-by: Peter Krempa 

Re: [libvirt PATCH 3/9] docs: html.in: Convert api to rst

[Peter Krempa]

On Fri, Mar 12, 2021 at 12:43:06 +0100, Erik Skultety wrote:
> There were a number of occurrences where we used nested inline markup
> (verbatim + refs) which is currently not possible with RST syntax [1].
> There is a possible workaround involving substitution definitions like
> 
>   .. |virConnectPtr| replace:: ``virConnectPtr``
>   .. _virConnectPtr: /html/libvirt-libvirt-host.html#virConnectPtr
> 
> Substitutions cannot be made generic, hence we cannot create a template
> for substitution and use a single template everywhere, so we'd end up
> with a lot of clutter and convolution. Therefore, we can make an
> exception and just link the data type without further style markup.
> 
> [1] https://docutils.sourceforge.io/FAQ.html#is-nested-inline-markup-possible
> 
> Signed-off-by: Erik Skultety 
> ---
>  docs/api.html.in | 380 ---
>  docs/api.rst | 265 +
>  docs/meson.build |   2 +-
>  3 files changed, 266 insertions(+), 381 deletions(-)
>  delete mode 100644 docs/api.html.in
>  create mode 100644 docs/api.rst

Reviewed-by: Peter Krempa 

Re: [libvirt PATCH 2/9] docs: html.in: Convert api_extension to rst

[Peter Krempa]

On Fri, Mar 12, 2021 at 12:43:05 +0100, Erik Skultety wrote:
> Signed-off-by: Erik Skultety 
> ---
>  docs/api_extension.html.in | 376 -
>  docs/api_extension.rst | 291 
>  docs/meson.build   |   2 +-
>  3 files changed, 292 insertions(+), 377 deletions(-)
>  delete mode 100644 docs/api_extension.html.in
>  create mode 100644 docs/api_extension.rst

Reviewed-by: Peter Krempa 

Re: [libvirt PATCH 1/9] docs: html.in: Convert aclpolkit to rst

[Peter Krempa]

On Fri, Mar 12, 2021 at 12:43:04 +0100, Erik Skultety wrote:
> Signed-off-by: Erik Skultety 
> ---
>  docs/aclpolkit.html.in | 523 -
>  docs/aclpolkit.rst | 310 
>  docs/meson.build   |   2 +-
>  3 files changed, 311 insertions(+), 524 deletions(-)
>  delete mode 100644 docs/aclpolkit.html.in
>  create mode 100644 docs/aclpolkit.rst

Reviewed-by: Peter Krempa 

Re: [PATCH] bhyve: add support

[Michal Privoznik]

On 2/27/21 5:34 AM, Roman Bogorodskiy wrote:

Implement "" support for bhyve driver.
As there are not really lot of options, try to find
"BHYVE_UEFI.fd" firmware which is installed by the
sysutils/uefi-edk2-bhyve FreeBSD port.

If not found, just use the first found firmware
in the firmwares directory (which is configurable via
config file).

Signed-off-by: Roman Bogorodskiy 
---
Not extremely happy about the LIBVIRT_BHYVE_FIRMWARE_DIR_OVERRIDE knob,
but not sure how to test this otherwise.

  po/POTFILES.in|  1 +
  src/bhyve/bhyve_domain.c  |  5 +
  src/bhyve/bhyve_firmware.c| 91 +++
  src/bhyve/bhyve_firmware.h| 30 ++
  src/bhyve/bhyve_process.c | 15 +++
  src/bhyve/bhyve_process.h |  5 +
  src/bhyve/meson.build |  1 +
  .../bhyvexml2argv-firmware-efi.args   | 11 +++
  .../bhyvexml2argv-firmware-efi.ldargs |  1 +
  .../bhyvexml2argv-firmware-efi.xml| 22 +
  tests/bhyvexml2argvtest.c | 83 ++---
  11 files changed, 254 insertions(+), 11 deletions(-)
  create mode 100644 src/bhyve/bhyve_firmware.c
  create mode 100644 src/bhyve/bhyve_firmware.h
  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.args
  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.ldargs
  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-firmware-efi.xml

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 80c5f145be..413783ee35 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -14,6 +14,7 @@
  @SRCDIR@src/bhyve/bhyve_command.c
  @SRCDIR@src/bhyve/bhyve_domain.c
  @SRCDIR@src/bhyve/bhyve_driver.c
+@SRCDIR@src/bhyve/bhyve_firmware.c
  @SRCDIR@src/bhyve/bhyve_monitor.c
  @SRCDIR@src/bhyve/bhyve_parse_command.c
  @SRCDIR@src/bhyve/bhyve_process.c
diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c
index 8fbc554a0a..209e4d3905 100644
--- a/src/bhyve/bhyve_domain.c
+++ b/src/bhyve/bhyve_domain.c
@@ -64,6 +64,9 @@ bhyveDomainDefNeedsISAController(virDomainDefPtr def)
  if (def->os.bootloader == NULL && def->os.loader)
  return true;
  
+if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_EFI)

+return true;
+
  if (def->nserials || def->nconsoles)
  return true;
  
@@ -230,6 +233,8 @@ virDomainDefParserConfig virBhyveDriverDomainDefParserConfig = {

  .domainPostParseCallback = bhyveDomainDefPostParse,
  .assignAddressesCallback = bhyveDomainDefAssignAddresses,
  .deviceValidateCallback = bhyveDomainDeviceDefValidate,
+
+.features = VIR_DOMAIN_DEF_FEATURE_FW_AUTOSELECT,
  };
  
  static void

diff --git a/src/bhyve/bhyve_firmware.c b/src/bhyve/bhyve_firmware.c
new file mode 100644
index 00..ccc3a5ffc8
--- /dev/null
+++ b/src/bhyve/bhyve_firmware.c
@@ -0,0 +1,91 @@
+/*
+ * bhyve_firmware.c: bhyve firmware management
+ *
+ * Copyright (C) 2021 Roman Bogorodskiy
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ */
+
+#include 
+#include 
+
+#include "viralloc.h"
+#include "virlog.h"
+#include "virfile.h"
+#include "bhyve_conf.h"
+#include "bhyve_firmware.h"
+
+#define VIR_FROM_THIS   VIR_FROM_BHYVE
+
+VIR_LOG_INIT("bhyve.bhyve_firmware");
+
+
+#define BHYVE_DEFAULT_FIRMWARE  "BHYVE_UEFI.fd"
+
+int
+bhyveFirmwareFillDomain(bhyveConnPtr driver,
+virDomainDefPtr def,
+unsigned int flags)
+{
+g_autoptr(DIR) dir = NULL;
+virBhyveDriverConfigPtr cfg = virBhyveDriverGetConfig(driver);


virBhyveDriverGetConfig() returns a reference, thus needs to be coupled 
with virObjectUnref(cfg); otherwise .. [1]



+const char *firmware_dir_cfg = cfg->firmwareDir;
+const char *firmware_dir_env = NULL, *firmware_dir = NULL;


One variable per line, please. It turned out to be useful (although in 
this specific case it's not using virXXXPtr type so doesn't matter):


https://listman.redhat.com/archives/libvir-list/2021-March/msg00542.html


+struct dirent *entry;
+char *matching_firmware = NULL;
+char *first_found = NULL;
+
+virCheckFlags(0, -1);


1: .. @cfg is leaked here .. [2]


+
+if (def->os.firmware == VIR_DOMAIN_OS_DEF_FIRMWARE_NONE)
+return 0;


2: .. or here. You get the idea. A

Re: [PATCH v3 27/30] hmp: QAPIfy object_add

[Markus Armbruster]

Paolo Bonzini  writes:

> On 13/03/21 14:28, Markus Armbruster wrote:
>> Kevin Wolf  writes:
>> 
>>> This switches the HMP command object_add from a QemuOpts-based parser to
>>> user_creatable_add_from_str() which uses a keyval parser and enforces
>>> the QAPI schema.
>>>
>>> Apart from being a cleanup, this makes non-scalar properties and help
>>> accessible. In order for help to be printed to the monitor instead of
>>> stdout, the printf() calls in the help functions are changed to
>>> qemu_printf().
>>>
>>> Signed-off-by: Kevin Wolf 
>>> Acked-by: Peter Krempa 
>>> Reviewed-by: Eric Blake 
>>> Reviewed-by: Dr. David Alan Gilbert 
>>> ---
>>>   monitor/hmp-cmds.c  | 17 ++---
>>>   qom/object_interfaces.c | 11 ++-
>>>   hmp-commands.hx |  2 +-
>>>   3 files changed, 9 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
>>> index 3c88a4faef..652cf9ff21 100644
>>> --- a/monitor/hmp-cmds.c
>>> +++ b/monitor/hmp-cmds.c
>>> @@ -1670,24 +1670,11 @@ void hmp_netdev_del(Monitor *mon, const QDict 
>>> *qdict)
>>>   
>>>   void hmp_object_add(Monitor *mon, const QDict *qdict)
>>>   {
>>> +const char *options = qdict_get_str(qdict, "object");
>>>   Error *err = NULL;
>>> -QemuOpts *opts;
>>> -Object *obj = NULL;
>>> -
>>> -opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
>>> -if (err) {
>>> -goto end;
>>> -}
>>>   
>>> -obj = user_creatable_add_opts(opts, &err);
>>> -qemu_opts_del(opts);
>>> -
>>> -end:
>>> +user_creatable_add_from_str(options, &err);
>>>   hmp_handle_error(mon, err);
>>> -
>>> -if (obj) {
>>> -object_unref(obj);
>>> -}
>>>   }
>> 
>> Doesn't this break the list-valued properties (Memdev member host-nodes,
>> NumaNodeOptions member cpus) exactly the same way that made us keep
>> QemuOpts for qemu-system-FOO -object?
>
> Yes, it does.  I guess it can just be documented, unlike for the command 
> line?

Maybe.  Judgement call, not mine to make.

Do people create such objects in HMP?  I figure we don't really know.
Educated guess?

If you try, how does it break?  Is it confusing?  Can you show an
example?

Re: [libvirt PATCH v2 1/4] util: Try to get limits from /proc

[Michal Privoznik]

On 3/9/21 2:47 PM, Andrea Bolognani wrote:

Calling prlimit() requires elevated privileges, specifically
CAP_SYS_RESOURCE, and getrlimit() only works for the current
process which is too limiting for our needs; /proc/$pid/limits,
on the other hand, can be read by any process, so implement
parsing that file as a fallback for when prlimit() fails.

This is useful in containerized environments.

Signed-off-by: Andrea Bolognani 
---
  src/util/virprocess.c | 102 ++
  1 file changed, 102 insertions(+)

diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 8428c91182..4fa854090d 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -757,6 +757,99 @@ virProcessSetRLimit(int resource,
  #endif /* WITH_SETRLIMIT */
  
  #if WITH_GETRLIMIT

+static const char*
+virProcessLimitResourceToLabel(int resource)
+{
+switch (resource) {
+# if defined(RLIMIT_MEMLOCK)
+case RLIMIT_MEMLOCK:
+return "Max locked memory";
+# endif /* defined(RLIMIT_MEMLOCK) */
+
+# if defined(RLIMIT_NPROC)
+case RLIMIT_NPROC:
+return "Max processes";
+# endif /* defined(RLIMIT_NPROC) */
+
+# if defined(RLIMIT_NOFILE)
+case RLIMIT_NOFILE:
+return "Max open files";
+# endif /* defined(RLIMIT_NOFILE) */
+
+# if defined(RLIMIT_CORE)
+case RLIMIT_CORE:
+return "Max core file size";
+# endif /* defined(RLIMIT_CORE) */
+
+default:
+return NULL;
+}
+}
+
+# if defined(__linux__)
+static int
+virProcessGetLimitFromProc(pid_t pid,
+   int resource,
+   struct rlimit *limit)
+{
+g_autofree char *procfile = NULL;
+g_autofree char *buf = NULL;
+g_auto(GStrv) lines = NULL;
+const char *label;
+size_t i;
+
+if (!(label = virProcessLimitResourceToLabel(resource))) {
+return -1;
+}


While I am in favor of curly braces, this is incosistent with the rest 
of the function. However, you'll need to add another line here, probably 
so in the end you'll need to keep them. See [1].



+
+procfile = g_strdup_printf("/proc/%lld/limits", (long long)pid);
+
+if (virFileReadAllQuiet(procfile, 2048, &buf) < 0)
+return -1;
+
+if (!(lines = g_strsplit(buf, "\n", 0)))
+return -1;
+
+for (i = 0; lines[i]; i++) {
+g_autofree char *softLimit = NULL;
+g_autofree char *hardLimit = NULL;
+char *line = lines[i];
+unsigned long long tmp;
+
+if (!(line = STRSKIP(line, label)))
+continue;
+
+if (sscanf(line, "%ms %ms %*s", &softLimit, &hardLimit) < 2)
+return -1;
+
+if (STREQ(softLimit, "unlimited")) {
+limit->rlim_cur = RLIM_INFINITY;
+} else {
+if (virStrToLong_ull(softLimit, NULL, 10, &tmp) < 0)
+return -1;
+limit->rlim_cur = tmp;
+}
+if (STREQ(hardLimit, "unlimited")) {
+limit->rlim_max = RLIM_INFINITY;
+} else {
+if (virStrToLong_ull(hardLimit, NULL, 10, &tmp) < 0)
+return -1;
+limit->rlim_max = tmp;
+}
+}
+
+return 0;
+}
+# else /* !defined(__linux__) */
+static int
+virProcessGetLimitFromProc(pid_t pid G_GNUC_UNUSED,
+   int resource G_GNUC_UNUSED,
+   struct rlimit *limit G_GNUC_UNUSED)
+{
+return -1;
+}
+# endif /* !defined(__linux__) */
+
  static int
  virProcessGetLimit(pid_t pid,
 int resource,
@@ -768,6 +861,15 @@ virProcessGetLimit(pid_t pid,
  if (virProcessPrLimit(pid, resource, NULL, old_limit) == 0)
  return 0;
  
+/* For whatever reason, using prlimit() on another process - even

+ * when it's just to obtain the current limit rather than changing
+ * it - requires CAP_SYS_RESOURCE, which we might not have in a
+ * containerized environment; on the other hand, no particular
+ * permission is needed to poke around /proc, so try that if going
+ * through the syscall didn't work */
+if (virProcessGetLimitFromProc(pid, resource, old_limit) == 0)
+return 0;


There is just a single caller of this virProcessGetLimit() function and 
it expects errno to be set on failure. But that's not always the case 
with your patch. Moreover, I'd expect the !Linux stub to set ENOSYS.



+
  if (same_process && virProcessGetRLimit(resource, old_limit) == 0)
  return 0;
  



Michal