Re: [PATCH v3 0/3] stdvga: fix screen blanking
On Wed, Jun 5, 2024 at 5:14 PM Gerd Hoffmann wrote: > > > > Gerd Hoffmann (3): > stdvga: fix screen blanking > ui+display: rename is_placeholder() -> surface_is_placeholder() > ui+display: rename is_buffer_shared() -> surface_is_allocated() > > include/ui/surface.h| 6 +++--- > hw/display/qxl-render.c | 2 +- > hw/display/vga.c| 24 +++- > hw/display/xenfb.c | 5 +++-- > ui/console.c| 3 ++- > ui/sdl2-2d.c| 2 +- > ui/sdl2-gl.c| 2 +- > 7 files changed, 26 insertions(+), 18 deletions(-) > for the series: Reviewed-by: Marc-André Lureau
Re: [PATCH v2 1/3] stdvga: fix screen blanking
Hi
On Wed, Jun 5, 2024 at 11:36 AM Gerd Hoffmann wrote:
>
> On Tue, Jun 04, 2024 at 10:27:18AM GMT, Marc-André Lureau wrote:
> > Hi
> >
> > > +if (is_buffer_shared(surface)) {
> >
> > Perhaps the suggestion to rename the function (in the following patch)
> > should instead be surface_is_allocated() ? that would match the actual
> > flag check. But callers would have to ! the result. Wdyt?
>
> surface_is_shadow() ? Comes closer to the typical naming in computer
> graphics.
If the underlying flag is renamed too, that's ok to me.
Re: [PATCH v2 1/3] stdvga: fix screen blanking
Hi
On Mon, Jun 3, 2024 at 7:18 PM Gerd Hoffmann wrote:
>
> In case the display surface uses a shared buffer (i.e. uses vga vram
> directly instead of a shadow) go unshare the buffer before clearing it.
>
> This avoids vga memory corruption, which in turn fixes unblanking not
> working properly with X11.
>
> Cc: qemu-sta...@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2067
> Signed-off-by: Gerd Hoffmann
> ---
> hw/display/vga.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> index 30facc6c8e33..474b6b14c327 100644
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -1762,6 +1762,12 @@ static void vga_draw_blank(VGACommonState *s, int
> full_update)
> if (s->last_scr_width <= 0 || s->last_scr_height <= 0)
> return;
>
> +if (is_buffer_shared(surface)) {
Perhaps the suggestion to rename the function (in the following patch)
should instead be surface_is_allocated() ? that would match the actual
flag check. But callers would have to ! the result. Wdyt?
> +/* unshare buffer, otherwise the blanking corrupts vga vram */
> +surface = qemu_create_displaysurface(s->last_scr_width,
> s->last_scr_height);
> +dpy_gfx_replace_surface(s->con, surface);
Ok, this looks safer than calling "resize".
thanks
> +}
> +
> w = s->last_scr_width * surface_bytes_per_pixel(surface);
> d = surface_data(surface);
> for(i = 0; i < s->last_scr_height; i++) {
> --
> 2.45.1
>
Re: [PATCH v2 1/1] virgl: Implement resource_query_layout
/
> #define VIRTIO_GPU_F_CONTEXT_INIT4
>
> +/*
> + * VIRTIO_GPU_CMD_RESOURCE_QUERY_LAYOUT
> + */
> +#define VIRTIO_GPU_F_RESOURCE_QUERY_LAYOUT 5
> +
> enum virtio_gpu_ctrl_type {
> VIRTIO_GPU_UNDEFINED = 0,
>
> @@ -95,6 +100,7 @@ enum virtio_gpu_ctrl_type {
> VIRTIO_GPU_CMD_SUBMIT_3D,
> VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB,
> VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB,
> + VIRTIO_GPU_CMD_RESOURCE_QUERY_LAYOUT,
>
> /* cursor commands */
> VIRTIO_GPU_CMD_UPDATE_CURSOR = 0x0300,
> @@ -108,6 +114,7 @@ enum virtio_gpu_ctrl_type {
> VIRTIO_GPU_RESP_OK_EDID,
> VIRTIO_GPU_RESP_OK_RESOURCE_UUID,
> VIRTIO_GPU_RESP_OK_MAP_INFO,
> + VIRTIO_GPU_RESP_OK_RESOURCE_LAYOUT,
>
> /* error responses */
> VIRTIO_GPU_RESP_ERR_UNSPEC = 0x1200,
> @@ -455,4 +462,27 @@ struct virtio_gpu_resource_unmap_blob {
> uint32_t padding;
> };
>
> +
> +/* VIRTIO_GPU_CMD_RESOURCE_QUERY_LAYOUT */
> +struct virtio_gpu_resource_query_layout {
> + struct virtio_gpu_ctrl_hdr hdr;
> + uint32_t resource_id;
> + uint32_t width;
> + uint32_t height;
> + uint32_t format;
> + uint32_t bind;
> +};
> +
> +/* VIRTIO_GPU_RESP_OK_RESOURCE_LAYOUT */
> +#define VIRTIO_GPU_RES_MAX_PLANES 4
> +struct virtio_gpu_resp_resource_layout {
> + struct virtio_gpu_ctrl_hdr hdr;
> + uint64_t modifier;
> + uint32_t num_planes;
> + struct virtio_gpu_resource_plane {
> + uint64_t offset;
> + uint32_t stride;
> + } planes[VIRTIO_GPU_RES_MAX_PLANES];
> +};
> +
> #endif
> diff --git a/meson.build b/meson.build
> index a739a62f2c..72024f5f01 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1058,6 +1058,10 @@ if not get_option('virglrenderer').auto() or
> have_system or have_vhost_user_gpu
>
> cc.has_function('virgl_renderer_resource_create_blob',
> prefix: '#include
> ',
> dependencies: virgl))
> +config_host_data.set('HAVE_VIRGL_RESOURCE_QUERY_LAYOUT',
> +
> cc.has_function('virgl_renderer_resource_query_layout',
> + prefix: '#include
> ',
> + dependencies: virgl))
>endif
> endif
> rutabaga = not_found
> --
> 2.34.1
>
>
--
Marc-André Lureau
Re: [PATCH v6 10/11] virtio-gpu: Initialize Venus
Hi
On Tue, Dec 19, 2023 at 11:55 AM Huang Rui wrote:
>
> From: Antonio Caggiano
>
> Request Venus when initializing VirGL.
>
> Signed-off-by: Antonio Caggiano
> Signed-off-by: Huang Rui
> ---
>
> Changes in v6:
> - Remove the unstable API flags check because virglrenderer is already 1.0.
> - Squash the render server flag support into "Initialize Venus".
>
> hw/display/virtio-gpu-virgl.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index f35a751824..c523a6717a 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -964,6 +964,10 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
> }
> #endif
>
> +#ifdef VIRGL_RENDERER_VENUS
> +flags |= VIRGL_RENDERER_VENUS | VIRGL_RENDERER_RENDER_SERVER;
> +#endif
> +
I wonder if it's a good idea to initialize venus by default. It
doesn't seem to require vulkan during initialization, but this may
evolve. Make it optional?
> ret = virgl_renderer_init(g, flags, _gpu_3d_cbs);
> if (ret != 0) {
> error_report("virgl could not be initialized: %d", ret);
> --
> 2.25.1
>
--
Marc-André Lureau
Re: [PATCH v6 08/11] virtio-gpu: Resource UUID
; },
> .post_load = virtio_gpu_post_load,
> @@ -1622,6 +1739,8 @@ static Property virtio_gpu_properties[] = {
> DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
> VIRTIO_GPU_FLAG_BLOB_ENABLED, false),
> DEFINE_PROP_SIZE("hostmem", VirtIOGPU, parent_obj.conf.hostmem, 0),
> +DEFINE_PROP_BIT("resource_uuid", VirtIOGPU, parent_obj.conf.flags,
> +VIRTIO_GPU_FLAG_RESOURCE_UUID_ENABLED, false),
why not enable it by default? (and set it to false for machine < 9.0
> #ifdef HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS
> DEFINE_PROP_BIT("context_init", VirtIOGPU, parent_obj.conf.flags,
> VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, true),
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 584ba2ed73..76b410fe91 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -98,6 +98,7 @@ enum virtio_gpu_base_conf_flags {
> VIRTIO_GPU_FLAG_BLOB_ENABLED,
> VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED,
> VIRTIO_GPU_FLAG_RUTABAGA_ENABLED,
> +VIRTIO_GPU_FLAG_RESOURCE_UUID_ENABLED,
> };
>
> #define virtio_gpu_virgl_enabled(_cfg) \
> @@ -114,6 +115,8 @@ enum virtio_gpu_base_conf_flags {
> (_cfg.flags & (1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED))
> #define virtio_gpu_rutabaga_enabled(_cfg) \
> (_cfg.flags & (1 << VIRTIO_GPU_FLAG_RUTABAGA_ENABLED))
> +#define virtio_gpu_resource_uuid_enabled(_cfg) \
> +(_cfg.flags & (1 << VIRTIO_GPU_FLAG_RESOURCE_UUID_ENABLED))
> #define virtio_gpu_hostmem_enabled(_cfg) \
> (_cfg.hostmem > 0)
>
> @@ -209,6 +212,8 @@ struct VirtIOGPU {
> QTAILQ_HEAD(, VGPUDMABuf) bufs;
> VGPUDMABuf *primary[VIRTIO_GPU_MAX_SCANOUTS];
> } dmabuf;
> +
> +GHashTable *resource_uuids;
> };
>
> struct VirtIOGPUClass {
> @@ -307,6 +312,8 @@ void virtio_gpu_cleanup_mapping_iov(VirtIOGPU *g,
> struct iovec *iov, uint32_t count);
> void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
> struct virtio_gpu_simple_resource *res);
> +void virtio_gpu_resource_assign_uuid(VirtIOGPU *g,
> + struct virtio_gpu_ctrl_command *cmd);
> void virtio_gpu_process_cmdq(VirtIOGPU *g);
> void virtio_gpu_device_realize(DeviceState *qdev, Error **errp);
> void virtio_gpu_reset(VirtIODevice *vdev);
> --
> 2.25.1
>
--
Marc-André Lureau
Re: [PATCH v6 07/11] virtio-gpu: Handle resource blob commands
qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already mapped %d\n",
> + __func__, mblob.resource_id);
> +cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> +return;
> +}
> +
> +ret = virgl_renderer_resource_map(vres->res.resource_id, , );
> +if (ret) {
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: resource map error: %s\n",
> + __func__, strerror(-ret));
> +cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> +return;
> +}
> +
> +vres_get_ref(vres);
> +vres->region = g_new0(MemoryRegion, 1);
> +memory_region_init_ram_ptr(vres->region, OBJECT(g), NULL, size, data);
> +vres->region->opaque = vres;
> +OBJECT(vres->region)->free = virgl_resource_blob_async_unmap;
> +memory_region_add_subregion(>hostmem, mblob.offset, vres->region);
> +memory_region_set_enabled(vres->region, true);
> +
> +memset(, 0, sizeof(resp));
> +resp.hdr.type = VIRTIO_GPU_RESP_OK_MAP_INFO;
> +virgl_renderer_resource_get_map_info(mblob.resource_id, _info);
> +virtio_gpu_ctrl_response(g, cmd, , sizeof(resp));
> +}
> +
> +static void virgl_cmd_resource_unmap_blob(VirtIOGPU *g,
> + struct virtio_gpu_ctrl_command
> *cmd)
> +{
> +struct virgl_gpu_resource *vres;
> +struct virtio_gpu_resource_unmap_blob ublob;
> +VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
> +MemoryRegion *mr;
> +
> +VIRTIO_GPU_FILL_CMD(ublob);
> +virtio_gpu_unmap_blob_bswap();
> +
> +if (ublob.resource_id == 0) {
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
> + __func__);
> +cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> +return;
> +}
> +
> +vres = virgl_gpu_find_resource(g, ublob.resource_id);
> +if (!vres) {
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
> + __func__, ublob.resource_id);
> +cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> +return;
> +}
> +
> +if (!vres->region) {
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already unmapped %d\n",
> + __func__, ublob.resource_id);
> +cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> +return;
> +}
> +
> +mr = vres->region;
> +vres->region = NULL;
> +memory_region_set_enabled(mr, false);
> +memory_region_del_subregion(>hostmem, mr);
> +object_unparent(OBJECT(mr));
> +}
> +
> +#endif /* HAVE_VIRGL_RESOURCE_BLOB */
> +
> void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>struct virtio_gpu_ctrl_command *cmd)
> {
> @@ -536,6 +789,17 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
> case VIRTIO_GPU_CMD_GET_EDID:
> virtio_gpu_get_edid(g, cmd);
> break;
> +#ifdef HAVE_VIRGL_RESOURCE_BLOB
> +case VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB:
> +virgl_cmd_resource_create_blob(g, cmd);
> +break;
> +case VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB:
> +virgl_cmd_resource_map_blob(g, cmd);
> +break;
> +case VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB:
> +virgl_cmd_resource_unmap_blob(g, cmd);
> +break;
> +#endif /* HAVE_VIRGL_RESOURCE_BLOB */
> default:
> cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> break;
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 4c3ec9d0ea..8189c392dc 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1449,10 +1449,12 @@ void virtio_gpu_device_realize(DeviceState *qdev,
> Error **errp)
> return;
> }
>
> +#ifndef HAVE_VIRGL_RESOURCE_BLOB
> if (virtio_gpu_virgl_enabled(g->parent_obj.conf)) {
> -error_setg(errp, "blobs and virgl are not compatible (yet)");
> +error_setg(errp, "Linked virglrenderer does not support blob
> resources");
> return;
> }
> +#endif
> }
>
> if (!virtio_gpu_base_device_realize(qdev,
> diff --git a/meson.build b/meson.build
> index ea52ef1b9c..629407128e 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1054,6 +1054,10 @@ if not get_option('virglrenderer').auto() or
> have_system or have_vhost_user_gpu
>
> cc.has_function('virgl_renderer_context_create_with_flags',
> prefix: '#include
> ',
> dependencies: virgl))
> +config_host_data.set('HAVE_VIRGL_RESOURCE_BLOB',
> +
> cc.has_function('virgl_renderer_resource_create_blob',
> + prefix: '#include
> ',
> + dependencies: virgl))
>endif
> endif
> rutabaga = not_found
> --
> 2.25.1
>
--
Marc-André Lureau
Re: [PATCH v6 05/11] virtio-gpu: Introduce virgl_gpu_resource structure
> -uint32_t res_niov;
> +struct virgl_gpu_resource *vres;
> int ret;
>
> VIRTIO_GPU_FILL_CMD(att_rb);
> trace_virtio_gpu_cmd_res_back_attach(att_rb.resource_id);
>
> +vres = virgl_gpu_find_resource(g, att_rb.resource_id);
> +if (!vres) {
> +cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> +return;
> +}
> +
> ret = virtio_gpu_create_mapping_iov(g, att_rb.nr_entries, sizeof(att_rb),
> -cmd, NULL, _iovs, _niov);
> +cmd, NULL, >res.iov,
> +>res.iov_cnt);
> if (ret != 0) {
> cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> return;
> }
>
> ret = virgl_renderer_resource_attach_iov(att_rb.resource_id,
> - res_iovs, res_niov);
> + vres->res.iov,
> vres->res.iov_cnt);
>
> -if (ret != 0)
> -virtio_gpu_cleanup_mapping_iov(g, res_iovs, res_niov);
> +if (ret != 0) {
> +virtio_gpu_cleanup_mapping(g, >res);
> +}
> }
>
> static void virgl_resource_detach_backing(VirtIOGPU *g,
>struct virtio_gpu_ctrl_command
> *cmd)
> {
> struct virtio_gpu_resource_detach_backing detach_rb;
> -struct iovec *res_iovs = NULL;
> -int num_iovs = 0;
> +struct virgl_gpu_resource *vres;
>
> VIRTIO_GPU_FILL_CMD(detach_rb);
> trace_virtio_gpu_cmd_res_back_detach(detach_rb.resource_id);
>
> -virgl_renderer_resource_detach_iov(detach_rb.resource_id,
> - _iovs,
> - _iovs);
> -if (res_iovs == NULL || num_iovs == 0) {
> +vres = virgl_gpu_find_resource(g, detach_rb.resource_id);
> +if (!vres) {
> +cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> return;
> }
> -virtio_gpu_cleanup_mapping_iov(g, res_iovs, num_iovs);
> +
> +virgl_renderer_resource_detach_iov(detach_rb.resource_id, NULL, NULL);
> +virtio_gpu_cleanup_mapping(g, >res);
> }
>
>
> --
> 2.25.1
>
>
--
Marc-André Lureau
Re: [PATCH v6 04/11] virtio-gpu: Don't require udmabuf when blobs and virgl are enabled
On Tue, Dec 19, 2023 at 11:54 AM Huang Rui wrote:
>
> From: Dmitry Osipenko
>
> The udmabuf usage is mandatory when virgl is disabled and blobs feature
> enabled in the Qemu machine configuration. If virgl and blobs are enabled,
> then udmabuf requirement is optional. Since udmabuf isn't widely supported
> by a popular Linux distros today, let's relax the udmabuf requirement for
> blobs=on,virgl=on. Now, a full-featured virtio-gpu acceleration is
> available to Qemu users without a need to have udmabuf available in the
> system.
>
> Reviewed-by: Antonio Caggiano
> Signed-off-by: Dmitry Osipenko
> Signed-off-by: Huang Rui
Reviewed-by: Marc-André Lureau
> ---
>
> No change in v6.
>
> hw/display/virtio-gpu.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 8b2f4c6be3..4c3ec9d0ea 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1443,6 +1443,7 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error
> **errp)
>
> if (virtio_gpu_blob_enabled(g->parent_obj.conf)) {
> if (!virtio_gpu_rutabaga_enabled(g->parent_obj.conf) &&
> +!virtio_gpu_virgl_enabled(g->parent_obj.conf) &&
> !virtio_gpu_have_udmabuf()) {
> error_setg(errp, "need rutabaga or udmabuf for blob resources");
> return;
> --
> 2.25.1
>
--
Marc-André Lureau
Re: [PATCH v6 03/11] virtio-gpu: Support context init feature with virglrenderer
Hi
On Tue, Dec 19, 2023 at 11:54 AM Huang Rui wrote:
>
> Patch "virtio-gpu: CONTEXT_INIT feature" has added the context_init
> feature flags.
> We would like to enable the feature with virglrenderer, so add to create
> virgl renderer context with flags using context_id when valid.
>
> Originally-by: Antonio Caggiano
> Signed-off-by: Huang Rui
> ---
>
> Changes in v6:
> - Handle the case while context_init is disabled.
> - Enable context_init by default.
>
> hw/display/virtio-gpu-virgl.c | 13 +++--
> hw/display/virtio-gpu.c | 4
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 8bb7a2c21f..5bbc8071b2 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -106,8 +106,17 @@ static void virgl_cmd_context_create(VirtIOGPU *g,
> trace_virtio_gpu_cmd_ctx_create(cc.hdr.ctx_id,
> cc.debug_name);
>
> -virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen,
> - cc.debug_name);
> +#ifdef HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS
> +if (cc.context_init &&
> virtio_gpu_context_init_enabled(g->parent_obj.conf)) {
> +virgl_renderer_context_create_with_flags(cc.hdr.ctx_id,
> + cc.context_init,
> + cc.nlen,
> + cc.debug_name);
> +return;
> +}
> +#endif
> +
> +virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, cc.debug_name);
> }
>
> static void virgl_cmd_context_destroy(VirtIOGPU *g,
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index b016d3bac8..8b2f4c6be3 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1619,6 +1619,10 @@ static Property virtio_gpu_properties[] = {
> DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
> VIRTIO_GPU_FLAG_BLOB_ENABLED, false),
> DEFINE_PROP_SIZE("hostmem", VirtIOGPU, parent_obj.conf.hostmem, 0),
> +#ifdef HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS
> +DEFINE_PROP_BIT("context_init", VirtIOGPU, parent_obj.conf.flags,
> +VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, true),
> +#endif
Does it make sense to make this configurable? If the context to be
created asked for a capset id but the one created doesn't respect it,
what's the outcome?
It looks like it should instead set cmd->error.
--
Marc-André Lureau
Re: [PATCH v6 01/11] linux-headers: Update to kernel headers to add venus capset
Hi On Thu, Dec 21, 2023 at 10:55 AM Akihiko Odaki wrote: > > On 2023/12/19 23:14, Peter Maydell wrote: > > On Tue, 19 Dec 2023 at 13:49, Huang Rui wrote: > >> > >> On Tue, Dec 19, 2023 at 08:20:22PM +0800, Akihiko Odaki wrote: > >>> On 2023/12/19 16:53, Huang Rui wrote: > >>>> Sync up kernel headers to update venus macro till they are merged into > >>>> mainline. > >>> > >>> Thanks for sorting things out with the kernel and spec. > >>> > >>>> > >>>> Signed-off-by: Huang Rui > >>>> --- > >>>> > >>>> Changes in v6: > >>>> - Venus capset is applied in kernel, so update it in qemu for future use. > >>>> > >>>> https://lore.kernel.org/lkml/b79dcf75-c9e8-490e-644f-3b97d95f7...@collabora.com/ > >>>> https://cgit.freedesktop.org/drm-misc/commit/?id=216d86b9a430f3280e5b631c51e6fd1a7774cfa0 > >>> Please include the link to the upstream commit in the commit message. > >> > >> So far, it's in drm maintainers' branch not in kernel mainline yet. Do I > >> need to wait it to be merged into kernel mainline? > > > > For an RFC patchset, no. For patches to be merged into QEMU > > the headers change must be in the kernel mainline, and the > > QEMU commit that updates our copy of the headers must be a > > full-sync done with scripts/update-linux-headers.sh, not a > > manual edit. > > Apparently the kernel change is unlikely to be merged to mainline before > QEMU 9.0 so we need to come up with some idea to deal with this. > > The release of Linux 6.7 is near and the development of 6.8 will start > soon. So it was nice if the change made into 6.8, but unfortunately it > missed the *probably last* drm-misc tree pull request for 6.8: > https://lore.kernel.org/all/aqpn5miejmkks7pbcfex7b6u63uwsruywxsnr3x5ljs45qatin@nbkkej2elk46/ > > It will still get into linux-next so we may retrieve headers from > linux-next. Or we may add the definition to > hw/display/virtio-gpu-virgl.c; the duplicate definition warning will > tell when the definition becomes no longer necessary. I'm fine with > either option. The second option seems better to me, as it can avoid pulling unwanted changes. thanks -- Marc-André Lureau
Re: [PATCH 2/3] vl: disable default serial when xen-console is enabled
Hi
On Tue, Nov 21, 2023 at 2:57 PM Marc-André Lureau
wrote:
>
> Hi
>
> On Wed, Nov 15, 2023 at 9:28 PM David Woodhouse wrote:
> >
> > From: David Woodhouse
> >
> > If a Xen console is configured on the command line, do not add a default
> > serial port.
> >
> > Signed-off-by: David Woodhouse
> > ---
> > system/vl.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/system/vl.c b/system/vl.c
> > index 5af7ced2a1..8109231834 100644
> > --- a/system/vl.c
> > +++ b/system/vl.c
> > @@ -198,6 +198,7 @@ static const struct {
> > const char *driver;
> > int *flag;
> > } default_list[] = {
> > +{ .driver = "xen-console", .flag = _serial},
> > { .driver = "isa-serial", .flag = _serial},
> > { .driver = "isa-parallel", .flag = _parallel },
> > { .driver = "isa-fdc", .flag = _floppy},
>
> Consistent with the rest of the lines (no conditional compilation nor
> driver #define..)
> Reviewed-by: Marc-André Lureau
>
> btw, while quickly testing this (do we have any test for xen-console?):
>
> $ qemu --accel kvm,xen-version=0x40011,kernel-irqchip=split -device
> xen-console,chardev=foo -chardev stdio,id=foo
> (and close gtk window)
>
> Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
> 0x55c11695 in qemu_free_net_client (nc=0x0) at ../net/net.c:387
> 387if (nc->incoming_queue) {
> (gdb) bt
> #0 0x55c11695 in qemu_free_net_client (nc=0x0) at ../net/net.c:387
> #1 0x55c11a14 in qemu_del_nic (nic=0x58b6f930) at
> ../net/net.c:459
> #2 0x559e398b in xen_netdev_unrealize (xendev=0x58b6b510)
> at ../hw/net/xen_nic.c:550
> #3 0x55b6e22f in xen_device_unrealize (dev=0x58b6b510) at
> ../hw/xen/xen-bus.c:973
> #4 0x55b6e351 in xen_device_exit (n=0x58b6b5e0, data=0x0)
> at ../hw/xen/xen-bus.c:1002
> #5 0x00005555560bc3fc in notifier_list_notify (list=0x570b5fc0
> , data=0x0) at ../util/notify.c:39
> #6 0x55ba1d49 in qemu_run_exit_notifiers () at
> ../system/runstate.c:800
Ok, I found related "[PATCH 1/3] net: do not delete nics in net_cleanup()"
--
Marc-André Lureau
Re: [PATCH 2/3] vl: disable default serial when xen-console is enabled
Hi
On Wed, Nov 15, 2023 at 9:28 PM David Woodhouse wrote:
>
> From: David Woodhouse
>
> If a Xen console is configured on the command line, do not add a default
> serial port.
>
> Signed-off-by: David Woodhouse
> ---
> system/vl.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/system/vl.c b/system/vl.c
> index 5af7ced2a1..8109231834 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -198,6 +198,7 @@ static const struct {
> const char *driver;
> int *flag;
> } default_list[] = {
> +{ .driver = "xen-console", .flag = _serial},
> { .driver = "isa-serial", .flag = _serial},
> { .driver = "isa-parallel", .flag = _parallel },
> { .driver = "isa-fdc", .flag = _floppy},
Consistent with the rest of the lines (no conditional compilation nor
driver #define..)
Reviewed-by: Marc-André Lureau
btw, while quickly testing this (do we have any test for xen-console?):
$ qemu --accel kvm,xen-version=0x40011,kernel-irqchip=split -device
xen-console,chardev=foo -chardev stdio,id=foo
(and close gtk window)
Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
0x55c11695 in qemu_free_net_client (nc=0x0) at ../net/net.c:387
387if (nc->incoming_queue) {
(gdb) bt
#0 0x55c11695 in qemu_free_net_client (nc=0x0) at ../net/net.c:387
#1 0x55c11a14 in qemu_del_nic (nic=0x58b6f930) at ../net/net.c:459
#2 0x559e398b in xen_netdev_unrealize (xendev=0x58b6b510)
at ../hw/net/xen_nic.c:550
#3 0x55b6e22f in xen_device_unrealize (dev=0x58b6b510) at
../hw/xen/xen-bus.c:973
#4 0x55b6e351 in xen_device_exit (n=0x58b6b5e0, data=0x0)
at ../hw/xen/xen-bus.c:1002
#5 0x560bc3fc in notifier_list_notify (list=0x570b5fc0
, data=0x0) at ../util/notify.c:39
#6 0x55ba1d49 in qemu_run_exit_notifiers () at ../system/runstate.c:800
--
Marc-André Lureau
Re: [PATCH] ui/input: Constify QemuInputHandler structure
On Tue, Oct 17, 2023 at 5:13 PM Philippe Mathieu-Daudé
wrote:
>
> Access to QemuInputHandlerState::handler are read-only.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> include/hw/virtio/virtio-input.h | 2 +-
> include/ui/input.h | 2 +-
> chardev/msmouse.c| 2 +-
> chardev/wctablet.c | 2 +-
> hw/char/escc.c | 2 +-
> hw/display/xenfb.c | 6 +++---
> hw/input/adb-kbd.c | 2 +-
> hw/input/hid.c | 6 +++---
> hw/input/ps2.c | 4 ++--
> hw/input/virtio-input-hid.c | 8
> ui/input-legacy.c| 2 +-
> ui/input.c | 4 ++--
> ui/vdagent.c | 2 +-
> 13 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-input.h
> b/include/hw/virtio/virtio-input.h
> index 08f1591424..a6c9703644 100644
> --- a/include/hw/virtio/virtio-input.h
> +++ b/include/hw/virtio/virtio-input.h
> @@ -84,7 +84,7 @@ struct VirtIOInputHID {
> VirtIOInput parent_obj;
> char *display;
> uint32_t head;
> -QemuInputHandler *handler;
> +const QemuInputHandler*handler;
> QemuInputHandlerState *hs;
> int ledstate;
> bool wheel_axis;
> diff --git a/include/ui/input.h b/include/ui/input.h
> index 24d8e4579e..8f9aac562e 100644
> --- a/include/ui/input.h
> +++ b/include/ui/input.h
> @@ -30,7 +30,7 @@ struct QemuInputHandler {
> };
>
> QemuInputHandlerState *qemu_input_handler_register(DeviceState *dev,
> - QemuInputHandler
> *handler);
> +const QemuInputHandler *handler);
> void qemu_input_handler_activate(QemuInputHandlerState *s);
> void qemu_input_handler_deactivate(QemuInputHandlerState *s);
> void qemu_input_handler_unregister(QemuInputHandlerState *s);
> diff --git a/chardev/msmouse.c b/chardev/msmouse.c
> index ab8fe981d6..a774c397b4 100644
> --- a/chardev/msmouse.c
> +++ b/chardev/msmouse.c
> @@ -171,7 +171,7 @@ static int msmouse_chr_write(struct Chardev *s, const
> uint8_t *buf, int len)
> return len;
> }
>
> -static QemuInputHandler msmouse_handler = {
> +static const QemuInputHandler msmouse_handler = {
> .name = "QEMU Microsoft Mouse",
> .mask = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
> .event = msmouse_input_event,
> diff --git a/chardev/wctablet.c b/chardev/wctablet.c
> index 43bdf6b608..f4008bf35b 100644
> --- a/chardev/wctablet.c
> +++ b/chardev/wctablet.c
> @@ -178,7 +178,7 @@ static void wctablet_input_sync(DeviceState *dev)
> }
> }
>
> -static QemuInputHandler wctablet_handler = {
> +static const QemuInputHandler wctablet_handler = {
> .name = "QEMU Wacom Pen Tablet",
> .mask = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_ABS,
> .event = wctablet_input_event,
> diff --git a/hw/char/escc.c b/hw/char/escc.c
> index 4be66053c1..48b30ee760 100644
> --- a/hw/char/escc.c
> +++ b/hw/char/escc.c
> @@ -845,7 +845,7 @@ static void sunkbd_handle_event(DeviceState *dev,
> QemuConsole *src,
> put_queue(s, keycode);
> }
>
> -static QemuInputHandler sunkbd_handler = {
> +static const QemuInputHandler sunkbd_handler = {
> .name = "sun keyboard",
> .mask = INPUT_EVENT_MASK_KEY,
> .event = sunkbd_handle_event,
> diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
> index 0074a9b6f8..b2130a0d70 100644
> --- a/hw/display/xenfb.c
> +++ b/hw/display/xenfb.c
> @@ -321,20 +321,20 @@ static void xenfb_mouse_sync(DeviceState *dev)
> xenfb->wheel = 0;
> }
>
> -static QemuInputHandler xenfb_keyboard = {
> +static const QemuInputHandler xenfb_keyboard = {
> .name = "Xen PV Keyboard",
> .mask = INPUT_EVENT_MASK_KEY,
> .event = xenfb_key_event,
> };
>
> -static QemuInputHandler xenfb_abs_mouse = {
> +static const QemuInputHandler xenfb_abs_mouse = {
> .name = "Xen PV Mouse",
> .mask = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_ABS,
> .event = xenfb_mouse_event,
> .sync = xenfb_mouse_sync,
> };
>
> -static QemuInputHandler xenfb_rel_mouse = {
> +static const QemuInputHandler xenfb_rel_mouse = {
> .name = "Xen PV Mouse",
> .mask = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
> .event = xenfb_mouse_event,
> diff --git a/hw/input/adb-kbd.c b/hw/inpu
Re: [QEMU PATCH v5 11/13] virtio-gpu: Support Venus capset
Hi
On Fri, Sep 15, 2023 at 3:14 PM Huang Rui wrote:
>
> From: Antonio Caggiano
>
> Add support for the Venus capset, which enables Vulkan support through
> the Venus Vulkan driver for virtio-gpu.
>
> Signed-off-by: Antonio Caggiano
> Signed-off-by: Huang Rui
> ---
>
> V4 -> V5:
> - Send kernel patch to define VIRTIO_GPU_CAPSET_VENUS and will use
> another patch to sync up linux headers. (Akihiko)
> - https://lore.kernel.org/lkml/20230915105918.3763061-1-ray.hu...@amd.com/
Ok but in the meantime, you should have that header update patch in
the series too, otherwise we can't compile it :)
thanks
>
> hw/display/virtio-gpu-virgl.c | 21 +
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 8a017dbeb4..7f95490e90 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -437,6 +437,11 @@ static void virgl_cmd_get_capset_info(VirtIOGPU *g,
> virgl_renderer_get_cap_set(resp.capset_id,
> _max_version,
> _max_size);
> +} else if (info.capset_index == 2) {
> +resp.capset_id = VIRTIO_GPU_CAPSET_VENUS;
> +virgl_renderer_get_cap_set(resp.capset_id,
> + _max_version,
> + _max_size);
> } else {
> resp.capset_max_version = 0;
> resp.capset_max_size = 0;
> @@ -901,10 +906,18 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
>
> int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
> {
> -uint32_t capset2_max_ver, capset2_max_size;
> +uint32_t capset2_max_ver, capset2_max_size, num_capsets;
> +num_capsets = 1;
> +
> virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2,
> - _max_ver,
> - _max_size);
> + _max_ver,
> + _max_size);
> +num_capsets += capset2_max_ver ? 1 : 0;
> +
> +virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VENUS,
> + _max_ver,
> + _max_size);
> +num_capsets += capset2_max_size ? 1 : 0;
>
> -return capset2_max_ver ? 2 : 1;
> +return num_capsets;
> }
> --
> 2.34.1
>
>
--
Marc-André Lureau
Re: [QEMU PATCH v5 10/13] virtio-gpu: Resource UUID
bh, qemu_bh_delete);
> g_clear_pointer(>reset_bh, qemu_bh_delete);
> @@ -1452,6 +1508,8 @@ void virtio_gpu_reset(VirtIODevice *vdev)
> g_free(cmd);
> }
>
> +g_hash_table_remove_all(g->resource_uuids);
> +
> virtio_gpu_base_reset(VIRTIO_GPU_BASE(vdev));
> }
>
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index b9adc28071..67b39fccec 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -63,6 +63,8 @@ struct virtio_gpu_simple_resource {
> MemoryRegion *region;
> #endif
>
> +bool has_uuid;
> +
> QTAILQ_ENTRY(virtio_gpu_simple_resource) next;
> };
>
> @@ -208,6 +210,8 @@ struct VirtIOGPU {
> QTAILQ_HEAD(, VGPUDMABuf) bufs;
> VGPUDMABuf *primary[VIRTIO_GPU_MAX_SCANOUTS];
> } dmabuf;
> +
> +GHashTable *resource_uuids;
> };
>
> struct VirtIOGPUClass {
> @@ -285,6 +289,8 @@ void virtio_gpu_cleanup_mapping_iov(VirtIOGPU *g,
> struct iovec *iov, uint32_t count);
> void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
> struct virtio_gpu_simple_resource *res);
> +void virtio_gpu_resource_assign_uuid(VirtIOGPU *g,
> + struct virtio_gpu_ctrl_command *cmd);
> void virtio_gpu_process_cmdq(VirtIOGPU *g);
> void virtio_gpu_device_realize(DeviceState *qdev, Error **errp);
> void virtio_gpu_reset(VirtIODevice *vdev);
> --
> 2.34.1
>
>
--
Marc-André Lureau
Re: [QEMU PATCH v5 09/13] virtio-gpu: Handle resource blob commands
) {
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
> + __func__);
> +cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> +return;
> +}
> +
> +res = virtio_gpu_find_resource(g, ublob.resource_id);
> +if (!res) {
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
> + __func__, ublob.resource_id);
> +cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
> +return;
> +}
> +
> +virtio_gpu_virgl_resource_unmap(g, res);
> +}
> +
> +#endif /* HAVE_VIRGL_RESOURCE_BLOB */
> +
> void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>struct virtio_gpu_ctrl_command *cmd)
> {
> @@ -492,6 +694,17 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
> case VIRTIO_GPU_CMD_GET_EDID:
> virtio_gpu_get_edid(g, cmd);
> break;
> +#ifdef HAVE_VIRGL_RESOURCE_BLOB
> +case VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB:
> +virgl_cmd_resource_create_blob(g, cmd);
> +break;
> +case VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB:
> +virgl_cmd_resource_map_blob(g, cmd);
> +break;
> +case VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB:
> +virgl_cmd_resource_unmap_blob(g, cmd);
> +break;
> +#endif /* HAVE_VIRGL_RESOURCE_BLOB */
> default:
> cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> break;
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 5b7a7eab4f..cc4c1f81bb 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1367,10 +1367,12 @@ void virtio_gpu_device_realize(DeviceState *qdev,
> Error **errp)
> return;
> }
>
> +#ifndef HAVE_VIRGL_RESOURCE_BLOB
> if (virtio_gpu_virgl_enabled(g->parent_obj.conf)) {
> -error_setg(errp, "blobs and virgl are not compatible (yet)");
> +error_setg(errp, "Linked virglrenderer does not support blob
> resources");
> return;
> }
> +#endif
> }
>
> if (!virtio_gpu_base_device_realize(qdev,
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 55973e112f..b9adc28071 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -58,6 +58,11 @@ struct virtio_gpu_simple_resource {
> int dmabuf_fd;
> uint8_t *remapped;
>
> +#ifdef HAVE_VIRGL_RESOURCE_BLOB
> +/* only blob resource needs this region to be mapped as guest mmio */
> +MemoryRegion *region;
> +#endif
> +
> QTAILQ_ENTRY(virtio_gpu_simple_resource) next;
> };
>
> diff --git a/meson.build b/meson.build
> index ff20d3c249..f7b744ab82 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1072,6 +1072,10 @@ if not get_option('virglrenderer').auto() or
> have_system or have_vhost_user_gpu
>
> cc.has_function('virgl_renderer_context_create_with_flags',
> prefix: '#include ',
> dependencies: virgl))
> + config_host_data.set('HAVE_VIRGL_RESOURCE_BLOB',
> + cc.has_function('virgl_renderer_resource_create_blob',
> + prefix: '#include ',
> + dependencies: virgl))
better moved under the if virgl.found() block
> endif
> blkio = not_found
> if not get_option('blkio').auto() or have_block
> --
> 2.34.1
>
>
--
Marc-André Lureau
Re: [QEMU PATCH v5 05/13] virtio-gpu: Configure context init for virglrenderer
Hi
On Fri, Sep 15, 2023 at 6:16 PM Huang Rui wrote:
>
> Configure context init feature flag for virglrenderer.
>
> Originally-by: Antonio Caggiano
> Signed-off-by: Huang Rui
> ---
>
> V4 -> V5:
> - Inverted patch 5 and 6 because we should configure
> HAVE_VIRGL_CONTEXT_INIT firstly. (Philippe)
>
> meson.build | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index 98e68ef0b1..ff20d3c249 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1068,6 +1068,10 @@ if not get_option('virglrenderer').auto() or
> have_system or have_vhost_user_gpu
> prefix: '#include ',
> dependencies: virgl))
>endif
> + config_host_data.set('HAVE_VIRGL_CONTEXT_INIT',
> +
> cc.has_function('virgl_renderer_context_create_with_flags',
> + prefix: '#include ',
> + dependencies: virgl))
Move it under the "if virgl.found()" block above.
I suggest to name it after what is actually checked:
HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS for ex
> endif
> blkio = not_found
> if not get_option('blkio').auto() or have_block
> --
> 2.34.1
>
>
--
Marc-André Lureau
Re: [QEMU PATCH 1/1] virtgpu: do not destroy resources when guest suspend
virtio_gpu_resource_destroy(g, res);
> +}
> }
>
> while (!QTAILQ_EMPTY(>cmdq)) {
> diff --git a/include/hw/virtio/virtio-gpu.h
> b/include/hw/virtio/virtio-gpu.h
> index 2e28507efe..c21c2990fb 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -173,6 +173,7 @@ struct VirtIOGPU {
>
> uint64_t hostmem;
>
> +bool freezing;
> bool processing_cmdq;
> QEMUTimer *fence_poll;
> QEMUTimer *print_stats;
> @@ -284,5 +285,7 @@ void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g);
> void virtio_gpu_virgl_reset(VirtIOGPU *g);
> int virtio_gpu_virgl_init(VirtIOGPU *g);
> int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g);
> +void virtio_gpu_cmd_status_freezing(VirtIOGPU *g,
> + struct virtio_gpu_ctrl_command *cmd);
>
> #endif
> diff --git a/include/standard-headers/linux/virtio_gpu.h
> b/include/standard-headers/linux/virtio_gpu.h
> index 2da48d3d4c..aefffbd751 100644
> --- a/include/standard-headers/linux/virtio_gpu.h
> +++ b/include/standard-headers/linux/virtio_gpu.h
> @@ -116,6 +116,9 @@ enum virtio_gpu_ctrl_type {
> VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID,
> VIRTIO_GPU_RESP_ERR_INVALID_CONTEXT_ID,
> VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER,
> +
> + /* status */
> + VIRTIO_GPU_CMD_STATUS_FREEZING = 0x1300,
> };
>
Adding a new command requires new feature flag (and maybe it should be in
the <0x1000 range instead)
But I am not sure we need a new message at the virtio-gpu level. Gerd, wdyt?
Maybe it's not a good place to reset all GPU resources during QEMU reset()
after all, if it's called during s3 and there is no mechanism to restore
it. Damien?
thanks
...
> enum virtio_gpu_shm_id {
> @@ -453,4 +456,10 @@ struct virtio_gpu_resource_unmap_blob {
> uint32_t padding;
> };
>
> +/* VIRTIO_GPU_CMD_STATUS_FREEZING */
> +struct virtio_gpu_status_freezing {
> + struct virtio_gpu_ctrl_hdr hdr;
> + __u32 freezing;
> +};
> +
> #endif
> --
> 2.34.1
>
>
>
--
Marc-André Lureau
Re: [PATCH v14 13/17] qemu-sockets: move and rename SocketAddress_to_str()
Hi
On Fri, Oct 21, 2022 at 2:03 PM Laurent Vivier wrote:
> Rename SocketAddress_to_str() to socket_uri() and move it to
> util/qemu-sockets.c close to socket_parse().
>
> socket_uri() generates a string from a SocketAddress while
> socket_parse() generates a SocketAddress from a string.
>
Tbh, as we are renaming functions here, I would follow good glib/C
conventions and use the type name in lower case as prefix. I would also not
use "URI", which should be reserved for RFC3986 compliant forms.
So instead, I'd suggest to rename to:
char *socket_address_to_string(SocketAddress *addr);
SocketAddress *socket_address_new_from_string(const char *str, Error **errp)
my 2c
> Signed-off-by: Laurent Vivier
> Reviewed-by: David Gibson
> Reviewed-by: Dr. David Alan Gilbert
> Acked-by: Michael S. Tsirkin
> ---
> include/qemu/sockets.h | 2 +-
> monitor/hmp-cmds.c | 23 +--
> util/qemu-sockets.c| 20
> 3 files changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> index db4bedb6fa20..214058d8e307 100644
> --- a/include/qemu/sockets.h
> +++ b/include/qemu/sockets.h
> @@ -58,6 +58,7 @@ NetworkAddressFamily inet_netfamily(int family);
> int unix_listen(const char *path, Error **errp);
> int unix_connect(const char *path, Error **errp);
>
> +char *socket_uri(SocketAddress *addr);
> SocketAddress *socket_parse(const char *str, Error **errp);
> int socket_connect(SocketAddress *addr, Error **errp);
> int socket_listen(SocketAddress *addr, int num, Error **errp);
> @@ -141,5 +142,4 @@ SocketAddress
> *socket_address_flatten(SocketAddressLegacy *addr);
> * Return 0 on success.
> */
> int socket_address_parse_named_fd(SocketAddress *addr, Error **errp);
> -
> #endif /* QEMU_SOCKETS_H */
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index bab86c5537e1..01b789a79e62 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -199,27 +199,6 @@ void hmp_info_mice(Monitor *mon, const QDict *qdict)
> qapi_free_MouseInfoList(mice_list);
> }
>
> -static char *SocketAddress_to_str(SocketAddress *addr)
> -{
> -switch (addr->type) {
> -case SOCKET_ADDRESS_TYPE_INET:
> -return g_strdup_printf("tcp:%s:%s",
> - addr->u.inet.host,
> - addr->u.inet.port);
> -case SOCKET_ADDRESS_TYPE_UNIX:
> -return g_strdup_printf("unix:%s",
> - addr->u.q_unix.path);
> -case SOCKET_ADDRESS_TYPE_FD:
> -return g_strdup_printf("fd:%s", addr->u.fd.str);
> -case SOCKET_ADDRESS_TYPE_VSOCK:
> -return g_strdup_printf("tcp:%s:%s",
> - addr->u.vsock.cid,
> - addr->u.vsock.port);
> -default:
> -return g_strdup("unknown address type");
> -}
> -}
> -
> void hmp_info_migrate(Monitor *mon, const QDict *qdict)
> {
> MigrationInfo *info;
> @@ -382,7 +361,7 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
> monitor_printf(mon, "socket address: [\n");
>
> for (addr = info->socket_address; addr; addr = addr->next) {
> -char *s = SocketAddress_to_str(addr->value);
> +char *s = socket_uri(addr->value);
> monitor_printf(mon, "\t%s\n", s);
> g_free(s);
> }
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index 83f4bd6fd211..9f6f655fd526 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -1077,6 +1077,26 @@ int unix_connect(const char *path, Error **errp)
> return sock;
> }
>
> +char *socket_uri(SocketAddress *addr)
> +{
> +switch (addr->type) {
> +case SOCKET_ADDRESS_TYPE_INET:
> +return g_strdup_printf("tcp:%s:%s",
> + addr->u.inet.host,
> + addr->u.inet.port);
> +case SOCKET_ADDRESS_TYPE_UNIX:
> +return g_strdup_printf("unix:%s",
> + addr->u.q_unix.path);
> +case SOCKET_ADDRESS_TYPE_FD:
> + return g_strdup_printf("fd:%s", addr->u.fd.str);
> +case SOCKET_ADDRESS_TYPE_VSOCK:
> +return g_strdup_printf("tcp:%s:%s",
> + addr->u.vsock.cid,
> + addr->u.vsock.port);
> +default:
> +return g_strdup("unknown address type");
> +}
> +}
>
> SocketAddress *socket_parse(const char *str, Error **errp)
> {
> --
> 2.37.3
>
>
>
--
Marc-André Lureau
Re: [PATCH 07/27] Replace GCC_FMT_ATTR with G_GNUC_PRINTF
Hi On Wed, Mar 16, 2022 at 5:28 PM Thomas Huth wrote: > > On 16/03/2022 14.16, Philippe Mathieu-Daudé wrote: > > On 16/3/22 10:52, marcandre.lur...@redhat.com wrote: > >> From: Marc-André Lureau > >> > >> One less qemu-specific macro. It also helps to make some headers/units > >> only depend on glib, and thus moved in standalone projects eventually. > >> > >> Signed-off-by: Marc-André Lureau > >> --- > >> audio/audio.h | 4 +-- > >> block/qcow2.h | 2 +- > >> bsd-user/qemu.h | 2 +- > >> hw/display/qxl.h| 2 +- > >> hw/net/rocker/rocker.h | 2 +- > >> hw/xen/xen_pt.h | 2 +- > >> include/chardev/char-fe.h | 2 +- > >> include/disas/dis-asm.h | 2 +- > >> include/hw/acpi/aml-build.h | 12 +++ > >> include/hw/core/cpu.h | 2 +- > >> include/hw/hw.h | 2 +- > >> include/hw/virtio/virtio.h | 2 +- > >> include/hw/xen/xen-bus-helper.h | 4 +-- > >> include/hw/xen/xen-bus.h| 4 +-- > >> include/hw/xen/xen_common.h | 2 +- > >> include/hw/xen/xen_pvdev.h | 2 +- > >> include/monitor/monitor.h | 4 +-- > >> include/qapi/error.h| 20 ++-- > >> include/qapi/qmp/qjson.h| 8 ++--- > >> include/qemu/buffer.h | 2 +- > >> include/qemu/compiler.h | 11 ++- > >> include/qemu/error-report.h | 24 +++--- > >> include/qemu/log-for-trace.h| 2 +- > >> include/qemu/log.h | 2 +- > >> include/qemu/qemu-print.h | 8 ++--- > >> include/qemu/readline.h | 2 +- > >> qga/guest-agent-core.h | 2 +- > >> qga/vss-win32/requester.h | 2 +- > >> scripts/cocci-macro-file.h | 2 +- > >> tests/qtest/libqos/libqtest.h | 42 - > >> tests/qtest/libqtest-single.h | 2 +- > >> tests/qtest/migration-helpers.h | 6 ++-- > >> audio/alsaaudio.c | 4 +-- > >> audio/dsoundaudio.c | 4 +-- > >> audio/ossaudio.c| 4 +-- > >> audio/paaudio.c | 2 +- > >> audio/sdlaudio.c| 2 +- > >> block/blkverify.c | 2 +- > >> block/ssh.c | 4 +-- > >> fsdev/9p-marshal.c | 2 +- > >> fsdev/virtfs-proxy-helper.c | 2 +- > >> hw/9pfs/9p.c| 2 +- > >> hw/acpi/aml-build.c | 4 +-- > >> hw/mips/fuloong2e.c | 2 +- > >> hw/mips/malta.c | 2 +- > >> hw/net/rtl8139.c| 2 +- > >> hw/virtio/virtio.c | 2 +- > >> io/channel-websock.c| 2 +- > >> monitor/hmp.c | 4 +-- > >> nbd/server.c| 10 +++--- > >> qemu-img.c | 4 +-- > >> qemu-io.c | 2 +- > >> qobject/json-parser.c | 2 +- > >> softmmu/qtest.c | 4 +-- > >> tests/qtest/libqtest.c | 2 +- > >> tests/unit/test-qobject-input-visitor.c | 4 +-- > >> audio/coreaudio.m | 4 +-- > >> scripts/checkpatch.pl | 2 +- > >> 58 files changed, 130 insertions(+), 137 deletions(-) > > > >> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h > >> index 3baa5e3790f7..f2bd050e3b9a 100644 > >> --- a/include/qemu/compiler.h > >> +++ b/include/qemu/compiler.h > >> @@ -79,19 +79,12 @@ > >> #define QEMU_BUILD_BUG_ON_ZERO(x) (sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)) - > >> \ > >> sizeof(QEMU_BUILD_BUG_ON_STRUCT(x))) > >> -#if defined(__clang__) > >> -/* clang doesn't support gnu_printf, so use printf. */ > >> -# define GCC_FMT_ATTR(n, m) __attribute__((format(printf, n, m))) >
Re: [PATCH 07/27] Replace GCC_FMT_ATTR with G_GNUC_PRINTF
Hi On Wed, Mar 16, 2022 at 5:30 PM Daniel P. Berrangé wrote: > > On Wed, Mar 16, 2022 at 01:52:48PM +0400, marcandre.lur...@redhat.com wrote: > > diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h > > index 3baa5e3790f7..f2bd050e3b9a 100644 > > --- a/include/qemu/compiler.h > > +++ b/include/qemu/compiler.h > > @@ -79,19 +79,12 @@ > > #define QEMU_BUILD_BUG_ON_ZERO(x) (sizeof(QEMU_BUILD_BUG_ON_STRUCT(x)) - \ > > sizeof(QEMU_BUILD_BUG_ON_STRUCT(x))) > > > > -#if defined(__clang__) > > -/* clang doesn't support gnu_printf, so use printf. */ > > -# define GCC_FMT_ATTR(n, m) __attribute__((format(printf, n, m))) > > -#else > > -/* Use gnu_printf (qemu uses standard format strings). */ > > -# define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m))) > > -# if defined(_WIN32) > > +#if !defined(__clang__) && defined(_WIN32) > > /* > > * Map __printf__ to __gnu_printf__ because we want standard format > > strings even > > * when MinGW or GLib include files use __printf__. > > */ > > -# define __printf__ __gnu_printf__ > > -# endif > > +# define __printf__ __gnu_printf__ > > #endif > > I'm not convinced we shold have this remaining define, even > before your patch. > > For code we've implemented, we should have used __gnu_printf__ > already if we know it uses GNU format on Windows. > > For code in GLib, its header file uses __gnu_printf__ for anything > that relies on its portable printf replacement, which is basically > everything in GLib. > > For anything else we should honour whatever they declare, and not > assume their impl is the GNU one. > > > I guess it is easy enough to validate by deleting this and seeing > if we get any warnings from the mingw CI jobs about printf/gnu_printf > mismatch. Please comment on that thread: https://patchew.org/QEMU/20220224183701.608720-1-marcandre.lur...@redhat.com/20220224183701.608720-6-marcandre.lur...@redhat.com/ > > With regards, > Daniel > -- > |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o-https://fstop138.berrange.com :| > |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| >
Re: [PATCH v3 13/13] compiler.h: remove QEMU_GNUC_PREREQ
Hi On Thu, Dec 10, 2020 at 6:07 PM wrote: > From: Marc-André Lureau > > When needed, the G_GNUC_CHECK_VERSION() glib macro can be used instead. > > Signed-off-by: Marc-André Lureau > --- > include/qemu/compiler.h| 11 --- > scripts/cocci-macro-file.h | 1 - > 2 files changed, 12 deletions(-) > > diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h > index 5e6cf2c8e8..1b9e58e82b 100644 > --- a/include/qemu/compiler.h > +++ b/include/qemu/compiler.h > @@ -11,17 +11,6 @@ > #define QEMU_STATIC_ANALYSIS 1 > #endif > > > -/* > -| The macro QEMU_GNUC_PREREQ tests for minimum version of the GNU C > compiler. > -| The code is a copy of SOFTFLOAT_GNUC_PREREQ, see softfloat-macros.h. > > -**/ > -#if defined(__GNUC__) && defined(__GNUC_MINOR__) > -# define QEMU_GNUC_PREREQ(maj, min) \ > - ((__GNUC__ << 16) + __GNUC_MINOR__ >= ((maj) << 16) + (min)) > -#else > -# define QEMU_GNUC_PREREQ(maj, min) 0 > -#endif > - > #define QEMU_NORETURN __attribute__ ((__noreturn__)) > > #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result)) > diff --git a/scripts/cocci-macro-file.h b/scripts/cocci-macro-file.h > index c6bbc05ba3..20eea6b708 100644 > --- a/scripts/cocci-macro-file.h > +++ b/scripts/cocci-macro-file.h > @@ -19,7 +19,6 @@ > */ > > /* From qemu/compiler.h */ > -#define QEMU_GNUC_PREREQ(maj, min) 1 > #define QEMU_NORETURN __attribute__ ((__noreturn__)) > #define QEMU_WARN_UNUSED_RESULT __attribute__((warn_unused_result)) > #define QEMU_SENTINEL __attribute__((sentinel)) > ping, thanks -- Marc-André Lureau
Re: [PATCH v3 11/13] compiler: remove GNUC check
On Thu, Dec 10, 2020 at 6:14 PM wrote:
> From: Marc-André Lureau
>
> QEMU requires Clang or GCC, that define and support __GNUC__ extensions.
>
> Signed-off-by: Marc-André Lureau
> ---
> include/qemu/compiler.h | 8 +---
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
> index 6212295e52..5e6cf2c8e8 100644
> --- a/include/qemu/compiler.h
> +++ b/include/qemu/compiler.h
> @@ -64,14 +64,10 @@
> (offsetof(container, field) + sizeof_field(container, field))
>
> /* Convert from a base type to a parent type, with compile time
> checking. */
> -#ifdef __GNUC__
> #define DO_UPCAST(type, field, dev) ( __extension__ ( { \
> char __attribute__((unused)) offset_must_be_zero[ \
> -offsetof(type, field)]; \
> container_of(dev, type, field);}))
> -#else
> -#define DO_UPCAST(type, field, dev) container_of(dev, type, field)
> -#endif
>
> #define typeof_field(type, field) typeof(((type *)0)->field)
> #define type_check(t1,t2) ((t1*)0 - (t2*)0)
> @@ -102,7 +98,7 @@
> #if defined(__clang__)
> /* clang doesn't support gnu_printf, so use printf. */
> # define GCC_FMT_ATTR(n, m) __attribute__((format(printf, n, m)))
> -#elif defined(__GNUC__)
> +#else
> /* Use gnu_printf (qemu uses standard format strings). */
> # define GCC_FMT_ATTR(n, m) __attribute__((format(gnu_printf, n, m)))
> # if defined(_WIN32)
> @@ -112,8 +108,6 @@
> */
> # define __printf__ __gnu_printf__
> # endif
> -#else
> -#define GCC_FMT_ATTR(n, m)
> #endif
>
> #ifndef __has_warning
> --
> 2.29.0
>
>
>
Peter, Paolo, anyone to give a review?
thanks
--
Marc-André Lureau
Re: [PATCH v3 08/13] audio: remove GNUC & MSVC check
Hi
On Thu, Dec 10, 2020 at 6:35 PM Philippe Mathieu-Daudé
wrote:
>
> On 12/10/20 3:27 PM, Peter Maydell wrote:
> > On Thu, 10 Dec 2020 at 14:26, Philippe Mathieu-Daudé
> > wrote:
> >>
> >> On 12/10/20 2:47 PM, marcandre.lur...@redhat.com wrote:
> >>> From: Marc-André Lureau
> >>>
> >>> QEMU requires either GCC or Clang, which both advertize __GNUC__.
> >>> Drop MSVC fallback path.
> >>>
> >>> Note: I intentionally left further cleanups for a later work.
> >>>
> >>> Signed-off-by: Marc-André Lureau
> >>> ---
> >>> audio/audio.c | 8 +---
> >>> 1 file changed, 1 insertion(+), 7 deletions(-)
> >>>
> >>> diff --git a/audio/audio.c b/audio/audio.c
> >>> index 46578e4a58..d7a00294de 100644
> >>> --- a/audio/audio.c
> >>> +++ b/audio/audio.c
> >>> @@ -122,13 +122,7 @@ int audio_bug (const char *funcname, int cond)
> >>>
> >>> #if defined AUDIO_BREAKPOINT_ON_BUG
> >>> # if defined HOST_I386
> >>> -#if defined __GNUC__
> >>> -__asm__ ("int3");
> >>> -#elif defined _MSC_VER
> >>> -_asm _emit 0xcc;
> >>> -#else
> >>> -abort ();
> >>> -#endif
> >>> + __asm__ ("int3");
> >>
> >> This was 15 years ago... Why not simply use abort() today?
> >
> > That's what I suggested when I looked at this patch in
> > the previous version of the patchset, yes...
>
> Ah, I went back to read v2 thread. Actually I even prefer
> Gerd's suggestion to remove this dead code.
>
And I totally agree. However, I don't want to mix concerns. I am just
removing dead-code.
Re: [PATCH v3 03/13] compiler.h: remove GCC < 3 __builtin_expect fallback
Hi
On Thu, Dec 10, 2020 at 6:47 PM Peter Maydell wrote:
>
> On Thu, 10 Dec 2020 at 14:32, Philippe Mathieu-Daudé
> wrote:
> >
> > On 12/10/20 2:47 PM, marcandre.lur...@redhat.com wrote:
> > > From: Marc-André Lureau
> > >
> > > Since commit efc6c07 ("configure: Add a test for the minimum compiler
> > > version"), QEMU explicitely depends on GCC >= 4.8.
> > >
> > > (clang >= 3.4 advertizes itself as GCC >= 4.2 compatible and supports
> > > __builtin_expect too)
> > >
> > > Signed-off-by: Marc-André Lureau
>
> > Shouldn't it becleaner to test in the configure script or Meson that
> > likely() and unlikely() are not defined, and define them here
> > unconditionally?
>
> That sounds like way more infrastructure than we need if
> just checking "is it already defined" is sufficient...
>
Eh, I am just removing dead-code "#if __GNUC__ < 3". Further cleanups
can be done after.
Re: [PATCH 14/36] qdev: Move dev->realized check to qdev_property_set()
On Fri, Oct 30, 2020 at 2:10 AM Eduardo Habkost wrote: > Every single qdev property setter function manually checks > dev->realized. We can just check dev->realized inside > qdev_property_set() instead. > > The check is being added as a separate function > (qdev_prop_allow_set()) because it will become a callback later. > > Signed-off-by: Eduardo Habkost > nice Reviewed-by: Marc-André Lureau -- Marc-André Lureau
Re: [PATCH 09/36] qdev: Make qdev_get_prop_ptr() get Object* arg
On Fri, Oct 30, 2020 at 11:29 AM Marc-André Lureau <
marcandre.lur...@gmail.com> wrote:
>
>
> On Fri, Oct 30, 2020 at 2:07 AM Eduardo Habkost
> wrote:
>
>> Make the code more generic and not specific to TYPE_DEVICE.
>>
>> Signed-off-by: Eduardo Habkost
>>
>
> Nice cleanup!, but fails to build atm
>
> ../hw/block/xen-block.c:403:9: error: ‘dev’ undeclared (first use in this
> function); did you mean ‘vdev’?
> 403 | if (dev->realized) {
>
>
That seems to be the only issue though, so with that fixed:
Reviewed-by: Marc-André Lureau
--
Marc-André Lureau
Re: [PATCH 09/36] qdev: Make qdev_get_prop_ptr() get Object* arg
On Fri, Oct 30, 2020 at 2:07 AM Eduardo Habkost wrote:
> Make the code more generic and not specific to TYPE_DEVICE.
>
> Signed-off-by: Eduardo Habkost
>
Nice cleanup!, but fails to build atm
../hw/block/xen-block.c:403:9: error: ‘dev’ undeclared (first use in this
function); did you mean ‘vdev’?
403 | if (dev->realized) {
--
Marc-André Lureau
Re: [Xen-devel] [RFC v5 026/126] python: add commit-per-subsystem.py
Hi
On Fri, Oct 11, 2019 at 9:11 PM Vladimir Sementsov-Ogievskiy
wrote:
>
> Add script to automatically commit tree-wide changes per-subsystem.
Oh interesting! I guess it could use a --help or a larger commit
message to explain a bit what it does (I imagine from the rest of the
series, but someone looking at the script without context may wonder;)
You could also fix some pep8/pylint/pycodestyle
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> ---
>
> CC: Gerd Hoffmann
> CC: "Gonglei (Arei)"
> CC: Eduardo Habkost
> CC: Igor Mammedov
> CC: Laurent Vivier
> CC: Amit Shah
> CC: Kevin Wolf
> CC: Max Reitz
> CC: John Snow
> CC: Ari Sundholm
> CC: Pavel Dovgalyuk
> CC: Paolo Bonzini
> CC: Stefan Hajnoczi
> CC: Fam Zheng
> CC: Stefan Weil
> CC: Ronnie Sahlberg
> CC: Peter Lieven
> CC: Eric Blake
> CC: "Denis V. Lunev"
> CC: Markus Armbruster
> CC: Alberto Garcia
> CC: Jason Dillaman
> CC: Wen Congyang
> CC: Xie Changlong
> CC: Liu Yuan
> CC: "Richard W.M. Jones"
> CC: Jeff Cody
> CC: "Marc-André Lureau"
> CC: "Daniel P. Berrangé"
> CC: Richard Henderson
> CC: Greg Kurz
> CC: "Michael S. Tsirkin"
> CC: Marcel Apfelbaum
> CC: Beniamino Galvani
> CC: Peter Maydell
> CC: "Cédric Le Goater"
> CC: Andrew Jeffery
> CC: Joel Stanley
> CC: Andrew Baumann
> CC: "Philippe Mathieu-Daudé"
> CC: Antony Pavlov
> CC: Jean-Christophe Dubois
> CC: Peter Chubb
> CC: Subbaraya Sundeep
> CC: Eric Auger
> CC: Alistair Francis
> CC: "Edgar E. Iglesias"
> CC: Stefano Stabellini
> CC: Anthony Perard
> CC: Paul Durrant
> CC: Paul Burton
> CC: Aleksandar Rikalo
> CC: Chris Wulff
> CC: Marek Vasut
> CC: David Gibson
> CC: Cornelia Huck
> CC: Halil Pasic
> CC: Christian Borntraeger
> CC: "Hervé Poussineau"
> CC: Xiao Guangrong
> CC: Aurelien Jarno
> CC: Aleksandar Markovic
> CC: Mark Cave-Ayland
> CC: Jason Wang
> CC: Laszlo Ersek
> CC: Yuval Shaia
> CC: Palmer Dabbelt
> CC: Sagar Karandikar
> CC: Bastian Koppelmann
> CC: David Hildenbrand
> CC: Thomas Huth
> CC: Eric Farman
> CC: Matthew Rosato
> CC: Hannes Reinecke
> CC: Michael Walle
> CC: Artyom Tarasenko
> CC: Stefan Berger
> CC: Samuel Thibault
> CC: Alex Williamson
> CC: Tony Krowiak
> CC: Pierre Morel
> CC: Michael Roth
> CC: Hailiang Zhang
> CC: Juan Quintela
> CC: "Dr. David Alan Gilbert"
> CC: Luigi Rizzo
> CC: Giuseppe Lettieri
> CC: Vincenzo Maffione
> CC: Jan Kiszka
> CC: Anthony Green
> CC: Stafford Horne
> CC: Guan Xuetao
> CC: Max Filippov
> CC: qemu-bl...@nongnu.org
> CC: integrat...@gluster.org
> CC: sheep...@lists.wpkg.org
> CC: qemu-...@nongnu.org
> CC: xen-devel@lists.xenproject.org
> CC: qemu-...@nongnu.org
> CC: qemu-s3...@nongnu.org
> CC: qemu-ri...@nongnu.org
>
> python/commit-per-subsystem.py | 204 +
> 1 file changed, 204 insertions(+)
> create mode 100755 python/commit-per-subsystem.py
>
> diff --git a/python/commit-per-subsystem.py b/python/commit-per-subsystem.py
> new file mode 100755
> index 00..2ccf84cb15
> --- /dev/null
> +++ b/python/commit-per-subsystem.py
> @@ -0,0 +1,204 @@
> +#!/usr/bin/env python3
> +#
> +# Copyright (c) 2019 Virtuozzo International GmbH
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program 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 General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +import subprocess
> +import sys
> +import os
> +import glob
> +
> +
> +def git_add(pattern):
> +subprocess.run(['git', 'add', pattern])
> +
> +
> +def git_commit(msg):
> +subprocess.run(['git', 'commit', '-m', msg], capture_output=True)
> +
> +
> +def git_changed_files():
> +ret = subprocess.check_output(['git', 'diff', '--name-only'],
> encoding='utf-8').split('\n')
> +if ret[-1] == '':
> +del ret[-1]
> +return ret
> +
> +
> +maintainers = sys.argv[1]
> +message = sys.argv[
Re: [Xen-devel] [RFC v5 024/126] error: auto propagated local_err
On Fri, Oct 11, 2019 at 10:11 PM Vladimir Sementsov-Ogievskiy
wrote:
>
> Here is introduced ERRP_AUTO_PROPAGATE macro, to be used at start of
> functions with errp OUT parameter.
>
> It has three goals:
>
> 1. Fix issue with error_fatal & error_prepend/error_append_hint: user
> can't see this additional information, because exit() happens in
> error_setg earlier than information is added. [Reported by Greg Kurz]
>
> 2. Fix issue with error_abort & error_propagate: when we wrap
> error_abort by local_err+error_propagate, resulting coredump will
> refer to error_propagate and not to the place where error happened.
> (the macro itself doesn't fix the issue, but it allows to [3.] drop all
> local_err+error_propagate pattern, which will definitely fix the issue)
> [Reported by Kevin Wolf]
>
> 3. Drop local_err+error_propagate pattern, which is used to workaround
> void functions with errp parameter, when caller wants to know resulting
> status. (Note: actually these functions could be merely updated to
> return int error code).
>
> To achieve these goals, we need to add invocation of the macro at start
> of functions, which needs error_prepend/error_append_hint (1.); add
> invocation of the macro at start of functions which do
> local_err+error_propagate scenario the check errors, drop local errors
> from them and just use *errp instead (2., 3.).
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> Reviewed-by: Eric Blake
> ---
>
> CC: Gerd Hoffmann
> CC: "Gonglei (Arei)"
> CC: Eduardo Habkost
> CC: Igor Mammedov
> CC: Laurent Vivier
> CC: Amit Shah
> CC: Kevin Wolf
> CC: Max Reitz
> CC: John Snow
> CC: Ari Sundholm
> CC: Pavel Dovgalyuk
> CC: Paolo Bonzini
> CC: Stefan Hajnoczi
> CC: Fam Zheng
> CC: Stefan Weil
> CC: Ronnie Sahlberg
> CC: Peter Lieven
> CC: Eric Blake
> CC: "Denis V. Lunev"
> CC: Markus Armbruster
> CC: Alberto Garcia
> CC: Jason Dillaman
> CC: Wen Congyang
> CC: Xie Changlong
> CC: Liu Yuan
> CC: "Richard W.M. Jones"
> CC: Jeff Cody
> CC: "Marc-André Lureau"
> CC: "Daniel P. Berrangé"
> CC: Richard Henderson
> CC: Greg Kurz
> CC: "Michael S. Tsirkin"
> CC: Marcel Apfelbaum
> CC: Beniamino Galvani
> CC: Peter Maydell
> CC: "Cédric Le Goater"
> CC: Andrew Jeffery
> CC: Joel Stanley
> CC: Andrew Baumann
> CC: "Philippe Mathieu-Daudé"
> CC: Antony Pavlov
> CC: Jean-Christophe Dubois
> CC: Peter Chubb
> CC: Subbaraya Sundeep
> CC: Eric Auger
> CC: Alistair Francis
> CC: "Edgar E. Iglesias"
> CC: Stefano Stabellini
> CC: Anthony Perard
> CC: Paul Durrant
> CC: Paul Burton
> CC: Aleksandar Rikalo
> CC: Chris Wulff
> CC: Marek Vasut
> CC: David Gibson
> CC: Cornelia Huck
> CC: Halil Pasic
> CC: Christian Borntraeger
> CC: "Hervé Poussineau"
> CC: Xiao Guangrong
> CC: Aurelien Jarno
> CC: Aleksandar Markovic
> CC: Mark Cave-Ayland
> CC: Jason Wang
> CC: Laszlo Ersek
> CC: Yuval Shaia
> CC: Palmer Dabbelt
> CC: Sagar Karandikar
> CC: Bastian Koppelmann
> CC: David Hildenbrand
> CC: Thomas Huth
> CC: Eric Farman
> CC: Matthew Rosato
> CC: Hannes Reinecke
> CC: Michael Walle
> CC: Artyom Tarasenko
> CC: Stefan Berger
> CC: Samuel Thibault
> CC: Alex Williamson
> CC: Tony Krowiak
> CC: Pierre Morel
> CC: Michael Roth
> CC: Hailiang Zhang
> CC: Juan Quintela
> CC: "Dr. David Alan Gilbert"
> CC: Luigi Rizzo
> CC: Giuseppe Lettieri
> CC: Vincenzo Maffione
> CC: Jan Kiszka
> CC: Anthony Green
> CC: Stafford Horne
> CC: Guan Xuetao
> CC: Max Filippov
> CC: qemu-bl...@nongnu.org
> CC: integrat...@gluster.org
> CC: sheep...@lists.wpkg.org
> CC: qemu-...@nongnu.org
> CC: xen-devel@lists.xenproject.org
> CC: qemu-...@nongnu.org
> CC: qemu-s3...@nongnu.org
> CC: qemu-ri...@nongnu.org
>
> include/qapi/error.h | 38 ++
> 1 file changed, 38 insertions(+)
>
> diff --git a/include/qapi/error.h b/include/qapi/error.h
> index d6898d833b..47238d9065 100644
> --- a/include/qapi/error.h
> +++ b/include/qapi/error.h
> @@ -345,6 +345,44 @@ void error_set_internal(Error **errp,
> ErrorClass err_class, const char *fmt, ...)
> GCC_FMT_ATTR(6, 7);
>
> +typedef struct ErrorPropagator {
> +Error *local_err;
> +Error **errp;
> +} ErrorPropagator;
> +
> +static inline void error_propagator_cleanup(ErrorPropagator *prop)
> +{
> +error_propagate(prop->errp, prop->local_err);
> +}
&g
Re: [Xen-devel] [RFC v5 000/126] error: auto propagated local_err
/cpu/core.c > > hw/cpu/core.c > > hw/sd/ssi-sd.c > > hw/sd/ssi-sd.c > > iothread.c > > iothread.c > > memory_mapping.c > > memory_mapping.c > > target/tilegx/cpu.c > > target/tilegx/cpu.c > > tests/test-image-locking.c > > tests/test-image-locking.c > > util/qemu-config.c > > util/qemu-config.c > > > > > > Vladimir Sementsov-Ogievskiy (126): > >hw/core/loader-fit: fix freeing errp in fit_load_fdt > >net/net: Clean up variable shadowing in net_client_init() > >error: rename errp to errp_in where it is IN-argument > >hmp: drop Error pointer indirection in hmp_handle_error > >vnc: drop Error pointer indirection in vnc_client_io_error > >qdev-monitor: well form error hint helpers > >nbd: well form nbd_iter_channel_error errp handler > >ppc: well form kvmppc_hint_smt_possible error hint helper > >9pfs: well form error hint helpers > >hw/core/qdev: cleanup Error ** variables > >block/snapshot: rename Error ** parameter to more common errp > >hw/i386/amd_iommu: rename Error ** parameter to more common errp > >qga: rename Error ** parameter to more common errp > >monitor/qmp-cmds: rename Error ** parameter to more common errp > >hw/s390x: rename Error ** parameter to more common errp > >hw/sd: rename Error ** parameter to more common errp > >hw/tpm: rename Error ** parameter to more common errp > >hw/usb: rename Error ** parameter to more common errp > >include/block/snapshot.h: rename Error ** parameter to more common > > errp > >include/qom/object.h: rename Error ** parameter to more common errp > >qapi/error: add (Error **errp) cleaning APIs > >backends/cryptodev: drop local_err from cryptodev_backend_complete() > >hw/vfio/ap: drop local_err from vfio_ap_realize > >error: auto propagated local_err > >scripts: add coccinelle script to use auto propagated errp > >python: add commit-per-subsystem.py > >misc: introduce ERRP_AUTO_PROPAGATE > >s390x: introduce ERRP_AUTO_PROPAGATE > >tcg: introduce ERRP_AUTO_PROPAGATE > >kvm: introduce ERRP_AUTO_PROPAGATE > >xen: introduce ERRP_AUTO_PROPAGATE > >Hosts: introduce ERRP_AUTO_PROPAGATE > >ARM Machines: introduce ERRP_AUTO_PROPAGATE > >MIPS Machines: introduce ERRP_AUTO_PROPAGATE > >PowerPC Machines: introduce ERRP_AUTO_PROPAGATE > >SPARC Machines: introduce ERRP_AUTO_PROPAGATE > >S390 Machines: introduce ERRP_AUTO_PROPAGATE > >X86 Machines: introduce ERRP_AUTO_PROPAGATE > >IDE: introduce ERRP_AUTO_PROPAGATE > >Floppy: introduce ERRP_AUTO_PROPAGATE > >IPack: introduce ERRP_AUTO_PROPAGATE > >PCI: introduce ERRP_AUTO_PROPAGATE > >ACPI/SMBIOS: introduce ERRP_AUTO_PROPAGATE > >Network devices: introduce ERRP_AUTO_PROPAGATE > >pflash: introduce ERRP_AUTO_PROPAGATE > >SCSI: introduce ERRP_AUTO_PROPAGATE > >SD (Secure Card): introduce ERRP_AUTO_PROPAGATE > >USB: introduce ERRP_AUTO_PROPAGATE > >USB (serial adapter): introduce ERRP_AUTO_PROPAGATE > >VFIO: introduce ERRP_AUTO_PROPAGATE > >vfio-ccw: introduce ERRP_AUTO_PROPAGATE > >vhost: introduce ERRP_AUTO_PROPAGATE > >virtio: introduce ERRP_AUTO_PROPAGATE > >virtio-9p: introduce ERRP_AUTO_PROPAGATE > >virtio-blk: introduce ERRP_AUTO_PROPAGATE > >virtio-ccw: introduce ERRP_AUTO_PROPAGATE > >virtio-input: introduce ERRP_AUTO_PROPAGATE > >virtio-serial: introduce ERRP_AUTO_PROPAGATE > >virtio-rng: introduce ERRP_AUTO_PROPAGATE > >megasas: introduce ERRP_AUTO_PROPAGATE > >NVDIMM: introduce ERRP_AUTO_PROPAGATE > >eepro100: introduce ERRP_AUTO_PROPAGATE > >virtio-gpu: introduce ERRP_AUTO_PROPAGATE > >fw_cfg: introduce ERRP_AUTO_PROPAGATE > >XIVE: introduce ERRP_AUTO_PROPAGATE > >Audio: introduce ERRP_AUTO_PROPAGATE > >block: introduce ERRP_AUTO_PROPAGATE > >scsi: introduce ERRP_AUTO_PROPAGATE > >chardev: introduce ERRP_AUTO_PROPAGATE > >cmdline: introduce ERRP_AUTO_PROPAGATE > >Dump: introduce ERRP_AUTO_PROPAGATE > >Memory API: introduce ERRP_AUTO_PROPAGATE > >SPICE: introduce ERRP_AUTO_PROPAGATE > >Graphics: introduce ERRP_AUTO_PROPAGATE > >Main loop: introduce ERRP_AUTO_PROPAGATE > >Human Monitor (HMP): introduce ERRP_AUTO_PROPAGATE > >net: introduce ERRP_AUTO_PROPAGATE > >hostmem: introduce ERRP_AUTO_PROPAGATE > >cryptodev: introduce
Re: [Xen-devel] [PATCH v3 24/25] chardev: Let qemu_chr_fe_write[_all] use size_t type argument
On Wed, Feb 20, 2019 at 2:08 AM Philippe Mathieu-Daudé
wrote:
>
> All caller have been audited and call these functions with
> unsigned arguments.
>
> Most of them use a size_t argument, or directly pass sizeof().
>
> One case is unclear: the mux_chr_write() call in chardev/char-mux.c.
> There we add an assert (which will be removed in few patches) and
> cast the parameter as size_t to make explicit this value is unsigned.
mux_chr_write() is called indirectly from qemu_chr_fe_write(), so the
same argument applies here. Fine with or without the assert().
>
> Suggested-by: Paolo Bonzini
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> chardev/char-fe.c | 4 ++--
> chardev/char-mux.c| 3 ++-
> include/chardev/char-fe.h | 4 ++--
> 3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/chardev/char-fe.c b/chardev/char-fe.c
> index f3530a90e6..ab2a01709d 100644
> --- a/chardev/char-fe.c
> +++ b/chardev/char-fe.c
> @@ -31,7 +31,7 @@
> #include "chardev/char-io.h"
> #include "chardev/char-mux.h"
>
> -int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len)
> +int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, size_t len)
> {
> Chardev *s = be->chr;
>
> @@ -42,7 +42,7 @@ int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf,
> int len)
> return qemu_chr_write(s, buf, len, false);
> }
>
> -int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, int len)
> +int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, size_t len)
> {
> Chardev *s = be->chr;
>
> diff --git a/chardev/char-mux.c b/chardev/char-mux.c
> index 23aa82125d..7a3ff21db4 100644
> --- a/chardev/char-mux.c
> +++ b/chardev/char-mux.c
> @@ -38,7 +38,8 @@ static int mux_chr_write(Chardev *chr, const uint8_t *buf,
> int len)
> MuxChardev *d = MUX_CHARDEV(chr);
> int ret;
> if (!d->timestamps) {
> -ret = qemu_chr_fe_write(>chr, buf, len);
> +assert(len >= 0);
> +ret = qemu_chr_fe_write(>chr, buf, (size_t)len);
> } else {
> int i;
>
> diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
> index aa1b864ccd..5fb2c2e7ec 100644
> --- a/include/chardev/char-fe.h
> +++ b/include/chardev/char-fe.h
> @@ -203,7 +203,7 @@ guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition
> cond,
> *
> * Returns: the number of bytes consumed (0 if no associated Chardev)
> */
> -int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, int len);
> +int qemu_chr_fe_write(CharBackend *be, const uint8_t *buf, size_t len);
>
> /**
> * qemu_chr_fe_write_all:
> @@ -217,7 +217,7 @@ int qemu_chr_fe_write(CharBackend *be, const uint8_t
> *buf, int len);
> *
> * Returns: the number of bytes consumed (0 if no associated Chardev)
> */
> -int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, int len);
> +int qemu_chr_fe_write_all(CharBackend *be, const uint8_t *buf, size_t len);
>
> /**
> * qemu_chr_fe_read_all:
> --
> 2.20.1
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 23/25] hw/ipmi: Assert outlen > outpos
Hi
On Wed, Feb 20, 2019 at 2:08 AM Philippe Mathieu-Daudé
wrote:
>
> A througfull audit show that all time data is added to outbuf[],
througfull? :) thorough?
> 'outlen' is incremented. Then at creation and each time
> continue_send() returns it pass thru check_reset which resets
> 'outpos', thus we always have 'outlen >= outpos'.
> Also due to the check on entry, we know outlen != 0.
> We can then add an assertion on 'outlen > outpos', which will
hmm, I think you could assert 'outlen >= outpos' only. I don't buy
your argument about 'outlen > outpos' (because outlen != 0?)
> helps the next patch to safely convert 'outlen - outpos' as an
> unsigned type (size_t).
>
> Make this assertion explicit by casting 'outlen - outpos' size_t.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/ipmi/ipmi_bmc_extern.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
> index bf0b7ee0f5..ca61b04942 100644
> --- a/hw/ipmi/ipmi_bmc_extern.c
> +++ b/hw/ipmi/ipmi_bmc_extern.c
> @@ -107,8 +107,9 @@ static void continue_send(IPMIBmcExtern *ibe)
> goto check_reset;
> }
> send:
> +assert(ibe->outlen > ibe->outpos);
> ret = qemu_chr_fe_write(>chr, ibe->outbuf + ibe->outpos,
> -ibe->outlen - ibe->outpos);
> +(size_t)(ibe->outlen - ibe->outpos));
> if (ret > 0) {
> ibe->outpos += ret;
> }
> --
> 2.20.1
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 04/25] chardev: Let qemu_chr_be_can_write() return a size_t types
Hi
On Wed, Feb 20, 2019 at 12:26 PM Philippe Mathieu-Daudé
wrote:
>
> On 2/20/19 11:40 AM, Marc-André Lureau wrote:
> > Hi
> >
> > On Wed, Feb 20, 2019 at 2:04 AM Philippe Mathieu-Daudé
> > wrote:
> >>
> >> In the previous commit we added an assert to be sure than
> >> qemu_chr_be_can_write() will never return a negative value.
> >> We can now change its prototype to return a size_t.
> >> Adapt the backends accordingly.
> >
> > Each variable you change to an unsigned type, we should check it isn't
> > used with negative values.
>
> Fortunately the preprocessor can help here!
>
> Oh I forgot to write down the steps I ran:
>
> # enable warnings
> $ configure \
> --extra-cflags='-Wtype-limits -Wsign-conversion -Wsign-compare' \
> --disable-werror
>
> # since there are many sign abuse, build blindly
> $ make 2>/dev/null
>
> # now refresh the source we modified
> $ git diff --name-only origin/master \
> | egrep \.c\$ \
> | xargs touch
>
> # build again and carefully watch the warnings
> # (there are many unuseful #include warnings, ignore them)
> $ make
>
> >>
> >> Suggested-by: Paolo Bonzini
> >> Signed-off-by: Philippe Mathieu-Daudé
> >> ---
> >> chardev/baum.c| 6 +++---
> >> chardev/char-fd.c | 2 +-
> >> chardev/char-pty.c| 4 ++--
> >> chardev/char-socket.c | 7 ---
> >> chardev/char-udp.c| 4 ++--
> >> chardev/char-win.c| 2 +-
> >> chardev/char.c| 2 +-
> >> chardev/msmouse.c | 4 ++--
> >> chardev/spice.c | 2 +-
> >> chardev/wctablet.c| 4 ++--
> >> hw/bt/hci-csr.c | 2 +-
> >> include/chardev/char-fd.h | 2 +-
> >> include/chardev/char.h| 2 +-
> >> ui/console.c | 6 +++---
> >> 14 files changed, 25 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/chardev/baum.c b/chardev/baum.c
> >> index 78b0c87625..1d69d62158 100644
> >> --- a/chardev/baum.c
> >> +++ b/chardev/baum.c
> >> @@ -265,7 +265,7 @@ static int baum_deferred_init(BaumChardev *baum)
> >> static void baum_chr_accept_input(struct Chardev *chr)
> >> {
> >> BaumChardev *baum = BAUM_CHARDEV(chr);
> >> -int room, first;
> >> +size_t room, first;
> >>
> >> if (!baum->out_buf_used)
> >> return;
> >> @@ -292,7 +292,7 @@ static void baum_write_packet(BaumChardev *baum, const
> >> uint8_t *buf, int len)
> >> {
> >> Chardev *chr = CHARDEV(baum);
> >> uint8_t io_buf[1 + 2 * len], *cur = io_buf;
> >> -int room;
> >> +size_t room;
> >> *cur++ = ESC;
> >> while (len--)
> >> if ((*cur++ = *buf++) == ESC)
> >> @@ -303,7 +303,7 @@ static void baum_write_packet(BaumChardev *baum, const
> >> uint8_t *buf, int len)
> >> /* Fits */
> >> qemu_chr_be_write(chr, io_buf, len);
> >> } else {
> >> -int first;
> >> +size_t first;
> >> uint8_t out;
> >> /* Can't fit all, send what can be, and store the rest. */
> >> qemu_chr_be_write(chr, io_buf, room);
> >
> > baum room & first are only used for non-negative capacity values. ack
> >
> >> diff --git a/chardev/char-fd.c b/chardev/char-fd.c
> >> index 2421d8e216..0fe2822869 100644
> >> --- a/chardev/char-fd.c
> >> +++ b/chardev/char-fd.c
> >> @@ -43,7 +43,7 @@ static gboolean fd_chr_read(QIOChannel *chan,
> >> GIOCondition cond, void *opaque)
> >> {
> >> Chardev *chr = CHARDEV(opaque);
> >> FDChardev *s = FD_CHARDEV(opaque);
> >> -int len;
> >> +size_t len;
> >> uint8_t buf[CHR_READ_BUF_LEN];
> >> ssize_t ret;
> >>
> >
> > fd len is only used for non-negative buffer size. ack
> >
> >> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> >> index f6ddef..eae25f043b 100644
> >> --- a/chardev/char-pty.c
> >> +++ b/chardev/char-pty.c
> >> @@ -34,7 +34,7 @@
> >> typedef struct {
> >> Chardev parent;
> >> QIOChannel *ioc;
> >> -int read_bytes;
> >> +size_t read_bytes;
> >>
> >
> > Only set with values returned from qemu_chr_be_can_write(), ack
> >
> >>
Re: [Xen-devel] [PATCH v3 17/25] net/filter-mirror: Use size_t
Hi
On Wed, Feb 20, 2019 at 2:07 AM Philippe Mathieu-Daudé
wrote:
>
> Since iov_size() returns a size_t, no need to use a signed type.
And it is the variable only purpose.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> net/filter-mirror.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/filter-mirror.c b/net/filter-mirror.c
> index 3a61cf21e8..97b52d0544 100644
> --- a/net/filter-mirror.c
> +++ b/net/filter-mirror.c
> @@ -48,7 +48,7 @@ static int filter_send(MirrorState *s,
> {
> NetFilterState *nf = NETFILTER(s);
> int ret = 0;
> -ssize_t size = 0;
> +size_t size = 0;
> uint32_t len = 0;
> char *buf;
>
> --
> 2.20.1
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 16/25] tpm: Use size_t to hold sizes
On Wed, Feb 20, 2019 at 2:06 AM Philippe Mathieu-Daudé
wrote:
>
> Avoid to use a signed type to hold an unsigned value.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> hw/tpm/tpm_emulator.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
> index 70f4b10284..931e56f6ed 100644
> --- a/hw/tpm/tpm_emulator.c
> +++ b/hw/tpm/tpm_emulator.c
> @@ -87,17 +87,18 @@ static int tpm_emulator_ctrlcmd(TPMEmulator *tpm,
> unsigned long cmd, void *msg,
> {
> CharBackend *dev = >ctrl_chr;
> uint32_t cmd_no = cpu_to_be32(cmd);
> -ssize_t n = sizeof(uint32_t) + msg_len_in;
> +size_t sz = sizeof(uint32_t) + msg_len_in;
> +ssize_t n;
> uint8_t *buf = NULL;
> int ret = -1;
>
> qemu_mutex_lock(>mutex);
>
> -buf = g_alloca(n);
> +buf = g_alloca(sz);
> memcpy(buf, _no, sizeof(cmd_no));
> memcpy(buf + sizeof(cmd_no), msg, msg_len_in);
>
> -n = qemu_chr_fe_write_all(dev, buf, n);
> +n = qemu_chr_fe_write_all(dev, buf, sz);
> if (n <= 0) {
> goto end;
> }
> --
> 2.20.1
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 14/25] virtio-serial: Let VirtIOSerialPortClass::have_data() use size_t
On Wed, Feb 20, 2019 at 2:06 AM Philippe Mathieu-Daudé
wrote:
>
> Both callers in hw/char/virtio-serial-bus.c provide unsigned values,
> even the trace event display an unsigned value.
> Convert the have_data() handler to take an unsigned value.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> It is funny/scary that there are big comments about how to treat
> errors to set the return value, then the return value is simply
> ignored by the caller.
have_data() return value? yes, the code may have change and the
comment doesn't seem to reflect accurately what's going on.
> ---
> hw/char/virtio-console.c | 2 +-
> include/hw/virtio/virtio-serial.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
> index 2cbe1d4ed5..19639dca3b 100644
> --- a/hw/char/virtio-console.c
> +++ b/hw/char/virtio-console.c
> @@ -45,7 +45,7 @@ static gboolean chr_write_unblocked(GIOChannel *chan,
> GIOCondition cond,
>
> /* Callback function that's called when the guest sends us data */
> static ssize_t flush_buf(VirtIOSerialPort *port,
> - const uint8_t *buf, ssize_t len)
> + const uint8_t *buf, size_t len)
> {
> VirtConsole *vcon = VIRTIO_CONSOLE(port);
> ssize_t ret;
> diff --git a/include/hw/virtio/virtio-serial.h
> b/include/hw/virtio/virtio-serial.h
> index 12657a9f39..f1a5ccf4f7 100644
> --- a/include/hw/virtio/virtio-serial.h
> +++ b/include/hw/virtio/virtio-serial.h
> @@ -81,7 +81,7 @@ typedef struct VirtIOSerialPortClass {
> * 'len'. In this case, throttling will be enabled for this port.
> */
> ssize_t (*have_data)(VirtIOSerialPort *port, const uint8_t *buf,
> - ssize_t len);
> + size_t len);
> } VirtIOSerialPortClass;
>
> /*
> --
> 2.20.1
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 12/25] xen: Let buffer_append() return the size consumed
Hi
On Wed, Feb 20, 2019 at 2:06 AM Philippe Mathieu-Daudé
wrote:
>
> The buffer.size and buffer.consumed fields are only updated within the
> buffer_append() body. We can simply let buffer_append() return the
> difference (the buffer consumed).
>
> Signed-off-by: Philippe Mathieu-Daudé
This is weird, why not introduce a buffer_size() function instead?
> ---
> hw/char/xen_console.c | 13 -
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index 083b2c8e2a..1a30014a11 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -48,7 +48,7 @@ struct XenConsole {
> int backlog;
> };
>
> -static void buffer_append(struct XenConsole *con)
> +static ssize_t buffer_append(struct XenConsole *con)
> {
> struct buffer *buffer = >buffer;
> XENCONS_RING_IDX cons, prod, size;
> @@ -59,8 +59,9 @@ static void buffer_append(struct XenConsole *con)
> xen_mb();
>
> size = prod - cons;
> -if ((size == 0) || (size > sizeof(intf->out)))
> -return;
> +if ((size == 0) || (size > sizeof(intf->out))) {
> +goto out;
> +}
>
> if ((buffer->capacity - buffer->size) < size) {
> buffer->capacity += (size + 1024);
> @@ -89,6 +90,9 @@ static void buffer_append(struct XenConsole *con)
> if (buffer->consumed > buffer->max_capacity - over)
> buffer->consumed = buffer->max_capacity - over;
> }
> +
> + out:
> +return buffer->size - buffer->consumed;
> }
>
> static void buffer_advance(struct buffer *buffer, size_t len)
> @@ -281,8 +285,7 @@ static void con_event(struct XenLegacyDevice *xendev)
> struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
> ssize_t size;
>
> -buffer_append(con);
> -size = con->buffer.size - con->buffer.consumed;
> +size = buffer_append(con);
> if (size) {
> xencons_send(con, size);
> }
> --
> 2.20.1
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 11/25] xen: Let xencons_send() take a 'size' argument
On Wed, Feb 20, 2019 at 2:05 AM Philippe Mathieu-Daudé
wrote:
>
> The single caller of xencons_send(), con_event() already use the
> difference 'con->buffer.size - con->buffer.consumed'.
> Deduplicate by passing the difference as an argument.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> hw/char/xen_console.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index 91f34ef06c..083b2c8e2a 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -144,11 +144,10 @@ static void xencons_receive(void *opaque, const uint8_t
> *buf, int len)
> xen_pv_send_notify(>xendev);
> }
>
> -static void xencons_send(struct XenConsole *con)
> +static void xencons_send(struct XenConsole *con, ssize_t size)
> {
> -ssize_t len, size;
> +ssize_t len;
>
> -size = con->buffer.size - con->buffer.consumed;
> if (qemu_chr_fe_backend_connected(>chr)) {
> len = qemu_chr_fe_write(>chr,
> con->buffer.data + con->buffer.consumed,
> @@ -280,10 +279,13 @@ static void con_disconnect(struct XenLegacyDevice
> *xendev)
> static void con_event(struct XenLegacyDevice *xendev)
> {
> struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
> +ssize_t size;
>
> buffer_append(con);
> -if (con->buffer.size - con->buffer.consumed)
> -xencons_send(con);
> +size = con->buffer.size - con->buffer.consumed;
> +if (size) {
> +xencons_send(con, size);
> +}
> }
>
> /* */
> --
> 2.20.1
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 09/25] vhost-user: Express sizeof with size_t
On Wed, Feb 20, 2019 at 2:05 AM Philippe Mathieu-Daudé
wrote:
>
> VHOST_USER_HDR_SIZE uses offsetof(), thus is an expression of type
> size_t. Update the format string accordingly.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> hw/virtio/vhost-user.c | 14 --
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 564a31d12c..2eb7143d3d 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -215,11 +215,12 @@ static int vhost_user_read(struct vhost_dev *dev,
> VhostUserMsg *msg)
> struct vhost_user *u = dev->opaque;
> CharBackend *chr = u->user->chr;
> uint8_t *p = (uint8_t *) msg;
> -int r, size = VHOST_USER_HDR_SIZE;
> +int r;
> +size_t size = VHOST_USER_HDR_SIZE;
>
> r = qemu_chr_fe_read_all(chr, p, size);
> if (r != size) {
> -error_report("Failed to read msg header. Read %d instead of %d."
> +error_report("Failed to read msg header. Read %d instead of %zu."
> " Original request %d.", r, size, msg->hdr.request);
> goto fail;
> }
> @@ -235,7 +236,7 @@ static int vhost_user_read(struct vhost_dev *dev,
> VhostUserMsg *msg)
> /* validate message size is sane */
> if (msg->hdr.size > VHOST_USER_PAYLOAD_SIZE) {
> error_report("Failed to read msg header."
> -" Size %d exceeds the maximum %zu.", msg->hdr.size,
> +" Size %u exceeds the maximum %zu.", msg->hdr.size,
> VHOST_USER_PAYLOAD_SIZE);
> goto fail;
> }
> @@ -246,7 +247,7 @@ static int vhost_user_read(struct vhost_dev *dev,
> VhostUserMsg *msg)
> r = qemu_chr_fe_read_all(chr, p, size);
> if (r != size) {
> error_report("Failed to read msg payload."
> - " Read %d instead of %d.", r, msg->hdr.size);
> + " Read %d instead of %u.", r, msg->hdr.size);
> goto fail;
> }
> }
> @@ -300,7 +301,8 @@ static int vhost_user_write(struct vhost_dev *dev,
> VhostUserMsg *msg,
> {
> struct vhost_user *u = dev->opaque;
> CharBackend *chr = u->user->chr;
> -int ret, size = VHOST_USER_HDR_SIZE + msg->hdr.size;
> +int ret;
> +size_t size = VHOST_USER_HDR_SIZE + msg->hdr.size;
>
> /*
> * For non-vring specific requests, like VHOST_USER_SET_MEM_TABLE,
> @@ -320,7 +322,7 @@ static int vhost_user_write(struct vhost_dev *dev,
> VhostUserMsg *msg,
> ret = qemu_chr_fe_write_all(chr, (const uint8_t *) msg, size);
> if (ret != size) {
> error_report("Failed to write msg."
> - " Wrote %d instead of %d.", ret, size);
> + " Wrote %d instead of %zu.", ret, size);
> return -1;
> }
>
> --
> 2.20.1
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 07/25] gdbstub: Let put_buffer() use size_t
On Wed, Feb 20, 2019 at 2:04 AM Philippe Mathieu-Daudé
wrote:
>
> All callers provide a size_t argument, we can safely use size_t
> for this function.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> gdbstub.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 69340d7cd1..860e9bb7c7 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -475,10 +475,10 @@ static int gdb_continue_partial(GDBState *s, char
> *newstates)
> return res;
> }
>
> -static void put_buffer(GDBState *s, const uint8_t *buf, int len)
> +static void put_buffer(GDBState *s, const uint8_t *buf, size_t len)
> {
> #ifdef CONFIG_USER_ONLY
> -int ret;
> +ssize_t ret;
>
> while (len > 0) {
> ret = send(s->fd, buf, len, 0);
> --
> 2.20.1
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 06/25] gdbstub: Use size_t to hold GDBState::last_packet_len
On Wed, Feb 20, 2019 at 2:04 AM Philippe Mathieu-Daudé
wrote:
>
> In put_packet_binary() we have:
>
> uint8_t *p;
> for(;;) {
> p = s->last_packet;
> *(p++) = ...
> s->last_packet_len = p - s->last_packet;
> put_buffer(s, (uint8_t *)s->last_packet, s->last_packet_len);
>
> The 'p' pointer start at s->last_packet, then is only incremented.
> Since we have "p >= s->last_packet", we are sure than
> "p - s->last_packet >= 0", thus "p - s->last_packet" is positive.
>
> The few other places where s->last_packet_len is set is with constant
> positive values.
>
> It makes sense to use size_t to hold last_packet_len values.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> gdbstub.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 76eca3bb7e..69340d7cd1 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -323,7 +323,7 @@ typedef struct GDBState {
> int line_sum; /* running checksum */
> int line_csum; /* checksum at the end of the packet */
> uint8_t last_packet[MAX_PACKET_LENGTH + 4];
> -int last_packet_len;
> +size_t last_packet_len;
> int signal;
> #ifdef CONFIG_USER_ONLY
> int fd;
> --
> 2.20.1
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 05/25] gdbstub: Use size_t for strlen() return value
On Wed, Feb 20, 2019 at 2:04 AM Philippe Mathieu-Daudé
wrote:
>
> Since strlen() returns an unsigned value, it is pointless to
> convert it to a signed one. Use size_t to hold its return value.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
(it looks like the variable is hiding len from outer scope btw)
> ---
> gdbstub.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index bc774ae992..76eca3bb7e 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -1693,7 +1693,7 @@ static int gdb_handle_packet(GDBState *s, const char
> *line_buf)
> }
> #else /* !CONFIG_USER_ONLY */
> else if (strncmp(p, "Rcmd,", 5) == 0) {
> -int len = strlen(p + 5);
> +size_t len = strlen(p + 5);
>
> if ((len % 2) != 0) {
> put_packet(s, "E01");
> --
> 2.20.1
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 00/25] chardev: Convert qemu_chr_write() to take a size_t argument
Hi On Wed, Feb 20, 2019 at 2:02 AM Philippe Mathieu-Daudé wrote: > > Hi, > > This series convert the chardev::qemu_chr_write() to take unsigned > length argument. To do so I went through all caller and checked if > there are no negative value possible. Changing signedness is problematic and can easily introduce bugs that are easy to miss during review. I agree with Cornelia about idiomatic use of int. Changing "int" for "size_t" isn't systematically a clear win. Even Google C++ style recommends to avoid unsigned types "(except for representing bitfields or modular arithmetic). Do not use an unsigned type merely to assert that a variable is non-negative." https://google.github.io/styleguide/cppguide.html#Integer_Types - see rationale Since Paolo you suggested the change, could you give some convincing arguments that it's worth taking the plunge? thanks > > I'm having headaches with the Xen backend, talking with Marc-André > he suggested I ask help to the Xen maintainers. > > Since the series is becoming quite big, I splitted it: > - part 1: convert qemu_chr_write() > - part 2: convert IOReadHandler > > part 1 can be reviewed and merged without part 2. > > The git diffstat is not huge, but there are various chardev subsystems > so many maintainers to ask for Ack. > > v2: https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02396.html > v1: https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02200.html > (from Prasad J Pandit) > Changes requested by Paolo: > https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02294.html > > Please review, > > Phil. > > Philippe Mathieu-Daudé (25): > chardev: Simplify IOWatchPoll::fd_can_read as a GSourceFunc > chardev: Assert IOCanReadHandler can not be negative > chardev/wctablet: Use unsigned type to hold unsigned value > chardev: Let qemu_chr_be_can_write() return a size_t types > gdbstub: Use size_t for strlen() return value > gdbstub: Use size_t to hold GDBState::last_packet_len > gdbstub: Let put_buffer() use size_t > ui/gtk: Remove pointless cast > vhost-user: Express sizeof with size_t > usb-redir: Verify usbredirparser_write get called with positive count > xen: Let xencons_send() take a 'size' argument > xen: Let buffer_append() return the size consumed > RFC xen: Let buffer_append() return a size_t > virtio-serial: Let VirtIOSerialPortClass::have_data() use size_t > spapr-vty: Let vty_putchars() use size_t > tpm: Use size_t to hold sizes > net/filter-mirror: Use size_t > s390x/3270: Let insert_IAC_escape_char() use size_t > s390/ebcdic: Use size_t to iterate over arrays > s390x/sclp: Use a const variable to improve readability > s390x/sclp: Use size_t in process_mdb() > s390x/sclp: Let write_console_data() take a size_t > hw/ipmi: Assert outlen > outpos > chardev: Let qemu_chr_fe_write[_all] use size_t type argument > chardev: Let qemu_chr_write[_all] use size_t > > chardev/baum.c| 6 +++--- > chardev/char-fd.c | 6 +++--- > chardev/char-fe.c | 4 ++-- > chardev/char-io.c | 6 +++--- > chardev/char-mux.c| 3 ++- > chardev/char-pty.c| 8 > chardev/char-socket.c | 13 +++-- > chardev/char-udp.c| 8 > chardev/char-win.c| 2 +- > chardev/char.c| 15 +-- > chardev/msmouse.c | 4 ++-- > chardev/spice.c | 2 +- > chardev/trace-events | 2 +- > chardev/wctablet.c| 9 + > gdbstub.c | 8 > hw/bt/hci-csr.c | 2 +- > hw/char/sclpconsole-lm.c | 12 +++- > hw/char/spapr_vty.c | 2 +- > hw/char/terminal3270.c| 7 --- > hw/char/virtio-console.c | 2 +- > hw/char/xen_console.c | 24 +++- > hw/ipmi/ipmi_bmc_extern.c | 3 ++- > hw/tpm/tpm_emulator.c | 7 --- > hw/usb/redirect.c | 6 +- > hw/virtio/vhost-user.c| 14 -- > include/chardev/char-fd.h | 2 +- > include/chardev/char-fe.h | 4 ++-- > include/chardev/char-io.h | 2 +- > include/chardev/char.h| 4 ++-- > include/hw/ppc/spapr_vio.h| 2 +- > include/hw/s390x/ebcdic.h | 8 > include/hw/virtio/virtio-serial.h | 2 +- > include/sysemu/replay.h | 2 +- > net/filter-mirror.c | 2 +- > replay/replay-char.c | 2 +- > stubs/replay.c| 2 +- > ui/console.c | 6 +++--- > ui/gtk.c | 2 +- > 38 files changed, 119 insertions(+), 96 deletions(-) > > -- > 2.20.1 > ___ Xen-devel mailing list Xen-devel@lists.xenproject.org
Re: [Xen-devel] [Qemu-devel] [PATCH v3 25/25] chardev: Let qemu_chr_write[_all] use size_t
Hi On Wed, Feb 20, 2019 at 11:39 AM Daniel P. Berrangé wrote: > > On Wed, Feb 20, 2019 at 02:02:32AM +0100, Philippe Mathieu-Daudé wrote: > > > diff --git a/include/chardev/char.h b/include/chardev/char.h > > index 0341dd1ba2..2e3b5a15ca 100644 > > --- a/include/chardev/char.h > > +++ b/include/chardev/char.h > > @@ -221,7 +221,7 @@ void qemu_chr_set_feature(Chardev *chr, > >ChardevFeature feature); > > QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename, > > bool permit_mux_mon); > > -int qemu_chr_write(Chardev *s, const uint8_t *buf, int len, bool > > write_all); > > +int qemu_chr_write(Chardev *s, const uint8_t *buf, size_t len, bool > > write_all); > > Seeing this cleanup reminds me that I think we ought to change > the chardev read & write functions to take "void *buf" instead. > as is done for regular libc read/write functions. This would > avoid casts in the callers between char */uint8_t * > > Something to think about for a future cleanup jobsame applies > for the QIOChannel APIs which take a 'char *buf', annoyingly > different from the chardev APIs :-( Both ought to have void *buf I fully agree, and I started that conversion some time ago, and lost it somewhere. That should be easier than changing the sign of return values though! > > > Regards, > Daniel > -- > |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o-https://fstop138.berrange.com :| > |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 04/25] chardev: Let qemu_chr_be_can_write() return a size_t types
_CHARDEV_FD "chardev-fd"
> diff --git a/include/chardev/char.h b/include/chardev/char.h
> index c0b57f7685..0341dd1ba2 100644
> --- a/include/chardev/char.h
> +++ b/include/chardev/char.h
> @@ -173,7 +173,7 @@ Chardev *qemu_chr_new_noreplay(const char *label, const
> char *filename,
> *
> * Returns: the number of bytes the front end can receive via
> @qemu_chr_be_write
> */
> -int qemu_chr_be_can_write(Chardev *s);
> +size_t qemu_chr_be_can_write(Chardev *s);
>
> /**
> * qemu_chr_be_write:
> diff --git a/ui/console.c b/ui/console.c
> index 6d2282d3e9..42f04e2b37 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -61,8 +61,8 @@ enum TTYState {
>
> typedef struct QEMUFIFO {
> uint8_t *buf;
> -int buf_size;
> -int count, wptr, rptr;
> +size_t buf_size, count;
> +int wptr, rptr;
Only used as non-negative buffer size, ack
> } QEMUFIFO;
>
> static int qemu_fifo_write(QEMUFIFO *f, const uint8_t *buf, int len1)
> @@ -1110,7 +1110,7 @@ static int vc_chr_write(Chardev *chr, const uint8_t
> *buf, int len)
> static void kbd_send_chars(void *opaque)
> {
> QemuConsole *s = opaque;
> -int len;
> +size_t len;
Only used as non-negative buffer size, ack
> uint8_t buf[16];
>
> len = qemu_chr_be_can_write(s->chr);
> --
> 2.20.1
>
That was painful, hopefully I didn't miss something...
Reviewed-by: Marc-André Lureau
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 03/25] chardev/wctablet: Use unsigned type to hold unsigned value
Hi
On Wed, Feb 20, 2019 at 2:03 AM Philippe Mathieu-Daudé
wrote:
>
> TabletChardev::query is an array of uint8_t.
> Use the same type to hold it (this also silent a -Wsign-conversion
> warning in the trace function).
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> chardev/trace-events | 2 +-
> chardev/wctablet.c | 5 +++--
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/chardev/trace-events b/chardev/trace-events
> index d0e5f3bbc1..562bfe70e9 100644
> --- a/chardev/trace-events
> +++ b/chardev/trace-events
> @@ -5,7 +5,7 @@ wct_init(void) ""
> wct_cmd_re(void) ""
> wct_cmd_st(void) ""
> wct_cmd_sp(void) ""
> -wct_cmd_ts(int input) "0x%02x"
> +wct_cmd_ts(uint8_t input) "0x%02x"
> wct_cmd_other(const char *cmd) "%s"
> wct_speed(int speed) "%d"
>
> diff --git a/chardev/wctablet.c b/chardev/wctablet.c
> index 35dbd29a33..cf7a08a363 100644
> --- a/chardev/wctablet.c
> +++ b/chardev/wctablet.c
> @@ -207,7 +207,8 @@ static int wctablet_chr_write(struct Chardev *chr,
>const uint8_t *buf, int len)
> {
> TabletChardev *tablet = WCTABLET_CHARDEV(chr);
> -unsigned int i, clen;
> +size_t i;
> +unsigned int clen;
> char *pos;
>
> if (tablet->line_speed != 9600) {
> @@ -269,7 +270,7 @@ static int wctablet_chr_write(struct Chardev *chr,
>
> } else if (strncmp((char *)tablet->query, "TS", 2) == 0 &&
> clen == 3) {
> -unsigned int input = tablet->query[2];
> +uint8_t input = tablet->query[2];
> uint8_t codes[7] = {
> 0xa3,
> ((input & 0x80) == 0) ? 0x7e : 0x7f,
> --
> 2.20.1
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 02/25] chardev: Assert IOCanReadHandler can not be negative
Hi
On Wed, Feb 20, 2019 at 2:03 AM Philippe Mathieu-Daudé
wrote:
>
> The backend should not return a negative length to read.
> We will later change the prototype of IOCanReadHandler to return an
> unsigned length. Meanwhile make sure the return length is positive.
>
> Suggested-by: Paolo Bonzini
> Signed-off-by: Philippe Mathieu-Daudé
In such patch, you should do extensive review of existing callbacks,
or find a convincing argument that this can't break.
The problem is there are a lot of can_read callbacks, and it's not
trivial. The *first* of git-grep is rng_egd_chr_can_read()
57 QSIMPLEQ_FOREACH(req, >parent.requests, next) {
58 size += req->size - req->offset;
59 }
60
61 return size;
Clearly not obvious if it returns >= 0.
Another approach is to look at the caller and the return value
handling. If none handle negative values (or would have wrong
behaviour with negative values), the assert() is perhaps justified, as
it could prevent from doing more harm.
> ---
> chardev/char.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index f6d61fa5f8..71ecd32b25 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -159,12 +159,15 @@ int qemu_chr_write(Chardev *s, const uint8_t *buf, int
> len, bool write_all)
> int qemu_chr_be_can_write(Chardev *s)
> {
> CharBackend *be = s->be;
> +int receivable_bytes;
>
> if (!be || !be->chr_can_read) {
> return 0;
> }
>
> -return be->chr_can_read(be->opaque);
> +receivable_bytes = be->chr_can_read(be->opaque);
> +assert(receivable_bytes >= 0);
> +return receivable_bytes;
> }
>
> void qemu_chr_be_write_impl(Chardev *s, uint8_t *buf, int len)
> --
> 2.20.1
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 01/25] chardev: Simplify IOWatchPoll::fd_can_read as a GSourceFunc
On Wed, Feb 20, 2019 at 2:03 AM Philippe Mathieu-Daudé
wrote:
>
> IOWatchPoll::fd_can_read() really is a GSourceFunc type, it simply
> returns a boolean value.
> Update the backends to return a boolean, whether there is data to
> read from the source or not.
>
> Suggested-by: Paolo Bonzini
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Marc-André Lureau
> ---
> chardev/char-fd.c | 4 ++--
> chardev/char-io.c | 6 +++---
> chardev/char-pty.c| 4 ++--
> chardev/char-socket.c | 6 +++---
> chardev/char-udp.c| 4 ++--
> include/chardev/char-io.h | 2 +-
> 6 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/chardev/char-fd.c b/chardev/char-fd.c
> index 2c9b2ce567..2421d8e216 100644
> --- a/chardev/char-fd.c
> +++ b/chardev/char-fd.c
> @@ -69,13 +69,13 @@ static gboolean fd_chr_read(QIOChannel *chan,
> GIOCondition cond, void *opaque)
> return TRUE;
> }
>
> -static int fd_chr_read_poll(void *opaque)
> +static gboolean fd_chr_read_poll(void *opaque)
> {
> Chardev *chr = CHARDEV(opaque);
> FDChardev *s = FD_CHARDEV(opaque);
>
> s->max_size = qemu_chr_be_can_write(chr);
> -return s->max_size;
> +return s->max_size > 0;
> }
>
> static GSource *fd_chr_add_watch(Chardev *chr, GIOCondition cond)
> diff --git a/chardev/char-io.c b/chardev/char-io.c
> index 8ced184160..2c1c69098e 100644
> --- a/chardev/char-io.c
> +++ b/chardev/char-io.c
> @@ -30,7 +30,7 @@ typedef struct IOWatchPoll {
> QIOChannel *ioc;
> GSource *src;
>
> -IOCanReadHandler *fd_can_read;
> +GSourceFunc fd_can_read;
> GSourceFunc fd_read;
> void *opaque;
> } IOWatchPoll;
> @@ -44,7 +44,7 @@ static gboolean io_watch_poll_prepare(GSource *source,
>gint *timeout)
> {
> IOWatchPoll *iwp = io_watch_poll_from_source(source);
> -bool now_active = iwp->fd_can_read(iwp->opaque) > 0;
> +bool now_active = iwp->fd_can_read(iwp->opaque);
> bool was_active = iwp->src != NULL;
> if (was_active == now_active) {
> return FALSE;
> @@ -76,7 +76,7 @@ static GSourceFuncs io_watch_poll_funcs = {
>
> GSource *io_add_watch_poll(Chardev *chr,
> QIOChannel *ioc,
> -IOCanReadHandler *fd_can_read,
> +GSourceFunc fd_can_read,
> QIOChannelFunc fd_read,
> gpointer user_data,
> GMainContext *context)
> diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> index b034332edd..f6ddef 100644
> --- a/chardev/char-pty.c
> +++ b/chardev/char-pty.c
> @@ -119,13 +119,13 @@ static GSource *pty_chr_add_watch(Chardev *chr,
> GIOCondition cond)
> return qio_channel_create_watch(s->ioc, cond);
> }
>
> -static int pty_chr_read_poll(void *opaque)
> +static gboolean pty_chr_read_poll(void *opaque)
> {
> Chardev *chr = CHARDEV(opaque);
> PtyChardev *s = PTY_CHARDEV(opaque);
>
> s->read_bytes = qemu_chr_be_can_write(chr);
> -return s->read_bytes;
> +return s->read_bytes > 0;
> }
>
> static gboolean pty_chr_read(QIOChannel *chan, GIOCondition cond, void
> *opaque)
> diff --git a/chardev/char-socket.c b/chardev/char-socket.c
> index 4fcdd8aedd..262a59b64f 100644
> --- a/chardev/char-socket.c
> +++ b/chardev/char-socket.c
> @@ -147,7 +147,7 @@ static void tcp_chr_accept(QIONetListener *listener,
> QIOChannelSocket *cioc,
> void *opaque);
>
> -static int tcp_chr_read_poll(void *opaque);
> +static gboolean tcp_chr_read_poll(void *opaque);
> static void tcp_chr_disconnect(Chardev *chr);
>
> /* Called with chr_write_lock held. */
> @@ -184,7 +184,7 @@ static int tcp_chr_write(Chardev *chr, const uint8_t
> *buf, int len)
> }
> }
>
> -static int tcp_chr_read_poll(void *opaque)
> +static gboolean tcp_chr_read_poll(void *opaque)
> {
> Chardev *chr = CHARDEV(opaque);
> SocketChardev *s = SOCKET_CHARDEV(opaque);
> @@ -192,7 +192,7 @@ static int tcp_chr_read_poll(void *opaque)
> return 0;
> }
> s->max_size = qemu_chr_be_can_write(chr);
> -return s->max_size;
> +return s->max_size > 0;
> }
>
> static void tcp_chr_process_IAC_bytes(Chardev *chr,
> diff --git a/chardev/char-udp.c b/chardev/char-udp.c
> index 097a2f0f42..b6e399e983 100644
> --- a/chardev/char-udp.c
> +++ b/chardev/char-udp.c
> @@ -65,7 +65,7 @@ static void udp_chr_flush_buffer(UdpChardev *s)
> }
> }
>
> -static int
Re: [Xen-devel] [Qemu-devel] [PATCH 3/3] machine: Use shorter format for GlobalProperty arrays
On Mon, Jan 7, 2019 at 11:33 PM Eduardo Habkost wrote:
>
> Instead of verbose arrays with 4 lines for each entry, make each
> entry take only one line. This makes long arrays that couldn't
> fit in the screen become short and readable.
>
> Signed-off-by: Eduardo Habkost
> ---
> include/hw/i386/pc.h | 18 +-
> hw/core/machine.c | 338 -
> hw/i386/pc.c | 720 +++--
> hw/i386/pc_piix.c | 192 ++
> hw/ppc/spapr.c | 72 +---
> hw/s390x/s390-virtio-ccw.c | 75 +---
> hw/xen/xen-common.c| 18 +-
> 7 files changed, 265 insertions(+), 1168 deletions(-)
Nice diff state, hopefully I didn't miss any before/after difference:
Reviewed-by: Marc-André Lureau
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 84720bede9..0abbe45637 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -354,21 +354,9 @@ extern const size_t pc_compat_1_4_len;
> * depending on QEMU versions up to QEMU 2.4.
> */
> #define PC_CPU_MODEL_IDS(v) \
> -{\
> -.driver = "qemu32-" TYPE_X86_CPU,\
> -.property = "model-id",\
> -.value= "QEMU Virtual CPU version " v,\
> -},\
> -{\
> -.driver = "qemu64-" TYPE_X86_CPU,\
> -.property = "model-id",\
> -.value= "QEMU Virtual CPU version " v,\
> -},\
> -{\
> -.driver = "athlon-" TYPE_X86_CPU,\
> -.property = "model-id",\
> -.value= "QEMU Virtual CPU version " v,\
> -},
> +{ "qemu32-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
> +{ "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
> +{ "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },
>
> #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
> static void pc_machine_##suffix##_class_init(ObjectClass *oc, void
> *data) \
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 4b4d6c23de..5530b71981 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -24,23 +24,10 @@
> #include "hw/pci/pci.h"
>
> GlobalProperty hw_compat_3_1[] = {
> -{
> -.driver = "pcie-root-port",
> -.property = "x-speed",
> -.value= "2_5",
> -},{
> -.driver = "pcie-root-port",
> -.property = "x-width",
> -.value= "1",
> -},{
> -.driver = "memory-backend-file",
> -.property = "x-use-canonical-path-for-ramblock-id",
> -.value= "true",
> -},{
> -.driver = "memory-backend-memfd",
> -.property = "x-use-canonical-path-for-ramblock-id",
> -.value= "true",
> -},
> +{ "pcie-root-port", "x-speed", "2_5" },
> +{ "pcie-root-port", "x-width", "1" },
> +{ "memory-backend-file", "x-use-canonical-path-for-ramblock-id", "true"
> },
> +{ "memory-backend-memfd", "x-use-canonical-path-for-ramblock-id", "true"
> },
> };
> const size_t hw_compat_3_1_len = G_N_ELEMENTS(hw_compat_3_1);
>
> @@ -48,269 +35,96 @@ GlobalProperty hw_compat_3_0[] = {};
> const size_t hw_compat_3_0_len = G_N_ELEMENTS(hw_compat_3_0);
>
> GlobalProperty hw_compat_2_12[] = {
> -{
> -.driver = "migration",
> -.property = "decompress-error-check",
> -.value= "off",
> -},{
> -.driver = "hda-audio",
> -.property = "use-timer",
> -.value= "false",
> -},{
> -.driver = "cirrus-vga",
> -.property = "global-vmstate",
> -.value= "true",
> -},{
> -.driver = "VGA",
> -.property = "global-vmstate",
> -.value= "true",
> -},{
> -.driver = "vmware-svga",
> -.property = "global-vmstate",
> -.value= "true",
> -},{
> -.driver = "qxl-vga",
> -.property = "global-vmstate",
> -.value= "true",
> -},
> +{ "migration", "decompress-error-check", "off" },
> +{ "
Re: [Xen-devel] [Qemu-devel] [PATCH 1/3] spapr: Eliminate SPAPR_PCI_2_7_MMIO_WIN_SIZE macro
On Mon, Jan 7, 2019 at 11:34 PM Eduardo Habkost wrote:
>
> The macro is only used in one place, where the purpose of the
> value is obvious. Eliminate the macro so we don't need to rely
> on stringify().
>
> Signed-off-by: Eduardo Habkost
Reviewed-by: Marc-André Lureau
> ---
> include/hw/pci-host/spapr.h | 1 -
> hw/ppc/spapr.c | 2 +-
> 2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index 7c66c3872f..a85a995b6c 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -99,7 +99,6 @@ struct sPAPRPHBState {
> #define SPAPR_PCI_BASE (1ULL << 45) /* 32 TiB */
> #define SPAPR_PCI_LIMIT (1ULL << 46) /* 64 TiB */
>
> -#define SPAPR_PCI_2_7_MMIO_WIN_SIZE 0xf8000
> #define SPAPR_PCI_IO_WIN_SIZE0x1
>
> #define SPAPR_PCI_MSI_WINDOW 0x400ULL
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5671608cea..bff42f0adb 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4225,7 +4225,7 @@ static void
> spapr_machine_2_7_class_options(MachineClass *mc)
> {
> .driver = TYPE_SPAPR_PCI_HOST_BRIDGE,
> .property = "mem_win_size",
> -.value= stringify(SPAPR_PCI_2_7_MMIO_WIN_SIZE),
> +.value= "0xf8000",
> },
> {
> .driver = TYPE_SPAPR_PCI_HOST_BRIDGE,
> --
> 2.18.0.rc1.1.g3f1ff2140
>
>
--
Marc-André Lureau
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH 2/3] machine: Eliminate unnecessary stringify() usage
On Mon, Jan 7, 2019 at 11:32 PM Eduardo Habkost wrote:
>
> stringify() is useful when we need to use macros in compat_props
> (like when we set virtio-baloon-pci.class=PCI_CLASS_MEMORY_RAM at
> pc_i440fx_1_0_machine_options()), but it is pointless when we are
> already providing a number literal.
>
> Replace stringify() with string literals when appropriate.
>
> Signed-off-by: Eduardo Habkost
Reviewed-by: Marc-André Lureau
> ---
> hw/core/machine.c | 8 ++--
> hw/i386/pc.c | 94 +++
> hw/i386/pc_piix.c | 30 +++
> hw/ppc/spapr.c| 2 +-
> 4 files changed, 67 insertions(+), 67 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index f8563efb86..4b4d6c23de 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -135,11 +135,11 @@ GlobalProperty hw_compat_2_8[] = {
> {
> .driver = "fw_cfg_mem",
> .property = "x-file-slots",
> -.value= stringify(0x10),
> +.value= "0x10",
> },{
> .driver = "fw_cfg_io",
> .property = "x-file-slots",
> -.value= stringify(0x10),
> +.value= "0x10",
> },{
> .driver = "pflash_cfi01",
> .property = "old-multiple-chip-handling",
> @@ -337,11 +337,11 @@ GlobalProperty hw_compat_2_1[] = {
> },{
> .driver = "usb-mouse",
> .property = "usb_version",
> -.value= stringify(1),
> +.value= "1",
> },{
> .driver = "usb-kbd",
> .property = "usb_version",
> -.value= stringify(1),
> +.value= "1",
> },{
> .driver = "virtio-pci",
> .property = "virtio-pci-bus-master-bug-migration",
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 4952feb476..ff14b6d4df 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -148,11 +148,11 @@ GlobalProperty pc_compat_2_12[] = {
> },{
> .driver = "EPYC-" TYPE_X86_CPU,
> .property = "xlevel",
> -.value= stringify(0x800a),
> +.value= "0x800a",
> },{
> .driver = "EPYC-IBPB-" TYPE_X86_CPU,
> .property = "xlevel",
> -.value= stringify(0x800a),
> +.value= "0x800a",
> },
> };
> const size_t pc_compat_2_12_len = G_N_ELEMENTS(pc_compat_2_12);
> @@ -191,7 +191,7 @@ GlobalProperty pc_compat_2_9[] = {
> {
> .driver = "mch",
> .property = "extended-tseg-mbytes",
> -.value= stringify(0),
> +.value= "0",
> },
> };
> const size_t pc_compat_2_9_len = G_N_ELEMENTS(pc_compat_2_9);
> @@ -365,75 +365,75 @@ GlobalProperty pc_compat_2_3[] = {
> },{
> .driver = "qemu64" "-" TYPE_X86_CPU,
> .property = "min-level",
> -.value= stringify(4),
> +.value= "4",
> },{
> .driver = "kvm64" "-" TYPE_X86_CPU,
> .property = "min-level",
> -.value= stringify(5),
> +.value= "5",
> },{
> .driver = "pentium3" "-" TYPE_X86_CPU,
> .property = "min-level",
> -.value= stringify(2),
> +.value= "2",
> },{
> .driver = "n270" "-" TYPE_X86_CPU,
> .property = "min-level",
> -.value= stringify(5),
> +.value= "5",
> },{
> .driver = "Conroe" "-" TYPE_X86_CPU,
> .property = "min-level",
> -.value= stringify(4),
> +.value= "4",
> },{
> .driver = "Penryn" "-" TYPE_X86_CPU,
> .property = "min-level",
> -.value= stringify(4),
> +.value= "4",
> },{
> .driver = "Nehalem" "-" TYPE_X86_CPU,
> .property = "min-level",
> -.value= stringify(4),
> +.value= "4",
> },{
> .driver = "n270" "-" TYPE_X86_CPU,
> .property = "min-xlevel",
> -.value= stringify(0x800a),
> +.value= "0x800a",
> },{
> .driver =
Re: [Xen-devel] [Qemu-devel] [PATCH for-3.2 v5 07/19] hw: apply accel compat properties without touching globals
Hi
On Mon, Dec 10, 2018 at 8:55 PM Igor Mammedov wrote:
>
> On Mon, 10 Dec 2018 17:45:22 +0100
> Igor Mammedov wrote:
>
> > On Tue, 4 Dec 2018 18:20:11 +0400
> > Marc-André Lureau wrote:
> >
> > > Instead of registering compat properties as globals, let's keep them
> > > in their own array, to avoid mixing with user globals.
> > >
> > > Introduce object_apply_global_props() function, to apply compatibility
> > > properties from a GPtrArray.
> > >
> > > Signed-off-by: Marc-André Lureau
> > > ---
> > > include/hw/qdev-core.h | 10 ++
> > > include/qom/object.h | 3 +++
> > > include/sysemu/accel.h | 4 +---
> > > accel/accel.c | 12
> > > hw/core/qdev.c | 9 +
> > > hw/xen/xen-common.c| 9 ++---
> > > qom/object.c | 25 +
> > > vl.c | 1 -
> > > 8 files changed, 54 insertions(+), 19 deletions(-)
> > >
> > [...]
> > > diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
> > > index 6ec14c73ca..4532aa8632 100644
> > > --- a/hw/xen/xen-common.c
> > > +++ b/hw/xen/xen-common.c
> > > @@ -174,18 +174,21 @@ static GlobalProperty xen_compat_props[] = {
> > > .driver = "migration",
> > > .property = "send-section-footer",
> > > .value = "off",
> > > -},
> > > -{ /* end of list */ },
> > > +}
> > > };
> > >
> > > static void xen_accel_class_init(ObjectClass *oc, void *data)
> > > {
> > > AccelClass *ac = ACCEL_CLASS(oc);
> > > +
> > > ac->name = "Xen";
> > > ac->init_machine = xen_init;
> > > ac->setup_post = xen_setup_post;
> > > ac->allowed = _allowed;
> > > -ac->global_props = xen_compat_props;
> > > +ac->compat_props = g_ptr_array_new();
> > where is matching free for that?
> can we at least annotate it somehow so that valgrind won't complain about
> this leak?
If you check my commits on qemu, you should see that I do care (too
much?) about leaks :)
In this case though, I don't see valgrind or asan complaining, I guess
it's still a reachable reference.
Do you think a FIXME comment would be helpful?
(/me wish we had a proper object system, GObject, but that ship as
long sailed..)
>
> >
> > > +
> > > +compat_props_add(ac->compat_props,
> > > + xen_compat_props, G_N_ELEMENTS(xen_compat_props));
> > > }
> > >
> > > #define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen")
> > > diff --git a/qom/object.c b/qom/object.c
> > > index 17921c0a71..dbdab0aead 100644
> > > --- a/qom/object.c
> > > +++ b/qom/object.c
> > > @@ -370,6 +370,31 @@ static void object_post_init_with_type(Object *obj,
> > > TypeImpl *ti)
> > > }
> > > }
> > >
> > > +void object_apply_global_props(Object *obj, const GPtrArray *props,
> > > Error **errp)
> > > +{
> > > +Error *err = NULL;
> > > +int i;
> > > +
> > > +if (!props) {
> > > +return;
> > > +}
> > > +
> > > +for (i = 0; i < props->len; i++) {
> > > +GlobalProperty *p = g_ptr_array_index(props, i);
> > > +
> > > +if (object_dynamic_cast(obj, p->driver) == NULL) {
> > > +continue;
> > > +}
> > > +p->used = true;
> > > +object_property_parse(obj, p->value, p->property, );
> > > + if (err != NULL) {
> > > +error_prepend(, "can't apply global %s.%s=%s: ",
> > > + p->driver, p->property, p->value);
> > > +error_propagate(errp, err);
> > > +}
> > > +}
> > > +}
> > > +
> > > static void object_initialize_with_type(void *data, size_t size,
> > > TypeImpl *type)
> > > {
> > > Object *obj = data;
> > > diff --git a/vl.c b/vl.c
> > > index a5ae5f23d2..88ba658572 100644
> > > --- a/vl.c
> > > +++ b/vl.c
> > > @@ -2968,7 +2968,6 @@ static void user_register_global_props(void)
> > > */
> > > static void register_global_properties(MachineState *ms)
> > > {
> > > -accel_register_compat_props(ms->accelerator);
> > > machine_register_compat_props(ms);
> > > user_register_global_props();
> > > }
> >
> >
>
>
--
Marc-André Lureau
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-3.2 v5 07/19] hw: apply accel compat properties without touching globals
Instead of registering compat properties as globals, let's keep them
in their own array, to avoid mixing with user globals.
Introduce object_apply_global_props() function, to apply compatibility
properties from a GPtrArray.
Signed-off-by: Marc-André Lureau
---
include/hw/qdev-core.h | 10 ++
include/qom/object.h | 3 +++
include/sysemu/accel.h | 4 +---
accel/accel.c | 12
hw/core/qdev.c | 9 +
hw/xen/xen-common.c| 9 ++---
qom/object.c | 25 +
vl.c | 1 -
8 files changed, 54 insertions(+), 19 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index a24d0dd566..aeaa6dbbb8 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -267,6 +267,16 @@ typedef struct GlobalProperty {
Error **errp;
} GlobalProperty;
+static inline void
+compat_props_add(GPtrArray *arr,
+ GlobalProperty props[], size_t nelem)
+{
+int i;
+for (i = 0; i < nelem; i++) {
+g_ptr_array_add(arr, (void *)[i]);
+}
+}
+
/*** Board API. This should go away once we have a machine config file. ***/
DeviceState *qdev_create(BusState *bus, const char *name);
diff --git a/include/qom/object.h b/include/qom/object.h
index 0139838b69..5183c587f3 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -676,6 +676,9 @@ Object *object_new_with_propv(const char *typename,
Error **errp,
va_list vargs);
+void object_apply_global_props(Object *obj, const GPtrArray *props,
+ Error **errp);
+
/**
* object_set_props:
* @obj: the object instance to set properties on
diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h
index 637358f430..f331d128e9 100644
--- a/include/sysemu/accel.h
+++ b/include/sysemu/accel.h
@@ -49,7 +49,7 @@ typedef struct AccelClass {
* global properties may be overridden by machine-type
* compat_props or user-provided global properties.
*/
-GlobalProperty *global_props;
+GPtrArray *compat_props;
} AccelClass;
#define TYPE_ACCEL "accel"
@@ -67,8 +67,6 @@ typedef struct AccelClass {
extern unsigned long tcg_tb_size;
void configure_accelerator(MachineState *ms);
-/* Register accelerator specific global properties */
-void accel_register_compat_props(AccelState *accel);
/* Called just before os_setup_post (ie just before drop OS privs) */
void accel_setup_post(MachineState *ms);
diff --git a/accel/accel.c b/accel/accel.c
index 3da26eb90f..6db5d8f4df 100644
--- a/accel/accel.c
+++ b/accel/accel.c
@@ -119,18 +119,6 @@ void configure_accelerator(MachineState *ms)
}
}
-void accel_register_compat_props(AccelState *accel)
-{
-AccelClass *class = ACCEL_GET_CLASS(accel);
-GlobalProperty *prop = class->global_props;
-
-for (; prop && prop->driver; prop++) {
-/* Any compat_props must never cause error */
-prop->errp = _abort;
-qdev_prop_register_global(prop);
-}
-}
-
void accel_setup_post(MachineState *ms)
{
AccelState *accel = ms->accelerator;
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6b3cc55b27..53b507164f 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -972,6 +972,15 @@ static void device_initfn(Object *obj)
static void device_post_init(Object *obj)
{
+if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
+MachineState *m = MACHINE(qdev_get_machine());
+AccelClass *ac = ACCEL_GET_CLASS(m->accelerator);
+
+if (ac->compat_props) {
+object_apply_global_props(obj, ac->compat_props, _abort);
+}
+}
+
qdev_prop_set_globals(DEVICE(obj));
}
diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
index 6ec14c73ca..4532aa8632 100644
--- a/hw/xen/xen-common.c
+++ b/hw/xen/xen-common.c
@@ -174,18 +174,21 @@ static GlobalProperty xen_compat_props[] = {
.driver = "migration",
.property = "send-section-footer",
.value = "off",
-},
-{ /* end of list */ },
+}
};
static void xen_accel_class_init(ObjectClass *oc, void *data)
{
AccelClass *ac = ACCEL_CLASS(oc);
+
ac->name = "Xen";
ac->init_machine = xen_init;
ac->setup_post = xen_setup_post;
ac->allowed = _allowed;
-ac->global_props = xen_compat_props;
+ac->compat_props = g_ptr_array_new();
+
+compat_props_add(ac->compat_props,
+ xen_compat_props, G_N_ELEMENTS(xen_compat_props));
}
#define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen")
diff --git a/qom/object.c b/qom/object.c
index 17921c0a71..dbdab0aead 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -370,6 +370,31 @@ static void object_post_init_with_type(Object *obj,
TypeImpl *ti)
}
}
+void object_apply_global_props(Object *obj, const GPtrArray *props, Error
**errp)
Re: [Xen-devel] [Qemu-devel] [PATCH for-3.2 v4 15/28] hw: apply accel compat properties without touching globals
Hi
On Tue, Nov 27, 2018 at 11:40 PM Eduardo Habkost wrote:
>
> On Tue, Nov 27, 2018 at 01:27:48PM +0400, Marc-André Lureau wrote:
> > Introduce object_apply_global_props() function, to apply compatibility
> > properties from a GPtrArray.
> >
> > For accel compatibility properties, apply them during
> > device_post_init(), after accel_register_compat_props() has set them.
> >
> > To populate the compatibility properties, introduce SET_COMPAT(), a
> > more generic version of SET_MACHINE_COMPAT() that can set compat
> > properties on other objects than Machine, and using GPtrArray.
> >
> > Signed-off-by: Marc-André Lureau
> > ---
> > include/hw/qdev-core.h | 13 +
> > include/qom/object.h | 3 +++
> > include/sysemu/accel.h | 4 +---
> > accel/accel.c | 12
> > hw/core/qdev.c | 11 +++
> > hw/xen/xen-common.c| 38 +++---
> > qom/object.c | 25 +
> > vl.c | 2 +-
> > 8 files changed, 73 insertions(+), 35 deletions(-)
> >
> > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> > index a24d0dd566..82afd3c50d 100644
> > --- a/include/hw/qdev-core.h
> > +++ b/include/hw/qdev-core.h
> > @@ -267,6 +267,19 @@ typedef struct GlobalProperty {
> > Error **errp;
> > } GlobalProperty;
> >
> > +#define SET_COMPAT(S, COMPAT) \
> > +do {\
> > +int i; \
> > +static GlobalProperty props[] = { \
> > +COMPAT \
> > +}; \
> > +for (i = 0; i < G_N_ELEMENTS(props); i++) { \
> > +g_ptr_array_add((S)->compat_props, (void *)[i]); \
> > +} \
> > +} while (0)
>
> I think this macro would be an acceptable alternative to the
> existing SET_MACHINE_COMPAT macro trickery, but:
>
> > +
> > +void accel_register_compat_props(const GPtrArray *props);
> [...]
> > @@ -185,7 +183,9 @@ static void xen_accel_class_init(ObjectClass *oc, void
> > *data)
> > ac->init_machine = xen_init;
> > ac->setup_post = xen_setup_post;
> > ac->allowed = _allowed;
> > -ac->global_props = xen_compat_props;
> > +ac->compat_props = g_ptr_array_new();
> > +
> > +SET_COMPAT(ac, XEN_COMPAT);
>
> I think this is a step backwards. I like us to be able to
> register compat properties without macro magic. The existence of
> SET_MACHINE_COMPAT is a bug and not a feature.
>
> If you really want to use GPtrArray instead of a simple
> GlobalProperty* field (I'm not sure I understand the reasoning
> behind the choice to use GPtrArray), what about:
Except in the Xen case, It needs to register multiple GlobalProperty*,
not necessarily from contiguous in memory. That's why we have an array
of ptr.
>
> static GPtrArray *build_compat_props_array(GlobalProperty *props)
> {
> GlobalProperty *p = props;
> GPtrArray *array = g_ptr_array_new();
> while (p->driver) {
> g_ptr_array_add(array, (void *)p);
> }
> return array;
> }
>
>
> static void xen_accel_class_init(ObjectClass *oc, void *data)
> {
> ...
> ac->compat_props = build_compat_props_array(xen_compat_props);
If we would register from one place, that would be fine.
We could replace the macro by a function, then we would have to
declare the GlobalProperty arrays manually basically.
> }
>
>
>
> > }
> >
> > #define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen")
> > diff --git a/qom/object.c b/qom/object.c
> > index 17921c0a71..dbdab0aead 100644
> > --- a/qom/object.c
> > +++ b/qom/object.c
> > @@ -370,6 +370,31 @@ static void object_post_init_with_type(Object *obj,
> > TypeImpl *ti)
> > }
> > }
> >
> > +void object_apply_global_props(Object *obj, const GPtrArray *props, Error
> > **errp)
> > +{
> > +Error *err = NULL;
> > +int i;
> > +
> > +if (!props) {
> > +return;
> > +}
> > +
> > +for (i = 0; i < props->len; i++) {
> > +GlobalProperty *p = g_ptr_array_index(props, i);
> > +
> > +if (object_dynamic_cast(obj, p->driver) == NULL)
[Xen-devel] [PATCH for-3.2 v4 15/28] hw: apply accel compat properties without touching globals
Introduce object_apply_global_props() function, to apply compatibility
properties from a GPtrArray.
For accel compatibility properties, apply them during
device_post_init(), after accel_register_compat_props() has set them.
To populate the compatibility properties, introduce SET_COMPAT(), a
more generic version of SET_MACHINE_COMPAT() that can set compat
properties on other objects than Machine, and using GPtrArray.
Signed-off-by: Marc-André Lureau
---
include/hw/qdev-core.h | 13 +
include/qom/object.h | 3 +++
include/sysemu/accel.h | 4 +---
accel/accel.c | 12
hw/core/qdev.c | 11 +++
hw/xen/xen-common.c| 38 +++---
qom/object.c | 25 +
vl.c | 2 +-
8 files changed, 73 insertions(+), 35 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index a24d0dd566..82afd3c50d 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -267,6 +267,19 @@ typedef struct GlobalProperty {
Error **errp;
} GlobalProperty;
+#define SET_COMPAT(S, COMPAT) \
+do {\
+int i; \
+static GlobalProperty props[] = { \
+COMPAT \
+}; \
+for (i = 0; i < G_N_ELEMENTS(props); i++) { \
+g_ptr_array_add((S)->compat_props, (void *)[i]); \
+} \
+} while (0)
+
+void accel_register_compat_props(const GPtrArray *props);
+
/*** Board API. This should go away once we have a machine config file. ***/
DeviceState *qdev_create(BusState *bus, const char *name);
diff --git a/include/qom/object.h b/include/qom/object.h
index 0139838b69..5183c587f3 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -676,6 +676,9 @@ Object *object_new_with_propv(const char *typename,
Error **errp,
va_list vargs);
+void object_apply_global_props(Object *obj, const GPtrArray *props,
+ Error **errp);
+
/**
* object_set_props:
* @obj: the object instance to set properties on
diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h
index 637358f430..f331d128e9 100644
--- a/include/sysemu/accel.h
+++ b/include/sysemu/accel.h
@@ -49,7 +49,7 @@ typedef struct AccelClass {
* global properties may be overridden by machine-type
* compat_props or user-provided global properties.
*/
-GlobalProperty *global_props;
+GPtrArray *compat_props;
} AccelClass;
#define TYPE_ACCEL "accel"
@@ -67,8 +67,6 @@ typedef struct AccelClass {
extern unsigned long tcg_tb_size;
void configure_accelerator(MachineState *ms);
-/* Register accelerator specific global properties */
-void accel_register_compat_props(AccelState *accel);
/* Called just before os_setup_post (ie just before drop OS privs) */
void accel_setup_post(MachineState *ms);
diff --git a/accel/accel.c b/accel/accel.c
index 3da26eb90f..6db5d8f4df 100644
--- a/accel/accel.c
+++ b/accel/accel.c
@@ -119,18 +119,6 @@ void configure_accelerator(MachineState *ms)
}
}
-void accel_register_compat_props(AccelState *accel)
-{
-AccelClass *class = ACCEL_GET_CLASS(accel);
-GlobalProperty *prop = class->global_props;
-
-for (; prop && prop->driver; prop++) {
-/* Any compat_props must never cause error */
-prop->errp = _abort;
-qdev_prop_register_global(prop);
-}
-}
-
void accel_setup_post(MachineState *ms)
{
AccelState *accel = ms->accelerator;
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6b3cc55b27..7066d28271 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -970,8 +970,19 @@ static void device_initfn(Object *obj)
QLIST_INIT(>gpios);
}
+static const GPtrArray *ac_compat_props;
+
+void accel_register_compat_props(const GPtrArray *props)
+{
+ac_compat_props = props;
+}
+
static void device_post_init(Object *obj)
{
+if (ac_compat_props) {
+object_apply_global_props(obj, ac_compat_props, _abort);
+}
+
qdev_prop_set_globals(DEVICE(obj));
}
diff --git a/hw/xen/xen-common.c b/hw/xen/xen-common.c
index 6ec14c73ca..4da0292b61 100644
--- a/hw/xen/xen-common.c
+++ b/hw/xen/xen-common.c
@@ -159,24 +159,22 @@ static int xen_init(MachineState *ms)
return 0;
}
-static GlobalProperty xen_compat_props[] = {
-{
-.driver = "migration",
-.property = "store-global-state",
-.value = "off",
-},
-{
-.driver = "migration",
-.property = "send-configuration",
-
Re: [Xen-devel] [Qemu-devel] [PATCH for-3.2 v3 10/14] qdev-props: call object_apply_global_props()
Hi
On Mon, Nov 26, 2018 at 5:27 PM Igor Mammedov wrote:
>
> On Wed, 7 Nov 2018 16:36:48 +0400
> Marc-André Lureau wrote:
>
> > It's now possible to use the common function.
> >
> > Teach object_apply_global_props() to warn if Error argument is NULL.
> >
> > Signed-off-by: Marc-André Lureau
> > ---
> > hw/core/qdev-properties.c | 24 ++--
> > qom/object.c | 6 +-
> > 2 files changed, 7 insertions(+), 23 deletions(-)
> >
> > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > index 8728cbab9f..239535a4cb 100644
> > --- a/hw/core/qdev-properties.c
> > +++ b/hw/core/qdev-properties.c
> > @@ -1223,28 +1223,8 @@ int qdev_prop_check_globals(void)
> >
> > void qdev_prop_set_globals(DeviceState *dev)
> > {
> > -int i;
> > -
> > -for (i = 0; i < global_props()->len; i++) {
> > -GlobalProperty *prop;
> > -Error *err = NULL;
> > -
> > -prop = g_array_index(global_props(), GlobalProperty *, i);
> > -if (object_dynamic_cast(OBJECT(dev), prop->driver) == NULL) {
> > -continue;
> > -}
> > -prop->used = true;
> > -object_property_parse(OBJECT(dev), prop->value, prop->property,
> > );
> > -if (err != NULL) {
> > -error_prepend(, "can't apply global %s.%s=%s: ",
> > - prop->driver, prop->property, prop->value);
> > -if (!dev->hotplugged) {
> > -error_propagate(_fatal, err);
> > -} else {
> > -warn_report_err(err);
> > -}
> > -}
> > -}
> > +object_apply_global_props(OBJECT(dev), global_props(),
> > + dev->hotplugged ? NULL : _fatal);
> arguably, it's up to caller to decide it warn or not.
> I'd leave it warning code out of object_apply_global_props() and let caller
> do the job
The problem is that there may be multiple errors, and the remaining
globals should be applied.
I'll add a comment.
> > }
> >
> > /* --- 64bit unsigned int 'size' type --- */
> > diff --git a/qom/object.c b/qom/object.c
> > index 9acdf9e16d..b1a7f70550 100644
> > --- a/qom/object.c
> > +++ b/qom/object.c
> > @@ -392,7 +392,11 @@ void object_apply_global_props(Object *obj, GArray
> > *props, Error **errp)
> > if (err != NULL) {
> > error_prepend(, "can't apply global %s.%s=%s: ",
> >p->driver, p->property, p->value);
> > -error_propagate(errp, err);
> > +if (errp) {
> > +error_propagate(errp, err);
> > +} else {
> > +warn_report_err(err);
> > +}
> > }
> > }
> > }
>
>
--
Marc-André Lureau
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH for-3.2 v3 13/14] hw/i386: add pc-i440fx-3.2 & pc-q35-3.2
Hi
On Wed, Nov 7, 2018 at 4:49 PM Marc-André Lureau
wrote:
>
> The following patch is going to add compatiblity parameters for
> qemu <= 3.1.
>
I realize this may be good enough for x86 i440/q35 machines, but what
about other machines & architectures?
What do we officially support, for migration, across different versions?
It seems we have versionized:
- arm "virt" machines
- "s390-ccw-virtio" machines
- ppc "pseries" machines
- x86 piix/q35 machines
At least, I think I should update this patch to add new 3.2 machines for those.
Is there any way to check compat properties are handled properly for
those various machines? It looks like it is generally lacking. For
example, if there is a new HW_COMPAT, it would be nice if something
failed if a corresponding machine hasn't been added.
It also looks like there is a bit of code duplication and a bit too
much macros :) unfortunately, I don't yet have a good idea how to
improve things...
> Signed-off-by: Marc-André Lureau
> ---
> include/hw/compat.h | 3 +++
> include/hw/i386/pc.h | 3 +++
> hw/i386/pc_piix.c| 21 ++---
> hw/i386/pc_q35.c | 19 +--
> 4 files changed, 41 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 6f4d5fc647..70958328fe 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -1,6 +1,9 @@
> #ifndef HW_COMPAT_H
> #define HW_COMPAT_H
>
> +#define HW_COMPAT_3_1 \
> +/* empty */
> +
> #define HW_COMPAT_3_0 \
> /* empty */
>
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 136fe497b6..c37d4333a0 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -294,6 +294,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
> int e820_get_num_entries(void);
> bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>
> +#define PC_COMPAT_3_1 \
> +HW_COMPAT_3_1
> +
> #define PC_COMPAT_3_0 \
> HW_COMPAT_3_0 \
> {\
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index dc09466b3e..ba371bfcd7 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -428,21 +428,36 @@ static void pc_i440fx_machine_options(MachineClass *m)
> machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE);
> }
>
> -static void pc_i440fx_3_0_machine_options(MachineClass *m)
> +static void pc_i440fx_3_2_machine_options(MachineClass *m)
> {
> pc_i440fx_machine_options(m);
> m->alias = "pc";
> m->is_default = 1;
> }
>
> +DEFINE_I440FX_MACHINE(v3_2, "pc-i440fx-3.2", NULL,
> + pc_i440fx_3_2_machine_options);
> +
> +static void pc_i440fx_3_1_machine_options(MachineClass *m)
> +{
> +pc_i440fx_3_2_machine_options(m);
> +m->is_default = 0;
> +m->alias = NULL;
> +SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
> +}
> +
> +static void pc_i440fx_3_0_machine_options(MachineClass *m)
> +{
> +pc_i440fx_3_1_machine_options(m);
> +SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
> +}
> +
> DEFINE_I440FX_MACHINE(v3_0, "pc-i440fx-3.0", NULL,
>pc_i440fx_3_0_machine_options);
>
> static void pc_i440fx_2_12_machine_options(MachineClass *m)
> {
> pc_i440fx_3_0_machine_options(m);
> -m->is_default = 0;
> -m->alias = NULL;
> SET_MACHINE_COMPAT(m, PC_COMPAT_2_12);
> }
>
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 532241e3f8..64d6ea65d5 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -311,19 +311,34 @@ static void pc_q35_machine_options(MachineClass *m)
> m->max_cpus = 288;
> }
>
> -static void pc_q35_3_0_machine_options(MachineClass *m)
> +static void pc_q35_3_2_machine_options(MachineClass *m)
> {
> pc_q35_machine_options(m);
> m->alias = "q35";
> }
>
> +DEFINE_Q35_MACHINE(v3_2, "pc-q35-3.2", NULL,
> + pc_q35_3_2_machine_options);
> +
> +static void pc_q35_3_1_machine_options(MachineClass *m)
> +{
> +pc_q35_3_2_machine_options(m);
> +m->alias = NULL;
> +SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
> +}
> +
> +static void pc_q35_3_0_machine_options(MachineClass *m)
> +{
> +pc_q35_3_1_machine_options(m);
> +SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
> +}
> +
> DEFINE_Q35_MACHINE(v3_0, "pc-q35-3.0", NULL,
> pc_q35_3_0_machine_options);
>
> static void pc_q35_2_12_machine_options(MachineClass *m)
> {
> pc_q35_3_0_machine_options(m);
> -m->alias = NULL;
> SET_MACHINE_COMPAT(m, PC_COMPAT_2_12);
> }
>
> --
> 2.19.1.708.g4ede3d42df
>
>
--
Marc-André Lureau
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-3.2 v3 14/14] hostmem: use object id for memory region name with >= 3.1
hostmem-file and hostmem-memfd use the whole object path for the
memory region name, and hostname-ram uses only the path component (the
object id, or canonical path basename):
qemu -m 1024 -object memory-backend-file,id=mem,size=1G,mem-path=/tmp/foo -numa
node,memdev=mem -monitor stdio
(qemu) info ramblock
Block NamePSize Offset Used
Total
/objects/mem4 KiB 0x 0x4000
0x4000
qemu -m 1024 -object memory-backend-memfd,id=mem,size=1G -numa node,memdev=mem
-monitor stdio
(qemu) info ramblock
Block NamePSize Offset Used
Total
/objects/mem4 KiB 0x 0x4000
0x4000
qemu -m 1024 -object memory-backend-ram,id=mem,size=1G -numa node,memdev=mem
-monitor stdio
(qemu) info ramblock
Block NamePSize Offset Used
Total
mem4 KiB 0x 0x4000
0x4000
Use the object id for -file and -memfd with >= 3.1 for consistency.
Having a consistent naming allow to migrate to different hostmem
backends.
Signed-off-by: Marc-André Lureau
---
include/hw/compat.h | 10 +-
include/sysemu/hostmem.h | 3 ++-
backends/hostmem-file.c | 8
backends/hostmem-memfd.c | 2 +-
backends/hostmem-ram.c | 9 -
backends/hostmem.c | 31 +++
6 files changed, 51 insertions(+), 12 deletions(-)
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 70958328fe..99cf6b1e03 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -2,7 +2,15 @@
#define HW_COMPAT_H
#define HW_COMPAT_3_1 \
-/* empty */
+{\
+.driver = "memory-backend-file",\
+.property = "x-use-canonical-path-for-ramblock-id",\
+.value= "true",\
+},{\
+.driver = "memory-backend-memfd",\
+.property = "x-use-canonical-path-for-ramblock-id",\
+.value= "true",\
+},
#define HW_COMPAT_3_0 \
/* empty */
diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
index 6e6bd2c1cb..a023b372a4 100644
--- a/include/sysemu/hostmem.h
+++ b/include/sysemu/hostmem.h
@@ -53,7 +53,7 @@ struct HostMemoryBackend {
/* protected */
uint64_t size;
-bool merge, dump;
+bool merge, dump, use_canonical_path;
bool prealloc, force_prealloc, is_mapped, share;
DECLARE_BITMAP(host_nodes, MAX_NODES + 1);
HostMemPolicy policy;
@@ -67,5 +67,6 @@ MemoryRegion
*host_memory_backend_get_memory(HostMemoryBackend *backend);
void host_memory_backend_set_mapped(HostMemoryBackend *backend, bool mapped);
bool host_memory_backend_is_mapped(HostMemoryBackend *backend);
size_t host_memory_backend_pagesize(HostMemoryBackend *memdev);
+char *host_memory_backend_get_name(HostMemoryBackend *backend);
#endif
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 639c8d4307..c01a7cdf8d 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -55,16 +55,16 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error
**errp)
error_setg(errp, "-mem-path not supported on this host");
#else
if (!host_memory_backend_mr_inited(backend)) {
-gchar *path;
+gchar *name;
backend->force_prealloc = mem_prealloc;
-path = object_get_canonical_path(OBJECT(backend));
+name = host_memory_backend_get_name(backend);
memory_region_init_ram_from_file(>mr, OBJECT(backend),
- path,
+ name,
backend->size, fb->align,
(backend->share ? RAM_SHARED : 0) |
(fb->is_pmem ? RAM_PMEM : 0),
fb->mem_path, errp);
-g_free(path);
+g_free(name);
}
#endif
}
diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
index b6836b28e5..c5a4a9b530 100644
--- a/backends/hostmem-memfd.c
+++ b/backends/hostmem-memfd.c
@@ -57,7 +57,7 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error
**errp)
return;
}
-name = object_get_canonical_path(OBJECT(backend));
+name = host_memory_backend_get_name(backend);
memory_region_init_ram_from_fd(>mr, OBJECT(backend),
name, backend->size, true, fd, errp);
g_free(name);
diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
index 7ddd08d370..24b65d9ae3 100644
--- a/backends/hostmem-ram.c
+++ b/backends/hostmem-ram.c
@@ -16,21 +16,20 @@
#define TYPE_MEMORY_BACKEND_RAM "memory-backend-ram"
-
static void
ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
{
-char *p
[Xen-devel] [PATCH for-3.2 v3 09/14] qdev-props: remove errp from GlobalProperty
All qdev_prop_register_global() set _fatal for errp, except
'-rtc driftfix=slew', which arguably should also use _fatal, as
otherwise failing to apply the property would only report a warning.
Signed-off-by: Marc-André Lureau
---
include/hw/qdev-core.h| 6 --
hw/core/qdev-properties.c | 4 ++--
qom/cpu.c | 1 -
target/i386/cpu.c | 1 -
target/sparc/cpu.c| 1 -
vl.c | 1 -
6 files changed, 2 insertions(+), 12 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index baaf097212..3a45889c34 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -250,18 +250,12 @@ struct PropertyInfo {
/**
* GlobalProperty:
* @used: Set to true if property was used when initializing a device.
- * @errp: Error destination, used like first argument of error_setg()
- *in case property setting fails later. If @errp is NULL, we
- *print warnings instead of ignoring errors silently. For
- *hotplugged devices, errp is always ignored and warnings are
- *printed instead.
*/
typedef struct GlobalProperty {
const char *driver;
const char *property;
const char *value;
bool used;
-Error **errp;
} GlobalProperty;
/*** Board API. This should go away once we have a machine config file. ***/
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 353e67c05a..8728cbab9f 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1238,8 +1238,8 @@ void qdev_prop_set_globals(DeviceState *dev)
if (err != NULL) {
error_prepend(, "can't apply global %s.%s=%s: ",
prop->driver, prop->property, prop->value);
-if (!dev->hotplugged && prop->errp) {
-error_propagate(prop->errp, err);
+if (!dev->hotplugged) {
+error_propagate(_fatal, err);
} else {
warn_report_err(err);
}
diff --git a/qom/cpu.c b/qom/cpu.c
index 9ad1372d57..5442a7323b 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -312,7 +312,6 @@ static void cpu_common_parse_features(const char *typename,
char *features,
prop->driver = typename;
prop->property = g_strdup(featurestr);
prop->value = g_strdup(val);
-prop->errp = _fatal;
qdev_prop_register_global(prop);
} else {
error_setg(errp, "Expected key=value format, found %s.",
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index af7e9f09cc..1d2dba671e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3568,7 +3568,6 @@ static void x86_cpu_parse_featurestr(const char
*typename, char *features,
prop->driver = typename;
prop->property = g_strdup(name);
prop->value = g_strdup(val);
-prop->errp = _fatal;
qdev_prop_register_global(prop);
}
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index 0f090ece54..4a4445bdf5 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -111,7 +111,6 @@ cpu_add_feat_as_prop(const char *typename, const char
*name, const char *val)
prop->driver = typename;
prop->property = g_strdup(name);
prop->value = g_strdup(val);
-prop->errp = _fatal;
qdev_prop_register_global(prop);
}
diff --git a/vl.c b/vl.c
index d11b070e70..8ee6f7a688 100644
--- a/vl.c
+++ b/vl.c
@@ -2931,7 +2931,6 @@ static int global_init_func(void *opaque, QemuOpts *opts,
Error **errp)
g->driver = qemu_opt_get(opts, "driver");
g->property = qemu_opt_get(opts, "property");
g->value= qemu_opt_get(opts, "value");
-g->errp = _fatal;
qdev_prop_register_global(g);
return 0;
}
--
2.19.1.708.g4ede3d42df
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-3.2 v3 11/14] qom: teach interfaces to implement post-init
The following patches are going to implement post_init callbacks for
settings properties. The interface post_init are called before the
instance post_init, so the default interface behaviour can be
overriden if necessary.
Signed-off-by: Marc-André Lureau
---
qom/object.c| 8 +++-
tests/check-qom-interface.c | 23 +--
2 files changed, 28 insertions(+), 3 deletions(-)
diff --git a/qom/object.c b/qom/object.c
index b1a7f70550..980eeb8283 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -290,7 +290,6 @@ static void type_initialize(TypeImpl *ti)
assert(ti->instance_size == 0);
assert(ti->abstract);
assert(!ti->instance_init);
-assert(!ti->instance_post_init);
assert(!ti->instance_finalize);
assert(!ti->num_interfaces);
}
@@ -363,6 +362,13 @@ static void object_init_with_type(Object *obj, TypeImpl
*ti)
static void object_post_init_with_type(Object *obj, TypeImpl *ti)
{
+GSList *e;
+
+for (e = ti->class->interfaces; e; e = e->next) {
+TypeImpl *itype = OBJECT_CLASS(e->data)->type;
+object_post_init_with_type(obj, itype);
+}
+
if (ti->instance_post_init) {
ti->instance_post_init(obj);
}
diff --git a/tests/check-qom-interface.c b/tests/check-qom-interface.c
index 2177f0dce5..cd2dd6dcee 100644
--- a/tests/check-qom-interface.c
+++ b/tests/check-qom-interface.c
@@ -31,9 +31,27 @@ typedef struct TestIfClass {
uint32_t test;
} TestIfClass;
+typedef struct DirectImpl {
+Object parent_obj;
+
+bool if_post_init;
+} DirectImpl;
+
+#define TYPE_DIRECT_IMPL "direct-impl"
+#define DIRECT_IMPL(obj) \
+OBJECT_CHECK(DirectImpl, (obj), TYPE_DIRECT_IMPL)
+
+static void test_if_post_init(Object *obj)
+{
+DirectImpl *d = DIRECT_IMPL(obj);
+
+d->if_post_init = true;
+}
+
static const TypeInfo test_if_info = {
.name = TYPE_TEST_IF,
.parent= TYPE_INTERFACE,
+.instance_post_init = test_if_post_init,
.class_size = sizeof(TestIfClass),
};
@@ -47,11 +65,10 @@ static void test_class_init(ObjectClass *oc, void *data)
tc->test = PATTERN;
}
-#define TYPE_DIRECT_IMPL "direct-impl"
-
static const TypeInfo direct_impl_info = {
.name = TYPE_DIRECT_IMPL,
.parent = TYPE_OBJECT,
+.instance_size = sizeof(DirectImpl),
.class_init = test_class_init,
.interfaces = (InterfaceInfo[]) {
{ TYPE_TEST_IF },
@@ -70,10 +87,12 @@ static void test_interface_impl(const char *type)
{
Object *obj = object_new(type);
TestIf *iobj = TEST_IF(obj);
+DirectImpl *d = DIRECT_IMPL(obj);
TestIfClass *ioc = TEST_IF_GET_CLASS(iobj);
g_assert(iobj);
g_assert(ioc->test == PATTERN);
+g_assert(d->if_post_init == true);
object_unref(obj);
}
--
2.19.1.708.g4ede3d42df
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-3.2 v3 10/14] qdev-props: call object_apply_global_props()
It's now possible to use the common function.
Teach object_apply_global_props() to warn if Error argument is NULL.
Signed-off-by: Marc-André Lureau
---
hw/core/qdev-properties.c | 24 ++--
qom/object.c | 6 +-
2 files changed, 7 insertions(+), 23 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 8728cbab9f..239535a4cb 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1223,28 +1223,8 @@ int qdev_prop_check_globals(void)
void qdev_prop_set_globals(DeviceState *dev)
{
-int i;
-
-for (i = 0; i < global_props()->len; i++) {
-GlobalProperty *prop;
-Error *err = NULL;
-
-prop = g_array_index(global_props(), GlobalProperty *, i);
-if (object_dynamic_cast(OBJECT(dev), prop->driver) == NULL) {
-continue;
-}
-prop->used = true;
-object_property_parse(OBJECT(dev), prop->value, prop->property, );
-if (err != NULL) {
-error_prepend(, "can't apply global %s.%s=%s: ",
- prop->driver, prop->property, prop->value);
-if (!dev->hotplugged) {
-error_propagate(_fatal, err);
-} else {
-warn_report_err(err);
-}
-}
-}
+object_apply_global_props(OBJECT(dev), global_props(),
+ dev->hotplugged ? NULL : _fatal);
}
/* --- 64bit unsigned int 'size' type --- */
diff --git a/qom/object.c b/qom/object.c
index 9acdf9e16d..b1a7f70550 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -392,7 +392,11 @@ void object_apply_global_props(Object *obj, GArray *props,
Error **errp)
if (err != NULL) {
error_prepend(, "can't apply global %s.%s=%s: ",
p->driver, p->property, p->value);
-error_propagate(errp, err);
+if (errp) {
+error_propagate(errp, err);
+} else {
+warn_report_err(err);
+}
}
}
}
--
2.19.1.708.g4ede3d42df
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-3.2 v3 13/14] hw/i386: add pc-i440fx-3.2 & pc-q35-3.2
The following patch is going to add compatiblity parameters for
qemu <= 3.1.
Signed-off-by: Marc-André Lureau
---
include/hw/compat.h | 3 +++
include/hw/i386/pc.h | 3 +++
hw/i386/pc_piix.c| 21 ++---
hw/i386/pc_q35.c | 19 +--
4 files changed, 41 insertions(+), 5 deletions(-)
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 6f4d5fc647..70958328fe 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -1,6 +1,9 @@
#ifndef HW_COMPAT_H
#define HW_COMPAT_H
+#define HW_COMPAT_3_1 \
+/* empty */
+
#define HW_COMPAT_3_0 \
/* empty */
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 136fe497b6..c37d4333a0 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -294,6 +294,9 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
int e820_get_num_entries(void);
bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
+#define PC_COMPAT_3_1 \
+HW_COMPAT_3_1
+
#define PC_COMPAT_3_0 \
HW_COMPAT_3_0 \
{\
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index dc09466b3e..ba371bfcd7 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -428,21 +428,36 @@ static void pc_i440fx_machine_options(MachineClass *m)
machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE);
}
-static void pc_i440fx_3_0_machine_options(MachineClass *m)
+static void pc_i440fx_3_2_machine_options(MachineClass *m)
{
pc_i440fx_machine_options(m);
m->alias = "pc";
m->is_default = 1;
}
+DEFINE_I440FX_MACHINE(v3_2, "pc-i440fx-3.2", NULL,
+ pc_i440fx_3_2_machine_options);
+
+static void pc_i440fx_3_1_machine_options(MachineClass *m)
+{
+pc_i440fx_3_2_machine_options(m);
+m->is_default = 0;
+m->alias = NULL;
+SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
+}
+
+static void pc_i440fx_3_0_machine_options(MachineClass *m)
+{
+pc_i440fx_3_1_machine_options(m);
+SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
+}
+
DEFINE_I440FX_MACHINE(v3_0, "pc-i440fx-3.0", NULL,
pc_i440fx_3_0_machine_options);
static void pc_i440fx_2_12_machine_options(MachineClass *m)
{
pc_i440fx_3_0_machine_options(m);
-m->is_default = 0;
-m->alias = NULL;
SET_MACHINE_COMPAT(m, PC_COMPAT_2_12);
}
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 532241e3f8..64d6ea65d5 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -311,19 +311,34 @@ static void pc_q35_machine_options(MachineClass *m)
m->max_cpus = 288;
}
-static void pc_q35_3_0_machine_options(MachineClass *m)
+static void pc_q35_3_2_machine_options(MachineClass *m)
{
pc_q35_machine_options(m);
m->alias = "q35";
}
+DEFINE_Q35_MACHINE(v3_2, "pc-q35-3.2", NULL,
+ pc_q35_3_2_machine_options);
+
+static void pc_q35_3_1_machine_options(MachineClass *m)
+{
+pc_q35_3_2_machine_options(m);
+m->alias = NULL;
+SET_MACHINE_COMPAT(m, PC_COMPAT_3_1);
+}
+
+static void pc_q35_3_0_machine_options(MachineClass *m)
+{
+pc_q35_3_1_machine_options(m);
+SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
+}
+
DEFINE_Q35_MACHINE(v3_0, "pc-q35-3.0", NULL,
pc_q35_3_0_machine_options);
static void pc_q35_2_12_machine_options(MachineClass *m)
{
pc_q35_3_0_machine_options(m);
-m->alias = NULL;
SET_MACHINE_COMPAT(m, PC_COMPAT_2_12);
}
--
2.19.1.708.g4ede3d42df
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-3.2 v3 12/14] machine: add compat-props interface
Let's make compatiblity properties an interface, so that objects other
than QDev can benefit from having machine compatiblity properties.
Signed-off-by: Marc-André Lureau
---
include/hw/boards.h| 2 ++
hw/core/compat-props.c | 43 ++
hw/core/qdev.c | 12
MAINTAINERS| 1 +
hw/core/Makefile.objs | 1 +
tests/Makefile.include | 1 +
6 files changed, 52 insertions(+), 8 deletions(-)
create mode 100644 hw/core/compat-props.c
diff --git a/include/hw/boards.h b/include/hw/boards.h
index c02190fc52..e09cbe70a4 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -9,6 +9,8 @@
#include "qom/object.h"
#include "qom/cpu.h"
+#define TYPE_COMPAT_PROPS "compat-props"
+
/**
* memory_region_allocate_system_memory - Allocate a board's main memory
* @mr: the #MemoryRegion to be initialized
diff --git a/hw/core/compat-props.c b/hw/core/compat-props.c
new file mode 100644
index 00..1edeadb6a3
--- /dev/null
+++ b/hw/core/compat-props.c
@@ -0,0 +1,43 @@
+/*
+ * QEMU Machine compat properties
+ *
+ * Copyright (C) 2018 Red Hat Inc
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/boards.h"
+#include "qapi/error.h"
+
+typedef struct CompatProps CompatProps;
+
+typedef struct CompatPropsClass {
+InterfaceClass parent_class;
+} CompatPropsClass;
+
+static void compat_props_post_init(Object *obj)
+{
+if (current_machine) {
+MachineClass *mc = MACHINE_GET_CLASS(current_machine);
+AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator);
+
+object_apply_global_props(obj, mc->compat_props, _abort);
+object_apply_global_props(obj, ac->compat_props, _abort);
+}
+}
+
+static void compat_props_register_types(void)
+{
+static const TypeInfo cp_interface_info = {
+.name = TYPE_COMPAT_PROPS,
+.parent= TYPE_INTERFACE,
+.class_size = sizeof(CompatPropsClass),
+.instance_post_init = compat_props_post_init,
+};
+
+type_register_static(_interface_info);
+}
+
+type_init(compat_props_register_types)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 30890f2c8d..b0ee05f837 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -972,14 +972,6 @@ static void device_initfn(Object *obj)
static void device_post_init(Object *obj)
{
-if (current_machine) {
-MachineClass *mc = MACHINE_GET_CLASS(current_machine);
-AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator);
-
-object_apply_global_props(obj, mc->compat_props, _abort);
-object_apply_global_props(obj, ac->compat_props, _abort);
-}
-
qdev_prop_set_globals(DEVICE(obj));
}
@@ -1112,6 +1104,10 @@ static const TypeInfo device_type_info = {
.class_init = device_class_init,
.abstract = true,
.class_size = sizeof(DeviceClass),
+.interfaces = (InterfaceInfo[]) {
+{ TYPE_COMPAT_PROPS },
+{ }
+}
};
static void qdev_register_types(void)
diff --git a/MAINTAINERS b/MAINTAINERS
index 0499e11593..60af287386 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1024,6 +1024,7 @@ Machine core
M: Eduardo Habkost
M: Marcel Apfelbaum
S: Supported
+F: hw/core/compat-props.c
F: hw/core/machine.c
F: hw/core/null-machine.c
F: include/hw/boards.h
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index a799c83815..f15b3c970a 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -1,5 +1,6 @@
# core qdev-related obj files, also used by *-user:
common-obj-y += qdev.o qdev-properties.o
+common-obj-y += compat-props.o
common-obj-y += bus.o reset.o
common-obj-$(CONFIG_SOFTMMU) += qdev-fw.o
common-obj-$(CONFIG_SOFTMMU) += fw-path-provider.o
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 074eece558..4bfde2e7b0 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -565,6 +565,7 @@ tests/test-qdev-global-props$(EXESUF):
tests/test-qdev-global-props.o \
hw/core/irq.o \
hw/core/fw-path-provider.o \
hw/core/reset.o \
+ hw/core/compat-props.o \
$(test-qapi-obj-y)
tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
migration/vmstate.o migration/vmstate-types.o migration/qemu-file.o \
--
2.19.1.708.g4ede3d42df
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-3.2 v3 07/14] qdev: all globals are now user-provided
Considering that CPU features are provided via command line, the
global_props are now all user-provided globals. No need to track this
anymore for qdev_prop_check_globals().
Signed-off-by: Marc-André Lureau
---
include/hw/qdev-core.h | 3 --
hw/core/qdev-properties.c | 4 ---
tests/test-qdev-global-props.c | 57 --
vl.c | 1 -
4 files changed, 6 insertions(+), 59 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index a24d0dd566..baaf097212 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -249,8 +249,6 @@ struct PropertyInfo {
/**
* GlobalProperty:
- * @user_provided: Set to true if property comes from user-provided config
- * (command-line or config file).
* @used: Set to true if property was used when initializing a device.
* @errp: Error destination, used like first argument of error_setg()
*in case property setting fails later. If @errp is NULL, we
@@ -262,7 +260,6 @@ typedef struct GlobalProperty {
const char *driver;
const char *property;
const char *value;
-bool user_provided;
bool used;
Error **errp;
} GlobalProperty;
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index bd84c4ea4c..43c30a57f4 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1192,9 +1192,6 @@ int qdev_prop_check_globals(void)
if (prop->used) {
continue;
}
-if (!prop->user_provided) {
-continue;
-}
oc = object_class_by_name(prop->driver);
oc = object_class_dynamic_cast(oc, TYPE_DEVICE);
if (!oc) {
@@ -1233,7 +1230,6 @@ void qdev_prop_set_globals(DeviceState *dev)
if (!dev->hotplugged && prop->errp) {
error_propagate(prop->errp, err);
} else {
-assert(prop->user_provided);
warn_report_err(err);
}
}
diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
index 3a8d3170a0..f49a1b70b5 100644
--- a/tests/test-qdev-global-props.c
+++ b/tests/test-qdev-global-props.c
@@ -215,12 +215,12 @@ static void test_dynamic_globalprop_subprocess(void)
{
MyType *mt;
static GlobalProperty props[] = {
-{ TYPE_DYNAMIC_PROPS, "prop1", "101", true },
-{ TYPE_DYNAMIC_PROPS, "prop2", "102", true },
-{ TYPE_DYNAMIC_PROPS"-bad", "prop3", "103", true },
-{ TYPE_UNUSED_HOTPLUG, "prop4", "104", true },
-{ TYPE_UNUSED_NOHOTPLUG, "prop5", "105", true },
-{ TYPE_NONDEVICE, "prop6", "106", true },
+{ TYPE_DYNAMIC_PROPS, "prop1", "101", },
+{ TYPE_DYNAMIC_PROPS, "prop2", "102", },
+{ TYPE_DYNAMIC_PROPS"-bad", "prop3", "103", },
+{ TYPE_UNUSED_HOTPLUG, "prop4", "104", },
+{ TYPE_UNUSED_NOHOTPLUG, "prop5", "105", },
+{ TYPE_NONDEVICE, "prop6", "106", },
{}
};
int global_error;
@@ -255,46 +255,6 @@ static void test_dynamic_globalprop(void)
g_test_trap_assert_stdout("");
}
-/* Test setting of dynamic properties using user_provided=false properties */
-static void test_dynamic_globalprop_nouser_subprocess(void)
-{
-MyType *mt;
-static GlobalProperty props[] = {
-{ TYPE_DYNAMIC_PROPS, "prop1", "101" },
-{ TYPE_DYNAMIC_PROPS, "prop2", "102" },
-{ TYPE_DYNAMIC_PROPS"-bad", "prop3", "103" },
-{ TYPE_UNUSED_HOTPLUG, "prop4", "104" },
-{ TYPE_UNUSED_NOHOTPLUG, "prop5", "105" },
-{ TYPE_NONDEVICE, "prop6", "106" },
-{}
-};
-int global_error;
-
-register_global_properties(props);
-
-mt = DYNAMIC_TYPE(object_new(TYPE_DYNAMIC_PROPS));
-qdev_init_nofail(DEVICE(mt));
-
-g_assert_cmpuint(mt->prop1, ==, 101);
-g_assert_cmpuint(mt->prop2, ==, 102);
-global_error = qdev_prop_check_globals();
-g_assert_cmpuint(global_error, ==, 0);
-g_assert(props[0].used);
-g_assert(props[1].used);
-g_assert(!props[2].used);
-g_assert(!props[3].used);
-g_assert(!props[4].used);
-g_assert(!props[5].used);
-}
-
-static void test_dynamic_globalprop_nouser(void)
-{
-
g_test_trap_subprocess("/qdev/properties/dynamic/global/nouser/subprocess", 0,
0);
-g_test_trap_assert_passed();
-g_test_trap_assert_stderr("");
-g_test_trap_assert_stdout("");
-}
-
/* Test if global props affecting subclasses are applied in the right order */
static void test_subclass_global_props(void)
[Xen-devel] [PATCH for-3.2 v3 08/14] qdev-props: convert global_props to GArray
A step towards being able to call object_apply_global_props().
Signed-off-by: Marc-André Lureau
---
hw/core/qdev-properties.c | 29 -
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 43c30a57f4..353e67c05a 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1173,22 +1173,32 @@ void qdev_prop_set_ptr(DeviceState *dev, const char
*name, void *value)
*ptr = value;
}
-static GList *global_props;
+static GArray *global_props(void)
+{
+static GArray *gp;
+
+if (!gp) {
+gp = g_array_new(false, false, sizeof(GlobalProperty *));
+}
+
+return gp;
+}
void qdev_prop_register_global(GlobalProperty *prop)
{
-global_props = g_list_append(global_props, prop);
+g_array_append_val(global_props(), prop);
}
int qdev_prop_check_globals(void)
{
-GList *l;
-int ret = 0;
+int i, ret = 0;
-for (l = global_props; l; l = l->next) {
-GlobalProperty *prop = l->data;
+for (i = 0; i < global_props()->len; i++) {
+GlobalProperty *prop;
ObjectClass *oc;
DeviceClass *dc;
+
+prop = g_array_index(global_props(), GlobalProperty *, i);
if (prop->used) {
continue;
}
@@ -1213,12 +1223,13 @@ int qdev_prop_check_globals(void)
void qdev_prop_set_globals(DeviceState *dev)
{
-GList *l;
+int i;
-for (l = global_props; l; l = l->next) {
-GlobalProperty *prop = l->data;
+for (i = 0; i < global_props()->len; i++) {
+GlobalProperty *prop;
Error *err = NULL;
+prop = g_array_index(global_props(), GlobalProperty *, i);
if (object_dynamic_cast(OBJECT(dev), prop->driver) == NULL) {
continue;
}
--
2.19.1.708.g4ede3d42df
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-3.2 v3 02/14] qom: make interface types abstract
Interfaces don't have instance, let's make the interface type really
abstract to avoid confusion.
Signed-off-by: Marc-André Lureau
---
include/hw/acpi/acpi_dev_interface.h | 6 +-
include/hw/arm/linux-boot-if.h | 5 +
include/hw/fw-path-provider.h| 4 +---
include/hw/hotplug.h | 6 +-
include/hw/intc/intc.h | 4 +---
include/hw/ipmi/ipmi.h | 4 +---
include/hw/isa/isa.h | 4
include/hw/mem/memory-device.h | 4 +---
include/hw/nmi.h | 4 +---
include/hw/stream.h | 4 +---
include/hw/timer/m48t59.h| 4 +---
include/qom/object_interfaces.h | 6 +-
include/sysemu/tpm.h | 4 +---
target/arm/idau.h| 4 +---
tests/check-qom-interface.c | 4 +---
15 files changed, 14 insertions(+), 53 deletions(-)
diff --git a/include/hw/acpi/acpi_dev_interface.h
b/include/hw/acpi/acpi_dev_interface.h
index dabf4c4fc9..43ff119179 100644
--- a/include/hw/acpi/acpi_dev_interface.h
+++ b/include/hw/acpi/acpi_dev_interface.h
@@ -25,11 +25,7 @@ typedef enum {
INTERFACE_CHECK(AcpiDeviceIf, (obj), \
TYPE_ACPI_DEVICE_IF)
-
-typedef struct AcpiDeviceIf {
-/* */
-Object Parent;
-} AcpiDeviceIf;
+typedef struct AcpiDeviceIf AcpiDeviceIf;
void acpi_send_event(DeviceState *dev, AcpiEventStatusBits event);
diff --git a/include/hw/arm/linux-boot-if.h b/include/hw/arm/linux-boot-if.h
index aba4479a14..7bbdfd1cc6 100644
--- a/include/hw/arm/linux-boot-if.h
+++ b/include/hw/arm/linux-boot-if.h
@@ -16,10 +16,7 @@
#define ARM_LINUX_BOOT_IF(obj) \
INTERFACE_CHECK(ARMLinuxBootIf, (obj), TYPE_ARM_LINUX_BOOT_IF)
-typedef struct ARMLinuxBootIf {
-/*< private >*/
-Object parent_obj;
-} ARMLinuxBootIf;
+typedef struct ARMLinuxBootIf ARMLinuxBootIf;
typedef struct ARMLinuxBootIfClass {
/*< private >*/
diff --git a/include/hw/fw-path-provider.h b/include/hw/fw-path-provider.h
index 050cb05d92..5df893a3d8 100644
--- a/include/hw/fw-path-provider.h
+++ b/include/hw/fw-path-provider.h
@@ -30,9 +30,7 @@
#define FW_PATH_PROVIDER(obj) \
INTERFACE_CHECK(FWPathProvider, (obj), TYPE_FW_PATH_PROVIDER)
-typedef struct FWPathProvider {
-Object parent_obj;
-} FWPathProvider;
+typedef struct FWPathProvider FWPathProvider;
typedef struct FWPathProviderClass {
InterfaceClass parent_class;
diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
index 1a0516a479..6321e292fd 100644
--- a/include/hw/hotplug.h
+++ b/include/hw/hotplug.h
@@ -23,11 +23,7 @@
#define HOTPLUG_HANDLER(obj) \
INTERFACE_CHECK(HotplugHandler, (obj), TYPE_HOTPLUG_HANDLER)
-
-typedef struct HotplugHandler {
-/* */
-Object Parent;
-} HotplugHandler;
+typedef struct HotplugHandler HotplugHandler;
/**
* hotplug_fn:
diff --git a/include/hw/intc/intc.h b/include/hw/intc/intc.h
index 27d9828943..fb3e8e621f 100644
--- a/include/hw/intc/intc.h
+++ b/include/hw/intc/intc.h
@@ -15,9 +15,7 @@
INTERFACE_CHECK(InterruptStatsProvider, (obj), \
TYPE_INTERRUPT_STATS_PROVIDER)
-typedef struct InterruptStatsProvider {
-Object parent;
-} InterruptStatsProvider;
+typedef struct InterruptStatsProvider InterruptStatsProvider;
typedef struct InterruptStatsProviderClass {
InterfaceClass parent;
diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h
index 0affe5a4d8..99661d2bf0 100644
--- a/include/hw/ipmi/ipmi.h
+++ b/include/hw/ipmi/ipmi.h
@@ -114,9 +114,7 @@ uint32_t ipmi_next_uuid(void);
#define IPMI_INTERFACE_GET_CLASS(class) \
OBJECT_GET_CLASS(IPMIInterfaceClass, (class), TYPE_IPMI_INTERFACE)
-typedef struct IPMIInterface {
-Object parent;
-} IPMIInterface;
+typedef struct IPMIInterface IPMIInterface;
typedef struct IPMIInterfaceClass {
InterfaceClass parent;
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index b9dbab24b4..e62ac91c19 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -43,10 +43,6 @@ static inline uint16_t applesmc_port(void)
#define ISADMA(obj) \
INTERFACE_CHECK(IsaDma, (obj), TYPE_ISADMA)
-struct IsaDma {
-Object parent;
-};
-
typedef enum {
ISADMA_TRANSFER_VERIFY,
ISADMA_TRANSFER_READ,
diff --git a/include/hw/mem/memory-device.h b/include/hw/mem/memory-device.h
index e904e194d5..0293a96abb 100644
--- a/include/hw/mem/memory-device.h
+++ b/include/hw/mem/memory-device.h
@@ -25,9 +25,7 @@
#define MEMORY_DEVICE(obj) \
INTERFACE_CHECK(MemoryDeviceState, (obj), TYPE_MEMORY_DEVICE)
-typedef struct MemoryDeviceState {
-Object parent_obj;
-} MemoryDeviceState;
+typedef struct MemoryDeviceState MemoryDeviceState;
/**
* MemoryDeviceClass:
diff --git a/include/hw/nmi.h b/include/hw/nmi.h
index d092c684a1..ad857f3832 100644
--- a/include/hw/nmi.h
+++ b/include/hw/nmi.h
@@ -34,9 +34,7 @@
#define NMI(obj) \
INTERFACE_CHECK(NMI, (obj), TYPE_NMI)
-typedef str
[Xen-devel] [PATCH for-3.2 v3 05/14] qdev: move qdev_prop_register_global_list() to tests
The function is only used by a test, move it there.
Signed-off-by: Marc-André Lureau
Reviewed-by: Eduardo Habkost
---
include/hw/qdev-properties.h | 1 -
hw/core/qdev-properties.c | 9 -
tests/test-qdev-global-props.c | 18 ++
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index a95f4a73eb..3ab9cd2eb6 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -249,7 +249,6 @@ void qdev_prop_set_enum(DeviceState *dev, const char *name,
int value);
void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
void qdev_prop_register_global(GlobalProperty *prop);
-void qdev_prop_register_global_list(GlobalProperty *props);
int qdev_prop_check_globals(void);
void qdev_prop_set_globals(DeviceState *dev);
void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index ab61d502fd..bd84c4ea4c 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1180,15 +1180,6 @@ void qdev_prop_register_global(GlobalProperty *prop)
global_props = g_list_append(global_props, prop);
}
-void qdev_prop_register_global_list(GlobalProperty *props)
-{
-int i;
-
-for (i = 0; props[i].driver != NULL; i++) {
-qdev_prop_register_global(props+i);
-}
-}
-
int qdev_prop_check_globals(void)
{
GList *l;
diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
index ccdf6c57c1..b1eb505442 100644
--- a/tests/test-qdev-global-props.c
+++ b/tests/test-qdev-global-props.c
@@ -89,6 +89,16 @@ static void test_static_prop(void)
g_test_trap_assert_stdout("");
}
+static void register_global_properties(GlobalProperty *props)
+{
+int i;
+
+for (i = 0; props[i].driver != NULL; i++) {
+qdev_prop_register_global(props + i);
+}
+}
+
+
/* Test setting of static property using global properties */
static void test_static_globalprop_subprocess(void)
{
@@ -98,7 +108,7 @@ static void test_static_globalprop_subprocess(void)
{}
};
-qdev_prop_register_global_list(props);
+register_global_properties(props);
mt = STATIC_TYPE(object_new(TYPE_STATIC_PROPS));
qdev_init_nofail(DEVICE(mt));
@@ -216,7 +226,7 @@ static void test_dynamic_globalprop_subprocess(void)
};
int global_error;
-qdev_prop_register_global_list(props);
+register_global_properties(props);
mt = DYNAMIC_TYPE(object_new(TYPE_DYNAMIC_PROPS));
qdev_init_nofail(DEVICE(mt));
@@ -261,7 +271,7 @@ static void test_dynamic_globalprop_nouser_subprocess(void)
};
int global_error;
-qdev_prop_register_global_list(props);
+register_global_properties(props);
mt = DYNAMIC_TYPE(object_new(TYPE_DYNAMIC_PROPS));
qdev_init_nofail(DEVICE(mt));
@@ -299,7 +309,7 @@ static void test_subclass_global_props(void)
{}
};
-qdev_prop_register_global_list(props);
+register_global_properties(props);
mt = STATIC_TYPE(object_new(TYPE_SUBCLASS));
qdev_init_nofail(DEVICE(mt));
--
2.19.1.708.g4ede3d42df
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-3.2 v3 06/14] qdev: do not mix compat props with global props
Machine & Accel props are not provided by user. Let's not mix them
with the global properties.
Call a new helper function object_apply_global_props() during
device_post_init().
Add a stub for current_machine, so qemu-user and tests can find a
fallback symbol when linking with QDev.
The following patches is going to reuse object_apply_global_props()
for qdev globals.
Signed-off-by: Marc-André Lureau
---
include/hw/boards.h| 1 -
include/qom/object.h | 2 ++
include/sysemu/accel.h | 4 +---
accel/accel.c | 12
hw/core/machine.c | 18 --
hw/core/qdev.c | 8
hw/xen/xen-common.c| 9 -
qom/object.c | 25 +
stubs/machine.c| 4
tests/test-qdev-global-props.c | 1 -
vl.c | 2 --
stubs/Makefile.objs| 1 +
12 files changed, 49 insertions(+), 38 deletions(-)
create mode 100644 stubs/machine.c
diff --git a/include/hw/boards.h b/include/hw/boards.h
index f82f28468b..c02190fc52 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -69,7 +69,6 @@ int machine_kvm_shadow_mem(MachineState *machine);
int machine_phandle_start(MachineState *machine);
bool machine_dump_guest_core(MachineState *machine);
bool machine_mem_merge(MachineState *machine);
-void machine_register_compat_props(MachineState *machine);
HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine);
void machine_set_cpu_numa_node(MachineState *machine,
const CpuInstanceProperties *props,
diff --git a/include/qom/object.h b/include/qom/object.h
index f0b0bf39cc..e58eeb280f 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -679,6 +679,8 @@ Object *object_new_with_propv(const char *typename,
Error **errp,
va_list vargs);
+void object_apply_global_props(Object *obj, GArray *props, Error **errp);
+
/**
* object_set_props:
* @obj: the object instance to set properties on
diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h
index 637358f430..f4f71134b5 100644
--- a/include/sysemu/accel.h
+++ b/include/sysemu/accel.h
@@ -49,7 +49,7 @@ typedef struct AccelClass {
* global properties may be overridden by machine-type
* compat_props or user-provided global properties.
*/
-GlobalProperty *global_props;
+GArray *compat_props;
} AccelClass;
#define TYPE_ACCEL "accel"
@@ -67,8 +67,6 @@ typedef struct AccelClass {
extern unsigned long tcg_tb_size;
void configure_accelerator(MachineState *ms);
-/* Register accelerator specific global properties */
-void accel_register_compat_props(AccelState *accel);
/* Called just before os_setup_post (ie just before drop OS privs) */
void accel_setup_post(MachineState *ms);
diff --git a/accel/accel.c b/accel/accel.c
index 3da26eb90f..6db5d8f4df 100644
--- a/accel/accel.c
+++ b/accel/accel.c
@@ -119,18 +119,6 @@ void configure_accelerator(MachineState *ms)
}
}
-void accel_register_compat_props(AccelState *accel)
-{
-AccelClass *class = ACCEL_GET_CLASS(accel);
-GlobalProperty *prop = class->global_props;
-
-for (; prop && prop->driver; prop++) {
-/* Any compat_props must never cause error */
-prop->errp = _abort;
-qdev_prop_register_global(prop);
-}
-}
-
void accel_setup_post(MachineState *ms)
{
AccelState *accel = ms->accelerator;
diff --git a/hw/core/machine.c b/hw/core/machine.c
index da50ad6de7..d45945 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -844,24 +844,6 @@ static void machine_class_finalize(ObjectClass *klass,
void *data)
g_free(mc->name);
}
-void machine_register_compat_props(MachineState *machine)
-{
-MachineClass *mc = MACHINE_GET_CLASS(machine);
-int i;
-GlobalProperty *p;
-
-if (!mc->compat_props) {
-return;
-}
-
-for (i = 0; i < mc->compat_props->len; i++) {
-p = g_array_index(mc->compat_props, GlobalProperty *, i);
-/* Machine compat_props must never cause errors: */
-p->errp = _abort;
-qdev_prop_register_global(p);
-}
-}
-
static const TypeInfo machine_info = {
.name = TYPE_MACHINE,
.parent = TYPE_OBJECT,
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6b3cc55b27..30890f2c8d 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -972,6 +972,14 @@ static void device_initfn(Object *obj)
static void device_post_init(Object *obj)
{
+if (current_machine) {
+MachineClass *mc = MACHINE_GET_CLASS(current_machine);
+AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator);
+
+object_apply_global_props(obj, mc->compat_props, _abort);
+object_apply_global_props(obj, ac->compat_props, _abort);
+}
+
qdev_prop_set_globals(
[Xen-devel] [PATCH for-3.2 v3 00/14] Generalize machine compatibility properties
Hi, During "[PATCH v2 05/10] qom/globals: generalize object_property_set_globals()" review, Eduardo suggested to rework the GlobalProperty handling, so that -global is limited to QDev only and we avoid mixing the machine compats and the user-provided -global properties (instead of generalizing -global to various object kinds, like I proposed in v2). "qdev: do not mix compat props with global props" patch decouples a bit user-provided -global from machine compat properties. This allows to get rid of "user_provided" and "errp" fields in following patches. Instead of explcitely calling object_apply_global_props() in the various object post_init, I opted for creating a new TYPE_COMPAT_PROPS interface. The interface approach gives a lot more flexibility on which objects can have compat props. This requires some interface improvments in "qom: teach interfaces to implement post-init". A new compat property "x-use-canonical-path-for-ramblock-id" is added to hostmem for legacy canonical path names, set to true for -file and -memfd with qemu < 3.2. (this series was initially titled "[PATCH v2 00/10] hostmem: use object "id" for memory region name with >= 3.1", but its focus is more in refactoring the global and compatilibity properties handling now) v3: - GlobalProperties improvements/cleanups - drop generalizing the -global idea - "replace" the set_globals flag with a TYPE_COMPAT_PROPS interface - update hw/i386 machine version to 3.2 - add "qom: make interface types abstract" interface cleanup v2: - replace "qom/user-creatable: add a few helper macros" patch for a more optimized "qom: make user_creatable_complete() specific to UserCreatable" - rename register_global_list() to register_global_properties() - call object_property_set_globals() after post-init - add and use a ObjectClass.set_globals flag, instead of dynamically check object class in object_property_set_globals() - use object "id" in >= 3.1 instead of canonical path, add compat property "x-use-canonical-path-for-ramblock-id" in base hostmem class. Marc-André Lureau (14): tests: qdev_prop_check_globals() doesn't return "all_used" qom: make interface types abstract qom: make user_creatable_complete() specific to UserCreatable accel: register global_props like machine globals qdev: move qdev_prop_register_global_list() to tests qdev: do not mix compat props with global props qdev: all globals are now user-provided qdev-props: convert global_props to GArray qdev-props: remove errp from GlobalProperty qdev-props: call object_apply_global_props() qom: teach interfaces to implement post-init machine: add compat-props interface hw/i386: add pc-i440fx-3.2 & pc-q35-3.2 hostmem: use object id for memory region name with >= 3.1 include/hw/acpi/acpi_dev_interface.h | 6 +-- include/hw/arm/linux-boot-if.h | 5 +- include/hw/boards.h | 3 +- include/hw/compat.h | 11 include/hw/fw-path-provider.h| 4 +- include/hw/hotplug.h | 6 +-- include/hw/i386/pc.h | 3 ++ include/hw/intc/intc.h | 4 +- include/hw/ipmi/ipmi.h | 4 +- include/hw/isa/isa.h | 4 -- include/hw/mem/memory-device.h | 4 +- include/hw/nmi.h | 4 +- include/hw/qdev-core.h | 9 include/hw/qdev-properties.h | 30 --- include/hw/stream.h | 4 +- include/hw/timer/m48t59.h| 4 +- include/qom/object.h | 2 + include/qom/object_interfaces.h | 10 ++-- include/sysemu/accel.h | 4 +- include/sysemu/hostmem.h | 3 +- include/sysemu/tpm.h | 4 +- target/arm/idau.h| 4 +- accel/accel.c| 7 +-- backends/hostmem-file.c | 8 +-- backends/hostmem-memfd.c | 2 +- backends/hostmem-ram.c | 9 ++-- backends/hostmem.c | 31 +++ hw/core/compat-props.c | 43 +++ hw/core/machine.c| 18 --- hw/core/qdev-properties.c| 73 ++--- hw/core/qdev.c | 4 ++ hw/i386/pc_piix.c| 21 ++-- hw/i386/pc_q35.c | 19 ++- hw/misc/ivshmem.c| 2 +- hw/virtio/virtio-rng.c | 2 +- hw/xen/xen-common.c | 9 +++- qom/cpu.c| 1 - qom/object.c | 49 +++-- qom/object_interfaces.c | 14 ++--- stubs/machine.c | 4 ++ target/i386/cpu.c| 1 - target/sparc/cpu.c | 1 - tests/check-qom-i
[Xen-devel] [PATCH for-3.2 v3 03/14] qom: make user_creatable_complete() specific to UserCreatable
Instead of accepting any Object*, change user_creatable_complete() to
require a UserCreatable*. Modify the callers to pass the appropriate
argument, removing redundant dynamic cast checks in object creation.
Signed-off-by: Marc-André Lureau
Reviewed-by: Igor Mammedov
---
include/qom/object_interfaces.h | 4 ++--
hw/misc/ivshmem.c | 2 +-
hw/virtio/virtio-rng.c | 2 +-
qom/object.c| 12
qom/object_interfaces.c | 14 +++---
5 files changed, 15 insertions(+), 19 deletions(-)
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 652a16d2ba..682ba1d9b0 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -51,14 +51,14 @@ typedef struct UserCreatableClass {
/**
* user_creatable_complete:
- * @obj: the object whose complete() method is called if defined
+ * @uc: the user-creatable object whose complete() method is called if defined
* @errp: if an error occurs, a pointer to an area to store the error
*
* Wrapper to call complete() method if one of types it's inherited
* from implements USER_CREATABLE interface, otherwise the call does
* nothing.
*/
-void user_creatable_complete(Object *obj, Error **errp);
+void user_creatable_complete(UserCreatable *uc, Error **errp);
/**
* user_creatable_can_be_deleted:
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index f88910e55c..478f41044c 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -1279,7 +1279,7 @@ static void desugar_shm(IVShmemState *s)
object_property_set_bool(obj, true, "share", _abort);
object_property_add_child(OBJECT(s), "internal-shm-backend", obj,
_abort);
-user_creatable_complete(obj, _abort);
+user_creatable_complete(USER_CREATABLE(obj), _abort);
s->hostmem = MEMORY_BACKEND(obj);
}
diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index 855f1b41d1..30493a2586 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -191,7 +191,7 @@ static void virtio_rng_device_realize(DeviceState *dev,
Error **errp)
if (vrng->conf.rng == NULL) {
vrng->conf.default_backend = RNG_RANDOM(object_new(TYPE_RNG_RANDOM));
-user_creatable_complete(OBJECT(vrng->conf.default_backend),
+user_creatable_complete(USER_CREATABLE(vrng->conf.default_backend),
_err);
if (local_err) {
error_propagate(errp, local_err);
diff --git a/qom/object.c b/qom/object.c
index 547dcf97c3..eb770dbf7f 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -417,6 +417,7 @@ void object_initialize_childv(Object *parentobj, const char
*propname,
{
Error *local_err = NULL;
Object *obj;
+UserCreatable *uc;
object_initialize(childobj, size, type);
obj = OBJECT(childobj);
@@ -431,8 +432,9 @@ void object_initialize_childv(Object *parentobj, const char
*propname,
goto out;
}
-if (object_dynamic_cast(obj, TYPE_USER_CREATABLE)) {
-user_creatable_complete(obj, _err);
+uc = (UserCreatable *)object_dynamic_cast(obj, TYPE_USER_CREATABLE);
+if (uc) {
+user_creatable_complete(uc, _err);
if (local_err) {
object_unparent(obj);
goto out;
@@ -590,6 +592,7 @@ Object *object_new_with_propv(const char *typename,
Object *obj;
ObjectClass *klass;
Error *local_err = NULL;
+UserCreatable *uc;
klass = object_class_by_name(typename);
if (!klass) {
@@ -612,8 +615,9 @@ Object *object_new_with_propv(const char *typename,
goto error;
}
-if (object_dynamic_cast(obj, TYPE_USER_CREATABLE)) {
-user_creatable_complete(obj, _err);
+uc = (UserCreatable *)object_dynamic_cast(obj, TYPE_USER_CREATABLE);
+if (uc) {
+user_creatable_complete(uc, _err);
if (local_err) {
object_unparent(obj);
goto error;
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 97b79b48bb..db85d1eb75 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -8,18 +8,10 @@
#include "qapi/opts-visitor.h"
#include "qemu/config-file.h"
-void user_creatable_complete(Object *obj, Error **errp)
+void user_creatable_complete(UserCreatable *uc, Error **errp)
{
+UserCreatableClass *ucc = USER_CREATABLE_GET_CLASS(uc);
-UserCreatableClass *ucc;
-UserCreatable *uc =
-(UserCreatable *)object_dynamic_cast(obj, TYPE_USER_CREATABLE);
-
-if (!uc) {
-return;
-}
-
-ucc = USER_CREATABLE_GET_CLASS(uc);
if (ucc->complete) {
ucc->complete(uc, errp);
}
@@ -89,7 +81,7 @@ Object *user_creatable_add_type(const char *type, const char
*id,
goto out;
}
-user_creatable_complete(obj, _err);
+user_creatable_complete(USER_CREATABLE(obj), _err);
if (loca
[Xen-devel] [PATCH for-3.2 v3 04/14] accel: register global_props like machine globals
global_props is only used for Xen xen_compat_props. It's a static
array of GlobalProperty, like machine globals in SET_MACHINE_COMPAT().
Let's register the globals the same way, without extra copy allocation.
Signed-off-by: Marc-André Lureau
Reviewed-by: Igor Mammedov
---
include/hw/qdev-properties.h | 29 -
accel/accel.c| 9 -
hw/core/qdev-properties.c| 21 -
3 files changed, 8 insertions(+), 51 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 4f60cc88f3..a95f4a73eb 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -255,35 +255,6 @@ void qdev_prop_set_globals(DeviceState *dev);
void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
Property *prop, const char *value);
-/**
- * register_compat_prop:
- *
- * Register internal (not user-provided) global property, changing the
- * default value of a given property in a device type. This can be used
- * for enabling machine-type compatibility or for enabling
- * accelerator-specific defaults in devices.
- *
- * The property values set using this function must be always valid and
- * never report setter errors, as the property will have
- * GlobalProperty::errp set to _abort.
- *
- * User-provided global properties should override internal global
- * properties, so callers of this function should ensure that it is
- * called before user-provided global properties are registered.
- *
- * @driver: Device type to be affected
- * @property: Property whose default value is going to be changed
- * @value: New default value for the property
- */
-void register_compat_prop(const char *driver, const char *property,
- const char *value);
-/*
- * register_compat_props_array(): using register_compat_prop(), which
- * only registers internal global properties (which has lower priority
- * than user-provided global properties)
- */
-void register_compat_props_array(GlobalProperty *prop);
-
/**
* qdev_property_add_static:
* @dev: Device to add the property to.
diff --git a/accel/accel.c b/accel/accel.c
index 966b2d8f53..3da26eb90f 100644
--- a/accel/accel.c
+++ b/accel/accel.c
@@ -34,6 +34,7 @@
#include "qom/object.h"
#include "qemu/error-report.h"
#include "qemu/option.h"
+#include "qapi/error.h"
static const TypeInfo accel_type = {
.name = TYPE_ACCEL,
@@ -121,7 +122,13 @@ void configure_accelerator(MachineState *ms)
void accel_register_compat_props(AccelState *accel)
{
AccelClass *class = ACCEL_GET_CLASS(accel);
-register_compat_props_array(class->global_props);
+GlobalProperty *prop = class->global_props;
+
+for (; prop && prop->driver; prop++) {
+/* Any compat_props must never cause error */
+prop->errp = _abort;
+qdev_prop_register_global(prop);
+}
}
void accel_setup_post(MachineState *ms)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 35072dec1e..ab61d502fd 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1180,27 +1180,6 @@ void qdev_prop_register_global(GlobalProperty *prop)
global_props = g_list_append(global_props, prop);
}
-void register_compat_prop(const char *driver,
- const char *property,
- const char *value)
-{
-GlobalProperty *p = g_new0(GlobalProperty, 1);
-
-/* Any compat_props must never cause error */
-p->errp = _abort;
-p->driver = driver;
-p->property = property;
-p->value = value;
-qdev_prop_register_global(p);
-}
-
-void register_compat_props_array(GlobalProperty *prop)
-{
-for (; prop && prop->driver; prop++) {
-register_compat_prop(prop->driver, prop->property, prop->value);
-}
-}
-
void qdev_prop_register_global_list(GlobalProperty *props)
{
int i;
--
2.19.1.708.g4ede3d42df
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH for-3.2 v3 01/14] tests: qdev_prop_check_globals() doesn't return "all_used"
Instead, it returns 1 if an error was detected, which is the case for:
/qdev/properties/dynamic/global/subprocess:
warning: global dynamic-prop-type-bad.prop3 has invalid class name
warning: global nohotplug-type.prop5=105 not used
warning: global nondevice-type.prop6 has invalid class name
Clarify the function return value.
Signed-off-by: Marc-André Lureau
---
tests/test-qdev-global-props.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
index d81b0862d5..ccdf6c57c1 100644
--- a/tests/test-qdev-global-props.c
+++ b/tests/test-qdev-global-props.c
@@ -214,7 +214,7 @@ static void test_dynamic_globalprop_subprocess(void)
{ TYPE_NONDEVICE, "prop6", "106", true },
{}
};
-int all_used;
+int global_error;
qdev_prop_register_global_list(props);
@@ -223,8 +223,8 @@ static void test_dynamic_globalprop_subprocess(void)
g_assert_cmpuint(mt->prop1, ==, 101);
g_assert_cmpuint(mt->prop2, ==, 102);
-all_used = qdev_prop_check_globals();
-g_assert_cmpuint(all_used, ==, 1);
+global_error = qdev_prop_check_globals();
+g_assert_cmpuint(global_error, ==, 1);
g_assert(props[0].used);
g_assert(props[1].used);
g_assert(!props[2].used);
@@ -259,7 +259,7 @@ static void test_dynamic_globalprop_nouser_subprocess(void)
{ TYPE_NONDEVICE, "prop6", "106" },
{}
};
-int all_used;
+int global_error;
qdev_prop_register_global_list(props);
@@ -268,8 +268,8 @@ static void test_dynamic_globalprop_nouser_subprocess(void)
g_assert_cmpuint(mt->prop1, ==, 101);
g_assert_cmpuint(mt->prop2, ==, 102);
-all_used = qdev_prop_check_globals();
-g_assert_cmpuint(all_used, ==, 0);
+global_error = qdev_prop_check_globals();
+g_assert_cmpuint(global_error, ==, 0);
g_assert(props[0].used);
g_assert(props[1].used);
g_assert(!props[2].used);
--
2.19.1.708.g4ede3d42df
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH] RFC: chardev: mark the calls that allow an implicit mux monitor
Hi On Mon, Aug 27, 2018 at 10:10 AM Markus Armbruster wrote: > > Marc-André Lureau writes: > > > Hi > > On Fri, Aug 24, 2018 at 9:37 AM Markus Armbruster wrote: > >> > >> Marc-André Lureau writes: > >> > >> > This is mostly for readability of the code. Let's make it clear which > >> > callers can create an implicit monitor when the chardev is muxed. > >> > > >> > This will also enforce a safer behaviour, as we don't really support > >> > creating monitor anywhere/anytime at the moment. > >> > > >> > There are documented cases, such as: -serial/-parallel/-virtioconsole > >> > and to less extent -debugcon. > >> > > >> > Less obvious and questionnable ones are -gdb and Xen console. Add a > >> > FIXME note for those, but keep the support for now. > >> > > >> > Other qemu_chr_new() callers either have a fixed parameter/filename > >> > string, or do preliminary checks on the string (such as slirp), or do > >> > not need it, such as -qtest. > >> > > >> > On a related note, the list of monitor creation places: > >> > > >> > - the chardev creators listed above: all from command line (except > >> > perhaps Xen console?) > >> > > >> > - -gdb & hmp gdbserver will create a "GDB monitor command" chardev > >> > that is wired to an HMP monitor. > >> > > >> > - -mon command line option > >> > >> Aside: the question "what does it mean to connect a monitor to a chardev > >> that has an implicit monitor" crosses my mind. Next thought is "the > >> day's too short to go there". > >> > >> > From this short study, I would like to think that a monitor may only > >> > be created in the main thread today, though I remain skeptical :) > >> > >> Hah! > >> > >> > Signed-off-by: Marc-André Lureau > >> > --- > >> > include/chardev/char.h | 18 +- > >> > chardev/char.c | 21 + > >> > gdbstub.c | 6 +- > >> > hw/char/xen_console.c | 5 - > >> > vl.c | 8 > >> > 5 files changed, 47 insertions(+), 11 deletions(-) > >> > > >> > diff --git a/include/chardev/char.h b/include/chardev/char.h > >> > index 6f0576e214..be2b9b817e 100644 > >> > --- a/ > >> > +++ b/include/chardev/char.h > >> > @@ -105,6 +105,7 @@ ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts, > >> > * @qemu_chr_new: > >> > * > >> > * Create a new character backend from a URI. > >> > + * Do not implicitely initialize a monitor if the chardev is muxed. > >> > * > >> > * @label the name of the backend > >> > * @filename the URI > >> > @@ -113,6 +114,19 @@ ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts, > >> > */ > >> > Chardev *qemu_chr_new(const char *label, const char *filename); > >> > > >> > +/** > >> > + * @qemu_chr_new_mux_mon: > >> > + * > >> > + * Create a new character backend from a URI. > >> > + * Implicitely initialize a monitor if the chardev is muxed. > >> > + * > >> > + * @label the name of the backend > >> > + * @filename the URI > >> > >> I'm no friend of GTK-Doc style, but where we use it, we should use it > >> correctly: put a colon after @param. > > > > I copied existing comment... Should I fixed all others in the headers? > > Do we expect to actually *use* doc comments for anything? We haven't in > a decade or so, but if we still expect to all the same, then not fixing > them up now merely delays the inevitable. > > Do we think adding the colons makes the comments easier to read? For > what it's worth, I do. > > Decision's up to you. Let's improve it. > > >> > + * > >> > + * Returns: a new character backend > >> > + */ > >> > +Chardev *qemu_chr_new_mux_mon(const char *label, const char *filename); > >> > + > >> > /** > >> > * @qemu_chr_change: > >> > * > >> > @@ -138,10 +152,12 @@ void qemu_chr_cleanup(void); > >> > * > >> > * @label the name of the backend > >> > * @filename the URI > >> > + * @with_mux_mon if chardev is muxed, initiali
Re: [Xen-devel] [Qemu-devel] [PATCH] RFC: chardev: mark the calls that allow an implicit mux monitor
Hi
On Fri, Aug 24, 2018 at 9:37 AM Markus Armbruster wrote:
>
> Marc-André Lureau writes:
>
> > This is mostly for readability of the code. Let's make it clear which
> > callers can create an implicit monitor when the chardev is muxed.
> >
> > This will also enforce a safer behaviour, as we don't really support
> > creating monitor anywhere/anytime at the moment.
> >
> > There are documented cases, such as: -serial/-parallel/-virtioconsole
> > and to less extent -debugcon.
> >
> > Less obvious and questionnable ones are -gdb and Xen console. Add a
> > FIXME note for those, but keep the support for now.
> >
> > Other qemu_chr_new() callers either have a fixed parameter/filename
> > string, or do preliminary checks on the string (such as slirp), or do
> > not need it, such as -qtest.
> >
> > On a related note, the list of monitor creation places:
> >
> > - the chardev creators listed above: all from command line (except
> > perhaps Xen console?)
> >
> > - -gdb & hmp gdbserver will create a "GDB monitor command" chardev
> > that is wired to an HMP monitor.
> >
> > - -mon command line option
>
> Aside: the question "what does it mean to connect a monitor to a chardev
> that has an implicit monitor" crosses my mind. Next thought is "the
> day's too short to go there".
>
> > From this short study, I would like to think that a monitor may only
> > be created in the main thread today, though I remain skeptical :)
>
> Hah!
>
> > Signed-off-by: Marc-André Lureau
> > ---
> > include/chardev/char.h | 18 +-
> > chardev/char.c | 21 +
> > gdbstub.c | 6 +-
> > hw/char/xen_console.c | 5 -
> > vl.c | 8
> > 5 files changed, 47 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/chardev/char.h b/include/chardev/char.h
> > index 6f0576e214..be2b9b817e 100644
> > --- a/
> > +++ b/include/chardev/char.h
> > @@ -105,6 +105,7 @@ ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts,
> > * @qemu_chr_new:
> > *
> > * Create a new character backend from a URI.
> > + * Do not implicitely initialize a monitor if the chardev is muxed.
> > *
> > * @label the name of the backend
> > * @filename the URI
> > @@ -113,6 +114,19 @@ ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts,
> > */
> > Chardev *qemu_chr_new(const char *label, const char *filename);
> >
> > +/**
> > + * @qemu_chr_new_mux_mon:
> > + *
> > + * Create a new character backend from a URI.
> > + * Implicitely initialize a monitor if the chardev is muxed.
> > + *
> > + * @label the name of the backend
> > + * @filename the URI
>
> I'm no friend of GTK-Doc style, but where we use it, we should use it
> correctly: put a colon after @param.
I copied existing comment... Should I fixed all others in the headers?
>
> > + *
> > + * Returns: a new character backend
> > + */
> > +Chardev *qemu_chr_new_mux_mon(const char *label, const char *filename);
> > +
> > /**
> > * @qemu_chr_change:
> > *
> > @@ -138,10 +152,12 @@ void qemu_chr_cleanup(void);
> > *
> > * @label the name of the backend
> > * @filename the URI
> > + * @with_mux_mon if chardev is muxed, initialize a monitor
> > *
> > * Returns: a new character backend
> > */
> > -Chardev *qemu_chr_new_noreplay(const char *label, const char *filename);
> > +Chardev *qemu_chr_new_noreplay(const char *label, const char *filename,
> > + bool with_mux_mon);
> >
> > /**
> > * @qemu_chr_be_can_write:
> > diff --git a/chardev/char.c b/chardev/char.c
> > index 76d866e6fe..2c295ad676 100644
> > --- a/chardev/char.c
> > +++ b/chardev/char.c
> > @@ -683,7 +683,8 @@ out:
> > return chr;
> > }
> >
> > -Chardev *qemu_chr_new_noreplay(const char *label, const char *filename)
> > +Chardev *qemu_chr_new_noreplay(const char *label, const char *filename,
> > + bool with_mux_mon)
> > {
> > const char *p;
> > Chardev *chr;
> > @@ -702,17 +703,19 @@ Chardev *qemu_chr_new_noreplay(const char *label,
> > const char *filename)
> > if (err) {
> > error_report_err(err);
> > }
> > -if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
> > +if (with_mux_mon && chr && qemu_opt_get_bool(opts,
[Xen-devel] [PATCH] RFC: chardev: mark the calls that allow an implicit mux monitor
This is mostly for readability of the code. Let's make it clear which
callers can create an implicit monitor when the chardev is muxed.
This will also enforce a safer behaviour, as we don't really support
creating monitor anywhere/anytime at the moment.
There are documented cases, such as: -serial/-parallel/-virtioconsole
and to less extent -debugcon.
Less obvious and questionnable ones are -gdb and Xen console. Add a
FIXME note for those, but keep the support for now.
Other qemu_chr_new() callers either have a fixed parameter/filename
string, or do preliminary checks on the string (such as slirp), or do
not need it, such as -qtest.
On a related note, the list of monitor creation places:
- the chardev creators listed above: all from command line (except
perhaps Xen console?)
- -gdb & hmp gdbserver will create a "GDB monitor command" chardev
that is wired to an HMP monitor.
- -mon command line option
From this short study, I would like to think that a monitor may only
be created in the main thread today, though I remain skeptical :)
Signed-off-by: Marc-André Lureau
---
include/chardev/char.h | 18 +-
chardev/char.c | 21 +
gdbstub.c | 6 +-
hw/char/xen_console.c | 5 -
vl.c | 8
5 files changed, 47 insertions(+), 11 deletions(-)
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 6f0576e214..be2b9b817e 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -105,6 +105,7 @@ ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts,
* @qemu_chr_new:
*
* Create a new character backend from a URI.
+ * Do not implicitely initialize a monitor if the chardev is muxed.
*
* @label the name of the backend
* @filename the URI
@@ -113,6 +114,19 @@ ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts,
*/
Chardev *qemu_chr_new(const char *label, const char *filename);
+/**
+ * @qemu_chr_new_mux_mon:
+ *
+ * Create a new character backend from a URI.
+ * Implicitely initialize a monitor if the chardev is muxed.
+ *
+ * @label the name of the backend
+ * @filename the URI
+ *
+ * Returns: a new character backend
+ */
+Chardev *qemu_chr_new_mux_mon(const char *label, const char *filename);
+
/**
* @qemu_chr_change:
*
@@ -138,10 +152,12 @@ void qemu_chr_cleanup(void);
*
* @label the name of the backend
* @filename the URI
+ * @with_mux_mon if chardev is muxed, initialize a monitor
*
* Returns: a new character backend
*/
-Chardev *qemu_chr_new_noreplay(const char *label, const char *filename);
+Chardev *qemu_chr_new_noreplay(const char *label, const char *filename,
+ bool with_mux_mon);
/**
* @qemu_chr_be_can_write:
diff --git a/chardev/char.c b/chardev/char.c
index 76d866e6fe..2c295ad676 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -683,7 +683,8 @@ out:
return chr;
}
-Chardev *qemu_chr_new_noreplay(const char *label, const char *filename)
+Chardev *qemu_chr_new_noreplay(const char *label, const char *filename,
+ bool with_mux_mon)
{
const char *p;
Chardev *chr;
@@ -702,17 +703,19 @@ Chardev *qemu_chr_new_noreplay(const char *label, const
char *filename)
if (err) {
error_report_err(err);
}
-if (chr && qemu_opt_get_bool(opts, "mux", 0)) {
+if (with_mux_mon && chr && qemu_opt_get_bool(opts, "mux", 0)) {
monitor_init(chr, MONITOR_USE_READLINE);
}
qemu_opts_del(opts);
return chr;
}
-Chardev *qemu_chr_new(const char *label, const char *filename)
+static Chardev *qemu_chr_new_with_mux_mon(const char *label,
+ const char *filename,
+ bool with_mux_mon)
{
Chardev *chr;
-chr = qemu_chr_new_noreplay(label, filename);
+chr = qemu_chr_new_noreplay(label, filename, with_mux_mon);
if (chr) {
if (replay_mode != REPLAY_MODE_NONE) {
qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_REPLAY);
@@ -726,6 +729,16 @@ Chardev *qemu_chr_new(const char *label, const char
*filename)
return chr;
}
+Chardev *qemu_chr_new(const char *label, const char *filename)
+{
+return qemu_chr_new_with_mux_mon(label, filename, false);
+}
+
+Chardev *qemu_chr_new_mux_mon(const char *label, const char *filename)
+{
+return qemu_chr_new_with_mux_mon(label, filename, true);
+}
+
static int qmp_query_chardev_foreach(Object *obj, void *data)
{
Chardev *chr = CHARDEV(obj);
diff --git a/gdbstub.c b/gdbstub.c
index d6ab95006c..963531ea90 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2038,7 +2038,11 @@ int gdbserver_start(const char *device)
sigaction(SIGINT, , NULL);
}
#endif
-chr = qemu_chr_new_noreplay("gdb", device);
+/*
+ * FIXME: it's a bit weird to allow using a mux chardev here
+ * and setup implici
Re: [Xen-devel] [Qemu-devel] [PATCH v2 5/6] xen: Add only xen-sysdev to dynamic sysbus device list
Hi, On Sat, Nov 25, 2017 at 4:16 PM, Eduardo Habkost <ehabk...@redhat.com> wrote: > There's no need to make the machine allow every possible sysbus > device. We can now just add xen-sysdev to the allowed list. > > Cc: Stefano Stabellini <sstabell...@kernel.org> > Cc: Anthony Perard <anthony.per...@citrix.com> > Cc: xen-devel@lists.xenproject.org > Cc: Juergen Gross <jgr...@suse.com> > Signed-off-by: Eduardo Habkost <ehabk...@redhat.com> > --- > Changes series v1 -> v2: > * New patch added to series > --- > hw/xen/xen_backend.c | 7 +-- > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c > index 82380ea9ee..7445b506ac 100644 > --- a/hw/xen/xen_backend.c > +++ b/hw/xen/xen_backend.c > @@ -564,12 +564,7 @@ static void xen_set_dynamic_sysbus(void) > ObjectClass *oc = object_get_class(machine); > MachineClass *mc = MACHINE_CLASS(oc); > > -/* > - * Emulate old mc->has_dynamic_sysbus=true assignment > - * > - *TODO: add only Xen devices to the list > - */ > -machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SYS_BUS_DEVICE); > +machine_class_allow_dynamic_sysbus_dev(mc, TYPE_XENSYSDEV); It looks like it's the last patch in the series without review. It would be nice if a Xen maintainer could look at it, or test it, as I dont know how to test it tbh. Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com> > } > > int xen_be_register(const char *type, struct XenDevOps *ops) > -- > 2.13.6 > > -- Marc-André Lureau ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [Qemu-devel] [PATCH v2 1/6] machine: Replace has_dynamic_sysbus with list of allowed devices
Hi
On Sat, Nov 25, 2017 at 4:16 PM, Eduardo Habkost <ehabk...@redhat.com> wrote:
> The existing has_dynamic_sysbus flag makes the machine accept
> every user-creatable sysbus device type on the command-line.
> Replace it with a list of allowed device types, so machines can
> easily accept some sysbus devices while rejecting others.
>
> To keep exactly the same behavior as before, the existing
> has_dynamic_sysbus=true assignments are replaced with a
> TYPE_SYS_BUS_DEVICE entry on the allowed list. Other patches
> will replace the TYPE_SYS_BUS_DEVICE entries with more specific
> lists of devices.
>
> Cc: Peter Maydell <peter.mayd...@linaro.org>
> Cc: Marcel Apfelbaum <mar...@redhat.com>
> Cc: "Michael S. Tsirkin" <m...@redhat.com>
> Cc: Alexander Graf <ag...@suse.de>
> Cc: David Gibson <da...@gibson.dropbear.id.au>
> Cc: Stefano Stabellini <sstabell...@kernel.org>
> Cc: Anthony Perard <anthony.per...@citrix.com>
> Cc: qemu-...@nongnu.org
> Cc: qemu-...@nongnu.org
> Cc: xen-devel@lists.xenproject.org
> Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> Changes v1 -> v2:
> * Replace "dynamic sysbus whitelist" with "allowed sysbus devices"
> * Simply add TYPE_SYS_BUS_DEVICE to the list on existing
> has_dynamic_sysbus=true machines, and make machine-types more
> strict in separate patches
> ---
> include/hw/boards.h | 5 -
> hw/arm/virt.c| 3 ++-
> hw/core/machine.c| 43 +--
> hw/i386/pc_q35.c | 3 ++-
> hw/ppc/e500plat.c| 4 +++-
> hw/ppc/spapr.c | 3 ++-
> hw/xen/xen_backend.c | 7 ++-
> 7 files changed, 48 insertions(+), 20 deletions(-)
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 156b16f7a6..041bc08971 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -76,6 +76,9 @@ void machine_set_cpu_numa_node(MachineState *machine,
> const CpuInstanceProperties *props,
> Error **errp);
>
> +void machine_class_allow_dynamic_sysbus_dev(MachineClass *mc, const char
> *type);
> +
> +
> /**
> * CPUArchId:
> * @arch_id - architecture-dependent CPU ID of present or possible CPU
> @@ -179,7 +182,6 @@ struct MachineClass {
> no_floppy:1,
> no_cdrom:1,
> no_sdcard:1,
> -has_dynamic_sysbus:1,
> pci_allow_0_address:1,
> legacy_fw_cfg_order:1;
> int is_default;
> @@ -197,6 +199,7 @@ struct MachineClass {
> bool ignore_memory_transaction_failures;
> int numa_mem_align_shift;
> const char **valid_cpu_types;
> +strList *allowed_dynamic_sysbus_devices;
> bool auto_enable_numa_with_memhp;
> void (*numa_auto_assign_ram)(MachineClass *mc, NodeInfo *nodes,
> int nb_nodes, ram_addr_t size);
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 9e18b410d7..fa6dc15fcd 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1591,7 +1591,8 @@ static void virt_machine_class_init(ObjectClass *oc,
> void *data)
> * configuration of the particular instance.
> */
> mc->max_cpus = 255;
> -mc->has_dynamic_sysbus = true;
> +/*TODO: allow only sysbus devices that really work with this machine */
cosmetic: why do you not leave a space between * and TODO ? (you did
that repeatedly)
> +machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SYS_BUS_DEVICE);
> mc->block_default_type = IF_VIRTIO;
> mc->no_cdrom = 1;
> mc->pci_allow_0_address = true;
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 36c2fb069c..ab2ec292f3 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -335,29 +335,44 @@ static bool machine_get_enforce_config_section(Object
> *obj, Error **errp)
> return ms->enforce_config_section;
> }
>
> -static void error_on_sysbus_device(SysBusDevice *sbdev, void *opaque)
> +void machine_class_allow_dynamic_sysbus_dev(MachineClass *mc, const char
> *type)
> {
> -error_report("Option '-device %s' cannot be handled by this machine",
> - object_class_get_name(object_get_class(OBJECT(sbdev;
> -exit(1);
> +strList *item = g_new0(strList, 1);
> +
> +item->value = g_strdup(type);
> +item->next = mc->allowed_dynamic_sysbus_devices;
> +mc->allowed_dynamic_sysbus_devices = item;
> }
>
> -static void machine_init_notify(Notifier *notifier, void *data)
> +static void validate_sysbus_device(SysBusDevice *sbdev,
